Skip to content

Commit 782fefe

Browse files
authored
fix(native): recover renamed import names in the FFI hybrid import-edge path
fix(native): recover renamed import names in the FFI hybrid import-edge path Fixes a silent edge-miss in the hybrid FFI import path (buildImportEdgesNative -> build_import_edges) where renamed import specifiers (import { X as Y }) caused symbol and barrel-through lookups to search for the local alias Y in the target file instead of the name actually declared there (X), silently dropping the edge. Generalizes Rust's import_name_pairs to a shared ImportNameSource trait implemented by both the native orchestrator's Import type and the FFI-facing ImportInfo wire struct, and threads the renamedImports field through the TS-to-Rust FFI payload construction. Greptile gave a clean review with zero actionable comments. While resolving conflicts on this PR's 3 owned files, caught a genuine silent-duplicate-declaration risk in build-edges.ts: a stale local importNamePairs function (predating PR #1997's extraction of that same function to a shared import-utils.ts module) would have collided with the already-imported shared function, a TypeScript compile error not flagged by git's conflict markers since the two definitions occupied non-overlapping line ranges. The Pre-publish benchmark gate failed three times in a row (the routine flaky "1-file rebuild" timing gate seen on prior PRs). Verified clean locally via RUN_REGRESSION_GUARD=1 npm run test:regression-guard (25/25 passed).
1 parent d90be6f commit 782fefe

3 files changed

Lines changed: 204 additions & 39 deletions

File tree

crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs

Lines changed: 156 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::collections::{HashMap, HashSet};
33
use napi_derive::napi;
44

55
use crate::domain::graph::builder::barrel_resolution::{self, BarrelContext, ReexportRef};
6+
use crate::domain::graph::builder::stages::import_edges::{import_name_pairs, ImportNameSource};
67
use crate::domain::graph::resolve;
78
use crate::types::{
89
ArrayCallbackBinding, ArrayElemBinding, FnRefBinding, ForOfBinding, ObjectPropBinding,
@@ -1536,6 +1537,28 @@ pub struct ImportInfo {
15361537
/// by `type_only`, #1813).
15371538
#[napi(js_name = "typeOnlyNames")]
15381539
pub type_only_names: Vec<String>,
1540+
/// `{ local, imported }` pairs for `import { X as Y }` specifiers —
1541+
/// mirrors `Import.renamedImports` (#1730). Without this, symbol-level
1542+
/// lookups in `emit_named_symbol_edges`/`emit_barrel_through_edges` would
1543+
/// search the target file for the local (post-rename) name instead of
1544+
/// the name actually declared there, silently failing to find it (#1847).
1545+
#[napi(js_name = "renamedImports")]
1546+
pub renamed_imports: Vec<RenamedImport>,
1547+
}
1548+
1549+
impl ImportNameSource for ImportInfo {
1550+
fn names(&self) -> &[String] {
1551+
&self.names
1552+
}
1553+
fn renamed_imports(&self) -> &[RenamedImport] {
1554+
&self.renamed_imports
1555+
}
1556+
fn is_type_only(&self) -> bool {
1557+
self.type_only
1558+
}
1559+
fn type_only_names(&self) -> &[String] {
1560+
&self.type_only_names
1561+
}
15391562
}
15401563

15411564
#[napi(object)]
@@ -1721,17 +1744,6 @@ pub fn build_import_edges(
17211744

17221745
// ── build_import_edges helpers ──────────────────────────────────────────
17231746

1724-
/// Strip a `"* as "` / `"*\tas "` prefix from an import name so the bare
1725-
/// symbol can be looked up against the target's exports. JS equivalent:
1726-
/// `name.replace(/^\*\s+as\s+/, '')`.
1727-
fn strip_star_as_prefix(name: &str) -> &str {
1728-
if name.starts_with("* as ") || name.starts_with("*\tas ") {
1729-
&name[5..]
1730-
} else {
1731-
name
1732-
}
1733-
}
1734-
17351747
/// Classify an import into its edge kind: reexports / imports-type /
17361748
/// dynamic-imports / imports. Mirrors the JS classifier in `build-edges.ts`.
17371749
fn classify_import_edge_kind(imp: &ImportInfo) -> &'static str {
@@ -1780,17 +1792,20 @@ fn is_wildcard_reexport(imp: &ImportInfo) -> bool {
17801792
/// `type` keyword) is the only consumption signal `codegraph exports` can
17811793
/// observe for them (#1833).
17821794
///
1783-
/// `imp.names` holds the *original* declaration name for export specifiers
1784-
/// (see `extractImportNames` in the JS extractor) even when renamed
1785-
/// externally, so this naturally resolves `export { X as Z }` against `X`'s
1786-
/// own node — the emitted edge (and downstream `reexportedSymbols` entry)
1787-
/// is reported under the symbol's own declared name, not the barrel alias.
1795+
/// Looks up each specifier's *original* declared name via `import_name_pairs`
1796+
/// rather than the local (possibly renamed) binding — for `export { X as Z }`
1797+
/// this is already `X` (`imp.names` holds the original for export
1798+
/// specifiers), but for a renamed value/type import (`import type { X as Y }`)
1799+
/// the original name only exists in `imp.renamed_imports`; searching under the
1800+
/// local alias `Y` would never find a match in the target file (#1847). The
1801+
/// emitted edge (and downstream `reexportedSymbols` entry) is reported under
1802+
/// the symbol's own declared name in both cases, not the local/barrel alias.
17881803
///
17891804
/// When `resolved_path` is itself a barrel that renamed the requested name
17901805
/// further down its own reexport chain (`export { X as Y } from …`),
17911806
/// `resolve_barrel_export` reports the name actually declared in the
1792-
/// resolved file — which may differ from `clean_name` — so the lookup below
1793-
/// must use that reported name, not `clean_name`, against the barrel target
1807+
/// resolved file — which may differ from `original` — so the lookup below
1808+
/// must use that reported name, not `original`, against the barrel target
17941809
/// (#1823).
17951810
fn emit_named_symbol_edges(
17961811
edges: &mut Vec<ComputedEdge>,
@@ -1803,12 +1818,10 @@ fn emit_named_symbol_edges(
18031818
if ctx.symbol_node_map.is_empty() {
18041819
return;
18051820
}
1806-
for name in &imp.names {
1807-
let clean_name = strip_star_as_prefix(name);
1808-
let type_only = imp.type_only || imp.type_only_names.iter().any(|n| n == clean_name);
1821+
for (_local, original, type_only) in import_name_pairs(imp) {
18091822
let barrel_target = if ctx.barrel_set.contains(resolved_path) {
18101823
let mut visited = HashSet::new();
1811-
barrel_resolution::resolve_barrel_export(ctx, resolved_path, clean_name, &mut visited)
1824+
barrel_resolution::resolve_barrel_export(ctx, resolved_path, &original, &mut visited)
18121825
} else {
18131826
None
18141827
};
@@ -1820,7 +1833,7 @@ fn emit_named_symbol_edges(
18201833
{
18211834
(resolved.name.clone(), resolved.file.clone())
18221835
}
1823-
_ => (clean_name.to_string(), resolved_path.to_string()),
1836+
_ => (original, resolved_path.to_string()),
18241837
};
18251838
let Some((id, sym_kind)) = ctx.symbol_node_map.get(&(target_name, target_file.clone()))
18261839
else {
@@ -1864,13 +1877,12 @@ fn emit_barrel_through_edges(
18641877
_ => "imports",
18651878
};
18661879
let mut resolved_sources: HashSet<String> = HashSet::new();
1867-
for name in &imp.names {
1868-
let clean_name = strip_star_as_prefix(name);
1880+
for (_local, original, _type_only) in import_name_pairs(imp) {
18691881
let mut visited = HashSet::new();
18701882
let actual = barrel_resolution::resolve_barrel_export(
18711883
ctx,
18721884
resolved_path,
1873-
clean_name,
1885+
&original,
18741886
&mut visited,
18751887
);
18761888
let actual_source = match actual {
@@ -1968,6 +1980,7 @@ mod import_edge_tests {
19681980
dynamic_import: dynamic,
19691981
wildcard_reexport: false,
19701982
type_only_names: vec![],
1983+
renamed_imports: vec![],
19711984
}
19721985
}
19731986

@@ -1986,6 +1999,35 @@ mod import_edge_tests {
19861999
dynamic_import: false,
19872000
wildcard_reexport: false,
19882001
type_only_names: type_only_names.into_iter().map(|s| s.to_string()).collect(),
2002+
renamed_imports: vec![],
2003+
}
2004+
}
2005+
2006+
/// A renamed import (`import { X as Y } from 'src'`, optionally `import
2007+
/// type`) — `names` carries the local (post-rename) binding `Y`, and
2008+
/// `renamed_imports` maps it back to the original declared name `X`
2009+
/// (#1730, #1847).
2010+
fn make_import_with_renames(
2011+
source: &str,
2012+
names: Vec<&str>,
2013+
renames: Vec<(&str, &str)>,
2014+
type_only: bool,
2015+
) -> ImportInfo {
2016+
ImportInfo {
2017+
source: source.to_string(),
2018+
names: names.into_iter().map(|s| s.to_string()).collect(),
2019+
reexport: false,
2020+
type_only,
2021+
dynamic_import: false,
2022+
wildcard_reexport: false,
2023+
type_only_names: vec![],
2024+
renamed_imports: renames
2025+
.into_iter()
2026+
.map(|(local, imported)| RenamedImport {
2027+
local: local.to_string(),
2028+
imported: imported.to_string(),
2029+
})
2030+
.collect(),
19892031
}
19902032
}
19912033

@@ -2082,6 +2124,7 @@ mod import_edge_tests {
20822124
dynamic_import: false,
20832125
wildcard_reexport: true,
20842126
type_only_names: vec![],
2127+
renamed_imports: vec![],
20852128
},
20862129
], vec![])];
20872130
let resolved = vec![make_resolved("/root/src/index.ts", "./utils", "src/utils.ts")];
@@ -2128,6 +2171,7 @@ mod import_edge_tests {
21282171
dynamic_import: false,
21292172
wildcard_reexport: true,
21302173
type_only_names: vec![],
2174+
renamed_imports: vec![],
21312175
},
21322176
], vec![])];
21332177
let resolved = vec![make_resolved("/root/src/index.ts", "./utils", "src/utils.ts")];
@@ -2215,6 +2259,92 @@ mod import_edge_tests {
22152259
assert_eq!(edges[0].kind, "imports-type");
22162260
}
22172261

2262+
#[test]
2263+
fn renamed_type_import_resolves_original_name() {
2264+
// `import type { Config as CfgType } from './types'` — `names` holds
2265+
// the local alias "CfgType", but `Config` is the name actually
2266+
// declared in types.ts. The symbol-level `imports-type` edge must
2267+
// resolve against Config's own node, not fail to find "CfgType"
2268+
// there (#1847).
2269+
let files = vec![make_file("src/app.ts", 1, vec![
2270+
make_import_with_renames("./types", vec!["CfgType"], vec![("CfgType", "Config")], true),
2271+
], vec![])];
2272+
let resolved = vec![make_resolved("/root/src/app.ts", "./types", "src/types.ts")];
2273+
let node_ids = vec![make_node_entry("src/app.ts", 1), make_node_entry("src/types.ts", 2)];
2274+
let symbol_nodes = vec![SymbolNodeEntry {
2275+
name: "Config".to_string(),
2276+
file: "src/types.ts".to_string(),
2277+
node_id: 77,
2278+
kind: "interface".to_string(),
2279+
}];
2280+
2281+
let edges = build_import_edges(
2282+
files,
2283+
resolved,
2284+
vec![],
2285+
node_ids,
2286+
vec![],
2287+
"/root".to_string(),
2288+
Some(symbol_nodes),
2289+
);
2290+
assert_eq!(edges.len(), 2);
2291+
assert_eq!(edges[0].kind, "imports-type");
2292+
assert_eq!(edges[0].target_id, 2);
2293+
assert_eq!(edges[1].kind, "imports-type");
2294+
assert_eq!(edges[1].target_id, 77);
2295+
}
2296+
2297+
#[test]
2298+
fn renamed_import_through_barrel_resolves_original_name() {
2299+
// `import { Config as CfgType } from './barrel'` where './barrel'
2300+
// does `export { Config } from './types'`. Barrel-through resolution
2301+
// must look up "Config" (the original name) in the barrel's own
2302+
// export map, not the local alias "CfgType", which only exists in
2303+
// app.ts (#1847).
2304+
let files = vec![
2305+
make_file("src/app.ts", 1, vec![
2306+
make_import_with_renames("./barrel", vec!["CfgType"], vec![("CfgType", "Config")], false),
2307+
], vec![]),
2308+
make_file("src/barrel.ts", 10, vec![], vec![]),
2309+
make_file("src/types.ts", 20, vec![], vec!["Config"]),
2310+
];
2311+
let resolved = vec![make_resolved("/root/src/app.ts", "./barrel", "src/barrel.ts")];
2312+
let reexports = vec![FileReexports {
2313+
file: "src/barrel.ts".to_string(),
2314+
reexports: vec![ReexportEntryInput {
2315+
source: "src/types.ts".to_string(),
2316+
names: vec!["Config".to_string()],
2317+
wildcard_reexport: false,
2318+
renames: None,
2319+
}],
2320+
}];
2321+
let node_ids = vec![
2322+
make_node_entry("src/app.ts", 1),
2323+
make_node_entry("src/barrel.ts", 10),
2324+
make_node_entry("src/types.ts", 20),
2325+
];
2326+
let barrels = vec!["src/barrel.ts".to_string()];
2327+
2328+
let edges = build_import_edges(
2329+
files,
2330+
resolved,
2331+
reexports,
2332+
node_ids,
2333+
barrels,
2334+
"/root".to_string(),
2335+
None,
2336+
);
2337+
assert_eq!(edges.len(), 2);
2338+
// First: direct import to the barrel.
2339+
assert_eq!(edges[0].kind, "imports");
2340+
assert_eq!(edges[0].target_id, 10);
2341+
// Second: barrel-through to the actual source (types.ts), resolved
2342+
// via the original name "Config", not the local alias "CfgType".
2343+
assert_eq!(edges[1].kind, "imports");
2344+
assert_eq!(edges[1].target_id, 20);
2345+
assert_eq!(edges[1].confidence, 0.9);
2346+
}
2347+
22182348
#[test]
22192349
fn mixed_import_inline_type_modifier_credits_only_flagged_name() {
22202350
// `import { value, type Foo } from './mixed'` — only `Foo` carries

crates/codegraph-core/src/domain/graph/builder/stages/import_edges.rs

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,35 @@ impl BarrelContext for ImportEdgeContext {
120120
}
121121
}
122122

123+
/// Adapter over the two distinct "import statement" Rust representations in
124+
/// this crate that [`import_name_pairs`] needs to read from: the native
125+
/// orchestrator's own parsed `crate::types::Import`, and the FFI-facing
126+
/// `ImportInfo` wire struct the hybrid native path (`build_edges.rs`)
127+
/// deserializes from JS. Both mirror TS `Import` — this trait exists only so
128+
/// `import_name_pairs` has one implementation instead of being duplicated
129+
/// per Rust type, since the two structs differ in `Option` wrapping.
130+
pub(crate) trait ImportNameSource {
131+
fn names(&self) -> &[String];
132+
fn renamed_imports(&self) -> &[RenamedImport];
133+
fn is_type_only(&self) -> bool;
134+
fn type_only_names(&self) -> &[String];
135+
}
136+
137+
impl ImportNameSource for crate::types::Import {
138+
fn names(&self) -> &[String] {
139+
&self.names
140+
}
141+
fn renamed_imports(&self) -> &[RenamedImport] {
142+
self.renamed_imports.as_deref().unwrap_or(&[])
143+
}
144+
fn is_type_only(&self) -> bool {
145+
self.type_only.unwrap_or(false)
146+
}
147+
fn type_only_names(&self) -> &[String] {
148+
self.type_only_names.as_deref().unwrap_or(&[])
149+
}
150+
}
151+
123152
/// Pairs each locally-bound name from an import statement with its original
124153
/// (pre-rename) exported name — identical to the local name unless the
125154
/// specifier renames a binding (`import { X as Y }`). Barrel tracing and
@@ -131,23 +160,20 @@ impl BarrelContext for ImportEdgeContext {
131160
/// either because the whole statement is (`import type { X }`) or because
132161
/// this specific specifier carries the inline modifier
133162
/// (`import { type X }`, #1813).
134-
pub(crate) fn import_name_pairs(imp: &crate::types::Import) -> Vec<(String, String, bool)> {
163+
pub(crate) fn import_name_pairs<T: ImportNameSource>(imp: &T) -> Vec<(String, String, bool)> {
135164
let mut original_name_for: HashMap<&str, &str> = HashMap::new();
136-
if let Some(renamed) = &imp.renamed_imports {
137-
for r in renamed {
138-
original_name_for.insert(&r.local, &r.imported);
139-
}
165+
for r in imp.renamed_imports() {
166+
original_name_for.insert(&r.local, &r.imported);
140167
}
141-
let statement_type_only = imp.type_only.unwrap_or(false);
142-
let type_only_names: HashSet<&str> = imp
143-
.type_only_names
144-
.as_ref()
145-
.map(|v| v.iter().map(|s| s.as_str()).collect())
146-
.unwrap_or_default();
147-
imp.names
168+
let statement_type_only = imp.is_type_only();
169+
let type_only_names: HashSet<&str> = imp.type_only_names().iter().map(|s| s.as_str()).collect();
170+
imp.names()
148171
.iter()
149172
.map(|name| {
150-
let local = name.strip_prefix("* as ").unwrap_or(name);
173+
let local = name
174+
.strip_prefix("* as ")
175+
.or_else(|| name.strip_prefix("*\tas "))
176+
.unwrap_or(name);
151177
let original = original_name_for.get(local).copied().unwrap_or(local);
152178
let type_only = statement_type_only || type_only_names.contains(local);
153179
(local.to_string(), original.to_string(), type_only)

src/domain/graph/builder/stages/build-edges.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,14 @@ interface NativeImportInfo {
333333
wildcardReexport: boolean;
334334
/** Local names (subset of `names`) marked type-only via inline `type`/`typeof` modifier (#1813). */
335335
typeOnlyNames: string[];
336+
/**
337+
* `{ local, imported }` pairs for `import { X as Y }` specifiers — mirrors
338+
* `Import.renamedImports` (#1730). Without this, the native `emit_named_symbol_edges`/
339+
* `emit_barrel_through_edges` FFI handlers would search the target file for
340+
* the local (post-rename) name instead of the name actually declared
341+
* there, silently dropping the edge for a renamed import (#1847).
342+
*/
343+
renamedImports: Array<{ local: string; imported: string }>;
336344
}
337345

338346
/** Native FFI input shape for a single file. */
@@ -384,6 +392,7 @@ function toNativeImportInfo(imp: Import): NativeImportInfo {
384392
dynamicImport: !!imp.dynamicImport,
385393
wildcardReexport: !!imp.wildcardReexport,
386394
typeOnlyNames: imp.typeOnlyNames ?? [],
395+
renamedImports: imp.renamedImports ?? [],
387396
};
388397
}
389398

0 commit comments

Comments
 (0)