From 75976f9929b1c515d250c7f7eb5fb7e15129ffee Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Wed, 30 Sep 2026 09:44:16 +0000 Subject: [PATCH] fix(kotlin): top-level functions resolve only where they're visible (#2146) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGELOG.md | 1 + __tests__/kotlin-top-level-visibility.test.ts | 148 ++++++++++++++++++ src/extraction/languages/kotlin.ts | 17 ++ src/resolution/name-matcher.ts | 56 +++++++ 4 files changed, 222 insertions(+) create mode 100644 __tests__/kotlin-top-level-visibility.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4364e23b8..eaa4f107b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/__tests__/kotlin-top-level-visibility.test.ts b/__tests__/kotlin-top-level-visibility.test.ts new file mode 100644 index 000000000..f5dce7ed0 --- /dev/null +++ b/__tests__/kotlin-top-level-visibility.test.ts @@ -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 …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 = { + 'jdbc/src/main/kotlin/app/jdbc/tests/Assert.kt': `package app.jdbc.tests + +class JdbcTransaction + +fun JdbcTransaction.assertEquals(exp: T, act: T) {} +`, + 'r2dbc/src/main/kotlin/app/r2dbc/tests/Assert.kt': `package app.r2dbc.tests + +class R2dbcTransaction + +fun 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.(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.'))); + }); +}); diff --git a/src/extraction/languages/kotlin.ts b/src/extraction/languages/kotlin.ts index e0b40927a..7b8d305a6 100644 --- a/src/extraction/languages/kotlin.ts +++ b/src/extraction/languages/kotlin.ts @@ -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 diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 30467d7af..7a1d5ecca 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -1431,6 +1431,57 @@ function dartHeadOf(decl: Node, context: ResolutionContext): { supers: string[]; }; } +const KOTLIN_FILE_SCOPES = new WeakMap; stars: Set }>>(); +/** Packages every Kotlin file imports without writing it. */ +const KOTLIN_DEFAULT_IMPORTS: ReadonlySet = 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 = 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; stars: Set } { + 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(); + const stars = new Set(); + 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 = new Set(['class', 'trait', 'interface', 'enum']); const PHP_SUPERS = new WeakMap>(); @@ -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));