From 311cc40f44cbb8033f3b443501244d59a9da5363 Mon Sep 17 00:00:00 2001 From: Aaron Queen Date: Sun, 11 Oct 2026 15:42:28 -0600 Subject: [PATCH 1/2] fix(resolve): follow the declared parent when an inherited method is looked up The conformance fallback in resolve_method_on_type_at walked supertypes by their bare names. From the first parent on, `Base::m` matched a method on any `Base` in the project, and the parents of every same-named `Base` were pooled, so a call on a `Child` receiver could link to an unrelated package's `Base.m()` or skip a grandparent's `m()` for it. For Java, Kotlin, C#, Scala and Swift the walk now follows the extends and implements edges' target declarations. Where qualified names omit the package (Scala, Swift) a same-named type in another file limits members to the type's own file; a Kotlin extension member, qualified by its receiver's written name, still counts when that name is unambiguous. Other languages keep the name walk. --- CHANGELOG.md | 1 + .../supertype-declaration-resolution.test.ts | 213 ++++++++++++++++ codegraph-kernel/src/resolve/bound.rs | 236 +++++++++++++++--- 3 files changed, 421 insertions(+), 29 deletions(-) create mode 100644 __tests__/supertype-declaration-resolution.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e4b18bd07..ffe279653 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- **An inherited method no longer links to a same-named class in another package.** When a call on a `Child` reached `m()` through `Child extends Base`, the lookup past the first parent matched any class called `Base` in the project, so with an unrelated `Base` in another package the call could land on that package's `Base.m()`, or skip a grandparent's `m()` for it. The lookup now follows the parent the declaration names, for Java, Kotlin, C#, Scala and Swift, where a Java or Kotlin receiver comes from a factory call, or a C#, Scala or Swift receiver is typed. Dart, TypeScript, Python and C++ are unchanged: their wrong links did not reproduce, or Dart's member lookup matches parents by name elsewhere. - **Python calls on a local built through a module, or through one of its fields, now resolve.** `bp = pkg.Blueprint()`, `c.Blueprint()` and `core.Blueprint()` type the local like a class imported by name, and `app.config.from_mapping()` resolves on the class the field holds, as `self.config.from_mapping()` already did. On Flask and requests this adds 98 and 103 call edges (almost all from tests, none removed); a 30-edge sample was all correct. - **A derived TypeScript class no longer treats an unforwarded constructor parameter as a field.** In `class B extends A { constructor(req: Request) { super(); } }`, a call through `this.req` was resolved as if the parameter declared `req`, even when nothing passed it on. A plain constructor parameter now names an inherited field only when the constructor hands it to `super(...)` as an argument; `super(options.req)` does not count. - **Explore's Relationships list marks a call matched by name alone.** `codegraph_node`'s trail and explore's Flow already label a hop the resolver could bind only by the callee's name as `[name match, unverified]`; the flat Relationships list (shown for repositories of 500 files or more) now does too, so every place explore draws a call edge shows the same warning. diff --git a/__tests__/supertype-declaration-resolution.test.ts b/__tests__/supertype-declaration-resolution.test.ts new file mode 100644 index 000000000..409f01ef6 --- /dev/null +++ b/__tests__/supertype-declaration-resolution.test.ts @@ -0,0 +1,213 @@ +/** + * A method inherited through `class Child extends Base` comes from the `Base` + * the declaration names, never from a same-named `Base` in another package or + * namespace. + * + * The conformance fallback walked supertypes by their bare names: from the + * first supertype on, `Base::m` matched a method on any `Base` in the project, + * and the supertypes of every same-named `Base` were pooled. A call on a + * `Child` receiver then linked to an unrelated package's `Base.m()`, or, when + * the real `Base` lacked `m` and a grandparent had it, skipped the grandparent + * for the decoy. The walk now follows the `extends`/`implements` edges' target + * declarations for the languages listed below. + * + * Each case runs with the decoy package sorting before and after the real + * one, since ties between same-named candidates fell to index order. + * Java and Kotlin reach the declaration through their bound-type path when a + * parameter or local holds the receiver, and are kept as guards there; a + * factory call's return type reaches the fallback. Scala and Swift keep a separate + * heuristic that links a method name only one type declares, so the + * "decoy has the only method" case is asserted for C#, Java and Kotlin alone. + */ +import { describe, it, expect, afterAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +const roots: string[] = []; +afterAll(() => { + for (const r of roots.splice(0)) fs.rmSync(r, { recursive: true, force: true }); +}); + +type Scenario = 'direct' | 'grand' | 'decoyOnly'; +interface Layout { + real: string; + decoy: string | null; + scenario: Scenario; +} + +const cap = (s: string) => s[0].toUpperCase() + s.slice(1); + +interface Lang { + name: string; + /** Files by relative path; `caller` names the one holding `c.m()`. */ + build(l: Layout): Record; + caller(l: Layout): string; + method: string; + /** Scenarios the language is held to. */ + scenarios: Scenario[]; +} + +const hasRoot = (l: Layout) => l.scenario === 'grand'; +const ownsMethod = (l: Layout) => l.scenario === 'direct'; +// 'decoyOnly': the real chain has no `m` at all; only the decoy declares one. + +const languages: Lang[] = [ + { + name: 'java', + method: 'm', + scenarios: ['direct', 'grand', 'decoyOnly'], + caller: () => 'main/Use.java', + build(l) { + const f: Record = {}; + if (hasRoot(l)) f[`${l.real}/Root.java`] = `package ${l.real};\npublic class Root { public void m() {} }\n`; + f[`${l.real}/Base.java`] = `package ${l.real};\npublic class Base${hasRoot(l) ? ' extends Root' : ''} { ${ownsMethod(l) ? 'public void m() {}' : ''} }\n`; + f[`${l.real}/Child.java`] = `package ${l.real};\npublic class Child extends Base { }\n`; + if (l.decoy) f[`${l.decoy}/Base.java`] = `package ${l.decoy};\npublic class Base { public void m() {} }\n`; + f['main/Use.java'] = `package main;\nimport ${l.real}.Child;\npublic class Use { void f(Child c) { c.m(); } }\n`; + return f; + }, + }, + { + name: 'kotlin', + method: 'm', + scenarios: ['direct', 'grand', 'decoyOnly'], + caller: () => 'main/Use.kt', + build(l) { + const f: Record = {}; + if (hasRoot(l)) f[`${l.real}/Root.kt`] = `package ${l.real}\nopen class Root { fun m() {} }\n`; + f[`${l.real}/Base.kt`] = `package ${l.real}\nopen class Base${hasRoot(l) ? '() : Root()' : ''} { ${ownsMethod(l) ? 'fun m() {}' : ''} }\n`; + f[`${l.real}/Child.kt`] = `package ${l.real}\nclass Child : Base()\n`; + if (l.decoy) f[`${l.decoy}/Base.kt`] = `package ${l.decoy}\nopen class Base { fun m() {} }\n`; + f['main/Use.kt'] = `package main\nimport ${l.real}.Child\nclass Use { fun f(c: Child) { c.m() } }\n`; + return f; + }, + }, + { + // A factory's return type names the receiver, which no binding holds. + name: 'java via a factory call', + method: 'm', + scenarios: ['direct', 'grand', 'decoyOnly'], + caller: () => 'main/Use.java', + build(l) { + const f = languages[0].build(l); + f['main/Use.java'] = `package main;\nimport ${l.real}.Child;\npublic class Use { static Child make() { return null; } void f() { Use.make().m(); } }\n`; + return f; + }, + }, + { + name: 'kotlin via a factory call', + method: 'm', + scenarios: ['direct', 'grand', 'decoyOnly'], + caller: () => 'main/Use.kt', + build(l) { + const f = languages[1].build(l); + f['main/Use.kt'] = `package main\nimport ${l.real}.Child\nobject Use { fun make(): Child = Child()\n fun f() { Use.make().m() } }\n`; + return f; + }, + }, + { + name: 'csharp', + method: 'M', + scenarios: ['direct', 'grand', 'decoyOnly'], + caller: () => 'main/Use.cs', + build(l) { + const f: Record = {}; + const ns = cap(l.real); + if (hasRoot(l)) f[`${l.real}/Root.cs`] = `namespace ${ns} { public class Root { public void M() {} } }\n`; + f[`${l.real}/Base.cs`] = `namespace ${ns} { public class Base${hasRoot(l) ? ' : Root' : ''} { ${ownsMethod(l) ? 'public void M() {}' : ''} } }\n`; + f[`${l.real}/Child.cs`] = `namespace ${ns} { public class Child : Base { } }\n`; + if (l.decoy) f[`${l.decoy}/Base.cs`] = `namespace ${cap(l.decoy)} { public class Base { public void M() {} } }\n`; + f['main/Use.cs'] = `using ${ns};\nnamespace Main { public class Use { void F(Child c) { c.M(); } } }\n`; + return f; + }, + }, + { + name: 'scala', + method: 'm', + scenarios: ['direct', 'grand'], + caller: () => 'main/Use.scala', + build(l) { + const f: Record = {}; + if (hasRoot(l)) f[`${l.real}/Root.scala`] = `package ${l.real}\nclass Root { def m(): Unit = {} }\n`; + f[`${l.real}/Base.scala`] = `package ${l.real}\nclass Base${hasRoot(l) ? ' extends Root' : ''} { ${ownsMethod(l) ? 'def m(): Unit = {}' : ''} }\n`; + f[`${l.real}/Child.scala`] = `package ${l.real}\nclass Child extends Base\n`; + if (l.decoy) f[`${l.decoy}/Base.scala`] = `package ${l.decoy}\nclass Base { def m(): Unit = {} }\n`; + f['main/Use.scala'] = `package main\nimport ${l.real}.Child\nclass Use { def f(c: Child): Unit = { c.m() } }\n`; + return f; + }, + }, + { + name: 'swift', + method: 'm', + scenarios: ['direct', 'grand'], + caller: () => 'main/Use.swift', + build(l) { + const f: Record = {}; + if (hasRoot(l)) f[`${l.real}/Root.swift`] = 'open class Root { public func m() {} }\n'; + f[`${l.real}/Base.swift`] = `open class Base${hasRoot(l) ? ': Root' : ''} { ${ownsMethod(l) ? 'public func m() {}' : ''} }\n`; + f[`${l.real}/Child.swift`] = 'open class Child: Base { }\n'; + if (l.decoy) f[`${l.decoy}/Base.swift`] = 'open class Base { public func m() {} }\n'; + f['main/Use.swift'] = 'func f(c: Child) { c.m() }\n'; + return f; + }, + }, +]; + +/** The file that must hold the callee, or none when the call has no project target. */ +function expectedFile(l: Layout, lang: Lang): string | null { + if (l.scenario === 'decoyOnly') return null; + const ext = Object.keys(lang.build(l))[0].split('.').pop(); + return `${l.real}/${l.scenario === 'grand' ? 'Root' : 'Base'}.${ext}`; +} + +async function callTargets(lang: Lang, l: Layout): Promise { + const root = fs.mkdtempSync(path.join(os.tmpdir(), `cg-supertype-${lang.name}-`)); + roots.push(root); + for (const [rel, content] of Object.entries(lang.build(l))) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + const cg = await CodeGraph.init(root, { index: true }); + try { + const out: string[] = []; + for (const n of cg.getNodesInFile(lang.caller(l))) { + for (const e of cg.getOutgoingEdges(n.id)) { + if (e.kind !== 'calls') continue; + const t = cg.getNode(e.target)!; + if (t.name === lang.method) out.push(t.filePath); + } + } + return out.sort(); + } finally { + cg.close(); + } +} + +// The decoy package sorts before the real one in one layout and after it in the other. +const orders = [ + { real: 'zreal', decoy: 'adecoy' }, + { real: 'areal', decoy: 'zdecoy' }, +]; + +describe('inherited method calls follow the declared supertype', () => { + for (const lang of languages) { + for (const scenario of lang.scenarios) { + for (const order of orders) { + const where = scenario === 'direct' ? 'on the parent' : scenario === 'grand' ? 'past a parent without it' : 'declared only by the other package'; + it(`${lang.name}: the method ${where} ignores a same-named type (${order.real} vs ${order.decoy})`, async () => { + const layout: Layout = { ...order, scenario }; + const expected = expectedFile(layout, lang); + expect(await callTargets(lang, layout)).toEqual(expected ? [expected] : []); + }); + if (scenario !== 'decoyOnly' && order === orders[0]) { + it(`${lang.name}: control without a same-named type keeps the edge (${scenario})`, async () => { + const layout: Layout = { real: order.real, decoy: null, scenario }; + expect(await callTargets(lang, layout)).toEqual([expectedFile(layout, lang)]); + }); + } + } + } + } +}); diff --git a/codegraph-kernel/src/resolve/bound.rs b/codegraph-kernel/src/resolve/bound.rs index 536ed641f..a9db3ca29 100644 --- a/codegraph-kernel/src/resolve/bound.rs +++ b/codegraph-kernel/src/resolve/bound.rs @@ -757,31 +757,8 @@ if site.language == "dart" && site.reference_kind == "calls" { /// class's parent is the one its declaration resolved to, never every /// same-named class in other namespaces. fn supertype_names(&mut self, type_name: &str, r: &ResolveRefIn, from_site: bool, qualified: bool) -> Res> { - let named = if qualified { self.nodes_by_qualified_name(type_name)? } else { self.nodes_by_name(type_name)? }; - let mut type_nodes: Vec> = named - .iter() - // Scala singletons can inherit members even though they cannot be parents. - .filter(|n| { - n.language == r.language - && (is_supertype_bearing_kind(&n.kind) || (n.language == "scala" && n.kind == "module")) - }) - .cloned() - .collect(); - // A name the site binds to one type keeps that type's declarations - // (partial or extended parts) and drops a same-named type in another - // namespace, package or module. - if from_site && type_nodes.len() > 1 { - if let Some(owner) = self.resolve_bound_type(type_name, r, 0)? { - if type_nodes.iter().any(|n| n.id == owner.id) { - type_nodes.retain(|n| same_declared_type(n, &owner)); - } - } - } let mut names: Vec = Vec::new(); - for tn in type_nodes { - if from_site && r.language == "java" && !self.java_type_visible(&tn, r)? { - continue; - } + for tn in self.supertype_type_nodes(type_name, r, from_site, qualified)? { // Ruby and PHP look in mixins before the parent class. let mut targets = self.mixin_nodes(&tn)?; for n in self.supertype_nodes(&tn.id)? { @@ -815,6 +792,40 @@ if site.language == "dart" && site.reference_kind == "calls" { Ok(names) } + /// The type declarations `type_name` names for the supertype walk: every + /// same-language, supertype-bearing node of that name (or qualified + /// name), narrowed as `supertype_names` describes. + fn supertype_type_nodes(&mut self, type_name: &str, r: &ResolveRefIn, from_site: bool, qualified: bool) -> Res>> { + let named = if qualified { self.nodes_by_qualified_name(type_name)? } else { self.nodes_by_name(type_name)? }; + let mut type_nodes: Vec> = named + .iter() + // Scala singletons can inherit members even though they cannot be parents. + .filter(|n| { + n.language == r.language + && (is_supertype_bearing_kind(&n.kind) || (n.language == "scala" && n.kind == "module")) + }) + .cloned() + .collect(); + // A name the site binds to one type keeps that type's declarations + // (partial or extended parts) and drops a same-named type in another + // namespace, package or module. + if from_site && type_nodes.len() > 1 { + if let Some(owner) = self.resolve_bound_type(type_name, r, 0)? { + if type_nodes.iter().any(|n| n.id == owner.id) { + type_nodes.retain(|n| same_declared_type(n, &owner)); + } + } + } + let mut out = Vec::with_capacity(type_nodes.len()); + for tn in type_nodes { + if from_site && r.language == "java" && !self.java_type_visible(&tn, r)? { + continue; + } + out.push(tn); + } + Ok(out) + } + /// swiftExtendedConformances (swift-type-visibility.ts): the types a /// Swift type's own declaration conforms to that the project only /// extends — `View` for `struct HomeView: View` when `extension View {}` @@ -1233,6 +1244,22 @@ Ok(self.dart_member_of(type_name, method, r, false)? // The conformance fallback: the method may live on a supertype // (transitively, depth-capped), still validated by name. if depth < 4 { + // A type in a language whose supertypes are told apart by + // their declaration is walked along its extends/implements + // edges, not by the bare names they carry. + if !qualified && walks_supertype_nodes(&r.language) { + let mut items: Vec = Vec::new(); + for tn in self.supertype_type_nodes(type_name, r, depth == 0, false)? { + let own = self.supertype_items(&tn)?; + for item in own { + if matches!(&item, SupertypeItem::Node(n) if n.id == tn.id) { + continue; + } + items.push(item); + } + } + return self.resolve_method_on_supertype_items(items, method, r, confidence, resolved_by, preferred_fqn, depth + 1); + } // PHP supertypes, and Ruby ones from a qualified start, come // back by qualified name. let php = r.language == "php" || (qualified && r.language == "ruby"); @@ -1246,29 +1273,161 @@ Ok(self.dart_member_of(type_name, method, r, false)? } return Ok(None); } + Ok(Self::pick_method_match(matches, r, confidence, resolved_by, preferred_fqn)) + } + + /// The method a non-empty set of same-named members settles on: the one + /// the call site's import names, else the one in its own file, else the + /// first. + fn pick_method_match( + matches: Vec>, + r: &ResolveRefIn, + confidence: f64, + resolved_by: &'static str, + preferred_fqn: Option<&str>, + ) -> Option { // A method value (#1820) never breaks a tie by call-site file. if r.reference_kind == "function_ref" && matches.len() != 1 { - return Ok(None); + return None; } if matches.len() > 1 { if let Some(fqn) = preferred_fqn { let ext = if r.language == "kotlin" { ".kt" } else { ".java" }; let fqn_path = format!("{}{}", fqn.replace('.', "/"), ext); if let Some(chosen) = matches.iter().find(|m| m.file_path.ends_with(&fqn_path)) { - return Ok(Some(KCand { + return Some(KCand { node: chosen.clone(), confidence, resolved_by, - })); + }); } } } let ordered = prefer_call_site_file(matches, &r.file_path); - Ok(Some(KCand { + Some(KCand { node: ordered[0].clone(), confidence, resolved_by, - })) + }) + } + + /// What a type's own declaration hands the supertype walk: the types its + /// `extends`/`implements` edges point at (mixins first), then, for Swift, + /// the conformances the project only extends, which have no node. + fn supertype_items(&mut self, tn: &KNode) -> Res> { + let mut targets = self.mixin_nodes(tn)?; + for n in self.supertype_nodes(&tn.id)? { + if !targets.iter().any(|t| t.id == n.id) { + targets.push(n); + } + } + let mut items: Vec = targets.into_iter().map(SupertypeItem::Node).collect(); + if tn.language == "swift" { + for name in self.swift_extended_conformances(tn)?.iter() { + if *name != tn.name && !items.iter().any(|i| matches!(i, SupertypeItem::Name(n) if n == name)) { + items.push(SupertypeItem::Name(name.clone())); + } + } + } + Ok(items) + } + + /// resolveMethodOnType's conformance fallback along the supertype nodes + /// themselves. A supertype is the declaration the edge points at, so a + /// same-named type in another package or namespace contributes neither + /// its members nor its own supertypes. + #[allow(clippy::too_many_arguments)] + fn resolve_method_on_supertype_items( + &mut self, + items: Vec, + method: &str, + r: &ResolveRefIn, + confidence: f64, + resolved_by: &'static str, + preferred_fqn: Option<&str>, + depth: u32, + ) -> Res> { + let mut tried: Vec = Vec::new(); + for item in items { + let node = match item { + SupertypeItem::Name(name) => { + if let Some(via) = self.resolve_method_on_type_at(&name, method, r, confidence, resolved_by, preferred_fqn, depth, false)? { + return Ok(Some(via)); + } + continue; + } + SupertypeItem::Node(n) => n, + }; + if tried.contains(&node.id) { + continue; + } + tried.push(node.id.clone()); + let own = self.type_node_methods(&node, method, r)?; + if !own.is_empty() { + return Ok(Self::pick_method_match(own, r, confidence, resolved_by, preferred_fqn)); + } + let inherits = node.language == r.language + && (is_supertype_bearing_kind(&node.kind) || (node.language == "scala" && node.kind == "module")); + if depth < 4 && inherits { + let next = self.supertype_items(&node)?; + if let Some(via) = self.resolve_method_on_supertype_items(next, method, r, confidence, resolved_by, preferred_fqn, depth + 1)? { + return Ok(Some(via)); + } + } + } + Ok(None) + } + + /// The methods named `method` that `ty` declares. Where qualified names + /// leave out the package (Scala, Swift), a second type of the same + /// qualified name elsewhere is a different type, so only the members in + /// `ty`'s own file count; with no such twin a member in another file is + /// an extension or companion of `ty` and counts. + fn type_node_methods(&mut self, ty: &KNode, method: &str, r: &ResolveRefIn) -> Res>> { + let mut members: Vec> = self + .nodes_by_qualified_name(&format!("{}::{}", ty.qualified_name, method))? + .iter() + .filter(|m| m.kind == "method" && same_language_family(&m.language, &r.language)) + .cloned() + .collect(); + // An extension member is qualified by its receiver type's written + // name (`Expression::castTo`), without the package. It is `ty`'s + // when no other declaration of that name could be the receiver. + if ty.qualified_name != ty.name { + let mut sole = true; + for n in self.nodes_by_name(&ty.name)?.iter() { + if n.language == ty.language && is_supertype_bearing_kind(&n.kind) && n.qualified_name != ty.qualified_name { + sole = false; + break; + } + } + if sole { + let extension: Vec> = self + .nodes_by_qualified_name(&format!("{}::{}", ty.name, method))? + .iter() + .filter(|m| m.kind == "method" && same_language_family(&m.language, &r.language)) + .cloned() + .collect(); + members.extend(extension); + } + } + if members.iter().any(|m| m.file_path != ty.file_path) && qualified_names_omit_package(&ty.language) { + let mut twins = 0; + let same_named: Vec> = self.nodes_by_qualified_name(&ty.qualified_name)?.iter().cloned().collect(); + for n in same_named { + if n.language == ty.language + && is_supertype_bearing_kind(&n.kind) + && n.file_path != ty.file_path + && !self.is_swift_extension(&n) + { + twins += 1; + } + } + if twins > 0 { + members.retain(|m| m.file_path == ty.file_path); + } + } + Ok(members) } /// inferJavaFieldReceiverType — the declared type of a `field` node in @@ -1494,6 +1653,25 @@ Ok(self.dart_member_of(type_name, method, r, false)? } } +/// One step of the supertype walk: the type an edge points at, or a Swift +/// conformance the project only extends and so has no declaration to point at. +enum SupertypeItem { + Node(Arc), + Name(String), +} + +/// Languages whose conformance fallback follows the edges' target nodes. The +/// rest still walk by bare type name. +fn walks_supertype_nodes(lang: &str) -> bool { + matches!(lang, "java" | "kotlin" | "csharp" | "scala" | "swift") +} + +/// Whether a type's qualified name leaves out its package or namespace, so +/// two types in different packages can share it. +fn qualified_names_omit_package(lang: &str) -> bool { + matches!(lang, "scala" | "swift") +} + /// Whether `n` declares the same type as `owner`: the same qualified name /// in the same scope. A TS/JS or Python type is scoped by its module file /// and a Go type by its package directory; qualified names carry the From d5855f5af22a5c11c717f1f640f2141db835c6fd Mon Sep 17 00:00:00 2001 From: Aaron Queen Date: Sun, 11 Oct 2026 16:00:12 -0600 Subject: [PATCH 2/2] fix(resolve): tell Scala twins apart by singleton and by lines, and say what the Swift case shows A Scala `object Base` in another package now counts as a same-named twin of the declared parent `class Base`, so its `Base::m` is no longer taken for the parent's. Scala package blocks that put two same-named types in one file leave only the members inside the declared parent's lines. The Swift test case no longer claims a separate package: the index does not model Swift modules, so its decoy is a same-named type in another file. --- CHANGELOG.md | 2 +- .../supertype-declaration-resolution.test.ts | 61 ++++++++++++++++--- codegraph-kernel/src/resolve/bound.rs | 34 +++++++---- 3 files changed, 77 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffe279653..e20fcf2bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,7 +99,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes -- **An inherited method no longer links to a same-named class in another package.** When a call on a `Child` reached `m()` through `Child extends Base`, the lookup past the first parent matched any class called `Base` in the project, so with an unrelated `Base` in another package the call could land on that package's `Base.m()`, or skip a grandparent's `m()` for it. The lookup now follows the parent the declaration names, for Java, Kotlin, C#, Scala and Swift, where a Java or Kotlin receiver comes from a factory call, or a C#, Scala or Swift receiver is typed. Dart, TypeScript, Python and C++ are unchanged: their wrong links did not reproduce, or Dart's member lookup matches parents by name elsewhere. +- **An inherited method no longer links to a same-named class in another package.** When a call on a `Child` reached `m()` through `Child extends Base`, the lookup past the first parent matched any class called `Base` in the project, so with an unrelated `Base` in another package the call could land on that package's `Base.m()`, or skip a grandparent's `m()` for it. The lookup now follows the parent the declaration names, for Java, Kotlin, C#, Scala and Swift, where a Java or Kotlin receiver comes from a factory call, or a C#, Scala or Swift receiver is typed. In Scala, a same-named object or a package block in the same file is told apart too; in Swift the other `Base` is one declared in another file, since modules are not modeled. Dart, TypeScript, Python and C++ are unchanged: their wrong links did not reproduce, or Dart's member lookup matches parents by name elsewhere. - **Python calls on a local built through a module, or through one of its fields, now resolve.** `bp = pkg.Blueprint()`, `c.Blueprint()` and `core.Blueprint()` type the local like a class imported by name, and `app.config.from_mapping()` resolves on the class the field holds, as `self.config.from_mapping()` already did. On Flask and requests this adds 98 and 103 call edges (almost all from tests, none removed); a 30-edge sample was all correct. - **A derived TypeScript class no longer treats an unforwarded constructor parameter as a field.** In `class B extends A { constructor(req: Request) { super(); } }`, a call through `this.req` was resolved as if the parameter declared `req`, even when nothing passed it on. A plain constructor parameter now names an inherited field only when the constructor hands it to `super(...)` as an argument; `super(options.req)` does not count. - **Explore's Relationships list marks a call matched by name alone.** `codegraph_node`'s trail and explore's Flow already label a hop the resolver could bind only by the callee's name as `[name match, unverified]`; the flat Relationships list (shown for repositories of 500 files or more) now does too, so every place explore draws a call edge shows the same warning. diff --git a/__tests__/supertype-declaration-resolution.test.ts b/__tests__/supertype-declaration-resolution.test.ts index 409f01ef6..bf36aa934 100644 --- a/__tests__/supertype-declaration-resolution.test.ts +++ b/__tests__/supertype-declaration-resolution.test.ts @@ -12,7 +12,9 @@ * declarations for the languages listed below. * * Each case runs with the decoy package sorting before and after the real - * one, since ties between same-named candidates fell to index order. + * one, since ties between same-named candidates fell to index order. Swift's + * decoy is a same-named type in another file, not another module: the index + * does not model Swift modules. * Java and Kotlin reach the declaration through their bound-type path when a * parameter or local holds the receiver, and are kept as guards there; a * factory call's return type reaches the fallback. Scala and Swift keep a separate @@ -139,7 +141,11 @@ const languages: Lang[] = [ }, }, { - name: 'swift', + // Swift has no package in the extractor's model (a directory does not make + // a module, and SwiftPM targets are not read), so the "decoy" is a second + // `Base` declared in another file. This shows the lookup follows the + // declaration the `extends` edge names, not module separation. + name: 'swift (same-named type in another file)', method: 'm', scenarios: ['direct', 'grand'], caller: () => 'main/Use.swift', @@ -162,21 +168,21 @@ function expectedFile(l: Layout, lang: Lang): string | null { return `${l.real}/${l.scenario === 'grand' ? 'Root' : 'Base'}.${ext}`; } -async function callTargets(lang: Lang, l: Layout): Promise { - const root = fs.mkdtempSync(path.join(os.tmpdir(), `cg-supertype-${lang.name}-`)); +async function callsFrom(name: string, files: Record, callerFile: string, method: string): Promise { + const root = fs.mkdtempSync(path.join(os.tmpdir(), `cg-supertype-${name.replace(/\W+/g, '-')}-`)); roots.push(root); - for (const [rel, content] of Object.entries(lang.build(l))) { + for (const [rel, content] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); fs.writeFileSync(path.join(root, rel), content); } const cg = await CodeGraph.init(root, { index: true }); try { const out: string[] = []; - for (const n of cg.getNodesInFile(lang.caller(l))) { + for (const n of cg.getNodesInFile(callerFile)) { for (const e of cg.getOutgoingEdges(n.id)) { if (e.kind !== 'calls') continue; const t = cg.getNode(e.target)!; - if (t.name === lang.method) out.push(t.filePath); + if (t.name === method) out.push(`${t.filePath}:${t.startLine}`); } } return out.sort(); @@ -185,6 +191,10 @@ async function callTargets(lang: Lang, l: Layout): Promise { } } +async function callTargets(lang: Lang, l: Layout): Promise { + return (await callsFrom(lang.name, lang.build(l), lang.caller(l), lang.method)).map((t) => t.replace(/:\d+$/, '')); +} + // The decoy package sorts before the real one in one layout and after it in the other. const orders = [ { real: 'zreal', decoy: 'adecoy' }, @@ -211,3 +221,40 @@ describe('inherited method calls follow the declared supertype', () => { } } }); + +/** + * Scala qualified names omit the package, so two `Base` types share `Base::m`. + * What tells them apart is the declaration: its file, or its lines when + * Scala 2 package blocks put both in one file. + */ +describe('Scala types that share a qualified name', () => { + const use = 'package main\nimport real.Child\nclass Use { def f(c: Child): Unit = { c.m() } }\n'; + const orders = [ + { real: 'zreal', decoy: 'adecoy' }, + { real: 'areal', decoy: 'zdecoy' }, + ]; + + for (const order of orders) { + it(`an object of the same name in another package is not the parent (${order.real} vs ${order.decoy})`, async () => { + const files = { + [`${order.real}/Base.scala`]: `package ${order.real}\nclass Root {\n def m(): Unit = {}\n}\nclass Base extends Root\n`, + [`${order.real}/Child.scala`]: `package ${order.real}\nclass Child extends Base\n`, + [`${order.decoy}/Base.scala`]: `package ${order.decoy}\nobject Base {\n def m(): Unit = {}\n}\n`, + 'main/Use.scala': use.replace('real.Child', `${order.real}.Child`), + }; + expect(await callsFrom('scala-object', files, 'main/Use.scala', 'm')).toEqual([`${order.real}/Base.scala:3`]); + }); + + it(`package blocks in one file keep the declared parent's members (${order.real} vs ${order.decoy})`, async () => { + const real = `package ${order.real} {\n class Root {\n def m(): Unit = {}\n }\n class Base extends Root\n class Child extends Base\n}\n`; + const decoy = `package ${order.decoy} {\n class Base {\n def m(): Unit = {}\n }\n}\n`; + for (const [first, second, rootLine] of [[real, decoy, 3], [decoy, real, 8]] as const) { + const files = { + 'lib/All.scala': first + second, + 'main/Use.scala': use.replace('real.Child', `${order.real}.Child`), + }; + expect(await callsFrom('scala-blocks', files, 'main/Use.scala', 'm')).toEqual([`lib/All.scala:${rootLine}`]); + } + }); + } +}); diff --git a/codegraph-kernel/src/resolve/bound.rs b/codegraph-kernel/src/resolve/bound.rs index a9db3ca29..86f4dfb0c 100644 --- a/codegraph-kernel/src/resolve/bound.rs +++ b/codegraph-kernel/src/resolve/bound.rs @@ -1380,9 +1380,10 @@ Ok(self.dart_member_of(type_name, method, r, false)? /// The methods named `method` that `ty` declares. Where qualified names /// leave out the package (Scala, Swift), a second type of the same - /// qualified name elsewhere is a different type, so only the members in - /// `ty`'s own file count; with no such twin a member in another file is - /// an extension or companion of `ty` and counts. + /// qualified name in another file is a different type, so only the members + /// in `ty`'s own file count, and one in the same file (a Scala package + /// block) leaves only the members inside `ty`'s lines; with no such twin a + /// member in another file is an extension or companion of `ty` and counts. fn type_node_methods(&mut self, ty: &KNode, method: &str, r: &ResolveRefIn) -> Res>> { let mut members: Vec> = self .nodes_by_qualified_name(&format!("{}::{}", ty.qualified_name, method))? @@ -1411,19 +1412,28 @@ Ok(self.dart_member_of(type_name, method, r, false)? members.extend(extension); } } - if members.iter().any(|m| m.file_path != ty.file_path) && qualified_names_omit_package(&ty.language) { - let mut twins = 0; + if !members.is_empty() && qualified_names_omit_package(&ty.language) { + let mut other_file = false; + let mut same_file = false; let same_named: Vec> = self.nodes_by_qualified_name(&ty.qualified_name)?.iter().cloned().collect(); for n in same_named { - if n.language == ty.language - && is_supertype_bearing_kind(&n.kind) - && n.file_path != ty.file_path - && !self.is_swift_extension(&n) - { - twins += 1; + // A Scala singleton of the same name is a twin too, unless it + // is the companion beside `ty`. + let type_like = is_supertype_bearing_kind(&n.kind) || (n.language == "scala" && n.kind == "module"); + if n.id == ty.id || n.language != ty.language || !type_like || self.is_swift_extension(&n) { + continue; + } + if n.file_path != ty.file_path { + other_file = true; + } else if n.kind == ty.kind { + same_file = true; } } - if twins > 0 { + if same_file { + // Scala package blocks put same-named types in one file, with + // the same qualified name: only the lines of `ty` are its own. + members.retain(|m| m.file_path == ty.file_path && m.start_line >= ty.start_line && m.end_line <= ty.end_line); + } else if other_file { members.retain(|m| m.file_path == ty.file_path); } }