diff --git a/CHANGELOG.md b/CHANGELOG.md index b71a8a17d..e2980670b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- A call on a variable declared with a type from outside the project (`List list`, `var sb = new StringBuilder()`, `signer = TimestampSigner()`) no longer lands on a project type's method with the same name. On gson, `list.add(…)` used to land on a project list wrapper; on commons-lang, `s.length()` and `buffer.append(…)` on project classes. In Python, a keyword argument inside a call (`prefix=IPNetwork(…)`) is no longer taken for an assignment to that name. - Kotlin classes whose primary constructor sits on the line after the class name, before a supertype list (`expect open class ByteString` / `internal constructor(…) : Comparable`, common in Kotlin Multiplatform code), no longer lose their members to loose functions. A call later in a chain to a Kotlin standard-library function (`apply`, `let`, `toString`, `map`, `contains`…) no longer lands on a project type's same-named method. On okhttp, `.apply { … }` used to land on `ConnectionSpec.apply`. Re-index after upgrading. - In Rust and Go, a call later in a chain, like `sym.filename().map(…)` or `c.Flags().String(…)`, no longer lands on the project's own method of that name when the name is a standard-library one (`unwrap`, `clone`, `map`, `iter`, Go's `String` or `Error`). A plain call like `get(handler)` or `drop(x)` never lands on a method. On tokio, `.map(…)` on an `Option` used to land on `MutexGuard::map`; on axum, the router's `get(handler)` on a cookie jar's `get`; on ripgrep, `.clone()` on an error type. - A call on a type from outside the project, like `Integer.valueOf(…)`, `Arrays.asList(…)`, `Object.assign(…)` or Delphi's `Exception.Create(…)`, no longer lands on the project's own method with that name. On commons-lang, `Integer.valueOf` used to land on `StringUtils.valueOf` over 800 times. Go's exported variables and Pascal's capitalized parameters and locals are not mistaken for type names. diff --git a/__tests__/external-declared-receiver.test.ts b/__tests__/external-declared-receiver.test.ts new file mode 100644 index 000000000..b2f035f27 --- /dev/null +++ b/__tests__/external-declared-receiver.test.ts @@ -0,0 +1,76 @@ +/** + * A receiver whose declared type is one the project doesn't declare — + * `List list`, `var sb = new StringBuilder()` — calls that outside + * type's method, never a project type's same-named one: gson's `list.add(…)` + * went to a project list wrapper's `add`, commons-lang's `s.length()` to a + * writer's. A Python keyword argument (`prefix=IPNetwork(…),` inside a call) + * is not a binding of `prefix`. + */ +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'; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-external-declared-')); + const files: Record = { + 'src/main/java/app/NonNullElementWrapperList.java': `package app; +public class NonNullElementWrapperList { + public boolean add(Object o) { return true; } +} +`, + 'src/main/java/app/Use.java': `package app; +import java.util.ArrayList; +import java.util.List; +public class Use { + public void run() { + List list = new ArrayList<>(); + list.add("x"); + } +} +`, + 'app/models.py': `class Prefix: + def get_available_ip_count(self): + return 0 +`, + 'app/tests.py': `from app.models import Prefix +from netaddr import IPNetwork + + +def test_count(): + prefix = Prefix( + prefix=IPNetwork('192.0.2.0/24'), + ) + return prefix.get_available_ip_count() +`, + }; + 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); + } + cg = await CodeGraph.init(root, { index: true }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +const callsIn = (file: string, fn: string): string[] => { + const node = cg.getNodesInFile(file).find((n) => n.name === fn)!; + return cg.getOutgoingEdges(node.id).filter((e) => e.kind === 'calls').map((e) => cg.getNode(e.target)!.qualifiedName); +}; + +describe('a receiver declared with an outside type', () => { + it('Java: `List list` calls List.add, not a project class’s add', () => { + expect(callsIn('src/main/java/app/Use.java', 'run').some((t) => t.endsWith('NonNullElementWrapperList::add'))).toBe(false); + }); + + it('Python: a keyword argument is not the receiver’s binding', () => { + expect(callsIn('app/tests.py', 'test_count').some((t) => t.endsWith('Prefix::get_available_ip_count'))).toBe(true); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index e0244cbdd..7d9d0bff3 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -3984,7 +3984,9 @@ function buildLocalReceiverTypePatterns(language: Language, r: string): RegExp[] ]; case 'python': return [ - new RegExp(`\\b${r}\\b\\s*=\\s*([A-Z][\\w.]*)\\s*\\(`), // lg = Logger(...) + // lg = Logger(...) — a statement of its own: `prefix=IPNetwork(…),` + // inside a call's arguments is a keyword argument, not a binding. + new RegExp(`(?:^|;)\\s*${r}\\s*=(?!=)\\s*([A-Z][\\w.]*)\\s*\\((?![^\\n]*,\\s*$)`), // A quoted forward reference (`lg: "Logger"`, `lg: 'pkg.Logger'`) is the // same annotation — and what every file under `from __future__ import // annotations` or with a not-yet-defined class writes. The unquoted @@ -4681,6 +4683,17 @@ export function matchMethodCall( ) { return null; } + // The receiver's declared type is one the project doesn't declare — + // `List list`, `String s`, `val sb = StringBuilder()` — so the + // method is that outside type's. gson's `list.add(…)` went to a project + // list wrapper's `add`, commons-lang's `s.length()` to a writer's. + // (Only a type name — `java.util.List`, not a call chain like Python's + // `Device.objects.create(…)` the initializer pattern also captures.) + const typeName = inferredType.split('.').pop()!; + if (/^[A-Z]/.test(typeName) && + !context.getNodesByName(typeName).some((n) => isMethodOwnerKind(n) && sameLanguageFamily(n.language, ref.language))) { + return null; + } } }