mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-02 09:45:39 +08:00
fix(js): a name the calling function binds is its local (#2226)
A JS/TS parameter or plain var/let/const above the reference shadows a same-named function declared elsewhere in the file, whichever strategy found it; destructured call results, member calls and followed aliases are left alone. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
179939055c
commit
83dac15cff
@@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
### Fixes
|
||||
|
||||
- In JavaScript and TypeScript, a name the calling function binds itself, as a parameter or a plain `var`/`let`/`const` above the reference, now means that local instead of a same-named function elsewhere in the file. Before, every lodash helper's `object` parameter linked to a `function object() {}` declared inside `runInContext`, and zod's `new Class(…)` linked to an unrelated `Class` when `Class` was the function's parameter. Destructured hook results like `const { t } = useI18n()` still link to the function they come from.
|
||||
- Framework name conventions for Django, FastAPI, Flask, ASP.NET, Gin, SwiftUI and Vapor now only pick a class the reference can see. Examples of these conventions are "a `…View` is a view", "a `…Service` is a service" and "a capitalized name is a model". The pick starts from the reference's own file and then its package, and never takes a class declared inside some function or nested in another file's class. Before, Django REST framework's tests linked each file's own `MockView` and `Serializer` to another file's, and netbox's tests linked `TestForm()` to a form declared inside a different test function.
|
||||
- In Rust projects, a struct, handler or service name no longer resolves to an item declared inside some function body, and a file's own item comes before any other crate's. Before, axum's examples' `Uri` linked to a `struct Uri` declared inside a routing test, tokio's runtime `Handle` linked to `tokio-test`'s, and ripgrep's `Glob { … }` in globset linked to the CLI flags' `Glob`.
|
||||
- A method that calls a same-named method on another expression's result is no longer linked to itself in any language. Examples are Scala's `requestToArmeria(request).execute()` inside `execute()`, Rust's `self.0.into_route(state)` inside `into_route()`, and Kotlin's `this@Buffer.write(…)` from an inner object. A value whose initializer chain calls a same-named method, like sttp's `val response = basicRequest.get(…).response(…)`, no longer links to itself either.
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
/**
|
||||
* A JS/TS name the calling function binds itself — a parameter, or a `var` /
|
||||
* `let` / `const` above the reference — is that local, never a same-named
|
||||
* function declared elsewhere in the file. Every lodash helper lives inside
|
||||
* `runInContext`, so `baseHas(object, key)`'s `object` and `mixin`'s
|
||||
* `object(this.__wrapped__)` reached a `function object() {}` an IIFE declares
|
||||
* there. A function that calls itself through its own `const` still does, and
|
||||
* `if (handler) handler()` is not a parameter list.
|
||||
*/
|
||||
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-js-local-'));
|
||||
const files: Record<string, string> = {
|
||||
'lodash.js': `function runInContext(context) {
|
||||
var baseCreate = (function() {
|
||||
function object() {}
|
||||
return function(proto) {
|
||||
object.prototype = proto;
|
||||
return new object;
|
||||
};
|
||||
}());
|
||||
|
||||
function baseHas(object, key) {
|
||||
return object != null && hasOwnProperty.call(object, key);
|
||||
}
|
||||
|
||||
function mixin(object, source) {
|
||||
var result = object(this.__wrapped__);
|
||||
return result;
|
||||
}
|
||||
|
||||
function handler() {}
|
||||
|
||||
function run() {
|
||||
if (handler) {
|
||||
handler();
|
||||
}
|
||||
const walk = (node) => (node ? walk(node.next) : null);
|
||||
return walk(context);
|
||||
}
|
||||
|
||||
return { baseCreate, baseHas, mixin, run };
|
||||
}
|
||||
`,
|
||||
};
|
||||
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 targetsFrom = (qualifiedName: string) => {
|
||||
const ids = cg.getNodesInFile('lodash.js').filter((n) => n.qualifiedName === qualifiedName).map((n) => n.id);
|
||||
return cg.getOutgoingEdgesFrom(ids).filter((e) => e.kind !== 'contains').map((e) => cg.getNode(e.target)!.qualifiedName);
|
||||
};
|
||||
|
||||
describe('JS names the calling function binds', () => {
|
||||
it('are its parameters, not a same-named function elsewhere in the file', () => {
|
||||
expect(targetsFrom('runInContext::baseHas')).not.toContain('runInContext::object');
|
||||
expect(targetsFrom('runInContext::mixin')).not.toContain('runInContext::object');
|
||||
});
|
||||
|
||||
it('leave unbound names and a const’s own recursion alone', () => {
|
||||
expect(targetsFrom('runInContext::run')).toContain('runInContext::handler');
|
||||
expect(targetsFrom('runInContext::run::walk')).toContain('runInContext::run::walk');
|
||||
});
|
||||
});
|
||||
@@ -546,6 +546,12 @@ export function matchFunctionRef(
|
||||
// test's parameter of its name receives.
|
||||
if (ref.language === 'python' && !candidates.some((n) => isFixtureInReach(n, ref.filePath, context)) &&
|
||||
isPythonLocallyBound(ref.referenceName, ref, context)) return null;
|
||||
// Likewise a JS/TS parameter or local: lodash's `baseHas(object, key)` passes its own `object`.
|
||||
const jsLocal = jsFunctionLocalScope(ref.referenceName, ref, context);
|
||||
if (jsLocal) {
|
||||
candidates = candidates.filter((n) => n.filePath === ref.filePath && n.startLine >= jsLocal.start && n.startLine <= jsLocal.end);
|
||||
if (candidates.length === 0) return null;
|
||||
}
|
||||
|
||||
// Swift implicit-self: a bare identifier can name a METHOD only of the
|
||||
// ENCLOSING type (`Button(action: handleTap)` written inside that type) —
|
||||
@@ -4230,6 +4236,51 @@ export function isRustNameInScope(candidate: Node, ref: UnresolvedRef, context:
|
||||
const TYPE_MEMBER_KINDS: ReadonlySet<string> = new Set(['method', 'property', 'field', 'enum_member']);
|
||||
|
||||
/** Per-context memo: `file\0name` → "the file binds this name locally". */
|
||||
/** Whether `n` lies outside the function that binds the reference's name itself (see jsFunctionLocalScope). */
|
||||
function isOutsideJsLocal(n: Node, ref: UnresolvedRef, context: ResolutionContext): boolean {
|
||||
// `const indexName = this.dataSource.namingStrategy.indexName(…)`: a member, whatever the local's name.
|
||||
if (ref.referenceKind === 'calls' && bareCallReceiver(ref, context) !== null) return false;
|
||||
const scope = jsFunctionLocalScope(ref.referenceName, ref, context);
|
||||
return scope !== null && !(n.filePath === ref.filePath && n.startLine >= scope.start && n.startLine <= scope.end);
|
||||
}
|
||||
|
||||
const JS_FN_LOCAL_MEMO = new WeakMap<ResolutionContext, Map<string, { start: number; end: number } | null>>();
|
||||
|
||||
/**
|
||||
* The lines of the JS/TS function a reference sits in when that function binds
|
||||
* the name itself — a parameter, or a `var`/`let`/`const` above the reference.
|
||||
* Such a name is the local, never a same-named function declared elsewhere:
|
||||
* every lodash helper lives inside `runInContext`, so `baseHas(object, key)`'s
|
||||
* `object` and `mixin`'s `object(this.__wrapped__)` reached a `function
|
||||
* object() {}` an IIFE declares there. Null when the function does not bind it.
|
||||
*/
|
||||
function jsFunctionLocalScope(name: string, ref: UnresolvedRef, context: ResolutionContext): { start: number; end: number } | null {
|
||||
if (!JS_FAMILY.has(ref.language) || !/^[A-Za-z_$][\w$]*$/.test(name)) return null;
|
||||
let memo = JS_FN_LOCAL_MEMO.get(context);
|
||||
if (!memo) JS_FN_LOCAL_MEMO.set(context, (memo = new Map()));
|
||||
const key = `${ref.fromNodeId}\0${name}\0${ref.line}`;
|
||||
const hit = memo.get(key);
|
||||
if (hit !== undefined) return hit;
|
||||
let scope: { start: number; end: number } | null = null;
|
||||
const fn = context.getNodeById?.(ref.fromNodeId);
|
||||
if (fn && (fn.kind === 'function' || fn.kind === 'method') && fn.startLine <= ref.line && fn.endLine >= ref.line) {
|
||||
const lines = context.getFileLines?.(ref.filePath) ?? context.readFile(ref.filePath)?.split(/\r?\n/) ?? [];
|
||||
const text = stripCommentsForRegex(lines.slice(fn.startLine - 1, ref.line).join('\n'), 'javascript');
|
||||
const { param } = localBindingPatterns(name, 'g');
|
||||
const n = name.replace(/\$/g, '\\$');
|
||||
// A plain declaration. Destructuring re-binds what a call returns under
|
||||
// the same name — `const { t } = useI18n()`, `const { getLabel } =
|
||||
// useProps(props)` — which is the same-named function more often than not.
|
||||
const declared = new RegExp(`\\b(?:const|let|var)\\s+${n}\\b(?!\\s*[,\\]}])`).test(text);
|
||||
// A parameter list — never a control-flow head (`if (openMarkerClose) {`).
|
||||
// A return type stays on its line, never a ternary's `: data.slice()` below `filter(canRowExpand)`.
|
||||
const parameter = new RegExp(`(?<!\\b(?:if|while|for|switch|with)\\s*)${param.source.replace('(?::[^=;{]*)?', '(?::[^=;{}()\\n]*)?')}`);
|
||||
if (declared || parameter.test(text)) scope = { start: fn.startLine, end: fn.endLine };
|
||||
}
|
||||
memo.set(key, scope);
|
||||
return scope;
|
||||
}
|
||||
|
||||
const LOCAL_BINDING_MEMO = new WeakMap<ResolutionContext, Map<string, boolean>>();
|
||||
|
||||
/**
|
||||
@@ -8387,6 +8438,15 @@ export function matchReference(
|
||||
// inside `execute()`: a member of what the receiver is, which is the calling
|
||||
// method only through a TS/JS field of the caller's own type.
|
||||
if (result && result.targetNodeId === ref.fromNodeId && isCollapsedNonRecursion(ref, context)) return null;
|
||||
// A name the calling JS/TS function binds itself shadows the file's own:
|
||||
// lodash's `mixin(object, …)` calling `object(this.__wrapped__)` is its
|
||||
// parameter, whichever strategy (fuzzy included) found a `function object`.
|
||||
if (result && JS_LOCAL_REF_KINDS.has(ref.referenceKind)) {
|
||||
const target = context.getNodeById?.(result.targetNodeId);
|
||||
// (A target of another name is what the local was followed to: `const
|
||||
// selected = useStore(s => s.reset); selected()` is the store's `reset`.)
|
||||
if (target && target.name === ref.referenceName && isOutsideJsLocal(target, ref, context)) return null;
|
||||
}
|
||||
// Nor does a value's initializer call the value: sttp's `val response =
|
||||
// basicRequest.get(…).response(asStringAlways)` is a request's `response`.
|
||||
if (result && result.targetNodeId === ref.fromNodeId && ref.referenceKind === 'calls' &&
|
||||
@@ -8394,6 +8454,9 @@ export function matchReference(
|
||||
return result ? retargetSelfOverload(result, ref, context) : result;
|
||||
}
|
||||
|
||||
/** Reference kinds a bare JS/TS local can be: a call, a value, a construction. */
|
||||
const JS_LOCAL_REF_KINDS: ReadonlySet<string> = new Set(['calls', 'references', 'function_ref', 'instantiates']);
|
||||
|
||||
/** Node kinds that hold a value rather than run code. */
|
||||
const VALUE_KINDS: ReadonlySet<string> = new Set(['variable', 'constant', 'field', 'property']);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user