From 8d6b4cfd15f3131e4cea7889d64defea1076fdf4 Mon Sep 17 00:00:00 2001 From: Mark Stuart Date: Thu, 1 Oct 2026 00:35:39 +0000 Subject: [PATCH 1/4] fix: reconcile dependency graph placeholders --- src/graph/analyzer.rs | 78 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/src/graph/analyzer.rs b/src/graph/analyzer.rs index a395b6f..14e0775 100644 --- a/src/graph/analyzer.rs +++ b/src/graph/analyzer.rs @@ -40,6 +40,26 @@ fn normalize_path(path: &str) -> String { parts.join("/") } +fn source_placeholders(file_path: &str) -> Vec { + let path = Path::new(file_path); + let mut placeholders = Vec::new(); + + if matches!( + path.extension().and_then(|extension| extension.to_str()), + Some("ts" | "tsx" | "js" | "jsx" | "py" | "rs" | "go") + ) { + placeholders.push(path.with_extension("").to_string_lossy().into_owned()); + } + + if path.file_stem().and_then(|stem| stem.to_str()) == Some("index") { + if let Some(parent) = path.parent() { + placeholders.push(parent.to_string_lossy().into_owned()); + } + } + + placeholders +} + /// Dependency graph built from import analysis. pub struct DependencyGraph { graph: DiGraph, @@ -64,7 +84,7 @@ impl DependencyGraph { /// Add a file and its imports to the graph. pub fn add_file(&mut self, file_path: &str, content: &str) { - let source_idx = self.get_or_create_node(file_path); + let source_idx = self.get_or_create_source_node(file_path); let imports = self.parser.parse(file_path, content); for import in &imports { @@ -79,6 +99,28 @@ impl DependencyGraph { } } + /// Reuse an extensionless node that an earlier import created for this file. + /// + /// Imports such as `./router` are discovered before we necessarily walk + /// `router.ts`. Without reconciling that placeholder, the import edge and + /// the real file end up on separate nodes and graph results depend on walk + /// order. + fn get_or_create_source_node(&mut self, file_path: &str) -> NodeIndex { + if let Some(&idx) = self.node_map.get(file_path) { + return idx; + } + + for placeholder in source_placeholders(file_path) { + if let Some(idx) = self.node_map.remove(&placeholder) { + self.graph[idx] = file_path.to_string(); + self.node_map.insert(file_path.to_string(), idx); + return idx; + } + } + + self.get_or_create_node(file_path) + } + /// Try to find an existing node that matches the import path, /// accounting for missing file extensions (common in JS/TS/Python). fn find_matching_node(&self, resolved: &str) -> Option { @@ -303,6 +345,40 @@ mod tests { assert_eq!(dependents.len(), 2); } + #[test] + fn reconciles_extensionless_imports_when_target_is_added_later() { + let mut graph = DependencyGraph::new(); + + graph.add_file("src/main.ts", "import { Router } from './router';\n"); + graph.add_file("src/router.ts", "import { handler } from './handler';\n"); + graph.add_file("src/handler.ts", "export function handler() {}\n"); + + assert_eq!( + graph.depends_on("src/main.ts"), + vec!["src/router.ts".to_string()] + ); + assert_eq!( + graph.depended_on_by("src/router.ts"), + vec!["src/main.ts".to_string()] + ); + let transitive = graph.transitive_dependencies("src/main.ts"); + assert!(transitive.contains(&"src/router.ts".to_string())); + assert!(transitive.contains(&"src/handler.ts".to_string())); + } + + #[test] + fn reconciles_directory_imports_when_index_file_is_added_later() { + let mut graph = DependencyGraph::new(); + + graph.add_file("src/main.ts", "import { api } from './api';\n"); + graph.add_file("src/api/index.ts", "export const api = {};\n"); + + assert_eq!( + graph.depends_on("src/main.ts"), + vec!["src/api/index.ts".to_string()] + ); + } + #[test] fn test_transitive_dependencies() { let mut graph = DependencyGraph::new(); From 6ff0dad3a908d360469d291a9409f12b95995cfb Mon Sep 17 00:00:00 2001 From: Mark Stuart Date: Thu, 1 Oct 2026 01:34:37 +0000 Subject: [PATCH 2/4] fix: make graph reconciliation deterministic --- src/graph/analyzer.rs | 213 +++++++++++++++++++++++++++++++----------- 1 file changed, 157 insertions(+), 56 deletions(-) diff --git a/src/graph/analyzer.rs b/src/graph/analyzer.rs index 14e0775..a31e3ed 100644 --- a/src/graph/analyzer.rs +++ b/src/graph/analyzer.rs @@ -2,7 +2,7 @@ use petgraph::graph::{DiGraph, NodeIndex}; use petgraph::visit::Bfs; use petgraph::Direction; use serde::{Deserialize, Serialize}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::Path; use super::imports::ImportParser; @@ -40,30 +40,20 @@ fn normalize_path(path: &str) -> String { parts.join("/") } -fn source_placeholders(file_path: &str) -> Vec { - let path = Path::new(file_path); - let mut placeholders = Vec::new(); - - if matches!( - path.extension().and_then(|extension| extension.to_str()), - Some("ts" | "tsx" | "js" | "jsx" | "py" | "rs" | "go") - ) { - placeholders.push(path.with_extension("").to_string_lossy().into_owned()); - } - - if path.file_stem().and_then(|stem| stem.to_str()) == Some("index") { - if let Some(parent) = path.parent() { - placeholders.push(parent.to_string_lossy().into_owned()); - } - } - - placeholders +#[derive(Debug, Clone)] +struct ImportEdge { + source: NodeIndex, + target: NodeIndex, + relative_path: Option, } /// Dependency graph built from import analysis. pub struct DependencyGraph { graph: DiGraph, node_map: HashMap, + source_paths: HashSet, + relative_placeholders: HashMap, + import_edges: Vec, parser: ImportParser, } @@ -78,6 +68,9 @@ impl DependencyGraph { Self { graph: DiGraph::new(), node_map: HashMap::new(), + source_paths: HashSet::new(), + relative_placeholders: HashMap::new(), + import_edges: Vec::new(), parser: ImportParser::new(), } } @@ -85,65 +78,103 @@ impl DependencyGraph { /// Add a file and its imports to the graph. pub fn add_file(&mut self, file_path: &str, content: &str) { let source_idx = self.get_or_create_source_node(file_path); + self.rebind_relative_imports(); let imports = self.parser.parse(file_path, content); for import in &imports { let resolved = self.resolve_import(file_path, &import.imported_path, import.is_relative); - // Try to match against a known node (with common extensions) - let target_key = self.find_matching_node(&resolved).unwrap_or(resolved); - let target_idx = self.get_or_create_node(&target_key); + let (target_idx, relative_path) = if import.is_relative { + let target_idx = self + .find_matching_source(&resolved) + .unwrap_or_else(|| self.get_or_create_relative_placeholder(&resolved)); + (target_idx, Some(resolved)) + } else { + (self.get_or_create_node(&resolved), None) + }; if !self.graph.contains_edge(source_idx, target_idx) { self.graph.add_edge(source_idx, target_idx, ()); } + self.import_edges.push(ImportEdge { + source: source_idx, + target: target_idx, + relative_path, + }); } } - /// Reuse an extensionless node that an earlier import created for this file. - /// - /// Imports such as `./router` are discovered before we necessarily walk - /// `router.ts`. Without reconciling that placeholder, the import edge and - /// the real file end up on separate nodes and graph results depend on walk - /// order. fn get_or_create_source_node(&mut self, file_path: &str) -> NodeIndex { - if let Some(&idx) = self.node_map.get(file_path) { - return idx; - } - - for placeholder in source_placeholders(file_path) { - if let Some(idx) = self.node_map.remove(&placeholder) { - self.graph[idx] = file_path.to_string(); - self.node_map.insert(file_path.to_string(), idx); - return idx; - } - } - - self.get_or_create_node(file_path) + let idx = self.get_or_create_node(file_path); + self.source_paths.insert(file_path.to_string()); + idx } - /// Try to find an existing node that matches the import path, - /// accounting for missing file extensions (common in JS/TS/Python). - fn find_matching_node(&self, resolved: &str) -> Option { - if self.node_map.contains_key(resolved) { - return Some(resolved.to_string()); + /// Find the deterministic best source for an import path. Extension files + /// take precedence over directory indexes regardless of walk order. + fn find_matching_source(&self, resolved: &str) -> Option { + if self.source_paths.contains(resolved) { + return self.node_map.get(resolved).copied(); } - // Try common extensions for ext in &[".ts", ".tsx", ".js", ".jsx", ".py", ".rs", ".go"] { let with_ext = format!("{}{}", resolved, ext); - if self.node_map.contains_key(&with_ext) { - return Some(with_ext); + if self.source_paths.contains(&with_ext) { + return self.node_map.get(&with_ext).copied(); } } - // Try index files (JS/TS convention) for ext in &["/index.ts", "/index.js", "/index.tsx"] { let with_index = format!("{}{}", resolved, ext); - if self.node_map.contains_key(&with_index) { - return Some(with_index); + if self.source_paths.contains(&with_index) { + return self.node_map.get(&with_index).copied(); } } None } + fn get_or_create_relative_placeholder(&mut self, path: &str) -> NodeIndex { + if let Some(&idx) = self.relative_placeholders.get(path) { + return idx; + } + let idx = self.graph.add_node(path.to_string()); + self.relative_placeholders.insert(path.to_string(), idx); + idx + } + + /// Redirect unresolved relative imports whenever a newly discovered source + /// is a better match. Keeping import bindings separate from graph nodes also + /// prevents bare packages with the same name from being claimed as files. + fn rebind_relative_imports(&mut self) { + for edge_index in 0..self.import_edges.len() { + let Some(relative_path) = self.import_edges[edge_index].relative_path.clone() else { + continue; + }; + let Some(new_target) = self.find_matching_source(&relative_path) else { + continue; + }; + let source = self.import_edges[edge_index].source; + let old_target = self.import_edges[edge_index].target; + if new_target == old_target { + continue; + } + + self.import_edges[edge_index].target = new_target; + let old_edge_is_still_used = self + .import_edges + .iter() + .any(|edge| edge.source == source && edge.target == old_target); + if !old_edge_is_still_used { + if let Some(edge) = self.graph.find_edge(source, old_target) { + self.graph.remove_edge(edge); + } + } + if !self.graph.contains_edge(source, new_target) { + self.graph.add_edge(source, new_target, ()); + } + if self.relative_placeholders.get(&relative_path) == Some(&old_target) { + self.relative_placeholders.remove(&relative_path); + } + } + } + fn get_or_create_node(&mut self, path: &str) -> NodeIndex { if let Some(&idx) = self.node_map.get(path) { idx @@ -154,6 +185,13 @@ impl DependencyGraph { } } + fn lookup_node(&self, path: &str) -> Option { + self.node_map + .get(path) + .or_else(|| self.relative_placeholders.get(path)) + .copied() + } + fn resolve_import(&self, source_file: &str, import_path: &str, is_relative: bool) -> String { if !is_relative { return import_path.to_string(); @@ -199,7 +237,7 @@ impl DependencyGraph { /// What does this file depend on? (direct) pub fn depends_on(&self, file_path: &str) -> Vec { - let Some(&idx) = self.node_map.get(file_path) else { + let Some(idx) = self.lookup_node(file_path) else { return Vec::new(); }; self.graph @@ -210,7 +248,7 @@ impl DependencyGraph { /// What depends on this file? (direct) pub fn depended_on_by(&self, file_path: &str) -> Vec { - let Some(&idx) = self.node_map.get(file_path) else { + let Some(idx) = self.lookup_node(file_path) else { return Vec::new(); }; self.graph @@ -221,7 +259,7 @@ impl DependencyGraph { /// Find all transitively related code (BFS from file). pub fn transitive_dependencies(&self, file_path: &str) -> Vec { - let Some(&idx) = self.node_map.get(file_path) else { + let Some(idx) = self.lookup_node(file_path) else { return Vec::new(); }; let mut bfs = Bfs::new(&self.graph, idx); @@ -248,6 +286,7 @@ impl DependencyGraph { pub fn all_nodes(&self) -> Vec { self.node_map .iter() + .chain(self.relative_placeholders.iter()) .map(|(path, &idx)| { let ext = Path::new(path) .extension() @@ -281,7 +320,10 @@ impl DependencyGraph { /// Total nodes and edges. pub fn stats(&self) -> (usize, usize) { - (self.graph.node_count(), self.graph.edge_count()) + ( + self.node_map.len() + self.relative_placeholders.len(), + self.graph.edge_count(), + ) } /// Files with most incoming dependencies. @@ -289,6 +331,7 @@ impl DependencyGraph { let mut counts: Vec<(String, usize)> = self .node_map .iter() + .chain(self.relative_placeholders.iter()) .map(|(path, &idx)| { ( path.clone(), @@ -308,6 +351,7 @@ impl DependencyGraph { let mut counts: Vec<(String, usize)> = self .node_map .iter() + .chain(self.relative_placeholders.iter()) .map(|(path, &idx)| { ( path.clone(), @@ -379,6 +423,63 @@ mod tests { ); } + #[test] + fn reconciles_all_aliases_for_an_index_source() { + let mut graph = DependencyGraph::new(); + + graph.add_file("src/a.ts", "import { api } from './api';\n"); + graph.add_file("src/b.ts", "import { api } from './api/index';\n"); + graph.add_file("src/api/index.ts", "export const api = {};\n"); + + let mut dependents = graph.depended_on_by("src/api/index.ts"); + dependents.sort(); + assert_eq!( + dependents, + vec!["src/a.ts".to_string(), "src/b.ts".to_string()] + ); + } + + #[test] + fn extension_file_wins_over_index_regardless_of_walk_order() { + for sources in [ + ["src/api.ts", "src/api/index.ts"], + ["src/api/index.ts", "src/api.ts"], + ] { + let mut graph = DependencyGraph::new(); + graph.add_file("src/main.ts", "import { api } from './api';\n"); + for source in sources { + graph.add_file(source, "export const api = {};\n"); + } + + assert_eq!( + graph.depends_on("src/main.ts"), + vec!["src/api.ts".to_string()] + ); + } + } + + #[test] + fn bare_packages_are_not_reconciled_with_local_sources() { + let mut graph = DependencyGraph::new(); + + graph.add_file("src/main.ts", "import express from 'express';\n"); + graph.add_file("express.js", "export default {};\n"); + + assert_eq!(graph.depends_on("src/main.ts"), vec!["express".to_string()]); + assert!(graph.depended_on_by("express.js").is_empty()); + } + + #[test] + fn unsupported_index_files_do_not_claim_module_imports() { + let mut graph = DependencyGraph::new(); + + graph.add_file("src/main.ts", "import { api } from './api';\n"); + graph.add_file("src/api/index.json", "{}"); + + assert_eq!(graph.depends_on("src/main.ts"), vec!["src/api".to_string()]); + assert!(graph.depended_on_by("src/api/index.json").is_empty()); + } + #[test] fn test_transitive_dependencies() { let mut graph = DependencyGraph::new(); From 168c7a413c6ebf8cc4bd5cae3ba18d4983195d89 Mon Sep 17 00:00:00 2001 From: Mark Stuart Date: Thu, 1 Oct 2026 01:44:37 +0000 Subject: [PATCH 3/4] perf: index unresolved graph imports --- src/graph/analyzer.rs | 138 +++++++++++++++++++++++++++++++----------- 1 file changed, 102 insertions(+), 36 deletions(-) diff --git a/src/graph/analyzer.rs b/src/graph/analyzer.rs index a31e3ed..89d3165 100644 --- a/src/graph/analyzer.rs +++ b/src/graph/analyzer.rs @@ -40,20 +40,44 @@ fn normalize_path(path: &str) -> String { parts.join("/") } +fn source_aliases(file_path: &str) -> Vec { + let path = Path::new(file_path); + let extension = path.extension().and_then(|extension| extension.to_str()); + let mut aliases = vec![file_path.to_string()]; + + if matches!( + extension, + Some("ts" | "tsx" | "js" | "jsx" | "py" | "rs" | "go") + ) { + aliases.push(path.with_extension("").to_string_lossy().into_owned()); + } + if matches!(extension, Some("ts" | "tsx" | "js")) + && path.file_stem().and_then(|stem| stem.to_str()) == Some("index") + { + if let Some(parent) = path.parent() { + aliases.push(parent.to_string_lossy().into_owned()); + } + } + + aliases +} + #[derive(Debug, Clone)] struct ImportEdge { source: NodeIndex, target: NodeIndex, - relative_path: Option, } /// Dependency graph built from import analysis. pub struct DependencyGraph { graph: DiGraph, node_map: HashMap, + external_nodes: HashMap, source_paths: HashSet, relative_placeholders: HashMap, import_edges: Vec, + relative_imports: HashMap>, + edge_ref_counts: HashMap<(NodeIndex, NodeIndex), usize>, parser: ImportParser, } @@ -68,9 +92,12 @@ impl DependencyGraph { Self { graph: DiGraph::new(), node_map: HashMap::new(), + external_nodes: HashMap::new(), source_paths: HashSet::new(), relative_placeholders: HashMap::new(), import_edges: Vec::new(), + relative_imports: HashMap::new(), + edge_ref_counts: HashMap::new(), parser: ImportParser::new(), } } @@ -78,7 +105,7 @@ impl DependencyGraph { /// Add a file and its imports to the graph. pub fn add_file(&mut self, file_path: &str, content: &str) { let source_idx = self.get_or_create_source_node(file_path); - self.rebind_relative_imports(); + self.rebind_relative_imports(file_path); let imports = self.parser.parse(file_path, content); for import in &imports { @@ -90,21 +117,29 @@ impl DependencyGraph { .unwrap_or_else(|| self.get_or_create_relative_placeholder(&resolved)); (target_idx, Some(resolved)) } else { - (self.get_or_create_node(&resolved), None) + (self.get_or_create_external_node(&resolved), None) }; - if !self.graph.contains_edge(source_idx, target_idx) { - self.graph.add_edge(source_idx, target_idx, ()); + self.increment_edge(source_idx, target_idx); + let edge_index = self.import_edges.len(); + if let Some(path) = &relative_path { + self.relative_imports + .entry(path.clone()) + .or_default() + .push(edge_index); } self.import_edges.push(ImportEdge { source: source_idx, target: target_idx, - relative_path, }); } } fn get_or_create_source_node(&mut self, file_path: &str) -> NodeIndex { - let idx = self.get_or_create_node(file_path); + if let Some(&idx) = self.node_map.get(file_path) { + return idx; + } + let idx = self.graph.add_node(file_path.to_string()); + self.node_map.insert(file_path.to_string(), idx); self.source_paths.insert(file_path.to_string()); idx } @@ -142,53 +177,67 @@ impl DependencyGraph { /// Redirect unresolved relative imports whenever a newly discovered source /// is a better match. Keeping import bindings separate from graph nodes also /// prevents bare packages with the same name from being claimed as files. - fn rebind_relative_imports(&mut self) { - for edge_index in 0..self.import_edges.len() { - let Some(relative_path) = self.import_edges[edge_index].relative_path.clone() else { - continue; - }; + fn rebind_relative_imports(&mut self, file_path: &str) { + for relative_path in source_aliases(file_path) { let Some(new_target) = self.find_matching_source(&relative_path) else { continue; }; - let source = self.import_edges[edge_index].source; - let old_target = self.import_edges[edge_index].target; - if new_target == old_target { - continue; - } - - self.import_edges[edge_index].target = new_target; - let old_edge_is_still_used = self - .import_edges - .iter() - .any(|edge| edge.source == source && edge.target == old_target); - if !old_edge_is_still_used { - if let Some(edge) = self.graph.find_edge(source, old_target) { - self.graph.remove_edge(edge); + let edge_indices = self + .relative_imports + .get(&relative_path) + .cloned() + .unwrap_or_default(); + for edge_index in edge_indices { + let source = self.import_edges[edge_index].source; + let old_target = self.import_edges[edge_index].target; + if new_target == old_target { + continue; } + + self.decrement_edge(source, old_target); + self.increment_edge(source, new_target); + self.import_edges[edge_index].target = new_target; } - if !self.graph.contains_edge(source, new_target) { - self.graph.add_edge(source, new_target, ()); - } - if self.relative_placeholders.get(&relative_path) == Some(&old_target) { - self.relative_placeholders.remove(&relative_path); - } + self.relative_placeholders.remove(&relative_path); } } - fn get_or_create_node(&mut self, path: &str) -> NodeIndex { - if let Some(&idx) = self.node_map.get(path) { + fn get_or_create_external_node(&mut self, path: &str) -> NodeIndex { + if let Some(&idx) = self.external_nodes.get(path) { idx } else { let idx = self.graph.add_node(path.to_string()); - self.node_map.insert(path.to_string(), idx); + self.external_nodes.insert(path.to_string(), idx); idx } } + fn increment_edge(&mut self, source: NodeIndex, target: NodeIndex) { + let count = self.edge_ref_counts.entry((source, target)).or_default(); + if *count == 0 { + self.graph.add_edge(source, target, ()); + } + *count += 1; + } + + fn decrement_edge(&mut self, source: NodeIndex, target: NodeIndex) { + let Some(count) = self.edge_ref_counts.get_mut(&(source, target)) else { + return; + }; + *count -= 1; + if *count == 0 { + self.edge_ref_counts.remove(&(source, target)); + if let Some(edge) = self.graph.find_edge(source, target) { + self.graph.remove_edge(edge); + } + } + } + fn lookup_node(&self, path: &str) -> Option { self.node_map .get(path) .or_else(|| self.relative_placeholders.get(path)) + .or_else(|| self.external_nodes.get(path)) .copied() } @@ -286,6 +335,7 @@ impl DependencyGraph { pub fn all_nodes(&self) -> Vec { self.node_map .iter() + .chain(self.external_nodes.iter()) .chain(self.relative_placeholders.iter()) .map(|(path, &idx)| { let ext = Path::new(path) @@ -321,7 +371,7 @@ impl DependencyGraph { /// Total nodes and edges. pub fn stats(&self) -> (usize, usize) { ( - self.node_map.len() + self.relative_placeholders.len(), + self.node_map.len() + self.external_nodes.len() + self.relative_placeholders.len(), self.graph.edge_count(), ) } @@ -331,6 +381,7 @@ impl DependencyGraph { let mut counts: Vec<(String, usize)> = self .node_map .iter() + .chain(self.external_nodes.iter()) .chain(self.relative_placeholders.iter()) .map(|(path, &idx)| { ( @@ -351,6 +402,7 @@ impl DependencyGraph { let mut counts: Vec<(String, usize)> = self .node_map .iter() + .chain(self.external_nodes.iter()) .chain(self.relative_placeholders.iter()) .map(|(path, &idx)| { ( @@ -469,6 +521,20 @@ mod tests { assert!(graph.depended_on_by("express.js").is_empty()); } + #[test] + fn exact_bare_specifiers_are_separate_from_source_paths() { + let mut graph = DependencyGraph::new(); + + graph.add_file("src/main.ts", "import value from 'index.js';\n"); + graph.add_file("index.js", "export default {};\n"); + + assert!(graph.depended_on_by("index.js").is_empty()); + assert_eq!( + graph.depends_on("src/main.ts"), + vec!["index.js".to_string()] + ); + } + #[test] fn unsupported_index_files_do_not_claim_module_imports() { let mut graph = DependencyGraph::new(); From dfe5593d3918f73203f3202d3cb672d929ef5361 Mon Sep 17 00:00:00 2001 From: Mark Stuart Date: Thu, 1 Oct 2026 03:04:25 +0000 Subject: [PATCH 4/4] fix: resolve project-local Python imports --- src/graph/analyzer.rs | 110 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 90 insertions(+), 20 deletions(-) diff --git a/src/graph/analyzer.rs b/src/graph/analyzer.rs index 89d3165..c25e6c2 100644 --- a/src/graph/analyzer.rs +++ b/src/graph/analyzer.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; use std::collections::{HashMap, HashSet}; use std::path::Path; -use super::imports::ImportParser; +use super::imports::{ImportParser, ImportType}; /// Metadata about a node in the dependency graph. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -66,6 +66,7 @@ fn source_aliases(file_path: &str) -> Vec { struct ImportEdge { source: NodeIndex, target: NodeIndex, + source_candidates: Vec, } /// Dependency graph built from import analysis. @@ -76,7 +77,7 @@ pub struct DependencyGraph { source_paths: HashSet, relative_placeholders: HashMap, import_edges: Vec, - relative_imports: HashMap>, + resolvable_imports: HashMap>, edge_ref_counts: HashMap<(NodeIndex, NodeIndex), usize>, parser: ImportParser, } @@ -96,7 +97,7 @@ impl DependencyGraph { source_paths: HashSet::new(), relative_placeholders: HashMap::new(), import_edges: Vec::new(), - relative_imports: HashMap::new(), + resolvable_imports: HashMap::new(), edge_ref_counts: HashMap::new(), parser: ImportParser::new(), } @@ -105,31 +106,42 @@ impl DependencyGraph { /// Add a file and its imports to the graph. pub fn add_file(&mut self, file_path: &str, content: &str) { let source_idx = self.get_or_create_source_node(file_path); - self.rebind_relative_imports(file_path); + self.rebind_resolvable_imports(file_path); let imports = self.parser.parse(file_path, content); for import in &imports { let resolved = self.resolve_import(file_path, &import.imported_path, import.is_relative); - let (target_idx, relative_path) = if import.is_relative { - let target_idx = self - .find_matching_source(&resolved) - .unwrap_or_else(|| self.get_or_create_relative_placeholder(&resolved)); - (target_idx, Some(resolved)) + let source_candidates = if import.is_relative { + vec![resolved.clone()] + } else if matches!( + import.import_type, + ImportType::PythonImport | ImportType::PythonFrom + ) { + self.python_source_candidates(file_path, &import.imported_path) } else { - (self.get_or_create_external_node(&resolved), None) + Vec::new() + }; + let target_idx = if import.is_relative { + self.find_matching_source(&resolved) + .unwrap_or_else(|| self.get_or_create_relative_placeholder(&resolved)) + } else if let Some(target_idx) = self.find_best_source(&source_candidates) { + target_idx + } else { + self.get_or_create_external_node(&resolved) }; self.increment_edge(source_idx, target_idx); let edge_index = self.import_edges.len(); - if let Some(path) = &relative_path { - self.relative_imports - .entry(path.clone()) + for candidate in &source_candidates { + self.resolvable_imports + .entry(candidate.clone()) .or_default() .push(edge_index); } self.import_edges.push(ImportEdge { source: source_idx, target: target_idx, + source_candidates, }); } } @@ -165,6 +177,26 @@ impl DependencyGraph { None } + fn find_best_source(&self, candidates: &[String]) -> Option { + candidates + .iter() + .find_map(|candidate| self.find_matching_source(candidate)) + } + + fn python_source_candidates(&self, source_file: &str, import_path: &str) -> Vec { + let module_path = import_path.replace('.', "/"); + let source_dir = Path::new(source_file) + .parent() + .unwrap_or_else(|| Path::new("")); + let sibling = normalize_path(&source_dir.join(&module_path).to_string_lossy()); + + if sibling == module_path { + vec![module_path] + } else { + vec![sibling, module_path] + } + } + fn get_or_create_relative_placeholder(&mut self, path: &str) -> NodeIndex { if let Some(&idx) = self.relative_placeholders.get(path) { return idx; @@ -174,22 +206,24 @@ impl DependencyGraph { idx } - /// Redirect unresolved relative imports whenever a newly discovered source - /// is a better match. Keeping import bindings separate from graph nodes also + /// Redirect unresolved imports whenever a newly discovered source is a + /// better match. Keeping import bindings separate from graph nodes also /// prevents bare packages with the same name from being claimed as files. - fn rebind_relative_imports(&mut self, file_path: &str) { + fn rebind_resolvable_imports(&mut self, file_path: &str) { for relative_path in source_aliases(file_path) { - let Some(new_target) = self.find_matching_source(&relative_path) else { - continue; - }; let edge_indices = self - .relative_imports + .resolvable_imports .get(&relative_path) .cloned() .unwrap_or_default(); for edge_index in edge_indices { let source = self.import_edges[edge_index].source; let old_target = self.import_edges[edge_index].target; + let Some(new_target) = + self.find_best_source(&self.import_edges[edge_index].source_candidates) + else { + continue; + }; if new_target == old_target { continue; } @@ -546,6 +580,42 @@ mod tests { assert!(graph.depended_on_by("src/api/index.json").is_empty()); } + #[test] + fn reconciles_python_absolute_imports_with_sibling_modules() { + for sources in [ + ["src/main.py", "src/utils.py"], + ["src/utils.py", "src/main.py"], + ] { + let mut graph = DependencyGraph::new(); + for source in sources { + let content = if source == "src/main.py" { + "from utils import helper\n" + } else { + "def helper(): pass\n" + }; + graph.add_file(source, content); + } + + assert_eq!( + graph.depends_on("src/main.py"), + vec!["src/utils.py".to_string()] + ); + assert_eq!( + graph.depended_on_by("src/utils.py"), + vec!["src/main.py".to_string()] + ); + } + } + + #[test] + fn leaves_unresolved_python_imports_as_external_modules() { + let mut graph = DependencyGraph::new(); + + graph.add_file("src/main.py", "import os\n"); + + assert_eq!(graph.depends_on("src/main.py"), vec!["os".to_string()]); + } + #[test] fn test_transitive_dependencies() { let mut graph = DependencyGraph::new();