unused_finder: consume multi_err to return multiple resolution errors (…

…#113)

Consumes multi_err in unused_finder to report all resolution errors when
something goes wrong during resolution

change/@good-fences-api-83397144-820e-4f75-82b2-00d3fdc75b2e.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "unused_finder: consume multi_err to report resolution errors",
+  "packageName": "@good-fences/api",
+  "email": "mhuan13@gmail.com",
+  "dependentChangeType": "patch"
+}

crates/unused_finder/Cargo.toml

@@ -38,6 +38,7 @@ abspath = { version = "0.2.0", path = "../abspath" }
 itertools = "0.13.0"
 schemars.workspace = true
 logger = { version = "0.2.0", path = "../logger" }
+multi_err = { version = "0.2.0", path = "../multi_err" }
 
 [dev-dependencies]
 stringreader = "0.1.1"

crates/unused_finder/src/graph.rs

@@ -57,11 +57,11 @@ impl GraphFile {
                 for (reexported_from, symbol) in self.import_export_info.iter_exported_symbols() {
                     match (reexported_from, symbol) {
                         (_, ExportedSymbol::Default | ExportedSymbol::Named(_)) => {
-                            // mark as used
-                            tag_named_or_default_symbol(&mut self.symbol_tags, symbol, tag);
+                            let old_tag = self.symbol_tags.entry(symbol.clone()).or_default();
+                            *old_tag = old_tag.union(tag);
                         }
                         _ => {
-                            // TODO: somehow handle re-exports of namespaces
+                            // TODO: somehow handle re-exports of namespaces?
                         }
                     }
                 }

crates/unused_finder/src/parse/data.rs

@@ -1,7 +1,10 @@
-use std::path::{Path, PathBuf};
+use std::{
+    fmt::Debug,
+    path::{Path, PathBuf},
+};
 
 use ahashmap::{AHashMap, AHashSet, ARandomState};
-use anyhow::Result;
+use multi_err::{MultiErr, MultiResult};
 use swc_common::{FileName, Span};
 use swc_ecma_ast::ModuleExportName;
 use swc_ecma_loader::resolve::Resolve;
@@ -216,54 +219,71 @@ fn resolve_hashmap<T>(
     from_file: &FileName,
     resolver: impl Resolve,
     mut map: AHashMap<String, T>,
-) -> Result<AHashMap<PathBuf, T>, anyhow::Error> {
+) -> MultiResult<AHashMap<PathBuf, T>, anyhow::Error> {
     let mut accum = AHashMap::with_capacity_and_hasher(map.len(), ARandomState::new());
+    let mut errs: MultiErr<anyhow::Error> = MultiErr::new();
     for (import_specifier, imported_symbols) in map.drain() {
-        let resolved = resolver.resolve(from_file, &import_specifier)?;
+        let resolved = match resolver.resolve(from_file, &import_specifier) {
+            Ok(resolved) => resolved,
+            Err(e) => {
+                errs.add_single(e);
+                continue;
+            }
+        };
+
         match resolved.filename {
             FileName::Real(resolved_path) => {
                 accum.insert(resolved_path, imported_symbols);
             }
             _ => {
-                return Err(anyhow::anyhow!(
+                errs.add_single(anyhow::anyhow!(
                     "resolved to a non-file path?: {:?}",
                     resolved
                 ));
             }
         }
     }
-    Ok(accum)
+    errs.with_value(accum)
 }
 
 fn resolve_hashset(
     from_file: &FileName,
     resolver: impl Resolve,
     mut set: AHashSet<String>,
-) -> Result<AHashSet<PathBuf>, anyhow::Error> {
+) -> MultiResult<AHashSet<PathBuf>, anyhow::Error> {
     let mut accum = AHashSet::with_capacity_and_hasher(set.len(), ARandomState::new());
+    let mut errs = MultiErr::<anyhow::Error>::new();
     for import_specifier in set.drain() {
-        let resolved = resolver.resolve(from_file, &import_specifier)?;
-        let resolved_str = match resolved.filename {
-            FileName::Real(path) => path,
+        let resolved = match resolver.resolve(from_file, &import_specifier) {
+            Ok(resolved) => resolved,
+            Err(e) => {
+                errs.add_single(e);
+                continue;
+            }
+        };
+
+        match resolved.filename {
+            FileName::Real(path) => {
+                accum.insert(path);
+            }
             _ => {
-                return Err(anyhow::anyhow!(
+                errs.add_single(anyhow::anyhow!(
                     "resolved to a non-file path?: {:?}",
                     resolved
                 ));
             }
-        };
-        accum.insert(resolved_str);
+        }
     }
 
-    Ok(accum)
+    errs.with_value(accum)
 }
 
 impl RawImportExportInfo {
     pub fn try_resolve(
         self,
         from_file_path: &Path,
         resolver: impl Resolve,
-    ) -> Result<ResolvedImportExportInfo, anyhow::Error> {
+    ) -> MultiResult<ResolvedImportExportInfo, anyhow::Error> {
         let RawImportExportInfo {
             imported_path_ids,
             require_paths,
@@ -275,13 +295,26 @@ impl RawImportExportInfo {
 
         let from_file = FileName::Real(from_file_path.to_path_buf());
 
-        Ok(ResolvedImportExportInfo {
-            imported_symbols: resolve_hashmap(&from_file, &resolver, imported_path_ids)?,
-            require_paths: resolve_hashset(&from_file, &resolver, require_paths)?,
-            imported_paths: resolve_hashset(&from_file, &resolver, imported_paths)?,
-            export_from_symbols: resolve_hashmap(&from_file, &resolver, export_from_ids)?,
-            exported_ids,
-            executed_paths: resolve_hashset(&from_file, &resolver, executed_paths)?,
-        })
+        let mut errs = MultiErr::<anyhow::Error>::new();
+
+        let imported_symbols =
+            errs.extract(resolve_hashmap(&from_file, &resolver, imported_path_ids));
+        let require_paths = errs.extract(resolve_hashset(&from_file, &resolver, require_paths));
+        let imported_paths = errs.extract(resolve_hashset(&from_file, &resolver, imported_paths));
+        let export_from_symbols =
+            errs.extract(resolve_hashmap(&from_file, &resolver, export_from_ids));
+        let executed_paths = errs.extract(resolve_hashset(&from_file, &resolver, executed_paths));
+
+        MultiResult::with_errs(
+            ResolvedImportExportInfo {
+                imported_symbols,
+                require_paths,
+                imported_paths,
+                export_from_symbols,
+                exported_ids,
+                executed_paths,
+            },
+            errs,
+        )
     }
 }

crates/unused_finder/src/unused_finder.rs

@@ -71,26 +71,25 @@ impl SourceFiles {
             walk_result
                 .source_files
                 .into_par_iter()
-                .map(
-                    |walked_file| -> anyhow::Result<(PathBuf, ResolvedSourceFile)> {
-                        Ok((
-                            walked_file.source_file_path.clone(),
-                            ResolvedSourceFile {
-                                import_export_info: walked_file
-                                    .import_export_info
-                                    .try_resolve(&walked_file.source_file_path, &resolver)
-                                    .with_context(|| {
-                                        format!(
-                                            "trying to resolve imports for file {}",
-                                            walked_file.source_file_path.display()
-                                        )
-                                    })?,
-                                owning_package: walked_file.owning_package,
-                                source_file_path: walked_file.source_file_path,
-                            },
-                        ))
-                    },
-                )
+                .map(|walked_file| -> Result<(PathBuf, ResolvedSourceFile)> {
+                    Ok((
+                        walked_file.source_file_path.clone(),
+                        ResolvedSourceFile {
+                            import_export_info: walked_file
+                                .import_export_info
+                                .try_resolve(&walked_file.source_file_path, &resolver)
+                                .into_anyhow()
+                                .with_context(|| {
+                                    format!(
+                                        "trying to resolve imports for file {}",
+                                        walked_file.source_file_path.display()
+                                    )
+                                })?,
+                            owning_package: walked_file.owning_package,
+                            source_file_path: walked_file.source_file_path,
+                        },
+                    ))
+                })
                 .partition_map::<AHashMap<PathBuf, ResolvedSourceFile>, _, _, _, _>(|r| match r {
                     Ok(x) => Either::Left(x),
                     Err(e) => Either::Right(e),
@@ -290,6 +289,7 @@ impl UnusedFinder {
             source_file_path: file_path.to_path_buf(),
             import_export_info: import_export_info
                 .try_resolve(file_path, resolver)
+                .into_anyhow()
                 .map_err(JsErr::generic_failure)?,
         };