syntax_tests: allow to update tests, and don't use a regexp (#8589)

* syntax_tests: allow to "bless" tests, and don't use a regexp

A regexp was used at the beginning because I thought we would want to
allow error to contains things that were not predictable or that would
often change. But this is not the case¹. It is better to actually test
for the full error message

¹ well actually it was the case for path, but there is another substitution to 
 `📂` for the manifest directory

* syntax_tests: Bless the tests

* syntax_tests: Manual adjust after bless

Because there used to be comments on the same line of the message which
bless don't support

* Fix error message with path on windows

 - The debug implementation of path make double slash, that's not what
   we want to show the user
 - normalize paths to use `/` so the test passes

internal/compiler/tests/syntax_tests.rs

@@ -7,14 +7,18 @@
 //! The .slint files can have comments like this:
 //! ```ignore
 //!  hi ho
-//!  // ^error{some_regexp}
+//!  // ^error{expected_message}
 //! ```
 //!
-//! Meaning that there must an error following with an error message for that regular expression in the position
-//! on the line above at the column pointed by the caret.
-//! If there are two carets: ` ^^error{some_regexp}`  then it means two line above, and so on with more carets.
-//! `^warning{regexp}` is also supported.
+//! Meaning that there must an error with that error message in the position on the line above at the column pointed by the caret.
+//! If there are two carets: ` ^^error{expected_message}`  then it means two line above, and so on with more carets.
+//! `^warning{expected_message}` is also supported.
+//!
+//! The newlines are replaced by `↵` in the error message. Also the manifest dir (CARGO_MANIFEST_DIR) is replaced by `📂`.
+//!
+//! When the env variable `SLINT_SYNTAX_TEST_UPDATE` is set to `1`, the source code will be modified to add the comments
 
+use i_slint_compiler::diagnostics::{BuildDiagnostics, Diagnostic, DiagnosticLevel};
 use i_slint_compiler::ComponentSelection;
 use std::path::{Path, PathBuf};
 
@@ -22,6 +26,9 @@ use std::path::{Path, PathBuf};
 fn syntax_tests() -> std::io::Result<()> {
     use rayon::prelude::*;
 
+    let update = std::env::args().any(|arg| arg == "--update")
+        || std::env::var("SLINT_SYNTAX_TEST_UPDATE").is_ok_and(|v| v == "1");
+
     if let Some(specific_test) = std::env::args()
         .skip(1)
         .skip_while(|arg| arg.starts_with("--") || arg == "syntax_tests")
@@ -30,7 +37,7 @@ fn syntax_tests() -> std::io::Result<()> {
         let mut path = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
         path.push("tests");
         path.push(specific_test);
-        assert!(process_file(&path)?);
+        assert!(process_file(&path, update)?);
         return Ok(());
     }
 
@@ -56,7 +63,7 @@ fn syntax_tests() -> std::io::Result<()> {
         .try_fold(
             || true,
             |mut success, path| {
-                success &= process_file(path)?;
+                success &= process_file(path, update)?;
                 Ok::<bool, std::io::Error>(success)
             },
         )
@@ -67,7 +74,7 @@ fn syntax_tests() -> std::io::Result<()> {
     Ok(())
 }
 
-fn process_file(path: &std::path::Path) -> std::io::Result<bool> {
+fn process_file(path: &std::path::Path, update: bool) -> std::io::Result<bool> {
     let source = std::fs::read_to_string(path)?;
     if path.to_str().unwrap_or("").contains("bom-") && !source.starts_with("\u{FEFF}") {
         // make sure that the file still contains BOM and it wasn't remove by some tools
@@ -75,17 +82,20 @@ fn process_file(path: &std::path::Path) -> std::io::Result<bool> {
             "{path:?} does not contains BOM while it should"
         )));
     }
-    std::panic::catch_unwind(|| process_file_source(path, source, false)).unwrap_or_else(|err| {
-        println!("Panic while processing {}: {:?}", path.display(), err);
-        Ok(false)
-    })
+    std::panic::catch_unwind(|| process_file_source(path, source, false, update)).unwrap_or_else(
+        |err| {
+            println!("Panic while processing {}: {:?}", path.display(), err);
+            Ok(false)
+        },
+    )
 }
 
 fn process_diagnostics(
-    compile_diagnostics: &i_slint_compiler::diagnostics::BuildDiagnostics,
+    compile_diagnostics: &BuildDiagnostics,
     path: &Path,
     source: &str,
     _silent: bool,
+    update: bool,
 ) -> std::io::Result<bool> {
     let mut success = true;
 
@@ -107,6 +117,9 @@ fn process_diagnostics(
         .filter_map(|(i, c)| if c == b'\n' { Some(i) } else { None })
         .collect::<Vec<usize>>();
 
+    let diag_copy = diags.clone();
+    let mut captures = Vec::new();
+
     // Find expected errors in the file. The first caret (^) points to the expected column. The number of
     // carets refers to the number of lines to go back. This is useful when one line of code produces multiple
     // errors or warnings.
@@ -116,14 +129,11 @@ fn process_diagnostics(
         let column = m.get(1).unwrap().start() - line_begin_offset;
         let lines_to_source = m.get(1).unwrap().as_str().len();
         let warning_or_error = m.get(2).unwrap().as_str();
-        let rx = m.get(3).unwrap().as_str();
-        let r = match regex::Regex::new(rx) {
-            Err(e) => {
-                eprintln!("{path:?}: Invalid regexp {rx:?} : {e:?}");
-                return Ok(false);
-            }
-            Ok(r) => r,
-        };
+        let expected_message =
+            m.get(3).unwrap().as_str().replace('↵', "\n").replace('📂', env!("CARGO_MANIFEST_DIR"));
+        if update {
+            captures.push(m.get(0).unwrap().range());
+        }
 
         let mut line_counter = 0;
         let mut line_offset = source[..line_begin_offset].rfind('\n').unwrap_or(0);
@@ -140,35 +150,46 @@ fn process_diagnostics(
         };
 
         let expected_diag_level = match warning_or_error {
-            "warning" => i_slint_compiler::diagnostics::DiagnosticLevel::Warning,
-            "error" => i_slint_compiler::diagnostics::DiagnosticLevel::Error,
+            "warning" => DiagnosticLevel::Warning,
+            "error" => DiagnosticLevel::Error,
             _ => panic!("Unsupported diagnostic level {warning_or_error}"),
         };
 
+        fn compare_message(message: &str, expected_message: &str) -> bool {
+            if message == expected_message {
+                return true;
+            }
+            // The error message might contain path that might have other character, so replace them on windows
+            #[cfg(target_os = "windows")]
+            if message.replace('\\', "/") == expected_message.replace('\\', "/") {
+                return true;
+            }
+            false
+        }
+
         match diags.iter().position(|e| {
             let (l, c) = e.line_column();
             let o = lines.get(l.wrapping_sub(2)).unwrap_or(&0) + c;
-            o == offset && r.is_match(e.message()) && e.level() == expected_diag_level
+            o == offset
+                && compare_message(e.message(), &expected_message)
+                && e.level() == expected_diag_level
         }) {
             Some(idx) => {
                 diags.remove(idx);
             }
             None => {
                 success = false;
-                println!("{path:?}: {warning_or_error} not found at offset {offset}: {rx:?}");
+                println!("{path:?}: {warning_or_error} not found at offset {offset}: {expected_message:?}");
             }
         }
     }
 
-    // Ignore deprecated warning about old syntax, because our tests still use the old syntax a lot
-    diags.retain(|d| !(d.message().contains("':='") && d.message().contains("deprecated")));
-
     if !diags.is_empty() {
         println!("{path:?}: Unexpected errors/warnings: {diags:#?}");
 
         #[cfg(feature = "display-diagnostics")]
         if !_silent {
-            let mut to_report = i_slint_compiler::diagnostics::BuildDiagnostics::default();
+            let mut to_report = BuildDiagnostics::default();
             for d in diags {
                 to_report.push_compiler_error(d.clone());
             }
@@ -177,9 +198,71 @@ fn process_diagnostics(
 
         success = false;
     }
+
+    if !success && update {
+        let mut source = source.to_string();
+        self::update(diag_copy, &mut source, lines, &captures);
+        std::fs::write(path, source).unwrap();
+    }
+
     Ok(success)
 }
 
+/// Rewrite the source to remove the old comments and add accurate error comments
+fn update(
+    mut diags: Vec<&Diagnostic>,
+    source: &mut String,
+    mut lines: Vec<usize>,
+    to_remove: &[std::ops::Range<usize>],
+) {
+    for to_remove in to_remove.iter().rev() {
+        source.drain(to_remove.clone());
+        for l in &mut lines {
+            if *l > to_remove.start {
+                *l -= to_remove.end - to_remove.start;
+            }
+        }
+    }
+
+    diags.sort_by_key(|d| {
+        let (l, c) = d.line_column();
+        (usize::MAX - l, c)
+    });
+
+    let mut last_line = 0;
+    let mut last_line_adjust = 0;
+
+    for d in diags {
+        let (l, c) = d.line_column();
+        if c < 3 {
+            panic!("Error message cannot be on the column < 3: {d:?}")
+        }
+
+        if last_line == l {
+            last_line_adjust += 1;
+        } else {
+            last_line = l;
+            last_line_adjust = 0;
+        }
+
+        let byte_offset = lines[l - 1] + 1;
+
+        let to_insert = format!(
+            "//{indent}^{adjust}{error_or_warning}{{{message}}}\n",
+            indent = " ".repeat(c - 3),
+            adjust = "^".repeat(last_line_adjust),
+            error_or_warning =
+                if d.level() == DiagnosticLevel::Error { "error" } else { "warning" },
+            message = d.message().replace('\n', "↵").replace(env!("CARGO_MANIFEST_DIR"), "📂")
+        );
+        if byte_offset > source.len() {
+            source.push('\n');
+        }
+        source.insert_str(byte_offset, &to_insert);
+        lines[l - 1] += to_insert.len();
+    }
+}
+
 fn canonical(path: &Path) -> PathBuf {
     path.canonicalize().unwrap_or_else(|_| path.to_owned())
 }
@@ -188,8 +271,9 @@ fn process_file_source(
     path: &std::path::Path,
     source: String,
     silent: bool,
+    update: bool,
 ) -> std::io::Result<bool> {
-    let mut parse_diagnostics = i_slint_compiler::diagnostics::BuildDiagnostics::default();
+    let mut parse_diagnostics = BuildDiagnostics::default();
     let syntax_node =
         i_slint_compiler::parser::parse(source.clone(), Some(path), &mut parse_diagnostics);
 
@@ -219,7 +303,7 @@ fn process_file_source(
     };
 
     let mut success = true;
-    success &= process_diagnostics(&compile_diagnostics, path, &source, silent)?;
+    success &= process_diagnostics(&compile_diagnostics, path, &source, silent, update)?;
 
     for p in &compile_diagnostics.all_loaded_files {
         let source = if p.is_absolute() {
@@ -228,7 +312,7 @@ fn process_file_source(
             // probably std-widgets.slint
             String::new()
         };
-        success &= process_diagnostics(&compile_diagnostics, p, &source, silent)?;
+        success &= process_diagnostics(&compile_diagnostics, p, &source, silent, update)?;
     }
 
     if has_parse_error {
@@ -247,19 +331,19 @@ fn process_file_source(
 /// Test that this actually fail when it should
 fn self_test() -> std::io::Result<()> {
     let fake_path = std::path::Path::new("fake.slint");
-    let process = |str: &str| process_file_source(fake_path, str.into(), true);
+    let process = |str: &str| process_file_source(fake_path, str.into(), true, false);
 
     // this should succeed
     assert!(process(
         r#"
-export Foo := Rectangle { x: 0px; }
+export component Foo inherits Window { width: 10px; }
     "#
     )?);
 
     // unless we expected an error
     assert!(!process(
         r#"
-export Foo := Rectangle { x: 0px; }
+export component Foo inherits Window { width: 10px; }
 //            ^error{i want an error}
     "#
     )?);
@@ -267,40 +351,40 @@ export Foo := Rectangle { x: 0px; }
     // An error should fail
     assert!(!process(
         r#"
-export Foo := Rectangle foo { x:0px; }
+export component Foo inherits Window foo { width: 10px; }
     "#
     )?);
 
     // An error with the proper comment should pass
     assert!(process(
         r#"
-Foo := Rectangle foo { x:0px; }
-//               ^error{expected '\{'}
+export component Foo inherits Window foo { width: 10px; }
+//                                   ^error{Syntax error: expected '{'}
     "#
     )?);
 
     // But not if it is at the wrong position
     assert!(!process(
         r#"
-Foo := Rectangle foo { x:0px; }
-//             ^error{expected '\{'}
+export component Foo inherits Window foo { width: 10px; }
+//                                    ^error{Syntax error: expected '{'}
     "#
     )?);
 
     // or the wrong line
     assert!(!process(
         r#"
-Foo := Rectangle foo { x:0px; }
+export component Foo inherits Window foo { width: 10px; }
 
-//               ^error{expected '\{'}
+//                                   ^error{Syntax error: expected '{'}
     "#
     )?);
 
     // or the wrong message
     assert!(!process(
         r#"
-Foo := Rectangle foo { x:0px; }
-//               ^error{foo_bar}
+export component Foo inherits Window foo { width: 10px; }
+//                                   ^error{foo_bar}
     "#
     )?);
 
@@ -308,14 +392,14 @@ Foo := Rectangle foo { x:0px; }
     assert!(!process(
         r#"
 
-Foo := Rectangle foo { x:0px; }
-//               ^^error{expected '\{'}
+export component Foo inherits Window foo { width: 10px; }
+//                                   ^^error{Syntax error: expected '{'}
     "#
     )?);
 
     // Even on windows, it should work
     assert!(process(
-        "\r\nFoo := Rectangle foo { x:0px; }\r\n//               ^error{expected '\\{'}\r\n"
+        "\r\nexport component Foo inherits Window foo { width: 10px; }\r\n//                                   ^error{Syntax error: expected '{'}\r\n"
     )?);
 
     Ok(())