diff --git a/CHANGELOG.md b/CHANGELOG.md index b8552372..40f08351 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 +- 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 max(…)` or `class Foo`, 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. - A minified or bundled JavaScript file is now recognized as generated even when it isn't named `.min.js`, for example a docs site's `bundle.js`, a readable webpack build or a vendored `underscore-min.js`. Its one-letter functions now rank below your own code in search and `codegraph_explore` answers. diff --git a/__tests__/python-receiver-guessing.test.ts b/__tests__/python-receiver-guessing.test.ts new file mode 100644 index 00000000..dcc94520 --- /dev/null +++ b/__tests__/python-receiver-guessing.test.ts @@ -0,0 +1,89 @@ +/** + * A Python call whose receiver the extractor could not keep — `User.objects.get(…)`, + * `self.client.login(…)` reach the resolver as `get`, `login` — is not a project + * method picked by name. netbox bound 3,610 `.all()` calls to one `UserConfig.all`, + * healthchecks 880 `objects.get` to a test case's `get`. A bare `get(1)` cannot be a + * method either (no implicit self); `self.helper()` still resolves to the class's own. + */ +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 = { + 'app/tests.py': `class AuthTestCase: + def get(self, url): + return url + + def login(self, name): + return name +`, + 'app/curl.py': `def get(url): + return url +`, + 'app/helpers.py': `def slugify(text): + return text +`, + 'app/tags.py': `def render(context): + return context +`, + 'app/pages.py': `import app.helpers +from django.shortcuts import render + +def page(request): + kind = request.POST.get("kind") + slug = app.helpers.slugify(kind) + return render(request, "page.html", {"slug": slug}) +`, + 'app/views.py': `from django.contrib.auth.models import User + +class Backend: + def authenticate(self, username): + user = User.objects.get(email=username) + self.client.login(username="a") + self.helper() + self.assertEqual(self.filterset(params, self.queryset).qs.get(), 2) + get(1) + return user + + def helper(self): + return 1 +`, +}; + +describe('Python: a method is reached through a receiver', () => { + it('not by its name alone', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-py-receiver-')); + 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 authenticate = cg.getNodesByName('authenticate')[0]!; + const targets = cg + .getOutgoingEdgesFrom([authenticate.id], ['calls']) + .map((e) => cg.getNode(e.target)!.qualifiedName); + expect(targets).not.toContain('AuthTestCase::get'); + expect(targets).not.toContain('AuthTestCase::login'); + expect(targets).toContain('Backend::helper'); + const page = cg.getNodesByName('page')[0]!; + const fromPage = cg.getOutgoingEdgesFrom([page.id], ['calls']).map((e) => cg.getNode(e.target)!); + const pageTargets = fromPage.map((n) => `${n.filePath}:${n.name}`); + // \`request.POST.get\` names no project module; \`render\` is Django's. + expect(pageTargets).not.toContain('app/curl.py:get'); + expect(pageTargets).not.toContain('app/tags.py:render'); + // A module path names the module's function. + expect(pageTargets).toContain('app/helpers.py:slugify'); + } finally { + cg.close(); + } + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index fb46cd5e..7f367d4b 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -616,9 +616,9 @@ function isLexicallyReachable( ref: UnresolvedRef, context: ResolutionContext ): boolean { - // A function — or a type (`case class B()` in a test method) — declared - // inside a function is only in scope in there. - if (candidate.kind !== 'function' && !LOCAL_TYPE_KINDS.has(candidate.kind)) return true; + // A function — or a type (`case class B()` in a test method), or a method + // of such a type — declared inside a function is only in scope in there. + if (candidate.kind !== 'function' && candidate.kind !== 'method' && !LOCAL_TYPE_KINDS.has(candidate.kind)) return true; // C and C++ have no nested named functions, so a function the graph shows // inside another is an extraction artifact, not a scope: tree-sitter-c // cannot parse a macro call whose arguments are designated initializers @@ -627,23 +627,50 @@ function isLexicallyReachable( // the file, nesting every function after it. Trusting that nesting rejected // 117 real calls into pid.c on that tree; the functions are reachable. if (NO_NESTED_FUNCTIONS.has(candidate.language)) return true; - const qn = candidate.qualifiedName; - if (!qn || !qn.includes('::')) return true; - const parentQn = qn.slice(0, qn.lastIndexOf('::')); - const containers = context - .getNodesByQualifiedName(parentQn) - .filter( - (p) => - p.filePath === candidate.filePath && - (p.kind === 'function' || p.kind === 'method') && - p.startLine <= candidate.startLine && - p.endLine >= candidate.endLine - ); - if (containers.length === 0) return true; - return ( - ref.filePath === candidate.filePath && - containers.some((p) => ref.line >= p.startLine && ref.line <= p.endLine) - ); + const scope = lexicalScopeOf(candidate, context); + return scope === null || (ref.filePath === candidate.filePath && ref.line >= scope.start && ref.line <= scope.end); +} + +/** Per context: a candidate's scoping function body, or null when nothing scopes it. */ +const LEXICAL_SCOPE_MEMO = new WeakMap>(); + +/** + * The innermost function or method BODY that scopes a declaration, walking out + * through its qualified name (`test_x::Request::User::has_perm` → `test_x`). + * A method directly on a class is reachable through its instances, and an + * object-literal method a function returns through the object — neither is + * scoped by the function it sits in. + */ +function lexicalScopeOf(candidate: Node, context: ResolutionContext): { start: number; end: number } | null { + let memo = LEXICAL_SCOPE_MEMO.get(context); + if (!memo) { + memo = new Map(); + LEXICAL_SCOPE_MEMO.set(context, memo); + } + const hit = memo.get(candidate.id); + if (hit !== undefined) return hit; + let scope: { start: number; end: number } | null = null; + const own = candidate.qualifiedName ?? ''; + const parentQn = own.includes('::') ? own.slice(0, own.lastIndexOf('::')) : ''; + let qn = own; + while (qn.includes('::')) { + qn = qn.slice(0, qn.lastIndexOf('::')); + const container = context + .getNodesByQualifiedName(qn) + .find( + (p) => + p.filePath === candidate.filePath && + (p.kind === 'function' || p.kind === 'method') && + p.startLine <= candidate.startLine && + p.endLine >= candidate.endLine + ); + if (!container) continue; + if (candidate.kind === 'method' && qn === parentQn) break; + scope = { start: container.startLine, end: container.endLine }; + break; + } + memo.set(candidate.id, scope); + return scope; } /** Languages whose module boundary is `import`/`export` (or CommonJS). */ @@ -1039,6 +1066,99 @@ function canNameInTypePosition(n: Node): boolean { return !(n.kind === 'property' || n.kind === 'method' || n.kind === 'enum_member' || n.kind === 'field'); } +/** + * The shape of a Python call that reaches the resolver as a bare name — the + * column is the call's start. `bare`: `get(1)`, which cannot mean a method + * (Python has no implicit self). `chained`: a receiver the extractor could not + * keep — `User.objects.get(…)`, `self.client.login(…)`, `request.POST.get(…)` + * arrive as `get` / `login`, and can only mean a member of what the chain + * names last (`POST`, `client`, `objects`). netbox bound 3,610 `.all()` calls + * to one `UserConfig.all`, healthchecks 880 `objects.get` to a test case's + * `get`. `self.x()` / `cls.x()`, and a chain split across lines, are null: + * today's behavior. + */ +type PythonCallShape = { kind: 'bare' } | { kind: 'chained'; owner: string }; + +function pythonCallShape(ref: UnresolvedRef, context: ResolutionContext): PythonCallShape | null { + if (ref.language !== 'python' || ref.referenceKind !== 'calls') return null; + const name = ref.referenceName; + if (!/^[A-Za-z_]\w*$/.test(name)) return null; + const line = context.getFileLines?.(ref.filePath)?.[ref.line - 1] ?? context.readFile(ref.filePath)?.split('\n')[ref.line - 1]; + if (line === undefined) return null; + const text = line.slice(ref.column); + if (text.startsWith(name) && /^\s*\(/.test(text.slice(name.length))) return { kind: 'bare' }; + if (new RegExp(String.raw`^(?:self|cls)\s*\.\s*${name}\s*\(`).test(text)) return null; + // The call starts at its receiver, so everything up to `.name(` is the receiver chain. + const chain = new RegExp(String.raw`^(.*?)\.\s*${name}\s*\(`).exec(text); + if (!chain) return null; + const owner = /(\w+)\s*(?:\([^()]*\)|\[[^\[\]]*\])?\s*$/.exec(chain[1]!)?.[1]; + return owner ? { kind: 'chained', owner } : null; +} + +/** Can a Python call of this shape mean the candidate? */ +function fitsPythonCallShape(n: Node, shape: PythonCallShape, ref: UnresolvedRef, context: ResolutionContext): boolean { + if (shape.kind === 'bare') { + if (n.kind === 'method') return false; + // `from django.shortcuts import render`: the call is the package's. + return !(n.filePath !== ref.filePath && isPythonNameImportedFromOutside(ref.referenceName, ref, context)); + } + // A member of what the chain names: a method of a class of that name, or a + // function / class in a module of that name (`helpers.slugify()`). + if (n.kind === 'method') { + // `self.store.fetch()` on a `Store`, `self.user_service.find()` on a `UserService`. + const cut = n.qualifiedName.lastIndexOf('::'); + const owner = cut >= 0 ? n.qualifiedName.slice(0, cut).split('::').pop()! : ''; + const plain = (s: string): string => s.replace(/_/g, '').toLowerCase(); + return owner !== '' && plain(owner) === plain(shape.owner); + } + const parts = n.filePath.split('/'); + const stem = parts[parts.length - 1]!.replace(/\.pyi?$/, ''); + return stem === shape.owner || (stem === '__init__' && parts[parts.length - 2] === shape.owner); +} + +const PY_IMPORTS = new WeakMap>>(); +const PY_MODULE_LOCAL = new WeakMap>(); + +/** `name` → the module a `from import name` line takes it from, per file. */ +function pythonFromImports(filePath: string, context: ResolutionContext): Map { + let memo = PY_IMPORTS.get(context); + if (!memo) { + memo = new Map(); + PY_IMPORTS.set(context, memo); + } + const hit = memo.get(filePath); + if (hit) return hit; + const names = new Map(); + const text = context.readFile(filePath) ?? ''; + for (const m of text.matchAll(/^\s*from\s+([\w.]+)\s+import\s+(\([^)]*\)|[^\n#]+)/gm)) { + const module = m[1]!; + for (const item of m[2]!.replace(/[()]/g, '').split(',')) { + const bound = /(\w+)\s*$/.exec(item.trim())?.[1]; + if (bound && bound !== '*') names.set(bound, module); + } + } + memo.set(filePath, names); + return names; +} + +function isPythonNameImportedFromOutside(name: string, ref: UnresolvedRef, context: ResolutionContext): boolean { + const module = pythonFromImports(ref.filePath, context).get(name); + if (!module || module.startsWith('.')) return false; + let memo = PY_MODULE_LOCAL.get(context); + if (!memo) { + memo = new Map(); + PY_MODULE_LOCAL.set(context, memo); + } + let local = memo.get(module); + if (local === undefined) { + const rel = module.replace(/\./g, '/'); + local = context.getAllFiles().some((f) => + f === `${rel}.py` || f.endsWith(`/${rel}.py`) || f === `${rel}/__init__.py` || f.endsWith(`/${rel}/__init__.py`)); + memo.set(module, local); + } + return !local; +} + /** 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', @@ -1328,7 +1448,9 @@ export function matchByExactName( sameName.some((n) => n.kind === 'constant' && CPP_DEFINE_SIGNATURE.test(n.signature ?? '')); const typeRef = isDotNetTypeRef(ref, context); const rustBare = ref.language === 'rust' && /^[A-Za-z_]\w*$/.test(ref.referenceName); + const pythonShape = pythonCallShape(ref, context); const candidates = sameName.filter((n) => + !(pythonShape && !fitsPythonCallShape(n, pythonShape, ref, context)) && !(rustBare && !isRustNameInScope(n, ref, context)) && !(cMacroCall && n.kind !== 'function' && n.kind !== 'method') && !(typeRef && !canNameInTypePosition(n)) && @@ -2430,6 +2552,9 @@ export function clearNameMatcherMemos(context: ResolutionContext): void { C_STATIC_MEMO.delete(context); RUST_TRAIT_IMPL_MEMO.delete(context); RUST_USES.delete(context); + LEXICAL_SCOPE_MEMO.delete(context); + PY_IMPORTS.delete(context); + PY_MODULE_LOCAL.delete(context); SEALED_MODULES.delete(context); LOCAL_BINDING_MEMO.delete(context); LOCAL_BINDING_SITES.delete(context); @@ -4335,9 +4460,13 @@ export function matchFuzzy( const callableKinds = new Set(['function', 'method', 'class']); const typeRef = isDotNetTypeRef(ref, context); const rustBare = ref.language === 'rust' && /^[A-Za-z_]\w*$/.test(ref.referenceName); - // Rust names are case-sensitive: `Bytes` is not the method `bytes`, `Ok` not a function `ok`. + const pythonShape = pythonCallShape(ref, context); + // 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)))) + !(rustBare && (n.name !== ref.referenceName || !isRustNameInScope(n, ref, context))) && + !(ref.language === 'python' && n.name !== ref.referenceName) && + !(pythonShape && !fitsPythonCallShape(n, pythonShape, ref, context))) .filter((n) => (ref.referenceKind !== 'references' && ref.referenceKind !== 'function_ref') || sameLanguageFamily(n.language, ref.language));