diff --git a/CHANGELOG.md b/CHANGELOG.md index 426611325..4f93060b9 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 +- Calls on a library's own types and modules no longer land on a project method that happens to share the name. In Rust, `Vec::new()`, `String::from(…)` and module paths like `task::spawn(…)` stay the standard library's. A later link of a multi-line Rust or Go chain now counts as a chained call, named after the chain's first line. In Go, `w.Header().Get(…)` is net/http's. In R, a plain call like `range(x)` is a function, not an object's method. In JavaScript, `response.text()` / `.json()` is fetch's. On tokio, `task::spawn(…)` used to land on `TaskTracker::spawn` 300 times. On ggplot2, `range(…)` used to land on a coordinate system's `range` method. Re-index after upgrading. - In C and C++, a qualified name like `fmt::format` now reaches a declaration inside a namespace the library opens with a macro, like fmt's `FMT_BEGIN_NAMESPACE`, pybind11's `PYBIND11_NAMESPACE_BEGIN(…)` or rapidjson's and spdlog's `…_NAMESPACE_BEGIN`. It also works through a namespace alias like `namespace py = pybind11;`. On fmt, 1,728 `fmt::format(…)` calls used to link to nothing, and so did about 1,400 `py::…` calls on pybind11. - A method guessed from a receiver's name no longer lands on a request handler the web framework dispatches to, such as a Django or DRF view's `post` / `get` or a controller's `update` / `destroy`. It also no longer lands on a test double the calling file never mentions. On django-allauth, tests' `client.post(…)` used to land on a `ClientRegistrationView` 425 times. On mealie, `response.json()` used to land on a test's `_FakeHTTPResponse` 424 times. In Python, `site = Site.objects.create(…)` now makes `site` a `Site`, so `site.save()` reaches the model's own `save`. Re-index after upgrading. - Kotlin infix calls, like `Users.id eq id1` or koin's `single { … } bind MyApi::class`, are now recorded as calls to the infix function. Before, they had no callers at all. On Exposed, `eq` now shows over a thousand callers. Bit operations on numbers (`x and 0xff`, `h shr 8`) are left to Kotlin's own operators, even where the project defines a same-named extension. Re-index after upgrading. diff --git a/__tests__/library-receiver-calls.test.ts b/__tests__/library-receiver-calls.test.ts new file mode 100644 index 000000000..72e3f8cd9 --- /dev/null +++ b/__tests__/library-receiver-calls.test.ts @@ -0,0 +1,129 @@ +/** + * Calls on a library's own types and modules stay the library's, not a + * same-named project method's: + * + * - Rust `Vec::new()` / `task::spawn(…)`: a type the project doesn't declare, + * and a module path (tokio's `task::spawn` went to TaskTracker's `spawn` + * 300 times, `Vec::new` to a VecWithInitialized); + * - a link further down a multi-line Rust chain is named after the chain's + * head: `bat()\n.stdout(…)` is assert_cmd's, `Arg::new("x")\n.value_hint(…)` + * the project's Arg; + * - Go's `w.Header().Get(…)` is net/http's Header, not gin's `Context.Get`; + * - R's bare `range(x)` is base R's, not a ggproto object's `range` method; + * - JS `response.text()` is fetch's Response, not a project class's `text`. + */ +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-library-receivers-')); + const files: Record = { + 'src/lib.rs': `pub struct VecWithInitialized; +impl VecWithInitialized { + pub fn new() -> Self { VecWithInitialized } +} + +pub struct TaskTracker; +impl TaskTracker { + pub fn spawn(&self) {} +} + +pub struct OutputType; +impl OutputType { + pub fn stdout(&self) {} +} + +pub struct Arg; +impl Arg { + pub fn new(name: &str) -> Arg { Arg } + pub fn value_hint(self, hint: u8) -> Arg { self } +} + +pub fn run() { + let v: Vec = Vec::new(); + task::spawn(async {}); + bat() + .assert() + .stdout("x"); + let a = Arg::new("cmd") + .value_hint(1); +} +`, + 'context.go': `package gin + +type Context struct{} + +func (c *Context) Get(key string) (any, bool) { return nil, false } + +func handler(w Writer) string { + return w.Header().Get("Content-Type") +} +`, + 'R/coord.R': `Coord <- ggproto("Coord", + range = function(panel_params) { + list(x = panel_params$x$dimension()) + } +) + +scale_limits <- function(x) { + range(x) +} +`, + 'src/file.js': `export class LazyFile { + text() { return ''; } +} + +export async function load(response) { + return await response.text(); +} +`, + }; + 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 }); +}); + +/** `Owner::member` of every call edge out of a file. */ +function callsFrom(file: string): string[] { + const ids = cg.getNodesInFile(file).map((n) => n.id); + return cg + .getOutgoingEdgesFrom(ids) + .filter((e) => e.kind === 'calls') + .map((e) => cg.getNode(e.target)!.qualifiedName) + .sort(); +} + +describe('calls on a library’s types and modules', () => { + it('Rust: outside types, module paths and multi-line chains', () => { + const calls = callsFrom('src/lib.rs'); + expect(calls).not.toContain('VecWithInitialized::new'); + expect(calls).not.toContain('TaskTracker::spawn'); + expect(calls).not.toContain('OutputType::stdout'); + expect(calls).toContain('Arg::value_hint'); + }); + + it('Go: net/http Header methods', () => { + expect(callsFrom('context.go')).not.toContain('Context::Get'); + }); + + it('R: a bare call is a function, not an object’s method', () => { + expect(callsFrom('R/coord.R').filter((q) => q.endsWith('range'))).toEqual([]); + }); + + it('JS: fetch Response bodies', () => { + expect(callsFrom('src/file.js')).not.toContain('LazyFile::text'); + }); +}); diff --git a/src/resolution/js-builtins.ts b/src/resolution/js-builtins.ts index ef0d88cd9..3d877759e 100644 --- a/src/resolution/js-builtins.ts +++ b/src/resolution/js-builtins.ts @@ -29,6 +29,8 @@ export const JS_BUILTIN_METHODS = new Set([ 'prependListener', 'prependOnceListener', 'listeners', 'rawListeners', 'listenerCount', 'eventNames', 'setMaxListeners', 'getMaxListeners', 'next', 'return', 'throw', 'drop', 'take', 'toArray', + // fetch's Response / Request / Blob bodies. + 'text', 'json', 'arrayBuffer', 'blob', 'formData', ]); /** diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index c82419138..c6a316ffb 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -1060,6 +1060,20 @@ function isBareGoCall(ref: UnresolvedRef, context: ResolutionContext): boolean { return ref.language === 'go' && isReceiverLessCall(ref, context); } +/** + * Whether an R call is a plain function call — `range(x)`, `vars(a)` — not a + * ggproto / R6 method through `obj$m(…)` or `self$m(…)`. A method is only + * reached through its object: ggplot2's `range(data$x)` (base R's) went to a + * Coord's `range` method. + */ +function isBareRCall(ref: UnresolvedRef, context: ResolutionContext): boolean { + if (ref.language !== 'r' || ref.referenceKind !== 'calls' || !/^[A-Za-z_.][\w.]*$/.test(ref.referenceName)) return false; + const line = context.getFileLines?.(ref.filePath)?.[ref.line - 1] ?? context.readFile(ref.filePath)?.split('\n')[ref.line - 1]; + if (!line) return false; + const m = new RegExp(`(?redirect()` / `$obj->view()` / @@ -1502,14 +1516,29 @@ function dartHeadOf(decl: Node, context: ResolutionContext): { supers: string[]; * `child.Flags().String("f", …)` arrive as bare `map` / `String`. */ function rustGoCallShape(ref: UnresolvedRef, context: ResolutionContext): { shape: 'path' } | { shape: 'bare' } | { shape: 'chained'; receiver: string } | null { - const line = context.getFileLines?.(ref.filePath)?.[ref.line - 1] ?? context.readFile(ref.filePath)?.split('\n')[ref.line - 1]; + const lines = context.getFileLines?.(ref.filePath) ?? context.readFile(ref.filePath)?.split('\n'); + let line = lines?.[ref.line - 1]; if (line === undefined) return null; const name = ref.referenceName; + const at = new RegExp(`(?): // `iter`, Go's `String` / `Get`) needs a receiver named after the owner; a // project-specific one keeps its match — clap's `flag("n").short('n')` is // `Arg::short`, cobra's `c.Root().Name()` `Command::Name`. + // Go's `w.Header().Get(…)` / `r.Header.Set(…)`: net/http's Header map. + if (n.language === 'go' && /(?:^|\.)Header$/.test(shape.receiver) && /^(?:Get|Set|Add|Del|Values|Clone|Write)$/.test(n.name)) return false; const std = (n.language === 'go' ? GO_STD_METHODS : RUST_STD_METHODS).has(n.name); return !std || /^(?:self|Self)$/.test(shape.receiver) || (shape.receiver !== '' && sharesReceiverWord(shape.receiver, n)); } @@ -2231,6 +2262,9 @@ const RUST_STD_METHODS: ReadonlySet = new Set([ 'as_bytes', 'as_slice', 'as_ptr', 'into', 'try_into', 'borrow', 'borrow_mut', 'deref', 'deref_mut', 'chars', 'bytes', 'lines', 'starts_with', 'ends_with', 'trim', 'to_lowercase', 'to_uppercase', 'windows', 'chunks', 'then', 'then_some', 'eq', 'cmp', 'partial_cmp', 'read_to_end', 'read_to_string', 'fetch_add', 'fetch_sub', + // std::process::Command's pipes and the assert_cmd assertions tests chain + // onto it (not `arg` / `env`, which clap's own builders carry) + 'current_dir', 'stdout', 'stderr', 'stdin', 'success', 'failure', ]); /** @@ -3354,7 +3388,7 @@ export function matchByExactName( // findBestMatch — O(K²) per package, the dominant cost of "Resolving refs" on // large import-heavy (front-end + back-end) repos (#915). const bareJs = isBareJsCall(ref, context); - const bareGo = isBareGoCall(ref, context); + const bareNoMembers = isBareGoCall(ref, context) || isBareRCall(ref, context); const barePhp = isBarePhpCall(ref, context); // A type, a value or an import the file binds from a package outside the // repository names nothing in it, whatever kind of reference it is. @@ -3442,7 +3476,7 @@ export function matchByExactName( // method (#1714, #1857), nor a property, field or case: mocha's global // `it(…)` bound to an interface's `it` property, `describe(…)` to a // command class's `describe` string. - !((bareJs || bareGo) && TYPE_MEMBER_KINDS.has(n.kind)) && + !((bareJs || bareNoMembers) && TYPE_MEMBER_KINDS.has(n.kind)) && // A bare PHP call is a function call: nothing else is callable without a receiver. !(barePhp && n.kind !== 'function') && // A Vue component's own method is `this.m()` inside that component — not @@ -5734,10 +5768,16 @@ export function matchMethodCall( // In C# and Java a capitalized receiver the class around it doesn't declare // is a type — a project property of that name elsewhere (a test object's // `DateTime`) does not make `DateTime.Parse(…)` the project's. - const typesOnly = ref.language === 'csharp' || ref.language === 'java'; + const typesOnly = ref.language === 'csharp' || ref.language === 'java' || ref.language === 'rust'; if (namesExternalType(objectOrClass!, ref.language) && !context.getNodesByName(objectOrClass!).some((n) => sameLanguageFamily(n.language, ref.language) && - (!typesOnly || isMethodOwnerKind(n) || n.kind === 'enum' || n.kind === 'namespace' || n.kind === 'module'))) { + (!typesOnly || isMethodOwnerKind(n) || n.kind === 'enum' || n.kind === 'namespace' || n.kind === 'module' || + n.kind === 'trait' || n.kind === 'type_alias'))) { + return null; + } + // Rust `task::spawn(…)` / `io::stdout()`: a function through a module path — + // never a method some type of that name owns. + if (ref.language === 'rust' && match === colonMatch && /^[a-z_][\w]*(?:::[a-z_]\w*)*$/.test(objectOrClass!)) { return null; } // `string.Equals(…)`, `object.ReferenceEquals(…)`: a C# keyword type. @@ -6746,7 +6786,10 @@ function hasParameterBinding(code: string, escapedName: string): boolean { function namesExternalType(receiver: string, language: string): boolean { if (!/^[A-Z][A-Za-z0-9_]*$/.test(receiver)) return false; if (language === 'pascal') return /^(?:[TEI][A-Z]\w*|Exception)$/.test(receiver); - return !['go', 'c', 'cpp', 'rust', 'cuda', 'metal'].includes(language); + // Rust: `Vec::new()`, `String::from(…)`, `Default::default()` — but `Self::` is the impl's own type, + // and a SCREAMING_CASE receiver (`REQ_ID.scope(…)`) a static. + if (language === 'rust') return receiver !== 'Self' && /[a-z]/.test(receiver); + return !['go', 'c', 'cpp', 'cuda', 'metal'].includes(language); } /** @@ -6994,7 +7037,9 @@ export function matchFuzzy( // `Node` not a `node()`. Only PHP, Pascal/Delphi, CFML, COBOL and VB.NET // resolve a name without regard to case, which is what this fallback's // lowercase index is for. + const bareR = isBareRCall(ref, context); const callableCandidates = candidates.filter((n) => callableKinds.has(n.kind) && !(typeRef && !canNameInTypePosition(n)) && + !(bareR && n.kind === 'method') && // `new …MockData()` makes an instance of a type; a method is never what it names. !(ref.referenceKind === 'instantiates' && n.kind === 'method') && !(rustBare && !isRustNameInScope(n, ref, context)) &&