mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-26 04:19:33 +08:00
fix(java,csharp): a call on a new object reaches the class the file imports, not a namesake (#2864)
PR #2854 sends Java's and C#'s no-argument new Foo().bar() calls, which
the chain conformance pass retries, to the constructed-receiver resolver.
That links them (mybatis-3 +265, Newtonsoft.Json, serilog and jellyfin
+36, all right), but the hint for which Foo it passed was a file path,
which the resolver reads for these languages as a dotted package name,
and C#'s using names namespaces, not classes. So of two classes named
Foo the first indexed one won, whatever the file imported: import b.Foo;
new Foo().bar() linked a.Foo::bar at 0.9, and so did a call the language
itself cannot resolve, after import a.*; import b.*;.
When several project classes share the name, Java and C# now pick as
the language does: Java the written package (new a.Foo()), the file's
single-type import, else the caller's own package; C# the one type the
file's namespaces and usings can see. Only that class's own method links,
otherwise nothing. A name only one class has resolves as before,
inherited methods included.
Validation: the new test fails 4 of 4 on main 44adef82 with the kernel
and with CODEGRAPH_KERNEL=0 (import b.Foo linked a.Foo::bar, and the
ambiguous calls linked a.Foo and A.Foo), and passes here with both;
new-expression-receiver and kernel-tsjs-parity pass. A/B against main,
kernel per arm: flask, mybatis-3, Newtonsoft.Json, serilog and jellyfin
byte-identical, so the 301 links PR #2854 added there stay, mybatis-3's
2 calls on a class with a same-named copy in another package among them,
which resolve to the caller's own package as before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
43a4f5b9f9
commit
d02a88d052
@@ -221,6 +221,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
- In Ruby, a class method, `def self.build` or a `def` inside `class << self`, is now told apart from an instance method of the same name. A call on the class, like `Widget.build`, links to the class method; a call on an instance, like `widget.build` after `widget = Widget.new`, links to the instance method; and a call without a receiver links to the one that matches the method it is written in. Before, both were stored as instance methods under one name, so a call linked to whichever the class declared first: in a service object with `def self.call` and `def call`, `card.call` on an instance linked to the class method. Re-index Ruby projects after upgrading.
|
||||
- In Java, Kotlin, C# and Scala, each overload of an interface or base-class method now links only to the overrides with the same parameters. Before, every overload linked to every method of that name in the implementing class, so `ExampleService.putTairValue(String)` also led to `putTairValue(List<String>, double)`, and `codegraph_explore`'s call path, callers and impact went into overloads that call never runs. Where a parameter is a type parameter, like `save(T entity)` implemented as `save(String entity)`, or the class is written in another language than its interface, the number of parameters decides; a method declared once on each side links as before. Re-index Java, Kotlin, C# and Scala projects after upgrading. Thanks @hxj1225 for the report and @danusha2345 for the pull request. (#2663)
|
||||
- In Java and TypeScript, a class that implements or extends a parameterized type, like `implements Store<String>`, `extends Base<Integer>`, an interface's `extends Store<T>` or TypeScript's `implements Store<string>`, is now linked to that type, so its methods get their interface-to-implementation links and `codegraph_explore` follows a call on the interface into the class. Before, such a supertype was read with its type arguments and matched nothing. Re-index Java and TypeScript projects after upgrading. Thanks @danusha2345, and @paiyahooJay for the earlier pull request. (#2705)
|
||||
- In Java and C#, a method called on a newly constructed object, like `new SqlSessionFactoryBuilder().build(reader)` or `new JsonSerializer().Serialize(writer, value)`, now links to that class's own or inherited method. When several packages or namespaces declare a class of that name, it links to the one the file imports, and to none when the file could mean either. Re-index Java and C# projects after upgrading.
|
||||
- In Kotlin, a call on what one of your functions returns, like `repository().pending(1)` or `val repo = repository(); repo.pending(1)` with a `private fun repository(): RealRepo` in a test class, now links to the method of the type that function declares, including one the type inherits or an extension on it. Before, the chained call went to whichever `pending` the name matched first, like a test double's in another package, and the local was guessed from its name, which reached the interface. The function is found in the calling class, a class it inherits from or one around it, or at the top level, and a nested type like `Request.Builder` is that type, not another class's `Builder`. When the declared type has no such method, the call now links to nothing rather than a method of the same name elsewhere; a type from outside your project is matched by name as before. Re-index Kotlin projects after upgrading. Thanks @mindcipherrr for the report. (#2805)
|
||||
- In JavaScript and TypeScript, a `super.run()` call now links to the method of the class's parent, or of the nearest ancestor that has it, like three.js's `super.copy(source)` reaching `Object3D.copy`, so `codegraph_explore`'s call path and impact follow an override into the method it builds on. Before, such a call inside a method of the same name was dropped, and elsewhere it went to whatever method had the name, often the class's own or a sibling class's in the same file. A call from a static method reaches the parent's static method, and a call to a parent from outside your project, like `HTMLElement`, `Set` or `Promise`, links nothing. Re-index JavaScript and TypeScript projects after upgrading. Thanks @Puma7 for the report and the pull request. (#2497)
|
||||
- In JavaScript and TypeScript, a named class expression, like `register(class Widget extends Base {…})`, `customElements.define('x-card', class XCard extends HTMLElement {…})` or `const Panel = class PanelImpl extends Base {…}`, is now indexed as a class with its own methods and the class it extends, so calls between its methods and its `super` calls link as a class declaration's do. Before, its methods were free functions or weren't indexed at all, and it extended nothing. Its name stays bound inside the expression alone, as JavaScript has it: `new Widget()` elsewhere links nothing, a constant, class or interface of the same name around it keeps its links, and code outside never reaches its methods by a guess. A class a CommonJS module exports, like webpack's `module.exports = class MultiCompiler {…}` or `exports.Store = class Store {…}`, is reached through `require` as a class declaration is, along with its static methods and the methods called on its instances. Re-index JavaScript and TypeScript projects after upgrading. Thanks @Puma7 for the report and the pull request. (#2499)
|
||||
|
||||
@@ -0,0 +1,97 @@
|
||||
/**
|
||||
* A Java or C# method called on a newly constructed object, `new Foo().bar()`,
|
||||
* links to the class the language picks when several packages or namespaces
|
||||
* declare one of that name: Java's single-type import, a written package
|
||||
* (`new a.Foo()`) or the caller's own package; C#'s one type the file's
|
||||
* namespaces and usings can see. When the language itself can't tell
|
||||
* (`import a.*; import b.*;`, `using A; using B;`), the call links nothing.
|
||||
* The no-argument `new Foo().bar()` reaches the constructed-receiver resolver
|
||||
* from the chain conformance pass (#2854), which took the first indexed class
|
||||
* of the name whatever the file imported.
|
||||
*/
|
||||
import { describe, it, expect, afterAll, beforeAll } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import CodeGraph from '../src/index';
|
||||
|
||||
const roots: string[] = [];
|
||||
const graphs: CodeGraph[] = [];
|
||||
|
||||
afterAll(() => {
|
||||
for (const graph of graphs) graph.close();
|
||||
for (const root of roots) fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
/** Index `files` and return, by calling file, the qualified names its calls link to. */
|
||||
async function callTargets(files: Record<string, string>): Promise<Map<string, string[]>> {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-new-receiver-twins-'));
|
||||
roots.push(root);
|
||||
for (const [file, text] of Object.entries(files)) {
|
||||
fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true });
|
||||
fs.writeFileSync(path.join(root, file), text);
|
||||
}
|
||||
const graph = await CodeGraph.init(root, { index: true });
|
||||
graphs.push(graph);
|
||||
const nodes = graph.getFiles().flatMap((f) => graph.getNodesInFile(f.path));
|
||||
const byId = new Map(nodes.map((n) => [n.id, n]));
|
||||
const out = new Map<string, string[]>();
|
||||
for (const e of graph.getOutgoingEdgesFrom(nodes.map((n) => n.id), ['calls'])) {
|
||||
const source = byId.get(e.source)!;
|
||||
const target = byId.get(e.target)!;
|
||||
out.set(source.filePath, [...(out.get(source.filePath) ?? []), target.qualifiedName].sort());
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
describe('a method called on a new object, when several classes share its name', () => {
|
||||
let java: Map<string, string[]>;
|
||||
let csharp: Map<string, string[]>;
|
||||
|
||||
beforeAll(async () => {
|
||||
java = await callTargets({
|
||||
'src/main/java/a/Foo.java': 'package a;\npublic class Foo { public int bar() { return 1; } }\n',
|
||||
'src/main/java/b/Foo.java': 'package b;\npublic class Foo { public int bar() { return 2; } }\n',
|
||||
'src/main/java/c/UseA.java': 'package c;\nimport a.Foo;\npublic class UseA { int f() { return new Foo().bar(); } }\n',
|
||||
'src/main/java/c/UseB.java': 'package c;\nimport b.Foo;\npublic class UseB { int f() { return new Foo().bar(); } }\n',
|
||||
'src/main/java/b/Local.java': 'package b;\npublic class Local { int f() { return new Foo().bar(); } }\n',
|
||||
'src/main/java/c/Written.java': 'package c;\npublic class Written { int f() { return new a.Foo().bar(); } }\n',
|
||||
'src/main/java/d/Neither.java': 'package d;\nimport a.*;\nimport b.*;\npublic class Neither { int f() { return new Foo().bar(); } }\n',
|
||||
'src/main/java/u/Unique.java': 'package u;\npublic class Unique { public int go() { return 3; } }\n',
|
||||
'src/main/java/c/UseUnique.java': 'package c;\nimport u.Unique;\npublic class UseUnique { int f() { return new Unique().go(); } }\n',
|
||||
});
|
||||
csharp = await callTargets({
|
||||
'A/Foo.cs': 'namespace A { public class Foo { public int Bar() { return 1; } } }\n',
|
||||
'B/Foo.cs': 'namespace B { public class Foo { public int Bar() { return 2; } } }\n',
|
||||
'C/UseA.cs': 'using A;\nnamespace C { public class UseA { int F() { return new Foo().Bar(); } } }\n',
|
||||
'C/UseB.cs': 'using B;\nnamespace C { public class UseB { int F() { return new Foo().Bar(); } } }\n',
|
||||
'B/Local.cs': 'namespace B { public class Local { int F() { return new Foo().Bar(); } } }\n',
|
||||
'D/Both.cs': 'using A;\nusing B;\nnamespace D { public class Both { int F() { return new Foo().Bar(); } } }\n',
|
||||
'U/Unique.cs': 'namespace U { public class Unique { public int Go() { return 3; } } }\n',
|
||||
'C/UseUnique.cs': 'using U;\nnamespace C { public class UseUnique { int F() { return new Unique().Go(); } } }\n',
|
||||
});
|
||||
}, 120_000);
|
||||
|
||||
it('Java: the class the file imports, a written package, or the caller\'s own package', () => {
|
||||
expect(java.get('src/main/java/c/UseA.java')).toEqual(['a::Foo::bar']);
|
||||
expect(java.get('src/main/java/c/UseB.java')).toEqual(['b::Foo::bar']);
|
||||
expect(java.get('src/main/java/b/Local.java')).toEqual(['b::Foo::bar']);
|
||||
expect(java.get('src/main/java/c/Written.java')).toEqual(['a::Foo::bar']);
|
||||
});
|
||||
|
||||
it('Java: nothing when the language can\'t pick, and a class of its own name still links', () => {
|
||||
expect(java.get('src/main/java/d/Neither.java')).toBeUndefined();
|
||||
expect(java.get('src/main/java/c/UseUnique.java')).toEqual(['u::Unique::go']);
|
||||
});
|
||||
|
||||
it('C#: the one type the file\'s namespaces and usings can see', () => {
|
||||
expect(csharp.get('C/UseA.cs')).toEqual(['A::Foo::Bar']);
|
||||
expect(csharp.get('C/UseB.cs')).toEqual(['B::Foo::Bar']);
|
||||
expect(csharp.get('B/Local.cs')).toEqual(['B::Foo::Bar']);
|
||||
});
|
||||
|
||||
it('C#: nothing when the language can\'t pick, and a class of its own name still links', () => {
|
||||
expect(csharp.get('D/Both.cs')).toBeUndefined();
|
||||
expect(csharp.get('C/UseUnique.cs')).toEqual(['U::Unique::Go']);
|
||||
});
|
||||
});
|
||||
@@ -26050,6 +26050,9 @@ export function matchNewReceiverCall(ref: UnresolvedRef, context: ResolutionCont
|
||||
const shape = ref.referenceName.match(NEW_RECEIVER_SHAPE);
|
||||
const method = shape?.[2];
|
||||
if (!cls || !method) return null;
|
||||
// Java's and C#'s no-argument `new Foo().bar()` reach here from the
|
||||
// conformance pass; their own rules pick among same-named classes.
|
||||
if (ref.language === 'java' || ref.language === 'csharp') return matchJavaCsharpNewReceiverCall(cls, shape![1]!, method, ref, context);
|
||||
// Several classes may share the name: the one the call's file imports
|
||||
// under the written name is the class constructed (tsImportedTypeFile).
|
||||
const onClass = resolveMethodOnType(cls, method, ref, context, 0.9, 'instance-method', tsImportedTypeFile(shape![1]!, ref, context));
|
||||
@@ -26067,6 +26070,36 @@ export function matchNewReceiverCall(ref: UnresolvedRef, context: ResolutionCont
|
||||
: null;
|
||||
}
|
||||
|
||||
/**
|
||||
* A Java or C# `new Foo().bar()`. With one project class of the name, its
|
||||
* method or an inherited one, as for any typed call. With several, the class
|
||||
* the language picks, or none: Java's written package (`new a.b.Foo()`), the
|
||||
* file's single-type import, else the caller's own package; C#'s one type the
|
||||
* file's namespaces and usings can see (isCsharpTypeVisible). Then only that
|
||||
* class's own method: an inherited one is looked up by name, across every
|
||||
* class of the name.
|
||||
*/
|
||||
function matchJavaCsharpNewReceiverCall(cls: string, written: string, method: string, ref: UnresolvedRef, context: ResolutionContext): ResolvedRef | null {
|
||||
const twins = context.getNodesByName(cls).filter((n) =>
|
||||
(n.kind === 'class' || n.kind === 'struct') && sameLanguageFamily(n.language, ref.language));
|
||||
if (twins.length <= 1) return resolveMethodOnType(cls, method, ref, context, 0.9, 'instance-method');
|
||||
const dirOf = (file: string) => file.slice(0, file.lastIndexOf('/'));
|
||||
let chosen: Node | undefined;
|
||||
if (ref.language === 'csharp') {
|
||||
const visible = twins.filter((n) => isCsharpTypeVisible(n, ref, context));
|
||||
if (visible.length === 1) chosen = visible[0];
|
||||
} else {
|
||||
const fqn = written.includes('.') ? written : importedFqnOf(cls, ref, context);
|
||||
const byPath = (n: Node) => n.filePath.replace(/\.java$/, '').endsWith('/' + fqn!.replace(/\./g, '/'));
|
||||
const ownPackage = twins.filter((n) => dirOf(n.filePath) === dirOf(ref.filePath));
|
||||
chosen = fqn ? twins.find(byPath) : ownPackage.length === 1 ? ownPackage[0] : undefined;
|
||||
}
|
||||
if (!chosen) return null;
|
||||
const own = context.getNodesInFile(chosen.filePath).find((n) =>
|
||||
n.kind === 'method' && n.name === method && n.qualifiedName === `${chosen!.qualifiedName}::${method}`);
|
||||
return own ? { original: ref, targetNodeId: own.id, confidence: 0.9, resolvedBy: 'instance-method' } : null;
|
||||
}
|
||||
|
||||
/** Resolve the implementation inside the identified store, not a namesake or
|
||||
* an interface signature elsewhere in the project. Import resolution already
|
||||
* follows aliases/barrels; containment already excludes nested action locals. */
|
||||
|
||||
Reference in New Issue
Block a user