diff --git a/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs b/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs index 1dc36549..6ae55061 100644 --- a/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs +++ b/crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs @@ -1536,14 +1536,18 @@ pub struct ResolvedImportEntry { } /// A symbol node entry for type-only import resolution. -/// Maps (name, file) → nodeId so the native engine can create symbol-level -/// `imports-type` edges (parity with the JS `buildImportEdges` path). +/// Maps (name, file) → (nodeId, kind) so the native engine can create +/// symbol-level `imports-type` edges (parity with the JS `buildImportEdges` +/// path) — `kind` lets it also credit plain imports of TypeScript +/// interface/type-alias declarations, not just `import type` statements +/// (#1833). #[napi(object)] pub struct SymbolNodeEntry { pub name: String, pub file: String, #[napi(js_name = "nodeId")] pub node_id: u32, + pub kind: String, } /// Shared lookup context for import edge building. @@ -1553,13 +1557,15 @@ struct ImportEdgeContext<'a> { file_node_map: HashMap<&'a str, u32>, barrel_set: HashSet<&'a str>, file_defs: HashMap<&'a str, HashSet<&'a str>>, - /// Symbol node lookup: (name, file) → node ID. - /// Used to create symbol-level `imports-type` edges for type-only imports. + /// Symbol node lookup: (name, file) → (node ID, kind). + /// Used to create symbol-level `imports-type` edges for type-only imports, + /// and — via `kind` — for plain imports resolving to a TypeScript + /// interface/type-alias declaration (#1833). /// /// Owned keys (rather than `&'a str`) because a barrel-rename lookup key /// (#1823) is a freshly-resolved name that doesn't borrow from `'a` input /// data. - symbol_node_map: HashMap<(String, String), u32>, + symbol_node_map: HashMap<(String, String), (u32, String)>, } impl<'a> ImportEdgeContext<'a> { @@ -1597,7 +1603,10 @@ impl<'a> ImportEdgeContext<'a> { let mut symbol_node_map = HashMap::with_capacity(symbol_nodes.len()); for entry in symbol_nodes { - symbol_node_map.insert((entry.name.clone(), entry.file.clone()), entry.node_id); + symbol_node_map.insert( + (entry.name.clone(), entry.file.clone()), + (entry.node_id, entry.kind.clone()), + ); } Self { resolved, reexport_map, file_node_map, barrel_set, file_defs, symbol_node_map } @@ -1698,13 +1707,6 @@ fn is_named_reexport(imp: &ImportInfo) -> bool { imp.reexport && !imp.wildcard_reexport } -/// True when an import carries any type-only signal — a whole-statement -/// `import type { X }` or at least one inline per-specifier `type` modifier -/// (`import { type X }`, #1813). -fn has_type_only_names(imp: &ImportInfo) -> bool { - imp.type_only || !imp.type_only_names.is_empty() -} - /// For a `type` import or a named re-export targeting a barrel or resolved /// file, emit one symbol-level edge per named symbol so the target symbols /// receive fan-in credit and aren't misclassified as dead code @@ -1712,9 +1714,14 @@ fn has_type_only_names(imp: &ImportInfo) -> bool { /// precise re-export surface instead of the target's full export list /// (`reexports`, #1742). `kind` selects which edge kind to emit. /// -/// For `kind == "imports-type"`, only specifiers actually marked type-only -/// (whole-statement or inline per-specifier, #1813) get an edge — a mixed -/// `import { value, type Foo }` must not credit `value`. +/// For `kind == "imports-type"`, a specifier gets an edge when either it's +/// actually marked type-only (whole-statement or inline per-specifier, +/// #1813 — a mixed `import { value, type Foo }` must not credit `value` on +/// this basis alone), or the resolved target is a TypeScript +/// interface/type-alias declaration (`is_type_erased_import_target`) — those +/// kinds are erased before runtime, so a plain `import { Foo } from 'y'` (no +/// `type` keyword) is the only consumption signal `codegraph exports` can +/// observe for them (#1833). /// /// `imp.names` holds the *original* declaration name for export specifiers /// (see `extractImportNames` in the JS extractor) even when renamed @@ -1741,35 +1748,41 @@ fn emit_named_symbol_edges( } for name in &imp.names { let clean_name = strip_star_as_prefix(name); - if kind == "imports-type" - && !imp.type_only - && !imp.type_only_names.iter().any(|n| n == clean_name) - { - continue; - } + let type_only = imp.type_only || imp.type_only_names.iter().any(|n| n == clean_name); let barrel_target = if ctx.barrel_set.contains(resolved_path) { let mut visited = HashSet::new(); barrel_resolution::resolve_barrel_export(ctx, resolved_path, clean_name, &mut visited) } else { None }; - let sym_id = barrel_target - .as_ref() - .and_then(|resolved| ctx.symbol_node_map.get(&(resolved.name.clone(), resolved.file.clone()))) - .or_else(|| { - ctx.symbol_node_map - .get(&(clean_name.to_string(), resolved_path.to_string())) - }); - if let Some(&id) = sym_id { - edges.push(ComputedEdge { - source_id: file_input.file_node_id, - target_id: id, - kind: kind.to_string(), - confidence: 1.0, - dynamic: 0, - dynamic_kind: None, - }); + let (target_name, target_file) = match &barrel_target { + Some(resolved) + if ctx + .symbol_node_map + .contains_key(&(resolved.name.clone(), resolved.file.clone())) => + { + (resolved.name.clone(), resolved.file.clone()) + } + _ => (clean_name.to_string(), resolved_path.to_string()), + }; + let Some((id, sym_kind)) = ctx.symbol_node_map.get(&(target_name, target_file.clone())) + else { + continue; + }; + if kind == "imports-type" + && !type_only + && !crate::shared::constants::is_type_erased_import_target(sym_kind, &target_file) + { + continue; } + edges.push(ComputedEdge { + source_id: file_input.file_node_id, + target_id: *id, + kind: kind.to_string(), + confidence: 1.0, + dynamic: 0, + dynamic_kind: None, + }); } } @@ -1854,7 +1867,10 @@ fn process_single_import( dynamic: 0, dynamic_kind: None, }); - if has_type_only_names(imp) { + // Always attempted (not just for `import type`/inline-`type` specifiers) — + // emit_named_symbol_edges also credits plain specifiers that resolve to a + // TypeScript interface/type-alias declaration (#1833). + if !imp.reexport { emit_named_symbol_edges(edges, file_input, imp, resolved_path, "imports-type", ctx); } if is_named_reexport(imp) { @@ -1962,6 +1978,7 @@ mod import_edge_tests { name: "foo".to_string(), file: "src/utils.ts".to_string(), node_id: 99, + kind: "function".to_string(), }]; let edges = build_import_edges( @@ -2004,6 +2021,7 @@ mod import_edge_tests { name: "foo".to_string(), file: "src/utils.ts".to_string(), node_id: 99, + kind: "function".to_string(), }]; let edges = build_import_edges( @@ -2032,9 +2050,19 @@ mod import_edge_tests { let resolved = vec![make_resolved("/root/src/index.ts", "./utils", "src/utils.ts")]; let node_ids = vec![make_node_entry("src/index.ts", 1), make_node_entry("src/utils.ts", 2)]; let symbol_nodes = vec![ - SymbolNodeEntry { name: "foo".to_string(), file: "src/utils.ts".to_string(), node_id: 99 }, + SymbolNodeEntry { + name: "foo".to_string(), + file: "src/utils.ts".to_string(), + node_id: 99, + kind: "function".to_string(), + }, // A decoy under the external alias name must NOT be matched. - SymbolNodeEntry { name: "bar".to_string(), file: "src/utils.ts".to_string(), node_id: 100 }, + SymbolNodeEntry { + name: "bar".to_string(), + file: "src/utils.ts".to_string(), + node_id: 100, + kind: "function".to_string(), + }, ]; let edges = build_import_edges( @@ -2077,8 +2105,18 @@ mod import_edge_tests { let resolved = vec![make_resolved("/root/src/app.ts", "./mixed", "src/mixed.ts")]; let node_ids = vec![make_node_entry("src/app.ts", 1), make_node_entry("src/mixed.ts", 2)]; let symbol_nodes = vec![ - SymbolNodeEntry { name: "Foo".to_string(), file: "src/mixed.ts".to_string(), node_id: 50 }, - SymbolNodeEntry { name: "value".to_string(), file: "src/mixed.ts".to_string(), node_id: 51 }, + SymbolNodeEntry { + name: "Foo".to_string(), + file: "src/mixed.ts".to_string(), + node_id: 50, + kind: "function".to_string(), + }, + SymbolNodeEntry { + name: "value".to_string(), + file: "src/mixed.ts".to_string(), + node_id: 51, + kind: "function".to_string(), + }, ]; let edges = build_import_edges( @@ -2096,6 +2134,114 @@ mod import_edge_tests { assert_eq!(edges[1].target_id, 50); } + #[test] + fn plain_import_of_ts_interface_credits_imports_type_edge() { + // `import { Foo } from './types'` — no `type` keyword — where `Foo` + // is a TypeScript interface. Interfaces are erased before runtime, so + // this plain import is the only observable consumption signal + // `codegraph exports` can rely on; it must be credited exactly like + // `import type { Foo }` would be (#1833). + let files = vec![make_file( + "src/app.ts", + 1, + vec![make_import("./types", vec!["Foo"], false, false, false)], + vec![], + )]; + let resolved = vec![make_resolved("/root/src/app.ts", "./types", "src/types.ts")]; + let node_ids = vec![make_node_entry("src/app.ts", 1), make_node_entry("src/types.ts", 2)]; + let symbol_nodes = vec![SymbolNodeEntry { + name: "Foo".to_string(), + file: "src/types.ts".to_string(), + node_id: 50, + kind: "interface".to_string(), + }]; + + let edges = build_import_edges( + files, + resolved, + vec![], + node_ids, + vec![], + "/root".to_string(), + Some(symbol_nodes), + ); + assert_eq!(edges.len(), 2); + assert_eq!(edges[0].kind, "imports"); + assert_eq!(edges[1].kind, "imports-type"); + assert_eq!(edges[1].target_id, 50); + } + + #[test] + fn plain_import_of_ts_value_symbol_gets_no_symbol_level_edge() { + // `import { helper } from './utils'` where `helper` is a plain + // function (not an interface/type alias). Consumption credit for a + // value symbol must still come exclusively from a real `calls` edge + // — merely importing it must not fabricate one (#1833 must not + // regress the existing value-import behaviour). + let files = vec![make_file( + "src/app.ts", + 1, + vec![make_import("./utils", vec!["helper"], false, false, false)], + vec![], + )]; + let resolved = vec![make_resolved("/root/src/app.ts", "./utils", "src/utils.ts")]; + let node_ids = vec![make_node_entry("src/app.ts", 1), make_node_entry("src/utils.ts", 2)]; + let symbol_nodes = vec![SymbolNodeEntry { + name: "helper".to_string(), + file: "src/utils.ts".to_string(), + node_id: 50, + kind: "function".to_string(), + }]; + + let edges = build_import_edges( + files, + resolved, + vec![], + node_ids, + vec![], + "/root".to_string(), + Some(symbol_nodes), + ); + assert_eq!(edges.len(), 1); + assert_eq!(edges[0].kind, "imports"); + } + + #[test] + fn plain_import_of_non_typescript_interface_gets_no_symbol_level_edge() { + // A plain import resolving to an 'interface'-kind node in a + // non-TypeScript file (e.g. a Go `type ... interface {}`) must not be + // credited by this heuristic — those kinds are runtime-observable in + // other languages, so crediting on mere import would mask genuinely + // dead code instead of fixing a false positive (#1833 is scoped to + // TypeScript's compile-time-only interfaces/type aliases). + let files = vec![make_file( + "src/app.ts", + 1, + vec![make_import("./iface", vec!["Reader"], false, false, false)], + vec![], + )]; + let resolved = vec![make_resolved("/root/src/app.ts", "./iface", "src/iface.go")]; + let node_ids = vec![make_node_entry("src/app.ts", 1), make_node_entry("src/iface.go", 2)]; + let symbol_nodes = vec![SymbolNodeEntry { + name: "Reader".to_string(), + file: "src/iface.go".to_string(), + node_id: 50, + kind: "interface".to_string(), + }]; + + let edges = build_import_edges( + files, + resolved, + vec![], + node_ids, + vec![], + "/root".to_string(), + Some(symbol_nodes), + ); + assert_eq!(edges.len(), 1); + assert_eq!(edges[0].kind, "imports"); + } + #[test] fn dynamic_import_edge() { let files = vec![make_file("src/app.ts", 1, vec![ diff --git a/crates/codegraph-core/src/domain/graph/builder/stages/import_edges.rs b/crates/codegraph-core/src/domain/graph/builder/stages/import_edges.rs index 03acf6dc..738a930c 100644 --- a/crates/codegraph-core/src/domain/graph/builder/stages/import_edges.rs +++ b/crates/codegraph-core/src/domain/graph/builder/stages/import_edges.rs @@ -6,6 +6,7 @@ use crate::domain::graph::builder::barrel_resolution::{self, BarrelContext, ReexportRef}; use crate::domain::graph::resolve; +use crate::shared::constants::{is_type_erased_import_target, TYPESCRIPT_EXTENSIONS}; use crate::types::{FileSymbols, PathAliases, RenamedImport}; use rusqlite::Connection; use std::collections::{BTreeMap, HashMap, HashSet}; @@ -229,20 +230,23 @@ fn load_file_node_ids(conn: &Connection) -> HashMap { map } -/// Load symbol node IDs for the supplied `(name, file)` pairs in one chunked -/// query. Mirrors the JS `nodesByNameAndFile` lookup map; preserves the -/// first-row semantics of the legacy `LIMIT 1` query by keeping the first ID -/// seen per key. +/// Load symbol node IDs (and kinds) for the supplied `(name, file)` pairs in +/// one chunked query. Mirrors the JS `nodesByNameAndFile` lookup map; +/// preserves the first-row semantics of the legacy `LIMIT 1` query by keeping +/// the first row seen per key. `kind` lets `emit_named_symbol_rows` credit +/// plain imports resolving to a TypeScript interface/type-alias declaration, +/// not just `import type` statements (#1833). /// -/// The pairs are pre-computed by walking the type-only imports in -/// `ctx.file_symbols`, so we never scan the entire `nodes` table — even on -/// monorepos with 100k+ symbols, only the small slice actually referenced by -/// type-only imports is hit (#1013, #1028 review). +/// The pairs are pre-computed by walking the type-only imports, named +/// re-exports, and TypeScript-file-targeting imports in `ctx.file_symbols`, +/// so we never scan the entire `nodes` table — even on monorepos with 100k+ +/// symbols, only the slice actually reachable by one of those import shapes +/// is hit (#1013, #1028 review). fn load_symbol_node_ids( conn: &Connection, needed_pairs: &HashSet<(String, String)>, -) -> HashMap<(String, String), i64> { - let mut map: HashMap<(String, String), i64> = HashMap::new(); +) -> HashMap<(String, String), (i64, String)> { + let mut map: HashMap<(String, String), (i64, String)> = HashMap::new(); if needed_pairs.is_empty() { return map; } @@ -260,7 +264,7 @@ fn load_symbol_node_ids( }) .collect(); let sql = format!( - "SELECT name, file, id FROM nodes WHERE kind != 'file' AND (name, file) IN ({})", + "SELECT name, file, id, kind FROM nodes WHERE kind != 'file' AND (name, file) IN ({})", placeholders.join(",") ); let mut params: Vec<&dyn rusqlite::ToSql> = Vec::with_capacity(chunk.len() * 2); @@ -275,10 +279,11 @@ fn load_symbol_node_ids( row.get::<_, String>(0)?, row.get::<_, String>(1)?, row.get::<_, i64>(2)?, + row.get::<_, String>(3)?, )) }) { for r in rows.flatten() { - map.entry((r.0, r.1)).or_insert(r.2); + map.entry((r.0, r.1)).or_insert((r.2, r.3)); } } } @@ -294,13 +299,23 @@ fn is_named_reexport(imp: &crate::types::Import) -> bool { imp.reexport.unwrap_or(false) && !imp.wildcard_reexport.unwrap_or(false) } -/// Walk type-only imports and named re-exports in `ctx.file_symbols` and -/// return the distinct `(name, file)` pairs that `build_import_edges` will +/// True when `file`'s extension means it *might* hold a TypeScript +/// interface/type-alias declaration (see `is_type_erased_import_target`) — +/// used to widen `collect_symbol_lookup_pairs` beyond syntactically +/// type-only imports without scanning every plain import in every language +/// (#1833). +fn maybe_type_erased_file(file: &str) -> bool { + TYPESCRIPT_EXTENSIONS.iter().any(|ext| file.ends_with(ext)) +} + +/// Walk type-only imports, named re-exports, and plain imports that might +/// target a TypeScript interface/type-alias declaration in `ctx.file_symbols`, +/// returning the distinct `(name, file)` pairs that `build_import_edges` will /// need to look up. Resolves barrel files the same way the edge-building /// loop does so the pre-computed set matches the actual lookup keys. -/// Shared by symbol-level `imports-type` (#1724) and `reexports` (#1742) -/// edges — both name specific symbols requiring a (name, file) → node-id -/// lookup. +/// Shared by symbol-level `imports-type` (#1724, #1833) and `reexports` +/// (#1742) edges — all three name specific symbols requiring a +/// (name, file) → node-id/kind lookup. fn collect_symbol_lookup_pairs(ctx: &ImportEdgeContext) -> HashSet<(String, String)> { let mut pairs = HashSet::new(); for (rel_path, symbols) in &ctx.file_symbols { @@ -308,12 +323,13 @@ fn collect_symbol_lookup_pairs(ctx: &ImportEdgeContext) -> HashSet<(String, Stri let abs_str = abs_file.to_str().unwrap_or(""); for imp in &symbols.imports { let is_reexport = is_named_reexport(imp); - if !has_type_only_names(imp) && !is_reexport { + let resolved_path = ctx.get_resolved(abs_str, &imp.source); + let maybe_type_erased = maybe_type_erased_file(&resolved_path); + if !has_type_only_names(imp) && !is_reexport && !maybe_type_erased { continue; } - let resolved_path = ctx.get_resolved(abs_str, &imp.source); for (_local, original, type_only) in import_name_pairs(imp) { - if !is_reexport && !type_only { + if !is_reexport && !type_only && !maybe_type_erased { continue; } let mut target_file = resolved_path.clone(); @@ -357,15 +373,21 @@ fn classify_import_kind(imp: &crate::types::Import) -> &'static str { } } -/// For a `type` import or a named re-export, emit one symbol-level edge per -/// name so the target symbols receive fan-in credit and aren't classified -/// dead (`imports-type`, #1724), or so `codegraph exports` can report the -/// precise re-export surface instead of the target's full export list -/// (`reexports`, #1742). `kind` selects which edge kind to emit. +/// For a `type` import, a plain import of a TypeScript interface/type-alias, +/// or a named re-export, emit one symbol-level edge per name so the target +/// symbols receive fan-in credit and aren't classified dead (`imports-type`, +/// #1724, #1833), or so `codegraph exports` can report the precise re-export +/// surface instead of the target's full export list (`reexports`, #1742). +/// `kind` selects which edge kind to emit. /// -/// For `kind == "imports-type"`, only specifiers actually marked type-only -/// (whole-statement or inline per-specifier, #1813) get an edge — a mixed -/// `import { value, type Foo }` must not credit `value`. +/// For `kind == "imports-type"`, a specifier gets an edge when either it's +/// actually marked type-only (whole-statement or inline per-specifier, +/// #1813 — a mixed `import { value, type Foo }` must not credit `value` on +/// this basis alone), or the resolved target is a TypeScript +/// interface/type-alias declaration (`is_type_erased_import_target`) — those +/// kinds are erased before runtime, so a plain `import { Foo } from 'y'` (no +/// `type` keyword) is the only consumption signal `codegraph exports` can +/// observe for them (#1833). fn emit_named_symbol_rows( edges: &mut Vec, file_node_id: i64, @@ -373,12 +395,9 @@ fn emit_named_symbol_rows( resolved_path: &str, kind: &str, ctx: &ImportEdgeContext, - symbol_node_ids: &HashMap<(String, String), i64>, + symbol_node_ids: &HashMap<(String, String), (i64, String)>, ) { for (_local, original, type_only) in import_name_pairs(imp) { - if kind == "imports-type" && !type_only { - continue; - } let mut target_file = resolved_path.to_string(); let mut target_name = original; if ctx.is_barrel_file(resolved_path) { @@ -390,15 +409,23 @@ fn emit_named_symbol_rows( target_name = resolved.name; } } - if let Some(&sym_id) = symbol_node_ids.get(&(target_name, target_file)) { - edges.push(EdgeRow { - source_id: file_node_id, - target_id: sym_id, - kind: kind.to_string(), - confidence: 1.0, - dynamic: 0, - }); + let Some((sym_id, sym_kind)) = symbol_node_ids.get(&(target_name, target_file.clone())) + else { + continue; + }; + if kind == "imports-type" + && !type_only + && !is_type_erased_import_target(sym_kind, &target_file) + { + continue; } + edges.push(EdgeRow { + source_id: file_node_id, + target_id: *sym_id, + kind: kind.to_string(), + confidence: 1.0, + dynamic: 0, + }); } } @@ -454,7 +481,7 @@ fn emit_edges_for_import( is_barrel_only: bool, ctx: &ImportEdgeContext, file_node_ids: &HashMap, - symbol_node_ids: &HashMap<(String, String), i64>, + symbol_node_ids: &HashMap<(String, String), (i64, String)>, ) { let is_reexport = imp.reexport.unwrap_or(false); if is_barrel_only && !is_reexport { @@ -473,7 +500,10 @@ fn emit_edges_for_import( confidence: 1.0, dynamic: 0, }); - if has_type_only_names(imp) { + // Always attempted (not just for `import type`/inline-`type` specifiers) — + // emit_named_symbol_rows also credits plain specifiers that resolve to a + // TypeScript interface/type-alias declaration (#1833). + if !is_reexport { emit_named_symbol_rows( edges, file_node_id, @@ -648,6 +678,21 @@ mod tests { use crate::types::{Definition, Import}; fn make_symbols(defs: Vec<&str>, reexport_imports: Vec<&str>) -> FileSymbols { + make_symbols_with_imports( + defs, + reexport_imports + .into_iter() + .map(|src| { + let mut imp = Import::new(src.to_string(), vec![], 1); + imp.reexport = Some(true); + imp.wildcard_reexport = Some(true); + imp + }) + .collect(), + ) + } + + fn make_symbols_with_imports(defs: Vec<&str>, imports: Vec) -> FileSymbols { FileSymbols { file: "test.ts".to_string(), definitions: defs @@ -663,15 +708,7 @@ mod tests { children: None, }) .collect(), - imports: reexport_imports - .into_iter() - .map(|src| { - let mut imp = Import::new(src.to_string(), vec![], 1); - imp.reexport = Some(true); - imp.wildcard_reexport = Some(true); - imp - }) - .collect(), + imports, calls: vec![], classes: vec![], exports: vec![], @@ -773,6 +810,134 @@ mod tests { assert!(pairs.iter().all(|(_, _, type_only)| !*type_only)); } + fn empty_ctx(file_symbols: BTreeMap) -> ImportEdgeContext { + ImportEdgeContext { + batch_resolved: HashMap::new(), + reexport_map: HashMap::new(), + barrel_only_files: HashSet::new(), + file_symbols, + root_dir: "/project".to_string(), + aliases: PathAliases { base_url: None, paths: vec![] }, + known_files: HashSet::new(), + } + } + + #[test] + fn emit_named_symbol_rows_credits_plain_import_of_ts_interface() { + // `import { Foo } from './types'` — no `type` keyword — where `Foo` + // is a TypeScript interface. Interfaces are erased before runtime, so + // this plain import is the only observable consumption signal + // `codegraph exports` can rely on; it must be credited exactly like + // `import type { Foo }` would be (#1833). + let ctx = empty_ctx(BTreeMap::new()); + let imp = Import::new("./types".to_string(), vec!["Foo".to_string()], 1); + let mut symbol_node_ids: HashMap<(String, String), (i64, String)> = HashMap::new(); + symbol_node_ids.insert( + ("Foo".to_string(), "src/types.ts".to_string()), + (50, "interface".to_string()), + ); + + let mut edges = Vec::new(); + emit_named_symbol_rows( + &mut edges, + 1, + &imp, + "src/types.ts", + "imports-type", + &ctx, + &symbol_node_ids, + ); + + assert_eq!(edges.len(), 1); + assert_eq!(edges[0].kind, "imports-type"); + assert_eq!(edges[0].target_id, 50); + } + + #[test] + fn emit_named_symbol_rows_skips_plain_import_of_value_symbol() { + // A plain import of a real function must NOT get a fabricated + // `imports-type` edge — value-symbol consumption credit still comes + // exclusively from an actual `calls` edge (#1833 must not regress + // the existing value-import behaviour). + let ctx = empty_ctx(BTreeMap::new()); + let imp = Import::new("./utils".to_string(), vec!["helper".to_string()], 1); + let mut symbol_node_ids: HashMap<(String, String), (i64, String)> = HashMap::new(); + symbol_node_ids.insert( + ("helper".to_string(), "src/utils.ts".to_string()), + (50, "function".to_string()), + ); + + let mut edges = Vec::new(); + emit_named_symbol_rows( + &mut edges, + 1, + &imp, + "src/utils.ts", + "imports-type", + &ctx, + &symbol_node_ids, + ); + + assert!(edges.is_empty()); + } + + #[test] + fn emit_named_symbol_rows_skips_non_typescript_interface() { + // An 'interface'-kind node outside a .ts/.tsx file (e.g. a Go + // `type ... interface {}`) is runtime-observable in its own + // language, so this heuristic — scoped to TypeScript's compile-time + // erasure — must not credit it on mere import (#1833). + let ctx = empty_ctx(BTreeMap::new()); + let imp = Import::new("./iface".to_string(), vec!["Reader".to_string()], 1); + let mut symbol_node_ids: HashMap<(String, String), (i64, String)> = HashMap::new(); + symbol_node_ids.insert( + ("Reader".to_string(), "src/iface.go".to_string()), + (50, "interface".to_string()), + ); + + let mut edges = Vec::new(); + emit_named_symbol_rows( + &mut edges, + 1, + &imp, + "src/iface.go", + "imports-type", + &ctx, + &symbol_node_ids, + ); + + assert!(edges.is_empty()); + } + + #[test] + fn collect_symbol_lookup_pairs_includes_plain_imports_targeting_ts_files() { + // A plain (non-type-only, non-reexport) import must still be + // collected when its resolved target is a TypeScript file — the + // target might be an interface/type-alias declaration, which + // `emit_named_symbol_rows` can only credit if this pre-pass fetched + // its (id, kind) from the DB (#1833). + let mut file_symbols = BTreeMap::new(); + let plain_ts_import = Import::new("./types".to_string(), vec!["Foo".to_string()], 1); + let plain_py_import = Import::new("./helpers".to_string(), vec!["util".to_string()], 2); + file_symbols.insert( + "src/app.ts".to_string(), + make_symbols_with_imports(vec![], vec![plain_ts_import, plain_py_import]), + ); + let mut ctx = empty_ctx(file_symbols); + ctx.batch_resolved.insert( + "/project/src/app.ts|./types".to_string(), + "src/types.ts".to_string(), + ); + ctx.batch_resolved.insert( + "/project/src/app.ts|./helpers".to_string(), + "src/helpers.py".to_string(), + ); + + let pairs = collect_symbol_lookup_pairs(&ctx); + assert!(pairs.contains(&("Foo".to_string(), "src/types.ts".to_string()))); + assert!(!pairs.contains(&("util".to_string(), "src/helpers.py".to_string()))); + } + /// Minimal in-memory `edges` schema covering only the columns /// `insert_edge_chunk` writes. fn edges_test_conn() -> Connection { diff --git a/crates/codegraph-core/src/shared/constants.rs b/crates/codegraph-core/src/shared/constants.rs index 4a111cbb..0ec53a22 100644 --- a/crates/codegraph-core/src/shared/constants.rs +++ b/crates/codegraph-core/src/shared/constants.rs @@ -50,3 +50,31 @@ pub const FAST_PATH_MAX_CHANGED_FILES: usize = 5; /// Minimum existing file count required before the fast path is considered. /// Typed as `i64` to match `get_existing_file_count()` return type (SQLite row count). pub const FAST_PATH_MIN_EXISTING_FILES: i64 = 20; + +// ─── Import edge classification ───────────────────────────────────── +// +// Mirrors TS's `shared/kinds.ts` (TYPE_ERASED_SYMBOL_KINDS, +// isTypeErasedImportTarget) and `domain/parser.ts` (TYPESCRIPT_EXTENSIONS). + +/// TypeScript source extensions — type annotations (and TS's compile-time-only +/// 'interface'/'type' declarations) only exist for these. +pub const TYPESCRIPT_EXTENSIONS: [&str; 2] = [".ts", ".tsx"]; + +/// Symbol kinds that are compile-time-only in TypeScript — interfaces and +/// type aliases are erased before runtime, so a symbol of one of these kinds +/// can never receive a `calls` edge. Importing one — with or without the +/// `type` keyword — is the only consumption signal `codegraph exports` can +/// observe for these kinds (#1833). +pub const TYPE_ERASED_SYMBOL_KINDS: [&str; 2] = ["interface", "type"]; + +/// True when a named import specifier resolving to `kind` in `file` can only +/// ever be consumed as a type — see TYPE_ERASED_SYMBOL_KINDS. Scoped to +/// `.ts`/`.tsx` files because other languages reuse the 'interface'/'type' +/// node kinds for constructs that *are* runtime-observable (e.g. a Go `type` +/// alias, a Java `interface` dispatched through at runtime) — crediting +/// those on mere import would mask genuinely dead code instead of fixing a +/// false positive. +pub fn is_type_erased_import_target(kind: &str, file: &str) -> bool { + TYPE_ERASED_SYMBOL_KINDS.contains(&kind) + && TYPESCRIPT_EXTENSIONS.iter().any(|ext| file.ends_with(ext)) +} diff --git a/src/domain/graph/builder/incremental.ts b/src/domain/graph/builder/incremental.ts index 7d2905d8..87ac8c4b 100644 --- a/src/domain/graph/builder/incremental.ts +++ b/src/domain/graph/builder/incremental.ts @@ -12,6 +12,7 @@ import path from 'node:path'; import { bulkNodeIdsByFile, purgeFileData } from '../../../db/index.js'; import { debug, warn } from '../../../infrastructure/logger.js'; import { normalizePath, TS_NATIVE_CONFIDENCE_FLOOR } from '../../../shared/constants.js'; +import { isTypeErasedImportTarget } from '../../../shared/kinds.js'; import type { BetterSqlite3Database, EngineOpts, @@ -410,9 +411,14 @@ function resolveBarrelImportEdges( * target's full export list for anything reached only by the file-level * edge. Mirrors `emitNamedSymbolEdges` in build-edges.ts (full-build path). * - * For `edgeKind === 'imports-type'`, only specifiers actually marked - * type-only (whole-statement or inline per-specifier, #1813) get an edge — - * a mixed `import { value, type Foo }` must not credit `value`. + * For `edgeKind === 'imports-type'`, a specifier gets an edge when either + * it's actually marked type-only (whole-statement or inline per-specifier, + * #1813 — a mixed `import { value, type Foo }` must not credit `value` on + * this basis alone), or the resolved target is a TypeScript + * interface/type-alias declaration (`isTypeErasedImportTarget`) — those + * kinds are erased before runtime, so a plain `import { Foo } from 'y'` (no + * `type` keyword) is the only consumption signal `codegraph exports` can + * observe for them (#1833). */ function emitNamedSymbolEdges( db: BetterSqlite3Database | null, @@ -424,7 +430,6 @@ function emitNamedSymbolEdges( ): number { let edgesAdded = 0; for (const { original, typeOnly } of importNamePairs(imp)) { - if (edgeKind === 'imports-type' && !typeOnly) continue; let targetFile = resolvedPath; let targetName = original; if (db && isBarrelFile(db, resolvedPath)) { @@ -437,9 +442,18 @@ function emitNamedSymbolEdges( const candidates = stmts.findNodeInFile.all(targetName, targetFile) as Array<{ id: number; file: string; + kind: string; }>; if (candidates.length === 0) continue; - stmts.insertEdge.run(fileNodeId, candidates[0]!.id, edgeKind, 1.0, 0); + const target = candidates[0]!; + if ( + edgeKind === 'imports-type' && + !typeOnly && + !isTypeErasedImportTarget(target.kind, targetFile) + ) { + continue; + } + stmts.insertEdge.run(fileNodeId, target.id, edgeKind, 1.0, 0); edgesAdded++; } return edgesAdded; @@ -472,7 +486,10 @@ function emitEdgesForImport( stmts.insertEdge.run(fileNodeId, targetRow.id, edgeKind, 1.0, 0); let edgesAdded = 1; - if (imp.typeOnly || (imp.typeOnlyNames && imp.typeOnlyNames.length > 0)) { + // Always attempted (not just for `import type`/inline-`type` specifiers) — + // emitNamedSymbolEdges also credits plain specifiers that resolve to a + // TypeScript interface/type-alias declaration (#1833). + if (!imp.reexport) { edgesAdded += emitNamedSymbolEdges(db, stmts, imp, resolvedPath, fileNodeId, 'imports-type'); } if (imp.reexport && !imp.wildcardReexport) { diff --git a/src/domain/graph/builder/stages/build-edges.ts b/src/domain/graph/builder/stages/build-edges.ts index 1907b45b..a3f2f7d2 100644 --- a/src/domain/graph/builder/stages/build-edges.ts +++ b/src/domain/graph/builder/stages/build-edges.ts @@ -13,6 +13,7 @@ import { debug } from '../../../../infrastructure/logger.js'; import { loadNative } from '../../../../infrastructure/native.js'; import { getOrCreatePerDbChunkStmt } from '../../../../shared/chunked-stmt-cache.js'; import { TS_NATIVE_CONFIDENCE_FLOOR } from '../../../../shared/constants.js'; +import { isTypeErasedImportTarget } from '../../../../shared/kinds.js'; import type { ArrayCallbackBinding, ArrayElemBinding, @@ -217,9 +218,14 @@ function importNamePairs( * out; the query layer falls back to the target's full export list for * anything reached only by the file-level edge (genuine wildcard semantics). * - * For `edgeKind === 'imports-type'`, only specifiers actually marked - * type-only (whole-statement or inline per-specifier, #1813) get an edge — - * a mixed `import { value, type Foo }` must not credit `value`. + * For `edgeKind === 'imports-type'`, a specifier gets an edge when either: + * - it's actually marked type-only (whole-statement or inline + * per-specifier, #1813) — a mixed `import { value, type Foo }` must not + * credit `value` on this basis alone; or + * - the resolved target is a TypeScript interface/type-alias declaration + * (`isTypeErasedImportTarget`) — those kinds are erased before runtime, + * so a plain `import { Foo } from 'y'` (no `type` keyword) is the only + * consumption signal `codegraph exports` can observe for them (#1833). */ function emitNamedSymbolEdges( ctx: PipelineContext, @@ -231,7 +237,6 @@ function emitNamedSymbolEdges( ): void { if (!ctx.nodesByNameAndFile) return; for (const { original, typeOnly } of importNamePairs(imp)) { - if (edgeKind === 'imports-type' && !typeOnly) continue; let targetFile = resolvedPath; let targetName = original; if (isBarrelFile(ctx, resolvedPath)) { @@ -242,9 +247,16 @@ function emitNamedSymbolEdges( } } const candidates = ctx.nodesByNameAndFile.get(`${targetName}|${targetFile}`); - if (candidates && candidates.length > 0) { - allEdgeRows.push([fileNodeId, candidates[0]!.id, edgeKind, 1.0, 0, null, null]); + if (!candidates || candidates.length === 0) continue; + const target = candidates[0]!; + if ( + edgeKind === 'imports-type' && + !typeOnly && + !isTypeErasedImportTarget(target.kind, targetFile) + ) { + continue; } + allEdgeRows.push([fileNodeId, target.id, edgeKind, 1.0, 0, null, null]); } } @@ -267,7 +279,10 @@ function emitEdgesForImport( const edgeKind = importEdgeKind(imp); allEdgeRows.push([fileNodeId, targetRow.id, edgeKind, 1.0, 0, null, null]); - if (imp.typeOnly || (imp.typeOnlyNames && imp.typeOnlyNames.length > 0)) { + // Always attempted (not just for `import type`/inline-`type` specifiers) — + // emitNamedSymbolEdges also credits plain specifiers that resolve to a + // TypeScript interface/type-alias declaration (#1833). + if (!imp.reexport) { emitNamedSymbolEdges(ctx, imp, resolvedPath, fileNodeId, allEdgeRows, 'imports-type'); } if (imp.reexport && !imp.wildcardReexport) { @@ -474,13 +489,13 @@ function collectBarrelFiles(ctx: PipelineContext): string[] { function collectSymbolNodes( ctx: PipelineContext, -): Array<{ name: string; file: string; nodeId: number }> { - const symbolNodes: Array<{ name: string; file: string; nodeId: number }> = []; +): Array<{ name: string; file: string; nodeId: number; kind: string }> { + const symbolNodes: Array<{ name: string; file: string; nodeId: number; kind: string }> = []; if (!ctx.nodesByNameAndFile) return symbolNodes; for (const [key, nodes] of ctx.nodesByNameAndFile) { if (nodes.length === 0) continue; const [name, file] = key.split('|'); - symbolNodes.push({ name: name!, file: file!, nodeId: nodes[0]!.id }); + symbolNodes.push({ name: name!, file: file!, nodeId: nodes[0]!.id, kind: nodes[0]!.kind }); } return symbolNodes; } diff --git a/src/domain/parser.ts b/src/domain/parser.ts index 5208222c..79f3c273 100644 --- a/src/domain/parser.ts +++ b/src/domain/parser.ts @@ -134,8 +134,11 @@ let _allParsersLoaded: boolean = false; // In-flight grammar loads keyed by language id — prevents concurrent duplicate loads const _loadingPromises: Map> = new Map(); -// Extensions that need typeMap backfill (type annotations only exist in TS/TSX) -const TS_BACKFILL_EXTS = new Set(['.ts', '.tsx']); +// TypeScript source extensions — type annotations (and TS's compile-time-only +// 'interface'/'type' declarations, see shared/kinds.ts's TYPE_ERASED_SYMBOL_KINDS) +// only exist for these. Used for typeMap backfill and, via shared/kinds.ts's +// isTypeErasedImportTarget, for import-edge classification (#1833). +export const TYPESCRIPT_EXTENSIONS: ReadonlySet = new Set(['.ts', '.tsx']); // Re-export for backward compatibility export type { LanguageRegistryEntry } from '../types.js'; @@ -1119,7 +1122,7 @@ export async function parseFileAuto( // Always backfill typeMap for TS/TSX from WASM — native parser's type // extraction can produce incorrect scope-collision results. Non-TS files // are skipped to stay consistent with the batch path (backfillTypeMapBatch). - if (TS_BACKFILL_EXTS.has(path.extname(filePath))) { + if (TYPESCRIPT_EXTENSIONS.has(path.extname(filePath))) { const { typeMap, backfilled } = await backfillTypeMap(filePath, source); if (backfilled) { patched.typeMap = typeMap; @@ -1140,7 +1143,7 @@ async function backfillTypeMapBatch( result: Map, ): Promise { const tsFiles = needsTypeMap.filter(({ filePath }) => - TS_BACKFILL_EXTS.has(path.extname(filePath)), + TYPESCRIPT_EXTENSIONS.has(path.extname(filePath)), ); if (tsFiles.length === 0) return; @@ -1329,7 +1332,7 @@ function ingestNativeResults( // appears at multiple scopes (e.g. `node: TreeSitterNode` in one function // vs `node: NodeRow` in another). The WASM JS extractor handles scope // traversal order more accurately. - if (TS_BACKFILL_EXTS.has(path.extname(r.file))) { + if (TYPESCRIPT_EXTENSIONS.has(path.extname(r.file))) { needsTypeMap.push({ filePath: r.file, relPath }); } } @@ -1432,7 +1435,7 @@ export async function parseFileIncremental( if (!result) return null; const patched = patchNativeResult(result); // Always backfill typeMap for TS/TSX from WASM (see parseFileAuto comment). - if (TS_BACKFILL_EXTS.has(path.extname(filePath))) { + if (TYPESCRIPT_EXTENSIONS.has(path.extname(filePath))) { const { typeMap, backfilled } = await backfillTypeMap(filePath, source); if (backfilled) { patched.typeMap = typeMap; diff --git a/src/shared/kinds.ts b/src/shared/kinds.ts index 3b9ec050..a82745f8 100644 --- a/src/shared/kinds.ts +++ b/src/shared/kinds.ts @@ -1,3 +1,5 @@ +import path from 'node:path'; +import { TYPESCRIPT_EXTENSIONS } from '../domain/parser.js'; import type { CoreEdgeKind, CoreSymbolKind, @@ -83,3 +85,27 @@ export const VALID_ROLES: readonly Role[] = [ 'leaf', ...DEAD_SUB_ROLES, ]; + +// ── TypeScript type-erasure classification ────────────────────────── +// Symbol kinds that are compile-time-only in TypeScript — interfaces and +// type aliases are erased before runtime, so a symbol of one of these kinds +// can never receive a `calls` edge. Importing one — with or without the +// `type` keyword — is the only consumption signal `codegraph exports` can +// observe for these kinds (#1833). +export const TYPE_ERASED_SYMBOL_KINDS: ReadonlySet = new Set(['interface', 'type']); + +/** + * True when a named import specifier resolving to `kind` in `file` can only + * ever be consumed as a type — i.e. it's a TypeScript interface/type-alias + * declaration, which no `calls` edge could ever target regardless of the + * `type` keyword on the importing statement. + * + * Scoped to `.ts`/`.tsx` files because other languages reuse the 'interface'/ + * 'type' node kinds for constructs that *are* runtime-observable (e.g. a Go + * `type` alias, a Java `interface` implemented and dispatched through at + * runtime) — crediting those on mere import would mask genuinely dead code + * instead of fixing a false positive. + */ +export function isTypeErasedImportTarget(kind: string, file: string): boolean { + return TYPE_ERASED_SYMBOL_KINDS.has(kind) && TYPESCRIPT_EXTENSIONS.has(path.extname(file)); +} diff --git a/src/types.ts b/src/types.ts index 45149095..5ce6801c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -2346,7 +2346,7 @@ export interface NativeAddon { fileNodeIds: unknown[], barrelFiles: string[], rootDir: string, - symbolNodes?: Array<{ name: string; file: string; nodeId: number }>, + symbolNodes?: Array<{ name: string; file: string; nodeId: number; kind: string }>, ): unknown[]; engineVersion(): string; analyzeComplexity( diff --git a/tests/fixtures/issue-1833-plain-type-import/consumer.ts b/tests/fixtures/issue-1833-plain-type-import/consumer.ts new file mode 100644 index 00000000..dca8628f --- /dev/null +++ b/tests/fixtures/issue-1833-plain-type-import/consumer.ts @@ -0,0 +1,10 @@ +// Plain import — no `type` keyword — of an interface (Config) and a type +// alias (Mode), used only in type position below. TypeScript allows +// importing type-level declarations this way; codegraph must still credit +// Config/Mode as consumed since interfaces/type aliases are erased before +// runtime and can never receive a `calls` edge (#1833). +import { Config, Mode } from './types.js'; + +export function useConfig(cfg: Config): Mode { + return cfg.name.length > 0 ? 'fast' : 'slow'; +} diff --git a/tests/fixtures/issue-1833-plain-type-import/types.ts b/tests/fixtures/issue-1833-plain-type-import/types.ts new file mode 100644 index 00000000..5b31d342 --- /dev/null +++ b/tests/fixtures/issue-1833-plain-type-import/types.ts @@ -0,0 +1,9 @@ +export interface Config { + name: string; +} + +export type Mode = 'fast' | 'slow'; + +export function helper(): number { + return 1; +} diff --git a/tests/integration/issue-1833-plain-type-import.test.ts b/tests/integration/issue-1833-plain-type-import.test.ts new file mode 100644 index 00000000..3cc6e7f6 --- /dev/null +++ b/tests/integration/issue-1833-plain-type-import.test.ts @@ -0,0 +1,124 @@ +/** + * Regression for #1833: `codegraph exports`/dead-export analysis never + * credited a plain (no `type` keyword) import of a TypeScript + * interface/type-alias as a consumer. Only a whole-statement + * `import type { X }` (#1724) or an inline per-specifier `import { type X }` + * (#1813) produced a symbol-level `imports-type` edge — but TypeScript + * allows importing type-level declarations without the `type` keyword at + * all, which is extremely common in codebases that don't enforce + * `import/consistent-type-imports`. Since interfaces/type aliases are erased + * before runtime, they can never receive a `calls` edge either — so a plain + * import was the only possible consumption signal, and it was being ignored, + * making every such interface/type alias look permanently dead. + * + * Fixture: + * types.ts — exports interface Config, type alias Mode, and function + * helper (helper is never imported anywhere — a genuinely + * dead export used as a baseline). + * consumer.ts — a single plain import statement (no `type` keyword): + * import { Config, Mode } from './types.js'; + * using both purely in type position. + * + * Config and Mode should each get a symbol-level `imports-type` edge from + * consumer.ts even though the import statement has no `type` keyword at all. + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { buildGraph } from '../../src/domain/graph/builder.js'; +import { exportsData } from '../../src/domain/queries.js'; +import type { EngineMode } from '../../src/types.js'; + +const FIXTURE_DIR = path.join( + import.meta.dirname, + '..', + 'fixtures', + 'issue-1833-plain-type-import', +); + +interface EdgeRow { + kind: string; + target_name: string; + target_file: string; +} + +function readImportEdgesFromConsumer(dbPath: string): EdgeRow[] { + const db = new Database(dbPath, { readonly: true }); + try { + return db + .prepare( + `SELECT e.kind, n2.name AS target_name, n2.file AS target_file + FROM edges e + JOIN nodes n1 ON e.source_id = n1.id + JOIN nodes n2 ON e.target_id = n2.id + WHERE n1.file = 'consumer.ts' AND n1.kind = 'file' + AND e.kind IN ('imports', 'imports-type') + ORDER BY e.kind, n2.name`, + ) + .all() as EdgeRow[]; + } finally { + db.close(); + } +} + +const ENGINES: EngineMode[] = ['wasm', 'native']; + +describe.each(ENGINES)('#1833 plain (no `type` keyword) type-only import (%s)', (engine) => { + let tmpDir: string; + let dbPath: string; + let edges: EdgeRow[]; + + beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), `codegraph-1833-${engine}-`)); + fs.cpSync(FIXTURE_DIR, tmpDir, { recursive: true }); + + await buildGraph(tmpDir, { incremental: false, skipRegistry: true, engine }); + dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + edges = readImportEdgesFromConsumer(dbPath); + }, 60_000); + + afterAll(() => { + if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('credits Config with a symbol-level imports-type edge despite the plain import', () => { + const hit = edges.find((e) => e.kind === 'imports-type' && e.target_name === 'Config'); + expect(hit).toBeDefined(); + expect(hit?.target_file).toBe('types.ts'); + }); + + it('credits Mode with a symbol-level imports-type edge despite the plain import', () => { + const hit = edges.find((e) => e.kind === 'imports-type' && e.target_name === 'Mode'); + expect(hit).toBeDefined(); + expect(hit?.target_file).toBe('types.ts'); + }); + + it('keeps the file-level edge as plain imports, not imports-type', () => { + const fileLevel = edges.filter((e) => e.target_name === 'types.ts'); + expect(fileLevel.length).toBeGreaterThan(0); + for (const e of fileLevel) { + expect(e.kind).toBe('imports'); + } + }); + + it('reports Config and Mode as consumed via codegraph exports', () => { + const data = exportsData('types.ts', dbPath, { noTests: true }) as { + results: Array<{ name: string; consumerCount: number }>; + }; + const config = data.results.find((r) => r.name === 'Config'); + const mode = data.results.find((r) => r.name === 'Mode'); + expect(config?.consumerCount).toBeGreaterThan(0); + expect(mode?.consumerCount).toBeGreaterThan(0); + }); + + it('still reports the genuinely unimported helper export as unused', () => { + const data = exportsData('types.ts', dbPath, { noTests: true }) as { + results: Array<{ name: string; consumerCount: number }>; + }; + const helper = data.results.find((r) => r.name === 'helper'); + expect(helper?.consumerCount).toBe(0); + }); +});