From 77b8da9d1f9d20372b4ac1d8a25b594542e7fd05 Mon Sep 17 00:00:00 2001
From: Joel Arvidsson <joel@oblador.se>
Date: Sun, 20 Oct 2024 21:34:41 +0200
Subject: [PATCH] Use relative paths and make import errors fancy

---
 src/affected.rs | 19 +++++++++++++-----
 src/imports.rs  | 51 ++++++++++++++++++++++++++++++++-----------------
 2 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/src/affected.rs b/src/affected.rs
index d923c8d..be71437 100644
--- a/src/affected.rs
+++ b/src/affected.rs
@@ -72,11 +72,11 @@ pub fn collect_affected(
             continue;
         };
         let Ok(source_text) = fs::read_to_string(&absolute_path) else {
-            errors.push(format!("Cannot read file: {}", absolute_path.display()));
+            errors.push(format!("Cannot read file: {absolute_path:?}",));
             continue;
         };
         let result =
-            imports::collect_imports(source_type, source_text.as_str(), absolute_path.to_str());
+            imports::collect_imports(source_type, source_text.as_str(), Some(&absolute_path));
         errors.extend(result.errors);
         if let Some(parent_path) = absolute_path.parent() {
             for import_path in result.imports_paths.iter() {
@@ -84,7 +84,12 @@ pub fn collect_affected(
                     Err(e) => match e {
                         ResolveError::Builtin(_) => {} // Skip builtins
                         _ => {
-                            errors.push(e.to_string());
+                            let relative_path = absolute_path
+                                .strip_prefix(&current_dir)
+                                .unwrap()
+                                .to_str()
+                                .unwrap_or("unknown file");
+                            errors.push(format!("[{relative_path}]\n{e}"));
                         }
                     },
                     Ok(resolution) => {
@@ -302,8 +307,12 @@ mod tests {
 
     #[test]
     fn test_bad_import() {
-        let ret = collect_affected(vec!["fixtures/bad-import.js"], vec![], Resolver::default());
-        assert_eq!(ret.errors, vec!["Cannot find module 'bad-import'"]);
+        let file_name = "fixtures/bad-import.js";
+        let ret = collect_affected(vec![file_name], vec![], Resolver::default());
+        assert_eq!(
+            ret.errors,
+            vec![format!("[{file_name}]\nCannot find module 'bad-import'")]
+        );
     }
 
     #[test]
diff --git a/src/imports.rs b/src/imports.rs
index a153265..c8eaf36 100644
--- a/src/imports.rs
+++ b/src/imports.rs
@@ -1,9 +1,9 @@
-use oxc::diagnostics::{Error, NamedSource};
+use oxc::diagnostics::{Error, NamedSource, OxcDiagnostic};
 use oxc_allocator::Allocator;
 use oxc_ast::{visit::walk, Visit};
 use oxc_parser::Parser;
 use oxc_span::SourceType;
-use std::{collections::HashSet, sync::Arc};
+use std::{collections::HashSet, env, path::PathBuf, sync::Arc};
 
 pub struct ImportsReturn {
     pub errors: Vec<String>,
@@ -13,7 +13,7 @@ pub struct ImportsReturn {
 pub fn collect_imports(
     source_type: SourceType,
     source_text: &str,
-    source_filename: Option<&str>,
+    source_filename: Option<&PathBuf>,
 ) -> ImportsReturn {
     let allocator = Allocator::default();
     let parsed = Parser::new(&allocator, &source_text, source_type).parse();
@@ -23,13 +23,22 @@ pub fn collect_imports(
     let mut ast_pass = CollectImports::default();
     ast_pass.visit_program(&program);
 
-    let parse_errors = if parsed.errors.is_empty() {
+    let errors = if parsed.errors.is_empty() && ast_pass.errors.is_empty() {
         vec![]
     } else {
-        let file_name = source_filename.clone().unwrap_or_default();
+        let file_name = if source_filename.is_none() {
+            "unknown file"
+        } else {
+            source_filename
+                .unwrap()
+                .strip_prefix(env::current_dir().unwrap())
+                .unwrap()
+                .to_str()
+                .unwrap_or_default()
+        };
         let source = Arc::new(NamedSource::new(file_name, source_text.to_string()));
-        parsed
-            .errors
+        [parsed.errors, ast_pass.errors]
+            .concat()
             .into_iter()
             .map(|diagnostic| Error::from(diagnostic).with_source_code(Arc::clone(&source)))
             .map(|error| format!("{error:?}"))
@@ -37,7 +46,7 @@ pub fn collect_imports(
     };
 
     let ret = ImportsReturn {
-        errors: [parse_errors, ast_pass.errors].concat(),
+        errors,
         imports_paths: ast_pass.import_paths.into_iter().collect(),
     };
     ret
@@ -45,7 +54,7 @@ pub fn collect_imports(
 
 #[derive(Debug, Default)]
 struct CollectImports {
-    errors: Vec<String>,
+    errors: Vec<OxcDiagnostic>,
     import_paths: HashSet<String>,
 }
 
@@ -66,13 +75,17 @@ impl<'a> Visit<'a> for CollectImports {
                         self.import_paths.insert(first.value.raw.to_string());
                     }
                 } else {
-                    self.errors
-                        .push("Import call must not have dynamic template literals".to_string());
+                    self.errors.push(
+                        OxcDiagnostic::error("Import call must not have dynamic template literals")
+                            .with_label(it.span),
+                    );
                 }
             }
             _ => {
-                self.errors
-                    .push("Import call must have a string literal argument".to_string());
+                self.errors.push(
+                    OxcDiagnostic::error("Import call must not have dynamic template literals")
+                        .with_label(it.span),
+                );
             }
         }
         walk::walk_import_expression(self, it);
@@ -97,13 +110,17 @@ impl<'a> Visit<'a> for CollectImports {
                     self.import_paths.insert(literal.value.to_string());
                 }
                 None => {
-                    self.errors
-                        .push("Require call must have a string literal argument".to_string());
+                    self.errors.push(
+                        OxcDiagnostic::error("Require call must have a string literal argument")
+                            .with_label(it.span),
+                    );
                 }
             }
         } else if it.callee_name().is_some_and(|n| n == "require") {
-            self.errors
-                .push("Require call must have a string literal argument".to_string());
+            self.errors.push(
+                OxcDiagnostic::error("Require call must have a string literal argument")
+                    .with_label(it.span),
+            );
         }
         walk::walk_call_expression(self, it);
     }