diff --git a/crates/core/src/analyze/unused_exports.rs b/crates/core/src/analyze/unused_exports.rs index 6cdfb7dd8..f1fbfb96a 100644 --- a/crates/core/src/analyze/unused_exports.rs +++ b/crates/core/src/analyze/unused_exports.rs @@ -243,15 +243,6 @@ pub fn find_unused_exports( None }; - // Precompute reachable FileIds so we can distinguish meaningful references - // (from reachable modules) from unreachable-to-unreachable references. - let reachable_files: FxHashSet = graph - .modules - .iter() - .filter(|m| m.is_reachable()) - .map(|m| m.file_id.0) - .collect(); - let module_results: Vec<(Vec, Vec, Vec)> = graph .modules .par_iter() @@ -311,10 +302,15 @@ pub fn find_unused_exports( let has_cross_file_ref = if module.is_reachable() { !export.references.is_empty() } else { - export - .references - .iter() - .any(|r| reachable_files.contains(&r.from_file.0)) + export.references.iter().any(|r| { + graph.modules.get(r.from_file.0 as usize).is_some_and(|m| { + debug_assert_eq!( + m.file_id, r.from_file, + "ModuleGraph::modules FileId-as-index invariant broken" + ); + m.is_reachable() + }) + }) }; // Treat side-effect-registered exports (Lit @customElement, // customElements.define) as referenced even when no other file @@ -595,12 +591,6 @@ pub fn find_private_type_leaks( suppressions: &SuppressionContext<'_>, line_offsets_by_file: &LineOffsetsMap<'_>, ) -> Vec { - let module_by_id: FxHashMap = graph - .modules - .iter() - .map(|module| (module.file_id, module)) - .collect(); - let mut leaks = Vec::new(); for module_info in modules { if module_info.public_signature_type_references.is_empty() @@ -608,9 +598,13 @@ pub fn find_private_type_leaks( { continue; } - let Some(module) = module_by_id.get(&module_info.file_id) else { + let Some(module) = graph.modules.get(module_info.file_id.0 as usize) else { continue; }; + debug_assert_eq!( + module.file_id, module_info.file_id, + "ModuleGraph::modules FileId-as-index invariant broken" + ); if is_storybook_file(&module.path) || is_route_convention_file(&module.path, &config.root) { continue; } @@ -665,17 +659,21 @@ pub fn find_private_type_leaks( fn collect_dynamic_reexport_sources( resolved_modules: &[crate::resolve::ResolvedModule], graph: &ModuleGraph, - module_idx_by_file_id: &FxHashMap, re_export_sources: &mut FxHashMap>, ) { use crate::extract::ExportName; use fallow_types::extract::ImportedName; for resolved in resolved_modules { - let Some(&wrapper_idx) = module_idx_by_file_id.get(&resolved.file_id) else { + let wrapper_idx = resolved.file_id.0 as usize; + let Some(wrapper) = graph.modules.get(wrapper_idx) else { continue; }; - let wrapper_exports = &graph.modules[wrapper_idx].exports; + debug_assert_eq!( + wrapper.file_id, resolved.file_id, + "ModuleGraph::modules FileId-as-index invariant broken" + ); + let wrapper_exports = &wrapper.exports; for dynamic_import in &resolved.resolved_dynamic_imports { let crate::resolve::ResolveResult::InternalModule(source_file_id) = @@ -698,9 +696,10 @@ fn collect_dynamic_reexport_sources( continue; } - let Some(&source_idx) = module_idx_by_file_id.get(source_file_id) else { + let source_idx = source_file_id.0 as usize; + if source_idx >= graph.modules.len() { continue; - }; + } re_export_sources .entry(wrapper_idx) .or_default() @@ -726,17 +725,16 @@ pub fn find_duplicate_exports( resolved_modules: &[crate::resolve::ResolvedModule], ) -> Vec { let ignore_matchers = config.compiled_ignore_exports.as_slice(); - // Build a map from FileId to module index for dynamic re-export source lookup. - let module_idx_by_file_id: FxHashMap = graph - .modules - .iter() - .enumerate() - .map(|(idx, m)| (m.file_id, idx)) - .collect(); - // Build a set of re-export relationships: (re-exporting module idx) -> set of (source module idx) + // Build a set of re-export relationships: (re-exporting module idx) -> set of (source module idx). + // Module idx and `file_id.0 as usize` are interchangeable; see the + // invariant doc on `ModuleGraph::modules`. let mut re_export_sources: FxHashMap> = FxHashMap::default(); for (idx, module) in graph.modules.iter().enumerate() { + debug_assert_eq!( + module.file_id.0 as usize, idx, + "ModuleGraph::modules FileId-as-index invariant broken" + ); for re in &module.re_exports { re_export_sources .entry(idx) @@ -753,12 +751,7 @@ pub fn find_duplicate_exports( // import targeting `InternalModule(S)` AND A also exports the name being // imported. The export-side check guards against false negatives where a // module dynamically imports something but does not actually re-export it. - collect_dynamic_reexport_sources( - resolved_modules, - graph, - &module_idx_by_file_id, - &mut re_export_sources, - ); + collect_dynamic_reexport_sources(resolved_modules, graph, &mut re_export_sources); struct ExportEntry { module_idx: usize, @@ -769,11 +762,6 @@ pub fn find_duplicate_exports( } let mut export_locations: FxHashMap> = FxHashMap::default(); - let file_id_by_path: FxHashMap<&std::path::Path, FileId> = graph - .modules - .iter() - .map(|module| (module.path.as_path(), module.file_id)) - .collect(); for (idx, module) in graph.modules.iter().enumerate() { if !module.is_reachable() || module.is_entry_point() { @@ -857,24 +845,28 @@ pub fn find_duplicate_exports( // For each pair (A, B), if A re-exports from B or B re-exports from A, // they are part of the same export chain, not true duplicates. let module_indices: FxHashSet = locations.iter().map(|e| e.module_idx).collect(); - let independent: Vec = locations - .into_iter() - .filter(|e| { - let sources = re_export_sources.get(&e.module_idx); - let has_source_in_set = - sources.is_some_and(|s| s.iter().any(|src| module_indices.contains(src))); - !has_source_in_set - }) - .map(|e| { - let (line, col) = - byte_offset_to_line_col(line_offsets_by_file, e.file_id, e.span_start); - DuplicateLocation { - path: e.path, - line, - col, - } - }) - .collect(); + let (independent_file_ids, independent): (Vec, Vec) = + locations + .into_iter() + .filter(|e| { + let sources = re_export_sources.get(&e.module_idx); + let has_source_in_set = sources + .is_some_and(|s| s.iter().any(|src| module_indices.contains(src))); + !has_source_in_set + }) + .map(|e| { + let (line, col) = + byte_offset_to_line_col(line_offsets_by_file, e.file_id, e.span_start); + ( + e.file_id, + DuplicateLocation { + path: e.path, + line, + col, + }, + ) + }) + .unzip(); if independent.len() <= 1 { return None; @@ -885,7 +877,7 @@ pub fn find_duplicate_exports( // modules in different directories) that happen to export the same name // are not actionable duplicates since they can never be confused at an // import site. - let has_shared_importer = has_common_importer(&independent, graph, &file_id_by_path); + let has_shared_importer = has_common_importer(&independent_file_ids, graph); if has_shared_importer { Some(DuplicateExport { export_name: name, @@ -904,21 +896,7 @@ pub fn find_duplicate_exports( /// from both. This filters out false positives from unrelated leaf modules (e.g., /// SvelteKit route files in different directories) that coincidentally export the /// same name but are never imported together. -fn has_common_importer( - locations: &[DuplicateLocation], - graph: &ModuleGraph, - file_id_by_path: &FxHashMap<&std::path::Path, FileId>, -) -> bool { - if locations.len() <= 1 { - return false; - } - - // Collect FileIds for the duplicate locations through a pre-built path index. - let file_ids: Vec = locations - .iter() - .filter_map(|loc| file_id_by_path.get(loc.path.as_path()).copied()) - .collect(); - +fn has_common_importer(file_ids: &[FileId], graph: &ModuleGraph) -> bool { if file_ids.len() <= 1 { return false; } @@ -926,7 +904,7 @@ fn has_common_importer( let duplicate_files: FxHashSet = file_ids.iter().copied().collect(); let mut importer_owner: FxHashMap = FxHashMap::default(); - for file_id in file_ids { + for &file_id in file_ids { let idx = file_id.0 as usize; if idx >= graph.reverse_deps.len() { continue; diff --git a/crates/graph/src/graph/mod.rs b/crates/graph/src/graph/mod.rs index 41c94ae9a..bcd9fa6d9 100644 --- a/crates/graph/src/graph/mod.rs +++ b/crates/graph/src/graph/mod.rs @@ -28,6 +28,14 @@ pub use types::{ExportSymbol, ModuleNode, ReExportEdge, ReferenceKind, SymbolRef #[derive(Debug)] pub struct ModuleGraph { /// All modules indexed by `FileId`. + /// + /// Invariant: `modules[file_id.0 as usize].file_id == file_id` for every + /// `FileId` in the graph. Holds because `discover/walk.rs` assigns FileIds + /// sequentially via `.enumerate()` after path-sorting, and + /// `build::populate_edges` pushes one `ModuleNode` per file in iteration + /// order. Detectors rely on this for O(1) FileId-to-module lookup + /// (`graph.modules.get(file_id.0 as usize)`) instead of building a + /// per-call `FxHashMap`. pub modules: Vec, /// Flat edge storage for cache-friendly iteration. edges: Vec,