mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-02 09:45:39 +08:00
fix(resolution): a bare Java call reaches its class, supertypes or static imports (#2126)
Found by the README-wide sweep: halo's most-depended-on symbols were an `EmailVerificationService.verify` (1,038 — Mockito's `verify(mock)`) and a builder's `eq` (845 — ArgumentMatchers `eq`); retrofit's a test helper's `assertThat` (1,357 — Truth); mall's a DTO's `hashCode` (Object's). A bare Java call (`verify(x)`, `helper()`, `this.x()`, `super.x()`) now has only method candidates declared on a class around it, on one of that class's supertypes (read transitively from the declarations' `extends` / `implements` — the resolved edges don't exist yet on the first pass), or imported statically (`import static a.B.m;` / `a.B.*`). Memoized per type and per file. A/B vs main (edge diffs, every change classified): halo −2,103/+145, retrofit −1,380/+18, commons-lang −202/+87, jsoup −92/+45, mall −76, gson −7/+4. Gains are the right target where one exists: halo's `and` / `equal` / `isNull` through `import static …Queries.*`, jsoup's `attr(…)` through `LeafNode`, commons-lang's `addExact` on its own class instead of a nested MathBridge, retrofit's `getRawType` through `CallAdapter.Factory`. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
99209b6f3d
commit
8987be4254
@@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
### Fixes
|
||||
|
||||
- A bare Java call like `verify(mock)`, `assertThat(x)` or `hashCode()` now links only to a method of the class it's in, of that class's parents, or one the file imports statically. It used to link to any class's method of that name: in halo, Mockito's `verify` and `eq` gave one service's `verify` over a thousand made-up callers, and in retrofit Truth's `assertThat` did the same to a test helper. Calls to inherited methods and static imports from the project now link to the right one, like jsoup's `attr(…)` through `LeafNode` or halo's `and(…)` / `equal(…)` through `import static …Queries.*`. Re-index Java projects after upgrading.
|
||||
- Python calls like `User.objects.get(…)`, `self.client.login(…)` or `request.POST.get(…)` no longer link to whichever class in the project has a method of that name. Such a call now links only to a member of what its receiver names, like `helpers.slugify()` to `slugify` in `helpers.py`, and a bare `get(1)` never links to a method. A name the file imports from a package outside the project, like `from django.shortcuts import render`, no longer links to a same-named project function. In Django projects these made-up links had given single test or serializer methods thousands of callers, for example netbox's `.all()` calls on one `UserConfig.all`. Re-index Python projects after upgrading.
|
||||
- In Rust, a bare `Ok(…)`, `Some(…)`, `Err(…)`, `Result<…>`, `Option<…>`, `Vec<…>` or `drop(…)` now means the standard library's, unless the file defines or imports a project item of that name. These used to link to any same-named enum variant or struct in the project: every `Some(x)` in ripgrep to one enum's `Some` variant, and every `Ok` and `Result` in serde to structs its macro-hygiene tests declare. An enum variant is now reached by a bare name only where the file `use`s it or its enum's `*`, and a variant never stands for a type. Re-index Rust projects after upgrading.
|
||||
- A generic type parameter, like `A` in `def zipWith[A, B](…)`, `T` in `<T> T max(…)` or `class Foo<T>`, no longer links to a project symbol that happens to share its name. On cats, one `implicit def A` had thousands of made-up dependents from every `A` in the library. The same goes for Rust, Dart, TypeScript, Java, Kotlin, Swift, Go, C# and C++. A class or other type declared inside a function is only linked from inside that function. In Scala, a `def`'s own parameter, like `f` in `(f: A => B)`, no longer links to a same-named field elsewhere. A bare name no longer reaches a type nested inside another type unless the code is inside that type, extends it or imports its members; the type in scope is linked instead, like cats' own `FlatMap` in place of `Eval`'s nested `FlatMap`. Re-index projects in these languages after upgrading.
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
/**
|
||||
* A bare Java call reaches a method of a class around it, a supertype of that
|
||||
* class, or a static import — not another class's method that shares the name.
|
||||
* halo's Mockito `verify(…)` bound 1,038 calls to `EmailVerificationService.verify`.
|
||||
*/
|
||||
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 });
|
||||
});
|
||||
|
||||
const FILES: Record<string, string> = {
|
||||
'src/main/java/app/EmailVerificationService.java': `package app;
|
||||
public class EmailVerificationService {
|
||||
public boolean verify(String token) { return true; }
|
||||
}
|
||||
`,
|
||||
'src/main/java/app/Util.java': `package app;
|
||||
public class Util {
|
||||
public static String format(String s) { return s; }
|
||||
}
|
||||
`,
|
||||
'src/main/java/app/Base.java': `package app;
|
||||
public class Base {
|
||||
protected void helper() {}
|
||||
}
|
||||
`,
|
||||
'src/main/java/app/Child.java': `package app;
|
||||
import static app.Util.format;
|
||||
public class Child extends Base {
|
||||
public void run() {
|
||||
helper();
|
||||
format("x");
|
||||
own();
|
||||
}
|
||||
private void own() {}
|
||||
}
|
||||
`,
|
||||
'src/test/java/app/ChildTest.java': `package app;
|
||||
import static org.mockito.Mockito.verify;
|
||||
public class ChildTest {
|
||||
public void sends(Object mock) {
|
||||
verify(mock);
|
||||
}
|
||||
}
|
||||
`,
|
||||
};
|
||||
|
||||
describe('Java: a bare call reaches what is in scope', () => {
|
||||
it('own class, a supertype, a static import — not a same-named method elsewhere', async () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-java-bare-'));
|
||||
roots.push(root);
|
||||
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 callsFrom = (name: string): string[] => {
|
||||
const from = cg.getNodesByName(name).find((n) => n.kind === 'method')!;
|
||||
return cg.getOutgoingEdgesFrom([from.id], ['calls']).map((e) => cg.getNode(e.target)!.name).sort();
|
||||
};
|
||||
expect(callsFrom('sends')).not.toContain('verify');
|
||||
expect(callsFrom('run')).toEqual(['format', 'helper', 'own']);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -1159,6 +1159,85 @@ function isPythonNameImportedFromOutside(name: string, ref: UnresolvedRef, conte
|
||||
return !local;
|
||||
}
|
||||
|
||||
const JAVA_TYPE_KINDS: ReadonlySet<string> = new Set(['class', 'interface', 'enum', 'struct', 'record', 'trait']);
|
||||
const JAVA_SUPERS = new WeakMap<ResolutionContext, Map<string, string[]>>();
|
||||
const JAVA_STATIC_IMPORTS = new WeakMap<ResolutionContext, Map<string, { owners: Set<string>; members: Set<string> }>>();
|
||||
|
||||
/**
|
||||
* A bare Java call — `verify(mock)`, `helper()`, `this.x()`, `super.x()` —
|
||||
* reaches a method of a class around it, of one of that class's supertypes, or
|
||||
* one the file imports statically. Not some other class's method of that name:
|
||||
* halo's tests' Mockito `verify(…)` and `eq(…)` bound 1,038 calls to an
|
||||
* `EmailVerificationService.verify` and 845 to a builder's `eq`. Supertypes are
|
||||
* read from the declarations — the resolved `extends` edges do not exist yet on
|
||||
* the first pass.
|
||||
*/
|
||||
function isJavaMethodInScope(method: Node, ref: UnresolvedRef, context: ResolutionContext): boolean {
|
||||
const cut = method.qualifiedName.lastIndexOf('::');
|
||||
if (cut < 0) return true;
|
||||
const owner = method.qualifiedName.slice(0, cut).split('::').pop()!;
|
||||
const imports = javaStaticImportsOf(ref.filePath, context);
|
||||
if (imports.owners.has(owner) || imports.members.has(`${owner}.${ref.referenceName}`)) return true;
|
||||
const around = context
|
||||
.getNodesInFile(ref.filePath)
|
||||
.filter((n) => JAVA_TYPE_KINDS.has(n.kind) && n.startLine <= ref.line && n.endLine >= ref.line);
|
||||
const seen = new Set<string>();
|
||||
const queue = around.map((n) => n.name);
|
||||
while (queue.length > 0 && seen.size < 40) {
|
||||
const name = queue.shift()!;
|
||||
if (seen.has(name)) continue;
|
||||
seen.add(name);
|
||||
if (name === owner) return true;
|
||||
queue.push(...javaSupertypesOf(name, context));
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/** The simple names a Java type's declarations extend or implement. */
|
||||
function javaSupertypesOf(typeName: string, context: ResolutionContext): string[] {
|
||||
let memo = JAVA_SUPERS.get(context);
|
||||
if (!memo) {
|
||||
memo = new Map();
|
||||
JAVA_SUPERS.set(context, memo);
|
||||
}
|
||||
const hit = memo.get(typeName);
|
||||
if (hit) return hit;
|
||||
const names: string[] = [];
|
||||
for (const decl of context.getNodesByName(typeName)) {
|
||||
if (decl.language !== 'java' || !JAVA_TYPE_KINDS.has(decl.kind)) continue;
|
||||
const lines = context.getFileLines?.(decl.filePath) ?? context.readFile(decl.filePath)?.split(/\r?\n/) ?? [];
|
||||
const head = lines.slice(decl.startLine - 1, decl.startLine + 5).join(' ');
|
||||
const clause = /\b(?:extends|implements)\b([^{]*)\{/.exec(head)?.[1] ?? '';
|
||||
const flat = clause.replace(/<[^<>]*(?:<[^<>]*>[^<>]*)*>/g, '');
|
||||
for (const m of flat.matchAll(/([A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)*)/g)) {
|
||||
const simple = m[1]!.split('.').pop()!;
|
||||
if (simple !== 'extends' && simple !== 'implements') names.push(simple);
|
||||
}
|
||||
}
|
||||
memo.set(typeName, names);
|
||||
return names;
|
||||
}
|
||||
|
||||
/** A Java file's `import static a.b.Owner.member;` / `import static a.b.Owner.*;`. */
|
||||
function javaStaticImportsOf(filePath: string, context: ResolutionContext): { owners: Set<string>; members: Set<string> } {
|
||||
let memo = JAVA_STATIC_IMPORTS.get(context);
|
||||
if (!memo) {
|
||||
memo = new Map();
|
||||
JAVA_STATIC_IMPORTS.set(context, memo);
|
||||
}
|
||||
const hit = memo.get(filePath);
|
||||
if (hit) return hit;
|
||||
const found = { owners: new Set<string>(), members: new Set<string>() };
|
||||
const text = context.readFile(filePath) ?? '';
|
||||
for (const m of text.matchAll(/^\s*import\s+static\s+([\w.$]+)\s*\.\s*(\*|[\w$]+)\s*;/gm)) {
|
||||
const owner = m[1]!.split('.').pop()!;
|
||||
if (m[2] === '*') found.owners.add(owner);
|
||||
else found.members.add(`${owner}.${m[2]}`);
|
||||
}
|
||||
memo.set(filePath, found);
|
||||
return found;
|
||||
}
|
||||
|
||||
/** Names the Rust prelude puts in every module; a project item of the same name needs a `use` to shadow one. */
|
||||
const RUST_PRELUDE = new Set([
|
||||
'Ok', 'Err', 'Some', 'None', 'Result', 'Option', 'Box', 'Vec', 'String', 'Default', 'Drop', 'Iterator',
|
||||
@@ -1449,7 +1528,9 @@ export function matchByExactName(
|
||||
const typeRef = isDotNetTypeRef(ref, context);
|
||||
const rustBare = ref.language === 'rust' && /^[A-Za-z_]\w*$/.test(ref.referenceName);
|
||||
const pythonShape = pythonCallShape(ref, context);
|
||||
const javaBare = ref.language === 'java' && ref.referenceKind === 'calls' && /^[A-Za-z_$][\w$]*$/.test(ref.referenceName);
|
||||
const candidates = sameName.filter((n) =>
|
||||
!(javaBare && n.kind === 'method' && !isJavaMethodInScope(n, ref, context)) &&
|
||||
!(pythonShape && !fitsPythonCallShape(n, pythonShape, ref, context)) &&
|
||||
!(rustBare && !isRustNameInScope(n, ref, context)) &&
|
||||
!(cMacroCall && n.kind !== 'function' && n.kind !== 'method') &&
|
||||
@@ -2553,6 +2634,8 @@ export function clearNameMatcherMemos(context: ResolutionContext): void {
|
||||
RUST_TRAIT_IMPL_MEMO.delete(context);
|
||||
RUST_USES.delete(context);
|
||||
LEXICAL_SCOPE_MEMO.delete(context);
|
||||
JAVA_SUPERS.delete(context);
|
||||
JAVA_STATIC_IMPORTS.delete(context);
|
||||
PY_IMPORTS.delete(context);
|
||||
PY_MODULE_LOCAL.delete(context);
|
||||
SEALED_MODULES.delete(context);
|
||||
@@ -4461,12 +4544,14 @@ export function matchFuzzy(
|
||||
const typeRef = isDotNetTypeRef(ref, context);
|
||||
const rustBare = ref.language === 'rust' && /^[A-Za-z_]\w*$/.test(ref.referenceName);
|
||||
const pythonShape = pythonCallShape(ref, context);
|
||||
const javaBare = ref.language === 'java' && ref.referenceKind === 'calls' && /^[A-Za-z_$][\w$]*$/.test(ref.referenceName);
|
||||
// Rust and Python names are case-sensitive: `Bytes` is not the method `bytes`,
|
||||
// Python's builtin `dir(…)` not a class `Dir`.
|
||||
const callableCandidates = candidates.filter((n) => callableKinds.has(n.kind) && !(typeRef && !canNameInTypePosition(n)) &&
|
||||
!(rustBare && (n.name !== ref.referenceName || !isRustNameInScope(n, ref, context))) &&
|
||||
!(ref.language === 'python' && n.name !== ref.referenceName) &&
|
||||
!(pythonShape && !fitsPythonCallShape(n, pythonShape, ref, context)))
|
||||
!(pythonShape && !fitsPythonCallShape(n, pythonShape, ref, context)) &&
|
||||
!(javaBare && n.kind === 'method' && !isJavaMethodInScope(n, ref, context)))
|
||||
.filter((n) => (ref.referenceKind !== 'references' && ref.referenceKind !== 'function_ref') ||
|
||||
sameLanguageFamily(n.language, ref.language));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user