mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-19 14:32:59 +08:00
fix(kotlin): top-level functions resolve only where they're visible (#2146)
A Kotlin call's top-level candidate (a function, an extension function, a property) must be in the caller's package, imported by name, or in a star-imported package. Class members aren't judged: a lambda's receiver can put any type's members in scope. A function type with a qualified receiver (`(A.Builder.() -> Unit)?`) broke the tree-sitter-kotlin parse badly enough to drop the enclosing class; an offset-preserving preParse blanks the qualifier (hoisted, so the kernel parses the same bytes — parity 0 diffs on Exposed, okhttp, nowinandroid). A/B (edges removed / added): Exposed -5370/+4256 (1,318 R2DBC assertEquals misroutes; DatabaseTestsBase's members and 250 extends/imports edges back), koin -965/+960, okhttp -690/+675 (567 MockResponse to the imported mockwebserver3 class), okio -98/+44, nowinandroid -14. 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 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.
|
||||
- In R, a variable assigned at the top of a test file is no longer linked from the package or from other tests, because testthat runs each test file in its own environment. On ggplot2, a test's `c <- ggplot(…)` used to collect 2,455 of the package's `c(…)` calls. Variables in `helper-*.R` and `setup-*.R`, which testthat loads for every test, are still shared.
|
||||
- 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.
|
||||
|
||||
@@ -0,0 +1,148 @@
|
||||
/**
|
||||
* A top-level Kotlin function — an extension function included — is visible
|
||||
* from its own package, and from a file that imports it by name or imports
|
||||
* its package with `*`. Nowhere else.
|
||||
*
|
||||
* Exposed keeps a JDBC and an R2DBC test suite, each with a
|
||||
* `fun <T> …Transaction.assertEquals(…)` helper. Tests that imported
|
||||
* kotlin.test's or JUnit's `assertEquals`, or the JDBC helper, all landed on
|
||||
* the R2DBC one: 3,075 calls. okhttp's tests constructed the deprecated
|
||||
* `okhttp3.mockwebserver.MockResponse` although every one imports
|
||||
* `mockwebserver3.MockResponse`.
|
||||
*
|
||||
* Also: a function type with a qualified receiver
|
||||
* (`(DatabaseConfig.Builder.() -> Unit)?`) no longer breaks the parse of the
|
||||
* class around it. Exposed's `DatabaseTestsBase` came out as loose functions.
|
||||
*/
|
||||
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';
|
||||
import { blankKotlinQualifiedReceivers } from '../src/extraction/languages/kotlin';
|
||||
|
||||
let root = '';
|
||||
let cg: CodeGraph;
|
||||
|
||||
beforeAll(async () => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-kotlin-visibility-'));
|
||||
const files: Record<string, string> = {
|
||||
'jdbc/src/main/kotlin/app/jdbc/tests/Assert.kt': `package app.jdbc.tests
|
||||
|
||||
class JdbcTransaction
|
||||
|
||||
fun <T> JdbcTransaction.assertEquals(exp: T, act: T) {}
|
||||
`,
|
||||
'r2dbc/src/main/kotlin/app/r2dbc/tests/Assert.kt': `package app.r2dbc.tests
|
||||
|
||||
class R2dbcTransaction
|
||||
|
||||
fun <T> R2dbcTransaction.assertEquals(exp: T, act: T) {}
|
||||
`,
|
||||
'jdbc/src/test/kotlin/app/jdbc/JdbcTest.kt': `package app.jdbc
|
||||
|
||||
import app.jdbc.tests.assertEquals
|
||||
|
||||
class JdbcTest {
|
||||
fun works() {
|
||||
assertEquals(1, 1)
|
||||
}
|
||||
}
|
||||
`,
|
||||
'r2dbc/src/test/kotlin/app/r2dbc/R2dbcTest.kt': `package app.r2dbc
|
||||
|
||||
import app.r2dbc.tests.*
|
||||
|
||||
class R2dbcTest {
|
||||
fun works() {
|
||||
assertEquals(1, 1)
|
||||
}
|
||||
}
|
||||
`,
|
||||
'core/src/test/kotlin/app/core/PlainTest.kt': `package app.core
|
||||
|
||||
import kotlin.test.assertEquals
|
||||
|
||||
class PlainTest {
|
||||
fun works() {
|
||||
assertEquals(1, 1)
|
||||
}
|
||||
}
|
||||
`,
|
||||
'jdbc/src/main/kotlin/app/jdbc/tests/Local.kt': `package app.jdbc.tests
|
||||
|
||||
fun check() {
|
||||
assertEquals(1, 1)
|
||||
}
|
||||
`,
|
||||
'jdbc/src/main/kotlin/app/jdbc/tests/DatabaseTestsBase.kt': `package app.jdbc.tests
|
||||
|
||||
class DatabaseConfig {
|
||||
class Builder
|
||||
}
|
||||
|
||||
@MethodSource("data")
|
||||
abstract class DatabaseTestsBase {
|
||||
fun withTables(configure: (DatabaseConfig.Builder.() -> Unit)? = null, statement: () -> Unit) {
|
||||
statement()
|
||||
}
|
||||
}
|
||||
`,
|
||||
};
|
||||
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 });
|
||||
});
|
||||
|
||||
/** The file each `assertEquals` call in `file` resolved to. */
|
||||
function assertTargets(file: string): string[] {
|
||||
const ids = cg.getNodesInFile(file).map((n) => n.id);
|
||||
return cg
|
||||
.getOutgoingEdgesFrom(ids)
|
||||
.filter((e) => e.kind === 'calls')
|
||||
.map((e) => cg.getNode(e.target)!)
|
||||
.filter((n) => n.name === 'assertEquals')
|
||||
.map((n) => n.filePath);
|
||||
}
|
||||
|
||||
describe('Kotlin top-level functions are visible where imported', () => {
|
||||
it('an explicit import picks that package’s extension', () => {
|
||||
expect(assertTargets('jdbc/src/test/kotlin/app/jdbc/JdbcTest.kt')).toEqual(['jdbc/src/main/kotlin/app/jdbc/tests/Assert.kt']);
|
||||
});
|
||||
|
||||
it('a star import of the package does too', () => {
|
||||
expect(assertTargets('r2dbc/src/test/kotlin/app/r2dbc/R2dbcTest.kt')).toEqual(['r2dbc/src/main/kotlin/app/r2dbc/tests/Assert.kt']);
|
||||
});
|
||||
|
||||
it('a file importing kotlin.test’s assertEquals reaches neither project helper', () => {
|
||||
expect(assertTargets('core/src/test/kotlin/app/core/PlainTest.kt')).toEqual([]);
|
||||
});
|
||||
|
||||
it('the same package needs no import', () => {
|
||||
expect(assertTargets('jdbc/src/main/kotlin/app/jdbc/tests/Local.kt')).toEqual(['jdbc/src/main/kotlin/app/jdbc/tests/Assert.kt']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('a function type with a qualified receiver', () => {
|
||||
it('keeps the class around it', () => {
|
||||
const nodes = cg.getNodesInFile('jdbc/src/main/kotlin/app/jdbc/tests/DatabaseTestsBase.kt');
|
||||
const withTables = nodes.find((n) => n.name === 'withTables')!;
|
||||
expect(withTables.kind).toBe('method');
|
||||
expect(withTables.qualifiedName).toBe('app.jdbc.tests::DatabaseTestsBase::withTables');
|
||||
});
|
||||
|
||||
it('is blanked to its simple name, offsets intact', () => {
|
||||
const src = 'fun f(c: (DatabaseConfig.Builder.() -> Unit)?, d: a.b.Response.Builder<T>.(Int) -> Unit, e: Builder.() -> Unit) {}';
|
||||
const out = blankKotlinQualifiedReceivers(src);
|
||||
expect(out.length).toBe(src.length);
|
||||
const blank = (s: string) => ' '.repeat(s.length);
|
||||
expect(out).toBe(src.replace('DatabaseConfig.', blank('DatabaseConfig.')).replace('a.b.Response.', blank('a.b.Response.')));
|
||||
});
|
||||
});
|
||||
@@ -161,7 +161,24 @@ function isFunInterfaceNode(node: SyntaxNode): boolean {
|
||||
return hasFun && hasInterfaceType;
|
||||
}
|
||||
|
||||
/**
|
||||
* The qualifier of a function type's receiver — `DatabaseConfig.` in
|
||||
* `configure: (DatabaseConfig.Builder.() -> Unit)?`. The grammar reads a
|
||||
* single-segment receiver (`Builder.() -> Unit`) but not a qualified one, and
|
||||
* its error recovery then drops the enclosing class: Exposed's
|
||||
* `DatabaseTestsBase` came out as a file of loose functions. `X.(` is not an
|
||||
* expression, so the shape only occurs in a type.
|
||||
*/
|
||||
const QUALIFIED_RECEIVER = /\b(?:[A-Za-z_]\w*(?:<[^<>()\n]*>)?\.)+(?=[A-Za-z_]\w*(?:<[^<>()\n]*>)?\.\()/g;
|
||||
|
||||
/** Blank a function type's receiver qualifier to spaces (offsets survive), leaving its simple type name. */
|
||||
export function blankKotlinQualifiedReceivers(source: string): string {
|
||||
if (!source.includes('.(')) return source;
|
||||
return source.replace(QUALIFIED_RECEIVER, (m) => ' '.repeat(m.length));
|
||||
}
|
||||
|
||||
export const kotlinExtractor: LanguageExtractor = {
|
||||
preParse: blankKotlinQualifiedReceivers,
|
||||
functionTypes: ['function_declaration'],
|
||||
classTypes: ['class_declaration'],
|
||||
methodTypes: ['function_declaration'], // Methods are functions inside classes
|
||||
|
||||
@@ -1431,6 +1431,57 @@ function dartHeadOf(decl: Node, context: ResolutionContext): { supers: string[];
|
||||
};
|
||||
}
|
||||
|
||||
const KOTLIN_FILE_SCOPES = new WeakMap<ResolutionContext, Map<string, { pkg: string; imports: Set<string>; stars: Set<string> }>>();
|
||||
/** Packages every Kotlin file imports without writing it. */
|
||||
const KOTLIN_DEFAULT_IMPORTS: ReadonlySet<string> = new Set([
|
||||
'kotlin', 'kotlin.annotation', 'kotlin.collections', 'kotlin.comparisons', 'kotlin.io', 'kotlin.ranges',
|
||||
'kotlin.sequences', 'kotlin.text', 'kotlin.jvm', 'java.lang', 'kotlin.js',
|
||||
]);
|
||||
const KOTLIN_ENCLOSING_KINDS: ReadonlySet<string> = new Set([
|
||||
'class', 'interface', 'enum', 'struct', 'trait', 'protocol', 'module', 'namespace', 'function', 'method',
|
||||
]);
|
||||
|
||||
/** A Kotlin file's `package` and the names and packages its `import`s bring in. */
|
||||
function kotlinFileScope(file: string, context: ResolutionContext): { pkg: string; imports: Set<string>; stars: Set<string> } {
|
||||
let memo = KOTLIN_FILE_SCOPES.get(context);
|
||||
if (!memo) KOTLIN_FILE_SCOPES.set(context, (memo = new Map()));
|
||||
const hit = memo.get(file);
|
||||
if (hit) return hit;
|
||||
const text = (context.readFile(file) ?? '').replace(/\/\*[\s\S]*?\*\//g, ' ').replace(/`/g, '');
|
||||
const pkg = /^\s*package\s+([\w.]+)/m.exec(text)?.[1] ?? '';
|
||||
const imports = new Set<string>();
|
||||
const stars = new Set<string>();
|
||||
for (const m of text.matchAll(/^\s*import\s+([\w.]+?)(\.\*)?(?:\s+as\s+\w+)?\s*;?\s*(?:\/\/.*)?$/gm)) {
|
||||
if (m[2]) stars.add(m[1]!);
|
||||
else imports.add(m[1]!);
|
||||
}
|
||||
const scope = { pkg, imports, stars };
|
||||
memo.set(file, scope);
|
||||
return scope;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a top-level Kotlin declaration — a function, an extension function
|
||||
* (indexed under its receiver type, `JdbcTransaction::assertEquals`), a
|
||||
* property — can be named from the file a call is written in: its own
|
||||
* package, an `import` of it, or a star import of its package. A member of a
|
||||
* class is not judged here; a lambda's receiver can put any type's members in
|
||||
* scope. Exposed's tests call `assertEquals(…)` from kotlin.test and JUnit, or
|
||||
* import the JDBC suite's extension; the calls went to the R2DBC suite's
|
||||
* extension 3,075 times.
|
||||
*/
|
||||
function isKotlinTopLevelVisible(n: Node, ref: UnresolvedRef, context: ResolutionContext): boolean {
|
||||
if (n.language !== 'kotlin' || n.filePath === ref.filePath) return true;
|
||||
const enclosed = context
|
||||
.getNodesInFile(n.filePath)
|
||||
.some((o) => o.id !== n.id && KOTLIN_ENCLOSING_KINDS.has(o.kind) && o.startLine <= n.startLine && o.endLine >= n.endLine &&
|
||||
(o.startLine < n.startLine || o.endLine > n.endLine));
|
||||
if (enclosed) return true;
|
||||
const pkg = kotlinFileScope(n.filePath, context).pkg;
|
||||
const here = kotlinFileScope(ref.filePath, context);
|
||||
return pkg === here.pkg || here.stars.has(pkg) || here.imports.has(pkg ? `${pkg}.${n.name}` : n.name) || KOTLIN_DEFAULT_IMPORTS.has(pkg);
|
||||
}
|
||||
|
||||
const PHP_TYPE_KINDS: ReadonlySet<string> = new Set(['class', 'trait', 'interface', 'enum']);
|
||||
const PHP_SUPERS = new WeakMap<ResolutionContext, Map<string, string[]>>();
|
||||
|
||||
@@ -1859,9 +1910,11 @@ export function matchByExactName(
|
||||
const pythonShape = pythonCallShape(ref, context);
|
||||
const javaBare = ref.language === 'java' && ref.referenceKind === 'calls' && /^[A-Za-z_$][\w$]*$/.test(ref.referenceName);
|
||||
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 phpSelf = phpSelfReceiver(ref, context);
|
||||
const filtered = sameName.filter((n) =>
|
||||
!(javaBare && n.kind === 'method' && !isJavaMethodInScope(n, ref, context)) &&
|
||||
!(kotlinCall && !isKotlinTopLevelVisible(n, ref, context)) &&
|
||||
!(dartBare && isDartMember(n) && !isDartMethodInScope(n, ref, context)) &&
|
||||
!(phpSelf && (n.kind !== 'method' || !isPhpMethodInScope(n, ref, phpSelf, context))) &&
|
||||
!(pythonShape && !fitsPythonCallShape(n, pythonShape, ref, context)) &&
|
||||
@@ -2975,6 +3028,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void {
|
||||
PHP_SUPERS.delete(context);
|
||||
DART_SUPERS.delete(context);
|
||||
DART_HIERARCHIES.delete(context);
|
||||
KOTLIN_FILE_SCOPES.delete(context);
|
||||
LUA_LOCALS.delete(context);
|
||||
PHP_FILE_SCOPES.delete(context);
|
||||
JAVA_STATIC_IMPORTS.delete(context);
|
||||
@@ -4929,6 +4983,7 @@ export function matchFuzzy(
|
||||
const pythonShape = pythonCallShape(ref, context);
|
||||
const javaBare = ref.language === 'java' && ref.referenceKind === 'calls' && /^[A-Za-z_$][\w$]*$/.test(ref.referenceName);
|
||||
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 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
|
||||
@@ -4944,6 +4999,7 @@ export function matchFuzzy(
|
||||
!(pythonShape && !fitsPythonCallShape(n, pythonShape, ref, context)) &&
|
||||
!(javaBare && n.kind === 'method' && !isJavaMethodInScope(n, ref, context)) &&
|
||||
!(dartBare && isDartMember(n) && !isDartMethodInScope(n, ref, context)) &&
|
||||
!(kotlinCall && !isKotlinTopLevelVisible(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