diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a0de5ef..b71a8a17 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 +- Kotlin classes whose primary constructor sits on the line after the class name, before a supertype list (`expect open class ByteString` / `internal constructor(…) : Comparable`, common in Kotlin Multiplatform code), no longer lose their members to loose functions. A call later in a chain to a Kotlin standard-library function (`apply`, `let`, `toString`, `map`, `contains`…) no longer lands on a project type's same-named method. On okhttp, `.apply { … }` used to land on `ConnectionSpec.apply`. Re-index after upgrading. - In Rust and Go, a call later in a chain, like `sym.filename().map(…)` or `c.Flags().String(…)`, no longer lands on the project's own method of that name when the name is a standard-library one (`unwrap`, `clone`, `map`, `iter`, Go's `String` or `Error`). A plain call like `get(handler)` or `drop(x)` never lands on a method. On tokio, `.map(…)` on an `Option` used to land on `MutexGuard::map`; on axum, the router's `get(handler)` on a cookie jar's `get`; on ripgrep, `.clone()` on an error type. - A call on a type from outside the project, like `Integer.valueOf(…)`, `Arrays.asList(…)`, `Object.assign(…)` or Delphi's `Exception.Create(…)`, no longer lands on the project's own method with that name. On commons-lang, `Integer.valueOf` used to land on `StringUtils.valueOf` over 800 times. Go's exported variables and Pascal's capitalized parameters and locals are not mistaken for type names. - Imports through a tsconfig or jsconfig path alias now use the config nearest the importing file, so each app in a monorepo resolves its own `@/…` or `~/…` alias. On bulletproof-react and trpc's examples, those imports used to fail or land in a different app. diff --git a/__tests__/kotlin-split-constructor-std-chain.test.ts b/__tests__/kotlin-split-constructor-std-chain.test.ts new file mode 100644 index 00000000..645c2f72 --- /dev/null +++ b/__tests__/kotlin-split-constructor-std-chain.test.ts @@ -0,0 +1,78 @@ +/** + * Two Kotlin fixes that travel together: + * - a primary constructor on the line after its class name with a supertype + * list (`expect open class ByteString` / `internal constructor(…) : + * Comparable {`, Kotlin Multiplatform's shape) dropped the class + * and scattered its members as loose functions; the constructor's modifiers + * and keyword are blanked (offsets kept) so the class parses; + * - with members back on their classes, a standard-library name on an + * untyped chain link (`builder.apply { … }`, `"x".toPath().toString()`) + * needs a receiver named after the owner — okhttp's `.apply { }` went to + * `ConnectionSpec.apply`. + */ +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 { joinKotlinSplitConstructors } from '../src/extraction/languages/kotlin'; + +let root = ''; +let cg: CodeGraph; + +const BYTE_STRING = `package okio + +expect open class ByteString +// Trusted internal constructor doesn't clone data. +internal constructor(data: ByteArray) : Comparable { + fun hex(): String + fun toByteArray(): ByteArray +} +`; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-kotlin-split-')); + const files: Record = { + 'src/commonMain/kotlin/okio/ByteString.kt': BYTE_STRING, + 'src/main/kotlin/app/ConnectionSpec.kt': `package app + +class ConnectionSpec { + fun apply(sslSocket: Any, isFallback: Boolean) {} +} +`, + 'src/main/kotlin/app/Builder.kt': `package app + +class Builder { + var name = "" + fun build(): String = StringBuilder().apply { append(name) }.toString() +} +`, + }; + 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('Kotlin split primary constructors and standard chain calls', () => { + it('keeps the class and its members', () => { + const nodes = cg.getNodesInFile('src/commonMain/kotlin/okio/ByteString.kt'); + const hex = nodes.find((n) => n.name === 'hex')!; + expect(hex.qualifiedName).toBe('okio::ByteString::hex'); + const out = joinKotlinSplitConstructors(BYTE_STRING); + expect(out.length).toBe(BYTE_STRING.length); + expect(out.split('\n')[4]).toMatch(/^\s+\(data: ByteArray\) : Comparable \{$/); + }); + + it('never sends a scope function to a project type', () => { + const build = cg.getNodesInFile('src/main/kotlin/app/Builder.kt').find((n) => n.name === 'build')!; + const targets = cg.getOutgoingEdges(build.id).filter((e) => e.kind === 'calls').map((e) => cg.getNode(e.target)!.qualifiedName); + expect(targets.some((t) => t.endsWith('ConnectionSpec::apply'))).toBe(false); + }); +}); diff --git a/src/extraction/languages/kotlin.ts b/src/extraction/languages/kotlin.ts index 7b8d305a..d2383bf0 100644 --- a/src/extraction/languages/kotlin.ts +++ b/src/extraction/languages/kotlin.ts @@ -177,8 +177,29 @@ export function blankKotlinQualifiedReceivers(source: string): string { return source.replace(QUALIFIED_RECEIVER, (m) => ' '.repeat(m.length)); } +/** + * A primary constructor written on the line after its class name — + * `expect open class ByteString` / `internal constructor(data: ByteArray) : + * Comparable {` (okio, and Kotlin Multiplatform code generally) — + * with a supertype list after it: the grammar reads it as two statements and + * drops the class, its members coming out as loose functions. Blanking the + * constructor's modifiers and keyword (`(data: ByteArray) : …` parses) keeps + * the class, its supertypes and every offset; only the visibility is lost. + */ +const SPLIT_PRIMARY_CONSTRUCTOR = + /(\bclass\s+[A-Za-z_]\w*(?:\s*<[^<>{}]*(?:<[^<>{}]*>[^<>{}]*)*>)?[ \t]*\r?\n(?:[ \t]*\/\/[^\n]*\n)*[ \t]*)((?:(?:public|private|protected|internal|actual|expect)\s+|@[\w.]+(?:\([^)]*\))?\s+)*constructor)(?=\s*\()/g; + +export function joinKotlinSplitConstructors(source: string): string { + if (!source.includes('constructor')) return source; + return source.replace(SPLIT_PRIMARY_CONSTRUCTOR, (_m, head: string, ctor: string) => head + ' '.repeat(ctor.length)); +} + +function preParseKotlin(source: string): string { + return joinKotlinSplitConstructors(blankKotlinQualifiedReceivers(source)); +} + export const kotlinExtractor: LanguageExtractor = { - preParse: blankKotlinQualifiedReceivers, + preParse: preParseKotlin, 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 f48afcd0..e0244cbd 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -1572,6 +1572,50 @@ function isRustGoCallTarget(n: Node, shape: ReturnType): return !std || /^(?:self|Self)$/.test(shape.receiver) || (shape.receiver !== '' && sharesReceiverWord(shape.receiver, n)); } +/** + * Kotlin's scope functions and standard collection / string / conversion + * methods: on an untyped chain link (`builder.apply { … }`, `list.map { … }`) + * they are the standard library's, never a project type's same-named member. + * Names project types commonly carry (`get`, `write`, `close`, `add`) are left + * out — okio's `sink.write(…)` is its own Buffer's. + */ +const KOTLIN_STD_METHODS: ReadonlySet = new Set([ + 'apply', 'also', 'let', 'run', 'takeIf', 'takeUnless', 'toString', 'equals', 'hashCode', 'map', 'mapNotNull', + 'mapIndexed', 'filter', 'filterNot', 'filterIsInstance', 'forEach', 'forEachIndexed', 'first', 'firstOrNull', + 'last', 'lastOrNull', 'single', 'singleOrNull', 'isEmpty', 'isNotEmpty', 'isNullOrEmpty', 'isNullOrBlank', + 'isBlank', 'isNotBlank', 'orEmpty', 'joinToString', 'toList', 'toMutableList', 'toSet', 'toMutableSet', 'toMap', + 'toTypedArray', 'any', 'all', 'none', 'count', 'sumOf', 'maxOf', 'minOf', 'maxOrNull', 'minOrNull', 'sortedBy', + 'sortedByDescending', 'sorted', 'sortedWith', 'reversed', 'drop', 'dropLast', 'take', 'takeLast', 'zip', + 'flatMap', 'flatten', 'distinct', 'groupBy', 'associate', 'associateBy', 'associateWith', 'partition', + 'contains', 'containsKey', 'getOrElse', 'getOrNull', 'getOrPut', 'getOrDefault', 'trim', 'trimEnd', 'trimStart', + 'split', 'substring', 'startsWith', 'endsWith', 'replace', 'lowercase', 'uppercase', 'toInt', 'toLong', + 'toDouble', 'toFloat', 'toIntOrNull', 'toLongOrNull', 'encodeToByteArray', 'decodeToString', + 'copyOf', 'copyOfRange', 'indexOf', 'lastIndexOf', 'withIndex', 'asSequence', 'asList', 'ifEmpty', 'ifBlank', + 'padStart', 'padEnd', 'repeat', 'lines', 'toCharArray', 'coerceAtLeast', 'coerceAtMost', 'coerceIn', +]); + +/** The receiver a Kotlin chain link `….name(` / `?.name {` is written on, or null for a call with none. */ +function kotlinChainReceiver(ref: UnresolvedRef, context: ResolutionContext): string | null { + const line = context.getFileLines?.(ref.filePath)?.[ref.line - 1] ?? context.readFile(ref.filePath)?.split('\n')[ref.line - 1]; + if (line === undefined) return null; + const name = ref.referenceName; + let start = line.startsWith(name, ref.column) ? ref.column : -1; + if (start < 0) { + const m = new RegExp(`(? + !(kotlinStdChain !== null && !isKotlinStdChainTarget(n, kotlinStdChain)) && !(rustGoShape && !isRustGoCallTarget(n, rustGoShape)) && !(scalaBare && !isScalaMemberInScope(n, ref, context)) && !(csharpBare && !isCsharpMemberInScope(n, ref, context)) && @@ -4894,9 +4941,9 @@ export function matchMethodCall( !((ref.language === 'lua' || ref.language === 'luau') && isLuaLibraryCall(objectOrClass!, methodName!, ref, targetMethods[0]!)) && // Rust / Go: a standard-library method name on an untyped receiver // (`sym.map(…)`, `w.Header().Get(…)`) is the library type's. - !((ref.language === 'rust' || ref.language === 'go') && - (ref.language === 'go' ? GO_STD_METHODS : RUST_STD_METHODS).has(methodName!) && - !/^(?:self|Self)$/.test(objectOrClass!) && !sharesReceiverWord(objectOrClass!, targetMethods[0]!)) && + !((ref.language === 'rust' || ref.language === 'go' || ref.language === 'kotlin') && + (ref.language === 'go' ? GO_STD_METHODS : ref.language === 'kotlin' ? KOTLIN_STD_METHODS : RUST_STD_METHODS).has(methodName!) && + !/^(?:self|Self|this)$/.test(objectOrClass!) && !sharesReceiverWord(objectOrClass!, targetMethods[0]!)) && !(UNTYPED_RECEIVER_LANGUAGES.has(ref.language) && !/^(?:self|self\.class|this|super|weak_?self|strong_?self)$/i.test(objectOrClass!) && !sharesReceiverWord(objectOrClass!, targetMethods[0]!) && !(ref.language === 'objc' && objcReceiverReaches(objectOrClass!, targetMethods[0]!, context)) && @@ -6014,6 +6061,8 @@ export function matchFuzzy( const scalaBare = ref.language === 'scala' && ref.referenceKind === 'calls' && /^[A-Za-z_$][\w$]*$/.test(ref.referenceName); const rustGoShape = (ref.language === 'rust' || ref.language === 'go') && ref.referenceKind === 'calls' && /^[A-Za-z_]\w*$/.test(ref.referenceName) ? rustGoCallShape(ref, context) : null; + const kotlinStdChain = ref.language === 'kotlin' && ref.referenceKind === 'calls' && KOTLIN_STD_METHODS.has(ref.referenceName) + ? kotlinChainReceiver(ref, context) : null; 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 @@ -6038,6 +6087,7 @@ export function matchFuzzy( !(csharpBare && !isCsharpMemberInScope(n, ref, context)) && !(scalaBare && !isScalaMemberInScope(n, ref, context)) && !(rustGoShape && !isRustGoCallTarget(n, rustGoShape)) && + !(kotlinStdChain !== null && !isKotlinStdChainTarget(n, kotlinStdChain)) && !(phpSelf && (n.kind !== 'method' || !isPhpMethodInScope(n, ref, phpSelf, context)))) .filter((n) => (ref.referenceKind !== 'references' && ref.referenceKind !== 'function_ref') || sameLanguageFamily(n.language, ref.language));