diff --git a/Cargo.lock b/Cargo.lock index 3f4e290a..daf7163e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2769,6 +2769,7 @@ dependencies = [ "itertools 0.13.0", "js_err", "logger", + "multi_err", "packagejson", "packagejson_exports", "path-clean", diff --git a/change/@good-fences-api-83397144-820e-4f75-82b2-00d3fdc75b2e.json b/change/@good-fences-api-83397144-820e-4f75-82b2-00d3fdc75b2e.json new file mode 100644 index 00000000..6d63c3e1 --- /dev/null +++ b/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" +} diff --git a/crates/unused_finder/Cargo.toml b/crates/unused_finder/Cargo.toml index fc029cbc..4af9a1b7 100644 --- a/crates/unused_finder/Cargo.toml +++ b/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" diff --git a/crates/unused_finder/src/graph.rs b/crates/unused_finder/src/graph.rs index dd2413ee..87dc6df0 100644 --- a/crates/unused_finder/src/graph.rs +++ b/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? } } } diff --git a/crates/unused_finder/src/parse/data.rs b/crates/unused_finder/src/parse/data.rs index 72d4ab82..c93d3043 100644 --- a/crates/unused_finder/src/parse/data.rs +++ b/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,46 +219,63 @@ fn resolve_hashmap( from_file: &FileName, resolver: impl Resolve, mut map: AHashMap, -) -> Result, anyhow::Error> { +) -> MultiResult, anyhow::Error> { let mut accum = AHashMap::with_capacity_and_hasher(map.len(), ARandomState::new()); + let mut errs: MultiErr = 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, -) -> Result, anyhow::Error> { +) -> MultiResult, anyhow::Error> { let mut accum = AHashSet::with_capacity_and_hasher(set.len(), ARandomState::new()); + let mut errs = MultiErr::::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 { @@ -263,7 +283,7 @@ impl RawImportExportInfo { self, from_file_path: &Path, resolver: impl Resolve, - ) -> Result { + ) -> MultiResult { 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::::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, + ) } } diff --git a/crates/unused_finder/src/unused_finder.rs b/crates/unused_finder/src/unused_finder.rs index 9bf8e785..f4921a9c 100644 --- a/crates/unused_finder/src/unused_finder.rs +++ b/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::, _, _, _, _>(|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)?, };