mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-18 22:02:49 +08:00
fix(cfml): a component method reached by name alone must be in reach (#2148)
- A bare call in a .cfc reaches only the component's own methods and those of the components it extends (extends="a.b.C" read from source, matched to indexed .cfc files by longest path suffix). Calls in .cfm templates and multi-line chain links (name not on the ref's line) are not judged. - The untyped-receiver rule from the Ruby fix now covers CFML too: the one project method named `m` needs a receiver named after its owner. The word match also accepts the receiver's whole name (`cbsecurity` → CBSecurity) and ignores trailing digits (`executor1` → Executor); Ruby is byte-identical to main. A/B (edges removed / added): coldbox -2429/+295, TestBox -255/+31, fw1 -103/+4, cbsecurity -8/+5, cfdocs 0. Deliberate losses: framework-repo calls through conventionally named, untyped receivers (ColdBox's `event.*` → RequestContext, TestBox's `variables.assert.*` → Assertion). 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
|
||||
|
||||
- In CFML, a call no longer lands on an unrelated component's method just because the name matches. A call with no receiver inside a component now reaches only that component's methods and those of the components it extends, so built-ins like `now()` stay built-ins. A call on an untyped variable now needs the variable to be named after the component, so `server.keyExists(…)` is no longer taken for some component's `keyExists`. On coldbox, `now()` used to land on a date helper over 200 times, and TestBox's `fail(…)` in specs now reaches the spec base class.
|
||||
- In Ruby, a call no longer lands on an unrelated class's method just because it's the only method with that name. A call on an untyped receiver now needs the receiver to be named after the method's class, like `web_push_request.legacy_encrypt`. A call with no receiver inside a class now reaches only that class's methods, its superclasses' methods, and those of the modules it includes, extends or prepends. On rubocop, `node.loc` on a syntax node used to land on the project's one `loc` method over 1,200 times, and Kernel's `format(…)` on the language server's `format`. On mastodon, `render` in controllers used to land on an inline renderer, and `authorize` now reaches the authorization concern.
|
||||
- In Kotlin, a call to a top-level or extension function now resolves only to one the file can see: one in its own package, one it imports by name, or one from a package it imports with `*`. On Exposed, tests that imported kotlin.test's or JUnit's `assertEquals`, or the JDBC suite's helper, used to land on the R2DBC suite's helper over 3,000 times. On okhttp, tests that import `mockwebserver3.MockResponse` used to construct the deprecated class of the same name. A function type with a qualified receiver, like `(DatabaseConfig.Builder.() -> Unit)?`, no longer breaks the parse of the class around it. Re-index after upgrading.
|
||||
- In Dart, a call written without a receiver now reaches only a method of the class it's written in, of a class it extends, mixes in or implements, or of an extension on one of those types, and picks the nearest one. On riverpod, package:test's `test(…)` in the test files used to land on a container class's `test` method over 1,100 times, shelf's and getx's `expect(…)` on unrelated classes' `expect`, and bloc's `emit(…)` in a Cubit on the wrong base class. Classes declared as `class A = B with C;` are now indexed under their name, with what they extend and mix in. Re-index after upgrading.
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
/**
|
||||
* CFML types no receiver, so a component method reached by name alone must
|
||||
* be in reach:
|
||||
*
|
||||
* - a bare call inside a component (`.cfc`) is the component's own method or
|
||||
* one of a component it `extends` — else a built-in: coldbox's `now()` went
|
||||
* to a date helper's `now` 228 times;
|
||||
* - `receiver.m()` through an untyped receiver takes the one method named
|
||||
* `m` only when the receiver is named after its component:
|
||||
* `server.keyExists(…)` is the struct member function, not an interceptor
|
||||
* buffer's `keyExists` (168 times).
|
||||
*/
|
||||
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-cfml-scope-'));
|
||||
const files: Record<string, string> = {
|
||||
'system/core/DateTimeHelper.cfc': `component {
|
||||
function now() {
|
||||
return createObject( "java", "java.time.LocalDateTime" ).now();
|
||||
}
|
||||
}
|
||||
`,
|
||||
'system/core/InterceptorBuffer.cfc': `component {
|
||||
function keyExists( required key ) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
`,
|
||||
'system/Base.cfc': `component {
|
||||
function getSetting( required name ) {
|
||||
return "";
|
||||
}
|
||||
}
|
||||
`,
|
||||
'system/models/UserService.cfc': `component {
|
||||
function list() {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
`,
|
||||
'system/Handler.cfc': `component extends="app.system.Base" {
|
||||
function index( event, rc, prc, userService ) {
|
||||
var stamp = now();
|
||||
var lucee = server.keyExists( "lucee" );
|
||||
var name = getSetting( "appName" );
|
||||
var users = userService.list();
|
||||
return stamp;
|
||||
}
|
||||
}
|
||||
`,
|
||||
};
|
||||
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 });
|
||||
});
|
||||
|
||||
describe('CFML calls reached by name alone', () => {
|
||||
it('keep the inherited method and the receiver named after its component; drop the built-ins', () => {
|
||||
const ids = cg.getNodesInFile('system/Handler.cfc').map((n) => n.id);
|
||||
const targets = cg
|
||||
.getOutgoingEdgesFrom(ids)
|
||||
.filter((e) => e.kind === 'calls')
|
||||
.map((e) => cg.getNode(e.target)!.qualifiedName)
|
||||
.sort();
|
||||
expect(targets).toEqual(['Base::getSetting', 'UserService::list']);
|
||||
});
|
||||
});
|
||||
@@ -1431,6 +1431,84 @@ function dartHeadOf(decl: Node, context: ResolutionContext): { supers: string[];
|
||||
};
|
||||
}
|
||||
|
||||
const CFML_CHAINS = new WeakMap<ResolutionContext, Map<string, Set<string>>>();
|
||||
|
||||
/**
|
||||
* Whether a bare CFML call written in a component can mean `method`: its own
|
||||
* component's, or one of the components it `extends` (read from source — a
|
||||
* dotted path matched against the indexed files by its longest suffix). A
|
||||
* call in a `.cfm` template is not judged: a ColdBox view runs inside the
|
||||
* renderer's scope. coldbox's `now()` — the built-in — went to a date
|
||||
* helper's `now` 228 times; chained `.then()` calls arrive bare too, and keep
|
||||
* their method.
|
||||
*/
|
||||
function isCfmlMethodInScope(method: Node, ref: UnresolvedRef, context: ResolutionContext): boolean {
|
||||
if (!/\.cfc$/i.test(ref.filePath) || !hasNoReceiverOnLine(ref, context)) return true;
|
||||
return cfmlChain(ref.filePath, context).has(method.filePath);
|
||||
}
|
||||
|
||||
/** A component file and every component file it extends. */
|
||||
function cfmlChain(file: string, context: ResolutionContext): Set<string> {
|
||||
let memo = CFML_CHAINS.get(context);
|
||||
if (!memo) CFML_CHAINS.set(context, (memo = new Map()));
|
||||
const hit = memo.get(file);
|
||||
if (hit) return hit;
|
||||
const chain = new Set<string>();
|
||||
const queue = [file];
|
||||
while (queue.length > 0 && chain.size < 30) {
|
||||
const f = queue.shift()!;
|
||||
if (chain.has(f)) continue;
|
||||
chain.add(f);
|
||||
const text = context.readFile(f) ?? '';
|
||||
const head = text.slice(0, text.search(/\bfunction\b|<cffunction/i) >>> 0 || text.length);
|
||||
const ext = /\bextends\s*=\s*["']?([\w.\/:-]+)["']?/i.exec(head)?.[1];
|
||||
if (ext) queue.push(...cfmlComponentFiles(ext, f, context));
|
||||
}
|
||||
memo.set(file, chain);
|
||||
return chain;
|
||||
}
|
||||
|
||||
/** The indexed `.cfc` files a component path names: the same directory first, else the longest path suffix. */
|
||||
function cfmlComponentFiles(dotted: string, from: string, context: ResolutionContext): string[] {
|
||||
const segments = dotted.replace(/\//g, '.').split('.').filter(Boolean);
|
||||
const name = segments[segments.length - 1]!.toLowerCase();
|
||||
const files = [...new Set(context.getNodesByLowerName(name)
|
||||
.filter((n) => n.kind === 'class' && /\.cfc$/i.test(n.filePath))
|
||||
.map((n) => n.filePath))];
|
||||
if (files.length === 0) return [];
|
||||
const dir = from.slice(0, from.lastIndexOf('/') + 1);
|
||||
if (segments.length === 1) {
|
||||
const local = files.filter((f) => f.slice(0, f.lastIndexOf('/') + 1) === dir);
|
||||
return local.length > 0 ? local : files;
|
||||
}
|
||||
for (let take = segments.length; take >= 1; take--) {
|
||||
const suffix = '/' + segments.slice(-take).join('/').toLowerCase() + '.cfc';
|
||||
const hits = files.filter((f) => ('/' + f.toLowerCase()).endsWith(suffix));
|
||||
if (hits.length > 0) return hits;
|
||||
}
|
||||
return files;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a bare reference is receiver-less at its call site, name case
|
||||
* aside: the name is not preceded by a `.` on its line (true when the line
|
||||
* can't tell). An extractor that keeps one receiver level hands the later
|
||||
* links of a chain (`newFuture(f).then(g)`) over bare.
|
||||
*/
|
||||
function hasNoReceiverOnLine(ref: UnresolvedRef, context: ResolutionContext): boolean {
|
||||
const line = context.getFileLines?.(ref.filePath)?.[ref.line - 1] ?? context.readFile(ref.filePath)?.split('\n')[ref.line - 1];
|
||||
if (line === undefined) return true;
|
||||
const lower = line.toLowerCase();
|
||||
const name = ref.referenceName.toLowerCase();
|
||||
let start = -1;
|
||||
if (lower.startsWith(name, ref.column)) start = ref.column;
|
||||
else if (ref.column >= name.length && lower.startsWith(name, ref.column - name.length)) start = ref.column - name.length;
|
||||
else start = lower.search(new RegExp(`(?<![\\w$])${name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\s*\\(`));
|
||||
// Not on its line at all: a link of a chain written across lines.
|
||||
if (start < 0) return false;
|
||||
return !/\.\s*$/.test(line.slice(0, start));
|
||||
}
|
||||
|
||||
const RUBY_ANCESTRY = new WeakMap<ResolutionContext, Map<string, Set<string>>>();
|
||||
|
||||
/**
|
||||
@@ -1976,9 +2054,11 @@ export function matchByExactName(
|
||||
const dartBare = ref.language === 'dart' && ref.referenceKind === 'calls' && /^[A-Za-z_$][\w$]*$/.test(ref.referenceName) && isReceiverLessDartCall(ref, context);
|
||||
const kotlinCall = ref.language === 'kotlin' && ref.referenceKind === 'calls' && /^[A-Za-z_$][\w$]*$/.test(ref.referenceName);
|
||||
const rubyBare = ref.language === 'ruby' && ref.referenceKind === 'calls' && /^[A-Za-z_]\w*[?!]?$/.test(ref.referenceName);
|
||||
const cfmlBare = (ref.language === 'cfml' || ref.language === 'cfscript') && ref.referenceKind === 'calls' && /^[A-Za-z_]\w*$/.test(ref.referenceName);
|
||||
const phpSelf = phpSelfReceiver(ref, context);
|
||||
const filtered = sameName.filter((n) =>
|
||||
!(rubyBare && n.kind === 'method' && !isRubyMethodInScope(n, ref, context)) &&
|
||||
!(cfmlBare && n.kind === 'method' && !isCfmlMethodInScope(n, ref, context)) &&
|
||||
!(javaBare && n.kind === 'method' && !isJavaMethodInScope(n, ref, context)) &&
|
||||
!(kotlinCall && !isKotlinTopLevelVisible(n, ref, context)) &&
|
||||
!(dartBare && isDartMember(n) && !isDartMethodInScope(n, ref, context)) &&
|
||||
@@ -3096,6 +3176,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void {
|
||||
DART_HIERARCHIES.delete(context);
|
||||
KOTLIN_FILE_SCOPES.delete(context);
|
||||
RUBY_ANCESTRY.delete(context);
|
||||
CFML_CHAINS.delete(context);
|
||||
LUA_LOCALS.delete(context);
|
||||
PHP_FILE_SCOPES.delete(context);
|
||||
JAVA_STATIC_IMPORTS.delete(context);
|
||||
@@ -4085,7 +4166,7 @@ export function matchMethodCall(
|
||||
// `node.loc` on a rubocop-ast node went to the one `loc` in the project
|
||||
// 1,201 times; lobsters' `value.to_s` to a short-id class's.
|
||||
if (targetMethods.length === 1 && !narrowed && targetMethods[0]!.language === ref.language &&
|
||||
!(ref.language === 'ruby' && objectOrClass !== 'self' && objectOrClass !== 'self.class' &&
|
||||
!(UNTYPED_RECEIVER_LANGUAGES.has(ref.language) && !/^(?:self|self\.class|this|super)$/i.test(objectOrClass!) &&
|
||||
!sharesReceiverWord(objectOrClass!, targetMethods[0]!))) {
|
||||
return {
|
||||
original: ref,
|
||||
@@ -4888,12 +4969,27 @@ function hasParameterBinding(code: string, escapedName: string): boolean {
|
||||
/**
|
||||
* Split a camelCase or PascalCase string into words.
|
||||
*/
|
||||
/** Whether a receiver's name and the owner of `method` have a word in common, case aside. */
|
||||
/**
|
||||
* Languages whose receivers nothing types, where a unique method name alone
|
||||
* is no evidence: CFML's `server.keyExists()` is the struct member function,
|
||||
* not the one component method named `keyExists`.
|
||||
*/
|
||||
const UNTYPED_RECEIVER_LANGUAGES: ReadonlySet<string> = new Set(['ruby', 'cfml', 'cfscript']);
|
||||
|
||||
/**
|
||||
* Whether a receiver is named after the owner of `method`, case aside: the
|
||||
* receiver's last segment is the owner's name (`cbsecurity` → CBSecurity), or
|
||||
* they share a word (`web_push_request` → WebPushRequest, `executor1` →
|
||||
* Executor).
|
||||
*/
|
||||
function sharesReceiverWord(receiver: string, method: Node): boolean {
|
||||
const cut = method.qualifiedName.lastIndexOf('::');
|
||||
if (cut < 0) return false;
|
||||
const owner = new Set(splitCamelCase(method.qualifiedName.slice(0, cut)).map((w) => w.toLowerCase()));
|
||||
return splitCamelCase(receiver).some((w) => owner.has(w.toLowerCase()));
|
||||
const ownerQn = method.qualifiedName.slice(0, cut);
|
||||
const flat = (w: string) => w.replace(/[^A-Za-z0-9]/g, '').replace(/\d+$/, '').toLowerCase();
|
||||
if (flat(receiver.split('.').pop()!) === flat(ownerQn.split(/::|\./).pop()!)) return true;
|
||||
const owner = new Set(splitCamelCase(ownerQn).map(flat));
|
||||
return splitCamelCase(receiver).some((w) => owner.has(flat(w)));
|
||||
}
|
||||
|
||||
function splitCamelCase(str: string): string[] {
|
||||
@@ -5067,6 +5163,7 @@ export function matchFuzzy(
|
||||
const dartBare = ref.language === 'dart' && ref.referenceKind === 'calls' && /^[A-Za-z_$][\w$]*$/.test(ref.referenceName) && isReceiverLessDartCall(ref, context);
|
||||
const kotlinCall = ref.language === 'kotlin' && ref.referenceKind === 'calls' && /^[A-Za-z_$][\w$]*$/.test(ref.referenceName);
|
||||
const rubyBare = ref.language === 'ruby' && ref.referenceKind === 'calls' && /^[A-Za-z_]\w*[?!]?$/.test(ref.referenceName);
|
||||
const cfmlBare = (ref.language === 'cfml' || ref.language === 'cfscript') && ref.referenceKind === 'calls' && /^[A-Za-z_]\w*$/.test(ref.referenceName);
|
||||
const phpSelf = phpSelfReceiver(ref, context);
|
||||
// Names are case-sensitive in every language but a handful: Rust's
|
||||
// `Bytes` is not the method `bytes`, Python's builtin `dir(…)` not a class
|
||||
@@ -5084,6 +5181,7 @@ export function matchFuzzy(
|
||||
!(dartBare && isDartMember(n) && !isDartMethodInScope(n, ref, context)) &&
|
||||
!(kotlinCall && !isKotlinTopLevelVisible(n, ref, context)) &&
|
||||
!(rubyBare && n.kind === 'method' && !isRubyMethodInScope(n, ref, context)) &&
|
||||
!(cfmlBare && n.kind === 'method' && !isCfmlMethodInScope(n, ref, context)) &&
|
||||
!(phpSelf && (n.kind !== 'method' || !isPhpMethodInScope(n, ref, phpSelf, context))))
|
||||
.filter((n) => (ref.referenceKind !== 'references' && ref.referenceKind !== 'function_ref') ||
|
||||
sameLanguageFamily(n.language, ref.language));
|
||||
|
||||
Reference in New Issue
Block a user