mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-16 20:53:57 +08:00
fix(resolution): a Python call reaches a method through its receiver, never by name alone (#2125)
Found by the README-wide sweep — Django/Flask/FastAPI apps' most-depended-on lists were test and serializer methods: netbox `UserConfig.all` (3,610 from `X.objects.all()`), `PortSerializer.create` (2,093 from `.objects.create`), a test-local `FakeQuerySet.count` (1,910); healthchecks `AuthTestCase.get` (880 from `Channel.objects.get()`), `ProfileAdmin.login` (702 from `self.client.login()`); mealie `_FakeHTTPResponse.json`. Python keeps a call through a receiver it cannot name (`User.objects.get`, `self.client.login`, `request.POST.get`) as the bare `get` / `login`, and exact-name matching then took any class's method by proximity. The call's shape is now read at its column (the call's start): - `bare` (`get(1)`): not a method — Python has no implicit self — and not a project symbol when the file takes that name `from` a module the project doesn't contain (`from django.shortcuts import render`); - `chained`: only a member of what the chain names last — a method of a class of that name (case/underscores ignored: `self.store` → `Store`, `self.user_service` → `UserService`) or a function/class in a module of that name (`app.helpers.slugify()` → helpers.py); - `self.x()` / `cls.x()`, and chains split across lines: unchanged. Fuzzy matching is case-exact for Python (`dir(…)` ≠ class `Dir`). Lexical reachability now walks every enclosing scope, so a method of a class declared inside a function is only reachable in there (memoized per candidate). A/B vs main (edge diffs, every change classified): netbox −18,091/+73 (indexes 8s → 5s), healthchecks −3,354/+361, pytest −2,950/+105, mealie −1,802/+6, allauth −1,086/+57, DRF −943/+50, flask −167/+17, httpx −147/+24. Gains are precise member links (`self.channel.notify` → Channel.notify, `item.stash.get` → Stash.get, `config.cache.get` → Cache.get, `self.provider.verify_token`); the one deliberate loss class is calls through an instance whose type needs inference (`flask.g.setdefault` in flask's own repo). excalidraw, typeorm, gson, okio, AutoMapper, gin and hono byte-identical. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -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> 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.
|
||||
- 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.
|
||||
|
||||
@@ -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<string, string> = {
|
||||
'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();
|
||||
}
|
||||
});
|
||||
});
|
||||
+151
-22
@@ -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<ResolutionContext, Map<string, { start: number; end: number } | null>>();
|
||||
|
||||
/**
|
||||
* 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<ResolutionContext, Map<string, Map<string, string>>>();
|
||||
const PY_MODULE_LOCAL = new WeakMap<ResolutionContext, Map<string, boolean>>();
|
||||
|
||||
/** `name` → the module a `from <module> import name` line takes it from, per file. */
|
||||
function pythonFromImports(filePath: string, context: ResolutionContext): Map<string, string> {
|
||||
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<string, string>();
|
||||
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));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user