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 c22642689..3a3c02d28 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 @@ -986,6 +986,12 @@ fn resolve_exact_global_match<'a>( /// `caller_name` is the enclosing function/method name (e.g. `"Shape.describe"`) used to scope /// `this`/`self`/`super` dispatch to the caller's own class before falling back to a broader scan. /// Mirrors `resolveCallTargets` / `resolveByMethodOrGlobal` in call-resolver.ts. +/// +/// Thin wrapper around `resolve_call_targets_core`: additionally attaches +/// constructor-call attribution (#1892) for bare (no-receiver) calls — see +/// `attach_constructor_targets`. Split out because the core resolver has many +/// early-return tiers, so a single post-processing pass at the call site is +/// simpler than threading the augmentation through every tier. fn resolve_call_targets<'a>( ctx: &EdgeContext<'a>, call: &CallInfo, @@ -994,6 +1000,30 @@ fn resolve_call_targets<'a>( type_map: &HashMap<&str, (&str, f64)>, caller_name: &str, imported_original_names: &HashMap<&str, &str>, +) -> Vec<&'a NodeInfo> { + let targets = resolve_call_targets_core( + ctx, call, rel_path, imported_from, type_map, caller_name, imported_original_names, + ); + if call.receiver.is_some() { + return targets; + } + let class_name = imported_original_names + .get(call.name.as_str()) + .copied() + .unwrap_or(call.name.as_str()); + attach_constructor_targets(ctx, targets, class_name) +} + +/// Core multi-strategy call target resolution — see `resolve_call_targets` for +/// the public entry point (which additionally applies constructor attribution). +fn resolve_call_targets_core<'a>( + ctx: &EdgeContext<'a>, + call: &CallInfo, + rel_path: &str, + imported_from: Option<&str>, + type_map: &HashMap<&str, (&str, f64)>, + caller_name: &str, + imported_original_names: &HashMap<&str, &str>, ) -> Vec<&'a NodeInfo> { // Flagged dynamic calls use synthetic names like "". Short-circuit // so they never accidentally match a real symbol via name lookup. @@ -1306,6 +1336,82 @@ fn resolve_call_targets<'a>( Vec::new() } +// ── Constructor-call attribution (#1892) ────────────────────────────────── + +/// Per-language constructor method identifier, keyed by file extension. Used +/// to build the qualified `ClassName.` lookup key that +/// attributes a `new ClassName()` (or bare `ClassName()`, for the keyword-less +/// languages below) call site to the class's own constructor **method**, +/// rather than only the class declaration node it already resolves to. +/// Returns `None` when the extension is unrecognised (or has no extension at +/// all). For Java/C#/Dart/Groovy the constructor's own identifier equals the +/// class name (`class Foo { Foo(...) {} }`), so those arms return +/// `class_name` itself rather than a fixed keyword. Mirrors +/// `CONSTRUCTOR_LOCAL_NAME_BY_EXTENSION` in strategy.ts. +/// +/// Deliberately excludes languages whose extractor does not emit an explicit +/// constructor definition at all (Kotlin, Swift, Scala) or does not track +/// object-construction call sites at all (C++) — for those, the class-node +/// edge already produced by `resolve_call_targets_core` is the only +/// attribution possible. +fn constructor_local_name<'b>(file: &str, class_name: &'b str) -> Option<&'b str> { + let ext = file.rsplit_once('.').map(|(_, e)| e)?; + match ext { + "js" | "mjs" | "cjs" | "jsx" | "ts" | "tsx" | "mts" | "cts" => Some("constructor"), + "py" | "pyi" => Some("__init__"), + "php" | "phtml" => Some("__construct"), + "java" | "cs" | "dart" | "groovy" | "gvy" => Some(class_name), + _ => None, + } +} + +/// Resolve the constructor **method** node for a class target, if the class +/// declares one explicitly. Scoped to the class's own file (`file`) so an +/// unrelated same-named constructor elsewhere can never match. +fn resolve_constructor_target<'a>( + ctx: &EdgeContext<'a>, + file: &str, + class_name: &str, +) -> Option<&'a NodeInfo> { + let local_name = constructor_local_name(file, class_name)?; + let qualified = format!("{}.{}", class_name, local_name); + ctx.nodes_by_name_and_file + .get(&(qualified.as_str(), file)) + .and_then(|v| v.iter().find(|n| n.kind == "method")) + .copied() +} + +/// Additive constructor-call attribution: for every `class`-kind target in +/// `targets`, also resolve that class's own constructor method (if one is +/// explicitly declared) and append it. Mirrors `attachConstructorTargets` in +/// strategy.ts. +/// +/// Additive, not a replacement: the class-node target is always left standing +/// — the DB-driven RTA fallback (incremental rebuilds, see `cha.ts`'s +/// `buildChaContextFromDb`) reads instantiation evidence from `calls` edges +/// targeting class-kind nodes, and a class with no explicit constructor +/// legitimately has nothing else to attribute the call to. +fn attach_constructor_targets<'a>( + ctx: &EdgeContext<'a>, + mut targets: Vec<&'a NodeInfo>, + class_name: &str, +) -> Vec<&'a NodeInfo> { + let mut seen_ids: HashSet = targets.iter().map(|t| t.id).collect(); + let mut extra = Vec::new(); + for target in &targets { + if target.kind != "class" { + continue; + } + if let Some(ctor) = resolve_constructor_target(ctx, target.file.as_str(), class_name) { + if seen_ids.insert(ctor.id) { + extra.push(ctor); + } + } + } + targets.extend(extra); + targets +} + /// Languages where bare `foo()` calls inside a class method are lexically scoped /// to the module, not the class — there is no implicit this/class binding. /// Mirrors `MODULE_SCOPED_BARE_CALL_EXTENSIONS` in call-resolver.ts. diff --git a/crates/codegraph-core/src/extractors/dart.rs b/crates/codegraph-core/src/extractors/dart.rs index 29d36e042..b424b1436 100644 --- a/crates/codegraph-core/src/extractors/dart.rs +++ b/crates/codegraph-core/src/extractors/dart.rs @@ -279,8 +279,20 @@ fn handle_dart_import(node: &Node, source: &[u8], symbols: &mut FileSymbols) { } fn handle_dart_constructor_call(node: &Node, source: &[u8], symbols: &mut FileSymbols) { + // tree-sitter-dart crate versions structure the type name differently: + // 0.2 (crates.io, current) nests it under an intermediate `type` field — + // `new_expression type: (type (type_identifier)) arguments: (...)` — + // while npm's tree-sitter-dart 1.x (used by the WASM engine, see + // check-grammar-versions.mjs's tracked exception for this crate) puts it + // directly under `new_expression` — `(new_expression (type_identifier) + // (arguments))`. Try the direct shape first, then unwrap one `type` level. let name_node = find_child(node, "type_identifier") - .or_else(|| find_child(node, "identifier")); + .or_else(|| find_child(node, "identifier")) + .or_else(|| { + node.child_by_field_name("type").or_else(|| find_child(node, "type")).and_then(|t| { + find_child(&t, "type_identifier").or_else(|| find_child(&t, "identifier")) + }) + }); if let Some(name) = name_node { symbols.calls.push(Call { name: node_text(&name, source).to_string(), diff --git a/src/domain/graph/builder/call-resolver.ts b/src/domain/graph/builder/call-resolver.ts index 466900162..a5878516a 100644 --- a/src/domain/graph/builder/call-resolver.ts +++ b/src/domain/graph/builder/call-resolver.ts @@ -12,6 +12,7 @@ import { CALLABLE_SYMBOL_KINDS } from '../../../shared/kinds.js'; import { computeConfidence, isSameLanguageFamily } from '../resolve.js'; import { + attachConstructorTargets, isModuleScopedLanguage, resolveByGlobal, resolveByReceiver, @@ -268,7 +269,10 @@ export function resolveCallTargets( typeMap: Map, callerName?: string | null, importedOriginalNames?: ReadonlyMap, -): { targets: Array<{ id: number; file: string }>; importedFrom: string | undefined } { +): { + targets: Array<{ id: number; file: string; kind?: string }>; + importedFrom: string | undefined; +} { // Flagged dynamic calls use synthetic names like ''. Short-circuit // so they never accidentally match a real symbol via lookup.byName. if (call.name.startsWith(' | undefined; + // Tracks the name actually used to find `targets`. Usually equal to + // `targetName`, but a barrel hop that itself renames the export + // (`export { Foo as Bar } from './foo'`, resolved below) reports the name + // truly declared in the origin file — the constructor-attribution lookup + // must key on that name, not the call site's (possibly barrel-aliased) + // `targetName`, or it builds a qualified name that doesn't exist (#1892). + let resolvedClassName = targetName; + let targets: ReadonlyArray<{ id: number; file: string; kind?: string }> | undefined; if (importedFrom) { targets = lookup.byNameAndFile(targetName, importedFrom); if (targets.length === 0 && lookup.isBarrel(importedFrom)) { - const resolved = lookup.resolveBarrel(importedFrom, targetName); - if (resolved) { - targets = lookup.byNameAndFile(resolved.name, resolved.file); + const barrelResolved = lookup.resolveBarrel(importedFrom, targetName); + if (barrelResolved) { + targets = lookup.byNameAndFile(barrelResolved.name, barrelResolved.file); + resolvedClassName = barrelResolved.name; } } } @@ -322,7 +334,15 @@ export function resolveCallTargets( } } - const resolved = [...(targets ?? [])]; + let resolved = [...(targets ?? [])]; + // #1892: `new ClassName()` / bare `ClassName()` (keyword-less languages) + // always resolves as a bare (no-receiver) call — augment any class-kind + // match with the class's own constructor method, if it declares one. + // Uses `resolvedClassName` (not `targetName`) so a barrel rename doesn't + // make the qualified constructor lookup miss (see comment above). + if (!call.receiver) { + resolved = attachConstructorTargets(lookup, resolved, resolvedClassName); + } if (resolved.length > 1) { resolved.sort((a, b) => { const confA = computeConfidence(relPath, a.file, importedFrom ?? null); diff --git a/src/domain/graph/builder/stages/build-edges.ts b/src/domain/graph/builder/stages/build-edges.ts index 2167023f6..d85c9f9b9 100644 --- a/src/domain/graph/builder/stages/build-edges.ts +++ b/src/domain/graph/builder/stages/build-edges.ts @@ -1349,14 +1349,7 @@ function resolveFallbackTargets( // either idiom commonly names a class. Applied once here, after every // fallback tier above, so it covers whichever tier produced the match. if (call.dynamicKind === 'value-ref') { - // `targets` is typed without `kind` when it flows straight through from - // resolveCallTargets (call-resolver.ts's declared return type omits it), - // but every underlying CallNodeLookup method actually populates it — the - // same gap the preQualifiedTargets cast above already works around. Kept - // as its own step (not folded into the filter callback) so the type-gap - // workaround and the actual filtering decision stay visually distinct. - const typedTargets = targets as ReadonlyArray<{ id: number; file: string; kind?: string }>; - targets = typedTargets.filter( + targets = targets.filter( (t) => t.kind === 'function' || t.kind === 'method' || t.kind === 'class', ); } diff --git a/src/domain/graph/resolver/strategy.ts b/src/domain/graph/resolver/strategy.ts index 8d6ae36b3..1416201c6 100644 --- a/src/domain/graph/resolver/strategy.ts +++ b/src/domain/graph/resolver/strategy.ts @@ -58,6 +58,111 @@ export function isModuleScopedLanguage(relPath: string): boolean { return MODULE_SCOPED_BARE_CALL_EXTENSIONS.has(ext); } +// ── Constructor-call attribution (#1892) ────────────────────────────────── + +/** + * Sentinel meaning "this language's constructor is named identically to its + * class" — Java, C#, Dart, and Groovy all require `class Foo { Foo(...) {} }`; + * the constructor's own identifier is the class name, not a fixed keyword. + */ +const CTOR_NAME_SAME_AS_CLASS = Symbol('ctor-name-same-as-class'); + +/** + * Per-language constructor method identifier, keyed by file extension. Used + * to build the qualified `ClassName.` lookup key that + * attributes a `new ClassName()` (or bare `ClassName()`, for the keyword-less + * languages below) call site to the class's own constructor **method**, + * rather than only the class declaration node it already resolves to (#1892). + * Mirrors `constructor_local_name` in build_edges.rs. + * + * Deliberately excludes languages whose extractor does not emit an explicit + * constructor definition at all (Kotlin, Swift, Scala — verified via their + * extractors' AST walk switches) or does not track object-construction call + * sites at all (C++) — for those, the class-node edge already produced by + * the tiers above is the only attribution possible. + */ +const CONSTRUCTOR_LOCAL_NAME_BY_EXTENSION = new Map< + string, + string | typeof CTOR_NAME_SAME_AS_CLASS +>([ + ['.js', 'constructor'], + ['.mjs', 'constructor'], + ['.cjs', 'constructor'], + ['.jsx', 'constructor'], + ['.ts', 'constructor'], + ['.tsx', 'constructor'], + ['.mts', 'constructor'], + ['.cts', 'constructor'], + ['.py', '__init__'], + ['.pyi', '__init__'], + ['.php', '__construct'], + ['.phtml', '__construct'], + ['.java', CTOR_NAME_SAME_AS_CLASS], + ['.cs', CTOR_NAME_SAME_AS_CLASS], + ['.dart', CTOR_NAME_SAME_AS_CLASS], + ['.groovy', CTOR_NAME_SAME_AS_CLASS], + ['.gvy', CTOR_NAME_SAME_AS_CLASS], +]); + +function constructorLocalName(file: string, className: string): string | null { + const dotIdx = file.lastIndexOf('.'); + if (dotIdx === -1) return null; + const entry = CONSTRUCTOR_LOCAL_NAME_BY_EXTENSION.get(file.slice(dotIdx)); + if (entry === undefined) return null; + return entry === CTOR_NAME_SAME_AS_CLASS ? className : entry; +} + +/** + * Resolve the constructor **method** node for a class target, if the class + * declares one explicitly. Scoped to the class's own file (`classTarget.file`) + * so an unrelated same-named constructor elsewhere can never match. + */ +function resolveConstructorTarget( + lookup: StrategyLookup, + classTarget: { file: string }, + className: string, +): { id: number; file: string; kind?: string } | null { + const ctorLocalName = constructorLocalName(classTarget.file, className); + if (!ctorLocalName) return null; + const found = lookup + .byNameAndFile(`${className}.${ctorLocalName}`, classTarget.file) + .find((n) => n.kind === 'method'); + return found ?? null; +} + +/** + * Additive constructor-call attribution (#1892): for every `class`-kind + * target in `targets`, also resolve that class's own constructor method (if + * one is explicitly declared) and append it. + * + * Additive, not a replacement: the class-node target is always left standing + * — `buildChaContextFromDb`'s RTA fallback (incremental rebuilds, see + * `builder/cha.ts`) reads instantiation evidence from `calls` edges targeting + * class-kind nodes, and a class with no explicit constructor legitimately has + * nothing else to attribute the call to. + * + * Only meaningful for bare (no-receiver) calls — a `new ClassName()` / + * `ClassName()` construction call never carries a receiver — so callers must + * gate on `!call.receiver` before invoking this. + */ +export function attachConstructorTargets( + lookup: StrategyLookup, + targets: readonly T[], + className: string, +): T[] { + const augmented = [...targets]; + const seenIds = new Set(targets.map((t) => t.id)); + for (const target of targets) { + if (target.kind !== 'class') continue; + const ctorTarget = resolveConstructorTarget(lookup, target, className); + if (ctorTarget && !seenIds.has(ctorTarget.id)) { + augmented.push(ctorTarget as T); + seenIds.add(ctorTarget.id); + } + } + return augmented; +} + // ── typeMap entry unwrapping ────────────────────────────────────────────────── /** diff --git a/src/extractors/dart.ts b/src/extractors/dart.ts index 2b166f8d5..0dd0955c3 100644 --- a/src/extractors/dart.ts +++ b/src/extractors/dart.ts @@ -241,7 +241,24 @@ function handleDartImport(node: TreeSitterNode, ctx: ExtractorOutput): void { } function handleDartConstructorCall(node: TreeSitterNode, ctx: ExtractorOutput): void { - const nameNode = findChild(node, 'type_identifier') || findChild(node, 'identifier'); + // tree-sitter-dart crate versions structure the type name differently: + // 0.2 (crates.io, used by the native engine — see check-grammar-versions.mjs's + // tracked exception for this crate) nests it under an intermediate `type` + // field — `new_expression type: (type (type_identifier)) arguments: (...)` — + // while npm's tree-sitter-dart 1.x (this WASM engine's grammar) puts it + // directly under `new_expression` — `(new_expression (type_identifier) + // (arguments))`. Try the direct shape first, then unwrap one `type` level, + // so both engines resolve identically once the crates.io release catches up. + const nameNode = + findChild(node, 'type_identifier') || + findChild(node, 'identifier') || + (() => { + const typeWrapper = node.childForFieldName('type') || findChild(node, 'type'); + return ( + typeWrapper && + (findChild(typeWrapper, 'type_identifier') || findChild(typeWrapper, 'identifier')) + ); + })(); if (!nameNode) return; ctx.calls.push({ diff --git a/tests/benchmarks/resolution/fixtures/csharp/expected-edges.json b/tests/benchmarks/resolution/fixtures/csharp/expected-edges.json index c7c149c45..08b3c17e2 100644 --- a/tests/benchmarks/resolution/fixtures/csharp/expected-edges.json +++ b/tests/benchmarks/resolution/fixtures/csharp/expected-edges.json @@ -66,6 +66,13 @@ "mode": "constructor", "notes": "new UserService(repo) — constructor with dependency injection" }, + { + "source": { "name": "Program.Main", "file": "Program.cs" }, + "target": { "name": "UserService.UserService", "file": "Service.cs" }, + "kind": "calls", + "mode": "constructor", + "notes": "new UserService(repo) — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "Program.Main", "file": "Program.cs" }, "target": { "name": "User", "file": "Repository.cs" }, @@ -115,6 +122,13 @@ "mode": "constructor", "notes": "new UserService(repo) — constructor with dependency injection" }, + { + "source": { "name": "Program.RunWithValidation", "file": "Program.cs" }, + "target": { "name": "UserService.UserService", "file": "Service.cs" }, + "kind": "calls", + "mode": "constructor", + "notes": "new UserService(repo) — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "Program.RunWithValidation", "file": "Program.cs" }, "target": { "name": "User", "file": "Repository.cs" }, diff --git a/tests/benchmarks/resolution/fixtures/dart/expected-edges.json b/tests/benchmarks/resolution/fixtures/dart/expected-edges.json index bc02074b3..8db6e94f7 100644 --- a/tests/benchmarks/resolution/fixtures/dart/expected-edges.json +++ b/tests/benchmarks/resolution/fixtures/dart/expected-edges.json @@ -17,6 +17,13 @@ "mode": "constructor", "notes": "UserService(repo) — constructor call" }, + { + "source": { "name": "main", "file": "main.dart" }, + "target": { "name": "UserService.UserService", "file": "service.dart" }, + "kind": "calls", + "mode": "constructor", + "notes": "UserService(repo) — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "main", "file": "main.dart" }, "target": { "name": "validateEmail", "file": "validators.dart" }, @@ -59,6 +66,13 @@ "mode": "constructor", "notes": "Order() — constructor call to create Order instance" }, + { + "source": { "name": "main", "file": "main.dart" }, + "target": { "name": "Order.Order", "file": "models.dart" }, + "kind": "calls", + "mode": "constructor", + "notes": "Order() — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "main", "file": "main.dart" }, "target": { "name": "validateAmount", "file": "validators.dart" }, @@ -87,6 +101,13 @@ "mode": "constructor", "notes": "User() — constructor call to create User instance" }, + { + "source": { "name": "UserService.createUser", "file": "service.dart" }, + "target": { "name": "User.User", "file": "models.dart" }, + "kind": "calls", + "mode": "constructor", + "notes": "User() — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "UserService.createUser", "file": "service.dart" }, "target": { "name": "UserRepository.save", "file": "repository.dart" }, diff --git a/tests/benchmarks/resolution/fixtures/dynamic-typescript/expected-edges.json b/tests/benchmarks/resolution/fixtures/dynamic-typescript/expected-edges.json index ff67e575d..72de91217 100644 --- a/tests/benchmarks/resolution/fixtures/dynamic-typescript/expected-edges.json +++ b/tests/benchmarks/resolution/fixtures/dynamic-typescript/expected-edges.json @@ -17,6 +17,13 @@ "mode": "dynamic", "notes": "Reflect.construct(UserService, args) — reflection kind, resolves to UserService class" }, + { + "source": { "name": "runReflectConstruct", "file": "reflect.ts" }, + "target": { "name": "UserService.constructor", "file": "reflect.ts" }, + "kind": "calls", + "mode": "dynamic", + "notes": "Reflect.construct(UserService, args) — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "runReflectGetLiteral", "file": "reflect.ts" }, "target": { "name": "greet", "file": "reflect.ts" }, diff --git a/tests/benchmarks/resolution/fixtures/javascript/expected-edges.json b/tests/benchmarks/resolution/fixtures/javascript/expected-edges.json index 581bfd3c7..bee806617 100644 --- a/tests/benchmarks/resolution/fixtures/javascript/expected-edges.json +++ b/tests/benchmarks/resolution/fixtures/javascript/expected-edges.json @@ -115,6 +115,13 @@ "mode": "constructor", "notes": "new UserService() — class instantiation tracked as consumption" }, + { + "source": { "name": "directInstantiation", "file": "index.js" }, + "target": { "name": "UserService.constructor", "file": "service.js" }, + "kind": "calls", + "mode": "constructor", + "notes": "new UserService() — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "UserService.constructor", "file": "service.js" }, "target": { "name": "Logger", "file": "logger.js" }, @@ -122,6 +129,13 @@ "mode": "constructor", "notes": "new Logger('UserService') — class instantiation in constructor" }, + { + "source": { "name": "UserService.constructor", "file": "service.js" }, + "target": { "name": "Logger.constructor", "file": "logger.js" }, + "kind": "calls", + "mode": "constructor", + "notes": "new Logger('UserService') — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "buildService", "file": "service.js" }, "target": { "name": "UserService", "file": "service.js" }, @@ -129,6 +143,13 @@ "mode": "constructor", "notes": "new UserService() — class instantiation tracked as consumption" }, + { + "source": { "name": "buildService", "file": "service.js" }, + "target": { "name": "UserService.constructor", "file": "service.js" }, + "kind": "calls", + "mode": "constructor", + "notes": "new UserService() — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "_defProp", "file": "define-property.js" }, "target": { "name": "f1", "file": "define-property.js" }, diff --git a/tests/benchmarks/resolution/fixtures/php/expected-edges.json b/tests/benchmarks/resolution/fixtures/php/expected-edges.json index e40b02877..7c852a8f0 100644 --- a/tests/benchmarks/resolution/fixtures/php/expected-edges.json +++ b/tests/benchmarks/resolution/fixtures/php/expected-edges.json @@ -66,6 +66,13 @@ "mode": "constructor", "notes": "new UserService($repo) — constructor with dependency injection" }, + { + "source": { "name": "main", "file": "index.php" }, + "target": { "name": "UserService.__construct", "file": "Service.php" }, + "kind": "calls", + "mode": "constructor", + "notes": "new UserService($repo) — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "main", "file": "index.php" }, "target": { "name": "User", "file": "Repository.php" }, @@ -73,6 +80,13 @@ "mode": "constructor", "notes": "new User('1', 'Alice', 'alice@example.com') — class instantiation" }, + { + "source": { "name": "main", "file": "index.php" }, + "target": { "name": "User.__construct", "file": "Repository.php" }, + "kind": "calls", + "mode": "constructor", + "notes": "new User('1', 'Alice', 'alice@example.com') — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "main", "file": "index.php" }, "target": { "name": "Validators.isValidEmail", "file": "Validators.php" }, @@ -115,6 +129,13 @@ "mode": "constructor", "notes": "new UserService($repo) — constructor with dependency injection" }, + { + "source": { "name": "runWithValidation", "file": "index.php" }, + "target": { "name": "UserService.__construct", "file": "Service.php" }, + "kind": "calls", + "mode": "constructor", + "notes": "new UserService($repo) — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "runWithValidation", "file": "index.php" }, "target": { "name": "User", "file": "Repository.php" }, @@ -122,6 +143,13 @@ "mode": "constructor", "notes": "new User('2', 'Bob', 'bob@example.com') — class instantiation" }, + { + "source": { "name": "runWithValidation", "file": "index.php" }, + "target": { "name": "User.__construct", "file": "Repository.php" }, + "kind": "calls", + "mode": "constructor", + "notes": "new User('2', 'Bob', 'bob@example.com') — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "runWithValidation", "file": "index.php" }, "target": { "name": "Validators.validateUser", "file": "Validators.php" }, diff --git a/tests/benchmarks/resolution/fixtures/python/expected-edges.json b/tests/benchmarks/resolution/fixtures/python/expected-edges.json index e036e5b32..aab0f8052 100644 --- a/tests/benchmarks/resolution/fixtures/python/expected-edges.json +++ b/tests/benchmarks/resolution/fixtures/python/expected-edges.json @@ -38,6 +38,13 @@ "mode": "constructor", "notes": "Order('o1', ...) — class instantiation" }, + { + "source": { "name": "run", "file": "main.py" }, + "target": { "name": "Order.__init__", "file": "models.py" }, + "kind": "calls", + "mode": "constructor", + "notes": "Order('o1', ...) — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "run", "file": "main.py" }, "target": { "name": "Order.validate", "file": "models.py" }, @@ -59,6 +66,13 @@ "mode": "constructor", "notes": "User(user_id, name, email) — class instantiation via import" }, + { + "source": { "name": "UserService.create_user", "file": "service.py" }, + "target": { "name": "User.__init__", "file": "models.py" }, + "kind": "calls", + "mode": "constructor", + "notes": "User(user_id, name, email) — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "UserService.create_user", "file": "service.py" }, "target": { "name": "User.validate", "file": "models.py" }, @@ -101,12 +115,26 @@ "mode": "constructor", "notes": "UserService(repo) — same-file class instantiation" }, + { + "source": { "name": "build_service", "file": "service.py" }, + "target": { "name": "UserService.__init__", "file": "service.py" }, + "kind": "calls", + "mode": "constructor", + "notes": "UserService(repo) — also attributes to the constructor method itself (#1892)" + }, { "source": { "name": "create_repository", "file": "repository.py" }, "target": { "name": "UserRepository", "file": "repository.py" }, "kind": "calls", "mode": "constructor", "notes": "UserRepository() — same-file class instantiation" + }, + { + "source": { "name": "create_repository", "file": "repository.py" }, + "target": { "name": "UserRepository.__init__", "file": "repository.py" }, + "kind": "calls", + "mode": "constructor", + "notes": "UserRepository() — also attributes to the constructor method itself (#1892)" } ] } diff --git a/tests/integration/issue-1892-constructor-call-attribution.test.ts b/tests/integration/issue-1892-constructor-call-attribution.test.ts new file mode 100644 index 000000000..dd5d5fb01 --- /dev/null +++ b/tests/integration/issue-1892-constructor-call-attribution.test.ts @@ -0,0 +1,372 @@ +/** + * Regression test for #1892: `new ClassName()` (and bare `ClassName()` for + * languages with no `new` keyword) resolved only to the class declaration + * node, never to the class's own constructor **method** — even when the + * class declares one explicitly. `fn-impact`/`roles --role dead` therefore + * always reported 0 dependents / role `dead` for every constructor in the + * codebase, no matter how many call sites actually constructed the class. + * + * Root cause: the bare-name call site (`{ name: 'Foo' }`, no receiver) always + * matched the class node (stored under the bare name `Foo`), never the + * constructor method (stored under a language-specific qualified name — + * `Foo.constructor` for JS/TS, `Foo.__init__` for Python, `Foo.Foo` for + * Java/C#/Dart/Groovy, whose constructor identifier equals the class name). + * + * Fix: `resolveCallTargets` (call-resolver.ts) / `resolve_call_targets` + * (build_edges.rs) now additionally resolve the class's own constructor + * method and, when found, emit a second `calls` edge to it alongside the + * pre-existing class-node edge (`attachConstructorTargets` / + * `attach_constructor_targets`). The class-node edge is intentionally kept — + * `buildChaContextFromDb`'s RTA fallback (incremental rebuilds) reads + * instantiation evidence from `calls` edges targeting class-kind nodes. + * + * A class with no explicit constructor (`Bar`/`Qux`/`Corge`/`Garply`/`Fred`/ + * `Xyzzy` below) must still resolve only to the class node — there is no + * method to attribute the call to, and the fix must not fabricate one. + * + * Covers all three constructor-naming families named in + * `CONSTRUCTOR_LOCAL_NAME_BY_EXTENSION` (strategy.ts): keyword-fixed + * (JS/TS `constructor`, Python `__init__`, PHP `__construct`) and + * class-name-identical (Java, Dart, Groovy — C# is covered by the shared + * benchmark fixtures, see resolution-benchmark.test.ts). + */ +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 { isNativeAvailable } from '../../src/infrastructure/native.js'; +import type { EngineMode } from '../../src/types.js'; + +const JS_FIXTURE = ` +class Foo { + constructor() { + this.value = 1; + } +} + +class Bar {} + +const foo = new Foo(); +const bar = new Bar(); +`; + +// Each construction is wrapped in its own top-level function rather than a +// bare module-level assignment: two SCREAMING_CASE module constants without +// individual endLine bounds collide in findEnclosingBinding's widest-span +// tie-break (see #2027, filed separately — unrelated to this fix), which +// would make this fixture assert the wrong caller name. +const PY_FIXTURE = ` +class Baz: + def __init__(self): + self.value = 1 + +class Qux: + pass + +def make_baz(): + return Baz() + +def make_qux(): + return Qux() +`; + +const JAVA_FIXTURE = ` +class Quux { + Quux() { + } +} + +class Corge { +} + +class Wrapper { + void run() { + Quux quux = new Quux(); + Corge corge = new Corge(); + } +} +`; + +// PHP's constructor keyword (__construct) is a fixed identifier unlike Java's +// "same as class name" convention — a typo in CONSTRUCTOR_LOCAL_NAME_BY_EXTENSION +// or a mismatch between the TS and Rust arms would go undetected without this. +const PHP_FIXTURE = ``) body rather than a `{ ... }` +// block spanning multiple lines — a block-bodied function/method's +// `endLine` is currently truncated to its signature line, which makes +// any call in its body (on a later line) fall outside the recorded +// [line, endLine] span and get attributed to the file instead of the +// enclosing function during graph build. +// 3. Waldo's constructor has an (empty) `{ }` block body rather than the +// semicolon-only `Waldo();` short form the benchmark fixtures use +// elsewhere — a bodyless constructor isn't extracted as a definition +// at all currently, so it would never be found for attribution. +const DART_FIXTURE = ` +class Waldo { + Waldo() { + } +} + +class Fred { +} + +Waldo makeWaldo() => new Waldo(); + +Fred makeFred() => new Fred(); +`; + +// Groovy is also "same as class name". Constructions wrapped in a method +// (mirroring the Java Wrapper.run() fixture) rather than a bare top-level +// function — Groovy scripts conventionally scope executable statements inside +// a class method (see the resolution-benchmark Main.groovy fixture). +const GROOVY_FIXTURE = ` +class Plugh { + Plugh() { + } +} + +class Xyzzy { +} + +class GroovyWrapper { + void run() { + def plugh = new Plugh() + def xyzzy = new Xyzzy() + } +} +`; + +interface CallEdgeRow { + src: string; + srcKind: string; + tgt: string; + tgtKind: string; +} + +function readCallEdges(dbPath: string): CallEdgeRow[] { + const db = new Database(dbPath, { readonly: true }); + try { + return db + .prepare( + `SELECT n1.name AS src, n1.kind AS srcKind, n2.name AS tgt, n2.kind AS tgtKind + FROM edges e + JOIN nodes n1 ON e.source_id = n1.id + JOIN nodes n2 ON e.target_id = n2.id + WHERE e.kind = 'calls' + ORDER BY n1.name, n2.name`, + ) + .all() as CallEdgeRow[]; + } finally { + db.close(); + } +} + +function runScenario(engine: EngineMode): void { + describe(`constructor-call attribution (#1892) — ${engine}`, () => { + let dir: string; + let edges: CallEdgeRow[]; + + beforeAll(async () => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), `cg-1892-${engine}-`)); + fs.writeFileSync(path.join(dir, 'repro.js'), JS_FIXTURE); + fs.writeFileSync(path.join(dir, 'repro.py'), PY_FIXTURE); + fs.writeFileSync(path.join(dir, 'Repro.java'), JAVA_FIXTURE); + fs.writeFileSync(path.join(dir, 'repro.php'), PHP_FIXTURE); + fs.writeFileSync(path.join(dir, 'repro.dart'), DART_FIXTURE); + fs.writeFileSync(path.join(dir, 'repro.groovy'), GROOVY_FIXTURE); + await buildGraph(dir, { engine, incremental: false, skipRegistry: true }); + edges = readCallEdges(path.join(dir, '.codegraph', 'graph.db')); + }, 30_000); + + afterAll(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('JS/TS: new Foo() resolves to both Foo (class) and Foo.constructor (method)', () => { + expect(edges).toContainEqual({ + src: 'foo', + srcKind: 'constant', + tgt: 'Foo', + tgtKind: 'class', + }); + expect(edges).toContainEqual({ + src: 'foo', + srcKind: 'constant', + tgt: 'Foo.constructor', + tgtKind: 'method', + }); + }); + + it('JS/TS: new Bar() resolves only to Bar (class) — no explicit constructor to attribute to', () => { + expect(edges).toContainEqual({ + src: 'bar', + srcKind: 'constant', + tgt: 'Bar', + tgtKind: 'class', + }); + expect(edges.some((e) => e.src === 'bar' && e.tgt === 'Bar.constructor')).toBe(false); + }); + + it('Python: Baz() resolves to both Baz (class) and Baz.__init__ (method)', () => { + expect(edges).toContainEqual({ + src: 'make_baz', + srcKind: 'function', + tgt: 'Baz', + tgtKind: 'class', + }); + expect(edges).toContainEqual({ + src: 'make_baz', + srcKind: 'function', + tgt: 'Baz.__init__', + tgtKind: 'method', + }); + }); + + it('Python: Qux() resolves only to Qux (class) — no explicit __init__ to attribute to', () => { + expect(edges).toContainEqual({ + src: 'make_qux', + srcKind: 'function', + tgt: 'Qux', + tgtKind: 'class', + }); + expect(edges.some((e) => e.src === 'make_qux' && e.tgt === 'Qux.__init__')).toBe(false); + }); + + it('Java: new Quux() resolves to both Quux (class) and Quux.Quux (constructor method)', () => { + expect(edges).toContainEqual({ + src: 'Wrapper.run', + srcKind: 'method', + tgt: 'Quux', + tgtKind: 'class', + }); + expect(edges).toContainEqual({ + src: 'Wrapper.run', + srcKind: 'method', + tgt: 'Quux.Quux', + tgtKind: 'method', + }); + }); + + it('Java: new Corge() resolves only to Corge (class) — no explicit constructor to attribute to', () => { + expect(edges).toContainEqual({ + src: 'Wrapper.run', + srcKind: 'method', + tgt: 'Corge', + tgtKind: 'class', + }); + expect(edges.some((e) => e.src === 'Wrapper.run' && e.tgt === 'Corge.Corge')).toBe(false); + }); + + it('PHP: new Grault() resolves to both Grault (class) and Grault.__construct (method)', () => { + expect(edges).toContainEqual({ + src: 'makeGrault', + srcKind: 'function', + tgt: 'Grault', + tgtKind: 'class', + }); + expect(edges).toContainEqual({ + src: 'makeGrault', + srcKind: 'function', + tgt: 'Grault.__construct', + tgtKind: 'method', + }); + }); + + it('PHP: new Garply() resolves only to Garply (class) — no explicit __construct to attribute to', () => { + expect(edges).toContainEqual({ + src: 'makeGarply', + srcKind: 'function', + tgt: 'Garply', + tgtKind: 'class', + }); + expect(edges.some((e) => e.src === 'makeGarply' && e.tgt === 'Garply.__construct')).toBe( + false, + ); + }); + + it('Dart: Waldo() resolves to both Waldo (class) and Waldo.Waldo (constructor method)', () => { + expect(edges).toContainEqual({ + src: 'makeWaldo', + srcKind: 'function', + tgt: 'Waldo', + tgtKind: 'class', + }); + expect(edges).toContainEqual({ + src: 'makeWaldo', + srcKind: 'function', + tgt: 'Waldo.Waldo', + tgtKind: 'method', + }); + }); + + it('Dart: Fred() resolves only to Fred (class) — no explicit constructor to attribute to', () => { + expect(edges).toContainEqual({ + src: 'makeFred', + srcKind: 'function', + tgt: 'Fred', + tgtKind: 'class', + }); + expect(edges.some((e) => e.src === 'makeFred' && e.tgt === 'Fred.Fred')).toBe(false); + }); + + it('Groovy: new Plugh() resolves to both Plugh (class) and Plugh.Plugh (constructor method)', () => { + expect(edges).toContainEqual({ + src: 'GroovyWrapper.run', + srcKind: 'method', + tgt: 'Plugh', + tgtKind: 'class', + }); + expect(edges).toContainEqual({ + src: 'GroovyWrapper.run', + srcKind: 'method', + tgt: 'Plugh.Plugh', + tgtKind: 'method', + }); + }); + + it('Groovy: new Xyzzy() resolves only to Xyzzy (class) — no explicit constructor to attribute to', () => { + expect(edges).toContainEqual({ + src: 'GroovyWrapper.run', + srcKind: 'method', + tgt: 'Xyzzy', + tgtKind: 'class', + }); + expect(edges.some((e) => e.src === 'GroovyWrapper.run' && e.tgt === 'Xyzzy.Xyzzy')).toBe( + false, + ); + }); + }); +} + +runScenario('wasm'); +describe.skipIf(!isNativeAvailable())('native engine coverage', () => { + runScenario('native'); +}); diff --git a/tests/unit/call-resolver.test.ts b/tests/unit/call-resolver.test.ts index 1747abc92..d9ba15409 100644 --- a/tests/unit/call-resolver.test.ts +++ b/tests/unit/call-resolver.test.ts @@ -719,3 +719,79 @@ describe('resolveCallTargets — same-file bare-name lookup kind filter (#1888)' expect(targets).toEqual([cls]); }); }); + +/** + * Regression test for #1892's barrel-rename gap (flagged in PR #2028 review): + * `attachConstructorTargets` must key its qualified `ClassName.ctorLocalName` + * lookup on the name truly declared in the target's own file, not the call + * site's (possibly barrel-aliased) name. `export { Foo as Bar } from './foo'` + * means `new Bar()` resolves the class node via `lookup.resolveBarrel`, + * which reports `{ file: 'foo.ts', name: 'Foo' }` — the constructor lookup + * must use that reported 'Foo', not the caller's 'Bar', or it builds a + * `Bar.constructor` key that can never match the stored `Foo.constructor`. + */ +describe('resolveCallTargets — constructor attribution through a renaming barrel (#1892)', () => { + function makeBarrelLookup( + sameFile: Record>, + barrelFiles: Set, + barrelExports: Record, + ): CallNodeLookup { + return { + byNameAndFile(name, file) { + return sameFile[`${name}:${file}`] ?? []; + }, + byName() { + return []; + }, + isBarrel(file) { + return barrelFiles.has(file); + }, + resolveBarrel(barrelFile, symbolName) { + return barrelExports[`${symbolName}:${barrelFile}`] ?? null; + }, + nodeId() { + return undefined; + }, + }; + } + + it('attributes new Bar() to Foo.constructor when the barrel renames Foo to Bar', () => { + const classFoo = { id: 10, file: 'foo.ts', kind: 'class' }; + const ctorFoo = { id: 11, file: 'foo.ts', kind: 'method' }; + const lookup = makeBarrelLookup( + { + 'Foo:foo.ts': [classFoo], + 'Foo.constructor:foo.ts': [ctorFoo], + }, + new Set(['barrel.ts']), + { 'Bar:barrel.ts': { file: 'foo.ts', name: 'Foo' } }, + ); + const importedNames = new Map([['Bar', 'barrel.ts']]); + const { targets } = resolveCallTargets( + lookup, + { name: 'Bar', receiver: undefined }, + 'caller.ts', + importedNames, + new Map(), + null, + ); + expect(targets).toEqual([classFoo, ctorFoo]); + }); + + it('does not fabricate a constructor edge when the barrel-renamed class has no explicit constructor', () => { + const classBaz = { id: 20, file: 'baz.ts', kind: 'class' }; + const lookup = makeBarrelLookup({ 'Baz:baz.ts': [classBaz] }, new Set(['barrel.ts']), { + 'Qux:barrel.ts': { file: 'baz.ts', name: 'Baz' }, + }); + const importedNames = new Map([['Qux', 'barrel.ts']]); + const { targets } = resolveCallTargets( + lookup, + { name: 'Qux', receiver: undefined }, + 'caller.ts', + importedNames, + new Map(), + null, + ); + expect(targets).toEqual([classBaz]); + }); +});