Friendlier diagnostics (#49)

* Improve diagnostics handling

* Refactor test harness to support test pragmas

* Friendly ambiguous pattern match diagnostic

* Friendly ambiguous path resolution error

* Adjust error message

Cargo.toml

@@ -32,6 +32,7 @@ smol = "2.0.2"
 toml_edit = "0.22.22"
 futures = "0.3.31"
 annotate-snippets = "0.11.5"
+anstream = "0.6.18"
 
 [workspace.lints.clippy]
 pedantic = { level = "warn", priority = -1 }

tests/Cargo.toml

@@ -22,6 +22,9 @@ toml_edit.workspace = true
 futures.workspace = true
 regex.workspace = true
 werk-util.workspace = true
+anstream.workspace = true
+# Hijacking winnow for the Offset trait
+winnow.workspace = true
 
 [dev-dependencies]
 criterion = "0.3"

tests/benches/bench_eval.rs

@@ -4,7 +4,13 @@ use tests::mock_io::Test;
 pub fn parse_c_example(c: &mut Criterion) {
     static SOURCE: &str = include_str!("../../examples/c/Werkfile");
     c.bench_function("parse c example", |b| {
-        b.iter(|| black_box(werk_parser::parse_werk(SOURCE)).unwrap())
+        b.iter(|| {
+            black_box(werk_parser::parse_werk(
+                std::path::Path::new("../../examples/c/Werkfile"),
+                SOURCE,
+            ))
+            .unwrap()
+        })
     });
 }
 
@@ -25,7 +31,13 @@ pub fn parse_1000_lets(c: &mut Criterion) {
         source.push_str(&format!("let a{} = a{}\n", i, i - 1));
     }
     c.bench_function("parse 1000 lets", |b| {
-        b.iter(|| black_box(werk_parser::parse_werk(&source)).unwrap())
+        b.iter(|| {
+            black_box(werk_parser::parse_werk(
+                std::path::Path::new("INPUT"),
+                &source,
+            ))
+            .unwrap()
+        })
     });
 }
 

tests/benches/bench_iai.rs

@@ -1,6 +1,10 @@
 fn iai_bench_parse() {
     static SOURCE: &str = include_str!("../../examples/c/Werkfile");
-    iai::black_box(werk_parser::parse_werk(SOURCE)).unwrap();
+    iai::black_box(werk_parser::parse_werk(
+        std::path::Path::new("../../examples/c/Werkfile"),
+        SOURCE,
+    ))
+    .unwrap();
 }
 
 iai::main!(iai_bench_parse);

tests/fail/ambiguous_build_recipe.txt

@@ -0,0 +1,11 @@
+error[R0011]: ambiguous pattern match: /foofoo
+ --> INPUT:3:7
+  |
+3 | build "%foo" {
+  |       ------ note: first pattern here
+  |
+ ::: INPUT:7:7
+  |
+7 | build "foo%" {
+  |       ------ note: second pattern here
+  |

tests/fail/ambiguous_build_recipe.werk

@@ -0,0 +1,9 @@
+config default = "foofoo"
+
+build "%foo" {
+    info "<out>"
+}
+
+build "foo%" {
+    info "<out>"
+}

tests/fail/ambiguous_path_resolution.txt

@@ -0,0 +1,12 @@
+error[E0032]: ambiguous path resolution: /bar exists in the workspace, but also matches a build recipe
+ --> INPUT:6:10
+  |
+6 |     info "<out>"
+  |          ^^^^^^^ ambiguous path resolution: /bar exists in the workspace, but also matches a build recipe
+  |
+ ::: INPUT:5:7
+  |
+5 | build "bar" {
+  |       ----- note: matched this build recipe
+  |
+  = help: use `<...:out-dir>` or `<...:workspace>` to disambiguate between paths in the workspace and the output directory

tests/fail/ambiguous_path_resolution.werk

@@ -0,0 +1,16 @@
+config default = "build"
+
+# Directories should not participate in the lookup that happens as part
+# of build recipe matching.
+build "bar" {
+    info "<out>"
+}
+build "foo" {
+    info "<out>"
+}
+
+task build {
+    build ["foo", "bar"]
+}
+
+#!dir bar

tests/fail/capture_group_out_of_bounds.txt

@@ -0,0 +1,7 @@
+error[E0009]: capture group with index 2 is out of bounds in the current scope
+ --> INPUT:2:21
+  |
+2 |     "(a|b)(c|d)" => "{2}"
+  |                     ^^^^^ capture group with index 2 is out of bounds in the current scope
+  |
+  = help: pattern capture groups are zero-indexed, starting from 0

tests/fail/capture_group_out_of_bounds.werk

@@ -0,0 +1,4 @@
+let a = "ac" | match {
+    "(a|b)(c|d)" => "{2}"
+    "%" => error "no match"
+}

tests/mock_io.rs

@@ -3,16 +3,19 @@ use std::{
     collections::{hash_map, HashMap},
     ffi::{OsStr, OsString},
     pin::Pin,
-    sync::{atomic::AtomicU64, Arc},
+    sync::{atomic::AtomicU64, Arc, OnceLock},
     time::SystemTime,
 };
 
 use parking_lot::Mutex;
 use werk_fs::Absolute;
+use werk_parser::parser::{Offset, Span};
 use werk_runner::{
     globset, BuildStatus, DirEntry, Env, Error, GlobSettings, Io, Metadata, Outdatedness,
     ShellCommandLine, TaskId, WhichError, WorkspaceSettings,
 };
+use werk_util::{Diagnostic as _, DiagnosticError, DiagnosticSource};
+use winnow::stream::Offset as _;
 
 #[inline]
 #[must_use]
@@ -45,6 +48,7 @@ pub struct TestBuilder<'a> {
     pub default_filesystem: bool,
     pub create_workspace_dir: bool,
     pub werkfile: &'a str,
+    pub werkfile_path: &'a std::path::Path,
 }
 
 pub fn native_path<I: IntoIterator<Item: AsRef<OsStr>>>(
@@ -70,6 +74,7 @@ impl Default for TestBuilder<'_> {
             default_filesystem: true,
             create_workspace_dir: true,
             werkfile: "",
+            werkfile_path: std::path::Path::new("INPUT"),
         }
     }
 }
@@ -96,7 +101,7 @@ impl<'a> TestBuilder<'a> {
     }
 
     pub fn build(&self) -> Result<Test<'a>, werk_parser::Error> {
-        let ast = werk_parser::parse_werk(self.werkfile)?;
+        let ast = werk_parser::parse_werk(self.werkfile_path, self.werkfile)?;
 
         let io = MockIo::default();
         io.set_env("PROFILE", "debug");
@@ -168,6 +173,8 @@ impl<'a> TestBuilder<'a> {
             workspace_dir: self.workspace_dir.clone(),
             output_dir: self.output_dir.clone(),
             ast,
+            source: self.werkfile,
+            pragma_check_files: vec![],
         })
     }
 }
@@ -178,22 +185,123 @@ pub struct Test<'a> {
     pub workspace_dir: Absolute<std::path::PathBuf>,
     pub output_dir: Absolute<std::path::PathBuf>,
     pub ast: werk_parser::Document<'a>,
+    pub source: &'a str,
+    pragma_check_files: Vec<(Span, String, Vec<u8>)>,
 }
 
 impl<'a> Test<'a> {
-    pub fn new(werk_source: &'a str) -> Result<Self, werk_parser::Error> {
-        TestBuilder::default().werkfile(werk_source).build()
+    pub fn new(
+        source: &'a str,
+    ) -> Result<Self, werk_util::DiagnosticError<'a, werk_parser::Error, DiagnosticSource<'a>>>
+    {
+        let mut test = TestBuilder::default()
+            .werkfile(source)
+            .build()
+            .map_err(|err| {
+                err.into_diagnostic_error(DiagnosticSource::new(
+                    std::path::Path::new("input"),
+                    source,
+                ))
+            })?;
+        test.reload_test_pragmas();
+        Ok(test)
+    }
+
+    pub fn reload(
+        &mut self,
+        source: &'a str,
+    ) -> Result<(), werk_util::DiagnosticError<'a, werk_parser::Error, DiagnosticSource<'a>>> {
+        self.ast = werk_parser::parse_werk(self.ast.origin, source).map_err(|err| {
+            err.into_diagnostic_error(DiagnosticSource::new(std::path::Path::new("input"), source))
+        })?;
+        self.source = source;
+        self.reload_test_pragmas();
+        Ok(())
     }
 
-    pub fn reload(&mut self, werkfile: &'a str) -> Result<(), werk_parser::Error> {
-        self.ast = werk_parser::parse_werk(werkfile)?;
+    fn reload_test_pragmas(&mut self) {
+        self.pragma_check_files.clear();
+
+        // Interpret pragmas in the trailing comment of the werkfile.
+        let trailing_whitespace = self.ast.get_whitespace(self.ast.root.ws_trailing);
+        let trailing_whitespace_lines = trailing_whitespace.trim().lines();
+        let trailing_ws_span_start = if self.ast.root.ws_trailing.0.is_ignored() {
+            0
+        } else {
+            self.ast.root.ws_trailing.0.start.0 as usize
+        };
+
+        let regexes = regexes();
+        {
+            let mut fs = self.io.filesystem.lock();
+            for line in trailing_whitespace_lines {
+                let offset = line.offset_from(&trailing_whitespace);
+                let span = Span {
+                    start: Offset((trailing_ws_span_start + offset) as u32),
+                    end: Offset((trailing_ws_span_start + offset + line.len()) as u32),
+                };
+
+                if let Some(captures) = regexes.file.captures(line) {
+                    let filename = captures.get(1).unwrap().as_str();
+                    let content = captures.get(2).unwrap().as_str();
+                    let path = self.workspace_path(filename.split('/'));
+                    insert_fs(
+                        &mut fs,
+                        &path,
+                        (
+                            Metadata {
+                                mtime: default_mtime(),
+                                is_file: true,
+                                is_symlink: false,
+                            },
+                            content.as_bytes().to_owned(),
+                        ),
+                    )
+                    .unwrap();
+                } else if let Some(captures) = regexes.dir.captures(line) {
+                    let dirname = captures.get(1).unwrap().as_str();
+                    let path = self.workspace_path(dirname.split('/'));
+                    create_dirs(&mut fs, &path).unwrap();
+                } else if let Some(captures) = regexes.assert_file.captures(line) {
+                    let filename = captures.get(1).unwrap().as_str();
+                    let content = captures.get(2).unwrap().as_str();
+                    self.pragma_check_files.push((
+                        span,
+                        filename.to_owned(),
+                        content.as_bytes().to_owned(),
+                    ));
+                } else if let Some(captures) = regexes.env.captures(line) {
+                    let key = captures.get(1).unwrap().as_str();
+                    let value = captures.get(2).unwrap().as_str();
+                    self.io.set_env(key, value);
+                }
+            }
+        }
+    }
+
+    pub fn run_pragma_tests(&self) -> Result<(), werk_runner::EvalError> {
+        let fs = self.io.filesystem.lock();
+        for (span, filename, expected) in &self.pragma_check_files {
+            let out_file = self.output_path(filename.split('/'));
+            let (_entry, actual) = read_fs(&fs, &out_file).unwrap();
+            if actual != expected {
+                return Err(werk_runner::EvalError::AssertCustomFailed(
+                    *span,
+                    format!("contents of output file `{filename}` do not match\nexpected: {expected:?}\n  actual: {actual:?}"),
+                ));
+            }
+        }
+
         Ok(())
     }
 
-    pub fn create_workspace(
-        &self,
+    pub fn create_workspace<'b>(
+        &'b self,
         defines: &[(&str, &str)],
-    ) -> Result<werk_runner::Workspace<'_>, werk_runner::Error> {
+    ) -> Result<
+        werk_runner::Workspace<'b>,
+        DiagnosticError<'b, werk_runner::Error, &'b werk_parser::Document<'b>>,
+    > {
         let mut settings = WorkspaceSettings::new(self.output_dir.clone());
 
         // Normally this would be covered by `.gitignore`, but we don't have that,
@@ -210,7 +318,7 @@ impl<'a> Test<'a> {
             settings.define(*key, *value);
         }
 
-        werk_runner::Workspace::new(
+        werk_runner::Workspace::new_with_diagnostics(
             &self.ast,
             &*self.io,
             &*self.render,
@@ -1093,3 +1201,26 @@ pub fn program_path(program: &str) -> Absolute<std::path::PathBuf> {
             .unwrap()
     }
 }
+
+struct PragmaRegexes {
+    pub file: regex::Regex,
+    pub dir: regex::Regex,
+    pub assert_file: regex::Regex,
+    pub env: regex::Regex,
+}
+
+impl Default for PragmaRegexes {
+    fn default() -> Self {
+        Self {
+            file: regex::Regex::new(r"^#\!file (.*)=(.*)$").unwrap(),
+            dir: regex::Regex::new(r"^#\!dir (.*)$").unwrap(),
+            assert_file: regex::Regex::new(r"^#\!assert-file (.*)=(.*)$").unwrap(),
+            env: regex::Regex::new(r"^#\!env (.*)=(.*)$").unwrap(),
+        }
+    }
+}
+
+fn regexes() -> &'static PragmaRegexes {
+    static REGEXES: OnceLock<PragmaRegexes> = OnceLock::new();
+    REGEXES.get_or_init(PragmaRegexes::default)
+}