mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-16 20:53:57 +08:00
fix(lua): a local is invisible outside its chunk (#2143)
A Lua `local` (variable or function) belongs to the file that declares it, but cross-file name matching treated it like a global. kong's spec helpers re-bind busted's globals (`local it = it`, `local assert = require "luassert"`), and every other spec file's `it(…)` / `assert(…)` / `describe(…)` linked to them — 15,776 edges; koreader's `local ipairs = ipairs` in one module took the project's `ipairs` / `pairs` / `type` / `tonumber` (2,329). isVisibleAcrossFiles now reads a Lua/Luau variable or function's declaration line and rejects a `local` one from another file (the reference stays unresolved rather than taking a runner-up). Globals stay visible. 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 Lua and Luau, a `local` variable or function is no longer linked from other files, since it's only visible in the file that declares it. On kong, a spec helper's `local it = it` used to collect every other spec file's `it(…)` (over 4,000 links), and one module's `local ipairs = ipairs` took the whole project's `ipairs(…)`. Global functions are still linked across files.
|
||||
- PHP calls on `$this`, `self`, `static` and `parent` now reach the method of the class they're written in, a class it extends, or a trait any of them uses, with parent classes and traits found through the file's `namespace` and `use` imports. On Drupal core, `$this->assertEquals()` (PHPUnit's) used to link to an unrelated comparator class 8,832 times, `$this->assertSession()` to a JavaScript-test base class, and `$this->t()` to `Views::t`. They now reach `UiHelperTrait` and `StringTranslationTrait`, or nothing when the method lives in a package outside the repository. A call inside a trait, and a base class calling a method its subclass defines, keep their links. Re-index PHP projects after upgrading.
|
||||
- In PHP, CFML, Pascal, COBOL and VB.NET, a call written in a different case than its definition now resolves, like `formatprice()` for `FormatPrice` or CFML's `ToMatchWithCase()` for `toMatchWithCase`. These languages ignore case in names, but such calls used to be dropped before resolution.
|
||||
- A PHP function call written after `=>` in an array, like `'count' => count($items)`, `'by' => user()->id` or `'label' => trans('…')`, is now read as a plain function call. It used to be taken for a method call and linked to whichever class had a method of that name. On BookStack, that meant 166 wrong links.
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
/**
|
||||
* A Lua `local` belongs to its chunk: another file can never name it.
|
||||
*
|
||||
* kong's spec helpers re-bind busted's globals (`local it = it`,
|
||||
* `local assert = require "luassert"`); every other spec file's `it(…)` and
|
||||
* `assert(…)` linked to them — 4,166 and 1,234 times. koreader's
|
||||
* `local ipairs = ipairs` in one module took the whole project's `ipairs`.
|
||||
* A global function stays reachable.
|
||||
*/
|
||||
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 });
|
||||
});
|
||||
|
||||
describe('Lua locals', () => {
|
||||
it('are invisible outside their file; globals are not', async () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-lua-local-'));
|
||||
roots.push(root);
|
||||
const files: Record<string, string> = {
|
||||
'spec/helpers.lua': `local it = it
|
||||
local ipairs = ipairs
|
||||
local function helper() return 1 end
|
||||
function global_helper() return 2 end
|
||||
return {}
|
||||
`,
|
||||
'spec/plugin_spec.lua': `describe("plugin", function()
|
||||
it("works", function()
|
||||
for _, v in ipairs({ 1 }) do helper(v) end
|
||||
global_helper()
|
||||
end)
|
||||
end)
|
||||
`,
|
||||
};
|
||||
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 targets = cg
|
||||
.getOutgoingEdgesFrom(cg.getNodesInFile('spec/plugin_spec.lua').map((n) => n.id), ['calls'])
|
||||
.map((e) => cg.getNode(e.target)!.name)
|
||||
.sort();
|
||||
expect(targets).toEqual(['global_helper']);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -784,6 +784,21 @@ function isCrossFileReachable(
|
||||
);
|
||||
}
|
||||
|
||||
const LUA_LOCALS = new WeakMap<ResolutionContext, Map<string, boolean>>();
|
||||
|
||||
/** Whether a Lua variable or function is declared `local` (`local x = …`, `local function f`, `local a, x = …`). */
|
||||
function isLuaLocal(candidate: Node, context: ResolutionContext): boolean {
|
||||
if (candidate.kind !== 'variable' && candidate.kind !== 'constant' && candidate.kind !== 'function') return false;
|
||||
let memo = LUA_LOCALS.get(context);
|
||||
if (!memo) LUA_LOCALS.set(context, (memo = new Map()));
|
||||
const hit = memo.get(candidate.id);
|
||||
if (hit !== undefined) return hit;
|
||||
const line = (context.getFileLines?.(candidate.filePath) ?? context.readFile(candidate.filePath)?.split(/\r?\n/))?.[candidate.startLine - 1] ?? '';
|
||||
const local = /^\s*local\b/.test(line);
|
||||
memo.set(candidate.id, local);
|
||||
return local;
|
||||
}
|
||||
|
||||
/**
|
||||
* Languages in which `visibility: 'private'` on a definition means no other
|
||||
* FILE can name it: a Kotlin `private fun` is file- or class-local, and the
|
||||
@@ -929,6 +944,9 @@ export function isVisibleAcrossFiles(candidate: Node, ref: UnresolvedRef, contex
|
||||
return ref.filePath.startsWith(owner + '/');
|
||||
}
|
||||
if (PRIVATE_IS_FILE_LOCAL.has(lang)) return candidate.visibility !== 'private';
|
||||
// A Lua `local` belongs to its chunk: kong's spec helpers' `local it = it`
|
||||
// took busted's `it(…)` in every other spec file, 4,166 times.
|
||||
if ((lang === 'lua' || lang === 'luau') && isLuaLocal(candidate, context)) return false;
|
||||
// JS/TS/ArkTS sealed modules + markdown/JSON call-target guards (#1719).
|
||||
// Same predicate matchByExactName / matchFuzzy apply to their survivors so a
|
||||
// rejection here cannot fall through to a promoted runner-up.
|
||||
@@ -2787,6 +2805,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void {
|
||||
LEXICAL_SCOPE_MEMO.delete(context);
|
||||
JAVA_SUPERS.delete(context);
|
||||
PHP_SUPERS.delete(context);
|
||||
LUA_LOCALS.delete(context);
|
||||
PHP_FILE_SCOPES.delete(context);
|
||||
JAVA_STATIC_IMPORTS.delete(context);
|
||||
PY_IMPORTS.delete(context);
|
||||
|
||||
Reference in New Issue
Block a user