lsp: Report the number of errors encountered during format

hunger · hunger · commit 1756dd5ad0bf · 2025-04-28T16:37:31.000Z
... in the return code. So 0 is "all is well", 1 - x is the number of slint errors in al files processed. 255 is a special value that gets reported for IO errors in the lsp code itself. Fixes: #8229
diff --git a/tools/lsp/fmt/fmt.rs b/tools/lsp/fmt/fmt.rs
@@ -1297,7 +1297,7 @@ fn format_member_access(
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::fmt::writer::FileWriter;
+    use crate::fmt::writer::StringWriter;
     use i_slint_compiler::diagnostics::BuildDiagnostics;
 
     // FIXME more descriptive errors when an assertion fails
@@ -1311,9 +1311,9 @@ mod tests {
         );
         // Turn the syntax node into a document
         let doc = syntax_nodes::Document::new(syntax_node).unwrap();
-        let mut file = Vec::new();
-        format_document(doc, &mut FileWriter { file: &mut file }).unwrap();
-        assert_eq!(String::from_utf8(file).unwrap(), formatted);
+        let mut writer = StringWriter { content: String::new() };
+        format_document(doc, &mut writer).unwrap();
+        assert_eq!(writer.content, formatted);
     }
 
     #[test]
diff --git a/tools/lsp/fmt/tool.rs b/tools/lsp/fmt/tool.rs
@@ -22,111 +22,180 @@ use std::path::Path;
 
 use super::{fmt, writer};
 
-pub fn run(files: Vec<std::path::PathBuf>, inplace: bool) -> std::io::Result<()> {
+pub fn run(files: Vec<std::path::PathBuf>, inplace: bool) -> std::io::Result<usize> {
+    let mut changed_or_error_files = 0;
+
     for path in files {
-        let source = std::fs::read_to_string(&path)?;
+        let source = std::fs::read(&path)?;
+
+        match process_file(source.clone(), &path) {
+            Ok(result) if result == source => {}
+            Ok(result) => {
+                changed_or_error_files += 1;
 
-        if inplace {
-            let file = BufWriter::new(std::fs::File::create(&path)?);
-            process_file(source, path, file)?
-        } else {
-            process_file(source, path, std::io::stdout())?
+                if inplace {
+                    let mut file = BufWriter::new(std::fs::File::create(&path)?);
+                    file.write_all(&result)?;
+                } else {
+                    std::io::stdout().write_all(&result)?;
+                }
+            }
+            Err(e) => {
+                println!("Error in {path:?}: {e}");
+                changed_or_error_files += 1;
+            }
         }
     }
-    Ok(())
+    Ok(changed_or_error_files)
 }
 
 /// FIXME! this is duplicated with the updater
-fn process_rust_file(source: String, mut file: impl Write) -> std::io::Result<()> {
+fn process_rust_file(source: Vec<u8>) -> std::io::Result<Vec<u8>> {
+    let mut result = String::new();
+    let source =
+        String::from_utf8(source).map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))?;
     let mut last = 0;
     for range in i_slint_compiler::lexer::locate_slint_macro(&source) {
-        file.write_all(source[last..=range.start].as_bytes())?;
+        result.push_str(&source[last..=range.start]);
         last = range.end;
         let code = &source[range];
 
         let mut diag = BuildDiagnostics::default();
         let syntax_node = i_slint_compiler::parser::parse(code.to_owned(), None, &mut diag);
         let len = syntax_node.text_range().end().into();
-        visit_node(syntax_node, &mut file)?;
+        result.push_str(&visit_node(syntax_node)?);
+
         if diag.has_errors() {
-            file.write_all(&code.as_bytes()[len..])?;
+            result.push_str(&code[len..]);
             diag.print();
+            return Err(std::io::Error::new(
+                std::io::ErrorKind::Other,
+                String::from("Failed to parse file"),
+            ));
+        }
+    }
+    result.push_str(&source[last..]);
+    Ok(result.as_bytes().to_vec())
+}
+
+fn find_slint_code_in_markdown(content: &[u8]) -> Option<(usize, usize)> {
+    const CODE_FENCE_START: &[u8] = b"```slint\n";
+    const CODE_FENCE_END: &[u8] = b"```\n";
+
+    let mut it = content.iter().enumerate();
+
+    let mut fence_offset = 0;
+
+    let mut code_start = usize::MAX;
+    let mut code_end = usize::MAX;
+
+    #[allow(clippy::while_let_on_iterator)]
+    while let Some((idx, b)) = it.next() {
+        if *b == CODE_FENCE_START[fence_offset] {
+            fence_offset += 1;
+        }
+
+        if fence_offset == CODE_FENCE_START.len() {
+            code_start = idx + 1;
+            break;
+        }
+    }
+
+    fence_offset = 0;
+    let mut possible_end_offset = 0;
+
+    #[allow(clippy::while_let_on_iterator)]
+    while let Some((idx, b)) = it.next() {
+        if *b == CODE_FENCE_END[fence_offset] {
+            if fence_offset == 0 {
+                possible_end_offset = idx - 1;
+            }
+            fence_offset += 1;
+        }
+
+        if fence_offset == CODE_FENCE_END.len() {
+            code_end = possible_end_offset;
+            break;
         }
     }
-    file.write_all(source[last..].as_bytes())
+
+    if code_end != usize::MAX {
+        Some((code_start, code_end))
+    } else {
+        None
+    }
 }
 
 /// FIXME! this is duplicated with the updater
-fn process_markdown_file(source: String, mut file: impl Write) -> std::io::Result<()> {
+fn process_markdown_file(source: Vec<u8>) -> std::io::Result<Vec<u8>> {
+    let mut result = Vec::new();
+
     let mut source_slice = &source[..];
-    const CODE_FENCE_START: &str = "```slint\n";
-    const CODE_FENCE_END: &str = "```\n";
-    'l: while let Some(code_start) =
-        source_slice.find(CODE_FENCE_START).map(|idx| idx + CODE_FENCE_START.len())
-    {
-        let code_end = if let Some(code_end) = source_slice[code_start..].find(CODE_FENCE_END) {
-            code_end
-        } else {
-            break 'l;
-        };
-        file.write_all(source_slice[..=code_start - 1].as_bytes())?;
-        source_slice = &source_slice[code_start..];
-        let code = &source_slice[..code_end];
+    while let Some((code_start, code_end)) = find_slint_code_in_markdown(source_slice) {
+        result.extend(&source_slice[..code_start]);
+        let code = Vec::from(&source_slice[code_start..code_end]);
+        let code = String::from_utf8(code)
+            .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))?;
         source_slice = &source_slice[code_end..];
 
         let mut diag = BuildDiagnostics::default();
-        let syntax_node = i_slint_compiler::parser::parse(code.to_owned(), None, &mut diag);
-        let len = syntax_node.text_range().end().into();
-        visit_node(syntax_node, &mut file)?;
+        let syntax_node = i_slint_compiler::parser::parse(code, None, &mut diag);
+
+        result.extend(visit_node(syntax_node)?.as_bytes());
+
         if diag.has_errors() {
-            file.write_all(&code.as_bytes()[len..])?;
             diag.print();
+            return Err(std::io::Error::new(
+                std::io::ErrorKind::Other,
+                String::from("Failed to parse file"),
+            ));
         }
     }
-    file.write_all(source_slice.as_bytes())
+    result.extend(source_slice);
+    Ok(result)
 }
 
-fn process_slint_file(
-    source: String,
-    path: std::path::PathBuf,
-    mut file: impl Write,
-) -> std::io::Result<()> {
+fn process_slint_file(source: Vec<u8>, path: &std::path::Path) -> std::io::Result<Vec<u8>> {
     let mut diag = BuildDiagnostics::default();
-    let syntax_node = i_slint_compiler::parser::parse(source.clone(), Some(&path), &mut diag);
-    let len = syntax_node.node.text_range().end().into();
-    visit_node(syntax_node, &mut file)?;
+    let source =
+        String::from_utf8(source).map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))?;
+
+    let syntax_node = i_slint_compiler::parser::parse(source, Some(path), &mut diag);
+
+    let result = visit_node(syntax_node)?.as_bytes().to_vec();
+
     if diag.has_errors() {
-        file.write_all(&source.as_bytes()[len..])?;
         diag.print();
+        return Err(std::io::Error::new(
+            std::io::ErrorKind::Other,
+            String::from("Failed to parse file"),
+        ));
     }
-    Ok(())
+    Ok(result)
 }
 
-fn process_file(
-    source: String,
-    path: std::path::PathBuf,
-    mut file: impl Write,
-) -> std::io::Result<()> {
+fn process_file(source: Vec<u8>, path: &std::path::Path) -> std::io::Result<Vec<u8>> {
     match path.extension() {
-        Some(ext) if ext == "rs" => process_rust_file(source, file),
-        Some(ext) if ext == "md" => process_markdown_file(source, file),
+        Some(ext) if ext == "rs" => process_rust_file(source),
+        Some(ext) if ext == "md" => process_markdown_file(source),
         // Formatting .60 files because of backwards compatibility (project was recently renamed)
-        Some(ext) if ext == "slint" || ext == ".60" => process_slint_file(source, path, file),
+        Some(ext) if ext == "slint" || ext == ".60" => process_slint_file(source, path),
         _ => {
-            // This allows usage like `cat x.slint | slint-lsp format /dev/stdin`
-            if path.as_path() == Path::new("/dev/stdin") {
-                return process_slint_file(source, path, file);
+            // This allows usage like `cat x.slint | slint-lsp format -`
+            if path == Path::new("/dev/stdin") || path == Path::new("-") {
+                return process_slint_file(source, path);
             }
-            // With other file types, we just output them in their original form.
-            file.write_all(source.as_bytes())
+
+            Ok(source.to_vec())
         }
     }
 }
 
-fn visit_node(node: SyntaxNode, file: &mut impl Write) -> std::io::Result<()> {
+fn visit_node(node: SyntaxNode) -> std::io::Result<String> {
     if let Some(doc) = syntax_nodes::Document::new(node) {
-        let mut writer = writer::FileWriter { file };
-        fmt::format_document(doc, &mut writer)
+        let mut writer = writer::StringWriter::default();
+        fmt::format_document(doc, &mut writer)?;
+        Ok(writer.finalize())
     } else {
         Err(std::io::Error::new(std::io::ErrorKind::Other, "Not a Document"))
     }
diff --git a/tools/lsp/fmt/writer.rs b/tools/lsp/fmt/writer.rs
@@ -2,7 +2,6 @@
 // SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
 
 use i_slint_compiler::parser::SyntaxToken;
-use std::io::Write;
 
 /// The idea is that each token need to go through this, either with no changes,
 /// or with a new content.
@@ -18,21 +17,31 @@ pub trait TokenWriter {
 }
 
 /// Just write the token stream to a file
-pub struct FileWriter<'a, W> {
-    pub file: &'a mut W,
+#[derive(Default)]
+pub struct StringWriter {
+    content: String,
 }
 
-impl<W: Write> TokenWriter for FileWriter<'_, W> {
+impl StringWriter {
+    pub fn finalize(self) -> String {
+        self.content
+    }
+}
+
+impl TokenWriter for StringWriter {
     fn no_change(&mut self, token: SyntaxToken) -> std::io::Result<()> {
-        self.file.write_all(token.text().as_bytes())
+        self.content.push_str(token.text());
+        Ok(())
     }
 
     fn with_new_content(&mut self, _token: SyntaxToken, contents: &str) -> std::io::Result<()> {
-        self.file.write_all(contents.as_bytes())
+        self.content.push_str(contents);
+        Ok(())
     }
 
     fn insert_before(&mut self, token: SyntaxToken, contents: &str) -> std::io::Result<()> {
-        self.file.write_all(contents.as_bytes())?;
-        self.file.write_all(token.text().as_bytes())
+        self.content.push_str(contents);
+        self.content.push_str(token.text());
+        Ok(())
     }
 }
diff --git a/tools/lsp/main.rs b/tools/lsp/main.rs
@@ -96,7 +96,6 @@ enum Commands {
 struct Format {
     #[arg(name = "path to .slint file(s)", action)]
     paths: Vec<std::path::PathBuf>,
-
     /// modify the file inline instead of printing to stdout
     #[arg(short, long, action)]
     inline: bool,
@@ -234,11 +233,13 @@ fn main() {
     }
 
     if let Some(Commands::Format(args)) = args.command {
-        let _ = fmt::tool::run(args.paths, args.inline).map_err(|e| {
-            eprintln!("{e}");
-            std::process::exit(1);
-        });
-        std::process::exit(0);
+        match fmt::tool::run(args.paths, args.inline) {
+            Ok(error_count) => std::process::exit(error_count.try_into().unwrap_or(254)),
+            Err(e) => {
+                eprintln!("{e}");
+                std::process::exit(255);
+            }
+        }
     }
 
     if let Ok(panic_log_file) = std::env::var("SLINT_LSP_PANIC_LOG") {
