diff --git a/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts b/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts index 20e4cccb3..37adece51 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts @@ -23,6 +23,7 @@ import { foldKotlinOperands, isKotlinConstantFile, parseKotlinConstOperands, + unquoteKotlinIdentifier, type ModuleConstants, type RepoConstants, } from '../../../ingestion/route-extractors/kotlin-const-resolver.js'; @@ -60,7 +61,9 @@ import { * those routes unprefixed would publish paths the application does not serve. * A prefix that resolves only PARTLY (Kotlin's vararg spelling * `@RequestMapping("/lit", ApiPaths.BASE)`) still publishes its resolvable arm: - * suppression exists to avoid wrong routes, not to discard right ones. On a + * suppression exists to avoid wrong routes, not to discard right ones. An EMPTY + * path array (`@RequestMapping(arrayOf())`) is not a prefix at all and + * suppresses nothing — see `classifyPathArgument`. On a * `@FeignClient` the same rule is applied to whichever prefix GOVERNS, in the * "path wins" order the URL is assembled in — `@FeignClient(path)` first, then * the interface's `@RequestMapping` — and to both consumer lanes, `@(Get|...)Mapping` @@ -165,7 +168,7 @@ const arrayOfArg = (cap: string): string => `(call_expression * shape missing from it yields no route, which is the skip floor. The * unfoldable-CLASS-PREFIX analysis must not be written this way — there a shape * missing from the list means "emit unprefixed", a wrong route — so it inverts - * the test instead (see `hasResolvableLiteralPathElement`). + * the test instead (see `classifyPathArgument`). */ const FOLDABLE_PATH_EXPRESSIONS: ReadonlySet = new Set([ 'simple_identifier', @@ -277,21 +280,75 @@ function kotlinArrayOfElements(node: Parser.SyntaxNode): Parser.SyntaxNode[] | n } /** - * Does this route-annotation path expression carry at least one element the - * literal prefix patterns can resolve to a real path? + * What a route-annotation path argument says about the prefix it designates. + * Three answers, and only one of them may suppress a route: * - * Accepts exactly the three shapes those patterns harvest — a bare literal, a - * `[…]` collection element, an `arrayOf(…)` element — and only when the literal - * is uninterpolated. Anything else (`ApiPaths.BASE`, `buildPath()`, - * `if (…) "/a" else "/b"`, a template) is NOT resolvable, which is the whole - * predicate the unfoldable-prefix analysis inverts. + * - `'literal'` — at least one element is a plain literal, so the literal + * prefix patterns already harvested a real path. Nothing to suppress. + * - `'none'` — the argument designates NO path at all. An EMPTY array is + * Spring's spelling for "no prefix": `@RequestMapping(arrayOf())` maps the + * class at the application root, so `@GetMapping("/lit")` beneath it really + * is served at `/lit`. "No prefix" is not "an unresolvable prefix", and + * conflating them dropped every route under such a class — including plain + * literal ones, which no constant fold was ever involved in. Measured on + * `@RequestMapping(arrayOf())` + `@GetMapping("/lit")`: `GET /lit` served, + * nothing emitted. The same conflation hit `@FeignClient(path = arrayOf())`, + * where it dropped `consumer GET /orders`. + * - `'unresolvable'` — there IS an argument, it is not empty, and no element of + * it resolves to a literal (`ApiPaths.BASE`, `buildPath()`, + * `if (…) "/a" else "/b"`, an interpolated template). Only here is the served + * path unknowable, and only here may the routes below be suppressed. + * + * The `'none'` arm is reachable through `arrayOf()` only. `@RequestMapping([])` + * is a third spelling of the same idea, but tree-sitter-kotlin (fwcd) does not + * parse the class that carries it as a `class_declaration` at all — the whole + * declaration degrades to an `infix_expression`, no class-prefix pattern + * matches, and the routes fall through unprefixed. That is measured, not + * assumed; it is also why the empty-`[…]` case needs no arm here, since a + * `collection_literal` never reaches this function from an annotation argument. */ -function hasResolvableLiteralPathElement(expr: Parser.SyntaxNode): boolean { - if (isPlainStringLiteral(expr)) return true; - if (expr.type === 'collection_literal') return expr.namedChildren.some(isPlainStringLiteral); +type PathArgumentPrefix = 'literal' | 'none' | 'unresolvable'; + +function classifyPathArgument(expr: Parser.SyntaxNode): PathArgumentPrefix { + if (isPlainStringLiteral(expr)) return 'literal'; + if (expr.type === 'collection_literal') { + return expr.namedChildren.some(isPlainStringLiteral) ? 'literal' : 'unresolvable'; + } const elements = kotlinArrayOfElements(expr); - if (elements) return elements.some(isPlainStringLiteral); - return false; + if (elements) { + if (elements.length === 0) return 'none'; + return elements.some(isPlainStringLiteral) ? 'literal' : 'unresolvable'; + } + return 'unresolvable'; +} + +/** + * The type declarations enclosing `node`, INNERMOST FIRST, by declared name. + * + * This is the scope a bare constant reference in a route annotation is resolved + * against (see `qualifyKotlinRefInEnclosingTypes`). Without it the fold is + * entered with a file key and a name, and a companion member — bound unqualified + * only inside its own class body — had to be recorded file-wide to be reachable + * at all, so it won every bare reference in the file: measured `/companion` + * where Kotlin serves the top-level `/top`, and `/h2` where Kotlin serves the + * referencing class's own `/h1`. + * + * Both `class_declaration` (which is also how tree-sitter-kotlin models an + * `interface`) and `object_declaration` are collected, because Kotlin binds the + * members of both unqualified inside their bodies, and the constant map keys + * both as `.`. A `companion_object` contributes no link of its own: + * its members are keyed under the ENCLOSING class, which the walk reaches one + * hop further up. An anonymous declaration has no `type_identifier` and is + * skipped rather than guessed at. + */ +function kotlinEnclosingTypeNames(node: Parser.SyntaxNode): string[] { + const out: string[] = []; + for (let cur = node.parent; cur; cur = cur.parent) { + if (cur.type !== 'class_declaration' && cur.type !== 'object_declaration') continue; + const ident = cur.children.find((c) => c.type === 'type_identifier'); + if (ident) out.push(unquoteKotlinIdentifier(ident.text)); + } + return out; } // ─── Kotlin OkHttp builder verb-walk (parity with java-static-path.ts) ── @@ -674,7 +731,7 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin { if (!argNode || !classNode) continue; if ((resolvedPrefixes.get(classNode.id) ?? []).length > 0) continue; const expr = kotlinRouteArgumentExpression(argNode); - if (!expr || hasResolvableLiteralPathElement(expr)) continue; + if (!expr || classifyPathArgument(expr) !== 'unresolvable') continue; ids.add(classNode.id); } return ids; @@ -701,7 +758,7 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin { const classNode = match.captures.class; if (!argNode || !classNode) continue; const expr = kotlinFeignPathArgumentExpression(argNode); - if (!expr || hasResolvableLiteralPathElement(expr)) continue; + if (!expr || classifyPathArgument(expr) !== 'unresolvable') continue; ids.add(classNode.id); } return ids; @@ -1413,7 +1470,15 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin { if (!constants) continue; const operands = parseKotlinConstOperands(expr); if (operands === null) continue; - const rawPath = foldKotlinOperands(fileKey, operands, constants); + // A bare reference means whatever the ENCLOSING types bind it to before + // it means anything at file level — Kotlin's rule for a companion + // member, which is in scope unqualified only inside its own class body. + const rawPath = foldKotlinOperands( + fileKey, + operands, + constants, + kotlinEnclosingTypeNames(methodNode), + ); if (rawPath === null) continue; methodRoutes.push({ httpMethod, diff --git a/gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts b/gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts index d19bf8a94..dc6f93306 100644 --- a/gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts +++ b/gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts @@ -51,9 +51,15 @@ * of a class `F` in package `a.b.C`. Nothing in the syntax says which, so * the fold tries both readings (see `resolveImportedName`) instead of * guessing from casing. + * 5. **Any identifier may be backtick-quoted.** `` package com.example.`api` `` + * and `package com.example.api` are the SAME package to the compiler, and a + * keyword segment (`` com.example.`fun` ``) can only be spelled the quoted + * way. The grammar keeps the backticks in the node text, so every identifier is + * read through {@link unquoteKotlinIdentifier} before it becomes a map key + * or a lookup name — see that function for what a verbatim comparison cost. * - * TWO PLACES THIS BINDING NO LONGER MIRRORS JAVA, both because the mirrored - * behavior was wrong rather than merely different, and both open as a Java + * THREE PLACES THIS BINDING NO LONGER MIRRORS JAVA, each because the mirrored + * behavior was wrong rather than merely different, and each open as a Java * follow-up rather than fixed here: * * * `java-const-resolver.ts` flattens nested types into one file-level @@ -68,6 +74,13 @@ * language enforces. Kotlin's cannot, and inferring the package from the path * lets a path-suffix twin outrank the real declaration — so * {@link resolveKotlinImport} reads the declared `package` instead. + * * Java's fold entry points take a file and a name, because a Java `static + * final` reachable by simple name is reachable that way from anywhere in the + * file. A Kotlin COMPANION member is not: it is bound unqualified only inside + * its enclosing class body. {@link foldKotlinOperands} therefore also takes + * the enclosing type chain of the reference site, which is what lets the + * binding answer a bare reference by Kotlin's scoping rather than by "whoever + * was walked last" — see {@link qualifyKotlinRefInEnclosingTypes}. * * Constant shapes this binding harvests: * @@ -174,6 +187,37 @@ function declaredPackageOf(mc: ModuleConstants | undefined): string | null { /** Source extensions a Kotlin declaration can live in. */ const KOTLIN_EXTENSIONS = ['.kt', '.kts'] as const; +/** + * The name a backtick-quoted Kotlin identifier denotes: `` `api` `` → `api`. + * + * Kotlin lets ANY identifier be quoted, and requires it when the name is a + * keyword (`` com.example.`fun` ``). The two spellings name the same thing — the + * quotes are lexical syntax, not part of the name — but tree-sitter-kotlin keeps + * them in the node text, so every `simple_identifier` / `type_identifier` this + * module turns into a map key or a lookup name is read through here first. + * + * Measured cost of comparing verbatim, on a file declaring + * `` package com.example.`api` `` imported as `com.example.api.ApiPaths`: + * {@link declaredPackage} recorded `` com.example.`api` ``, + * {@link resolveKotlinImport} required an exact match on `com.example.api`, the + * one real candidate was rejected, and the route was dropped. Both sides are + * normalized because either side alone can carry the quotes — an import + * specifier may spell `` import com.example.`api`.ApiPaths `` while the + * declaration spells it plainly. + * + * Applied per DOT-SEPARATED SEGMENT, never to a whole dotted name: a quoted + * identifier cannot contain `.` (nor a newline, nor a backtick), so splitting + * first is exact. + */ +export function unquoteKotlinIdentifier(text: string): string { + return text.length >= 2 && text.startsWith('`') && text.endsWith('`') ? text.slice(1, -1) : text; +} + +/** {@link unquoteKotlinIdentifier} applied to every segment of a dotted name. */ +function unquoteKotlinDottedName(text: string): string { + return text.includes('`') ? text.split('.').map(unquoteKotlinIdentifier).join('.') : text; +} + /** * Recursion ceiling for {@link parseKotlinConstOperands}, counted in `+` links. * @@ -282,7 +326,9 @@ function declaresTopLevelName(mc: ModuleConstants, name: string): boolean { * runs in three steps, all of them "unique or nothing": * * 0. **Declared package** — only files whose `package` header is EXACTLY the - * sought package can carry the declaration. This is the authority, and it + * sought package can carry the declaration (compared after + * {@link unquoteKotlinIdentifier}, since backtick quoting is spelling and + * not identity). This is the authority, and it * is checked first. Kotlin does not require a file's directory to match its * package, so the reverse test — "does this path end with the package?" — * answers a different question, one any decoy directory can satisfy: a file @@ -326,10 +372,13 @@ function declaresTopLevelName(mc: ModuleConstants, name: string): boolean { */ export function resolveKotlinImport( _importingFileKey: string, - moduleSpec: string, + rawModuleSpec: string, candidateKeys: ReadonlySet, repo: RepoConstants, ): string | null { + // Normalized here as well as at extraction, so the function answers the same + // question however a caller spells the specifier. + const moduleSpec = unquoteKotlinDottedName(rawModuleSpec); const lastDot = moduleSpec.lastIndexOf('.'); const packageName = lastDot < 0 ? '' : moduleSpec.slice(0, lastDot); const simpleName = lastDot < 0 ? moduleSpec : moduleSpec.slice(lastDot + 1); @@ -401,14 +450,14 @@ function stringLiteralValue(node: Parser.SyntaxNode): string | null { * `this`, indexing, safe navigation — not a constant shape). */ function flattenNavigation(node: Parser.SyntaxNode): string | null { - if (node.type === 'simple_identifier') return node.text; + if (node.type === 'simple_identifier') return unquoteKotlinIdentifier(node.text); if (node.type === 'navigation_expression') { const target = node.namedChild(0); const suffix = node.namedChildren.find((c) => c.type === 'navigation_suffix'); const field = suffix?.namedChildren.find((c) => c.type === 'simple_identifier'); if (target && field) { const head = flattenNavigation(target); - return head === null ? null : `${head}.${field.text}`; + return head === null ? null : `${head}.${unquoteKotlinIdentifier(field.text)}`; } } return null; @@ -446,7 +495,7 @@ export function parseKotlinConstOperands( return value === null ? null : [{ kind: 'literal', value }]; } if (node.type === 'simple_identifier') { - return [{ kind: 'ref', name: node.text }]; + return [{ kind: 'ref', name: unquoteKotlinIdentifier(node.text) }]; } if (node.type === 'navigation_expression') { const name = flattenNavigation(node); @@ -512,13 +561,26 @@ interface KotlinConstDeclaration { */ readonly scopes: readonly string[]; /** - * Is the simple name a binding Kotlin actually exposes to the rest of the - * file? True for a top-level `val`, and for a companion member (visible - * unqualified throughout its enclosing class, which is where route - * annotations sit). FALSE for a member of a named `object`, which every - * caller outside that object's body must qualify. + * Is the simple name a FILE-LEVEL binding — one any reference in the file can + * use unqualified? True only for a top-level `val`. FALSE for a member of a + * named `object` (which every caller outside that object's body must qualify) + * and FALSE for a companion member, whose unqualified binding exists only + * inside its enclosing class body and is reached through + * {@link qualifyKotlinRefInEnclosingTypes} instead. */ - readonly bareVisible: boolean; + readonly fileLevelName: boolean; + /** + * Does an unfoldable initializer here take a same-named IMPORT down with it? + * + * True wherever the declaration binds the simple name for at least some of the + * file — a top-level `val` (everywhere) or a companion member (inside its + * class). Deliberately wider than {@link fileLevelName}: a companion's shadow + * is scoped, but this map is not, and over-deleting an import can only cost a + * route, whereas under-deleting one publishes the imported value at a + * reference the compiler resolves to the unfoldable member. An `object` member + * shadows nothing and is false. + */ + readonly shadowsImport: boolean; /** The parsed initializer, or null when it is not a foldable string. */ readonly operands: readonly Operand[] | null; } @@ -527,6 +589,10 @@ interface KotlinConstDeclaration { * The file's declared `package`, or `''` when it declares none (default * package). Shaped exactly like the import walk below: `package_header` holds * one `identifier` whose `simple_identifier` children are the dotted segments. + * + * Each segment is unquoted (see {@link unquoteKotlinIdentifier}), so a package + * declared `` com.example.`api` `` is recorded — and therefore matched — as the + * same package an import spells `com.example.api`. */ function declaredPackage(root: Parser.SyntaxNode): string { const header = root.children.find((c) => c.type === 'package_header'); @@ -534,7 +600,7 @@ function declaredPackage(root: Parser.SyntaxNode): string { if (!identifier) return ''; return identifier.namedChildren .filter((c) => c.type === 'simple_identifier') - .map((c) => c.text) + .map((c) => unquoteKotlinIdentifier(c.text)) .join('.'); } @@ -553,15 +619,32 @@ function declaredPackage(root: Parser.SyntaxNode): string { * is recorded under `.`, the spelling a qualified reference * uses, with a companion member keyed under its ENCLOSING CLASS (`Holder.NAME`) * because that is how Kotlin source refers to it — `Companion` never appears in - * a reference. The SIMPLE name is recorded only when Kotlin really binds it: - * for a top-level `val`, and for a companion member. A member of a named + * a reference. The SIMPLE name is recorded only for a TOP-LEVEL `val`, the one + * carrier whose bare binding really does span the file. A member of a named * `object` gets no bare key, because `BASE` alone does not name `A.BASE` from * anywhere outside `object A`'s own body. Writing one anyway (as this binding * and the Java one both used to) fabricates a binding the language does not * have, and a fabricated key outranks the genuine `import com.example.api.Paths.ORDERS` * that {@link computeKotlinFold} consults only after literals and expressions. * - * An initializer that names a SIBLING is therefore resolved against its own + * A COMPANION member gets no bare key either, for the same reason at a smaller + * radius: it is bound unqualified inside its enclosing class BODY and nowhere + * else. A file-level bare key put it in the same namespace as top-level + * declarations, and companions are recorded last, so it won every unqualified + * reference in the file. Measured, on an app that serves `/top`: + * + * const val ORDERS = "/top" + * class Holder { companion object { const val ORDERS = "/companion" } } + * @RestController class OrderController { + * @GetMapping(ORDERS) fun get() = "ok" // emitted /companion + * } + * + * The unqualified binding is instead reached from the reference site, by + * {@link qualifyKotlinRefInEnclosingTypes}, which rewrites a bare name to + * `.` when an enclosing type declares it — so the companion + * wins inside its own class and loses everywhere else, which is Kotlin's rule. + * + * An initializer that names a SIBLING is resolved the same way, against its own * scope chain, innermost first, before the file level: inside * `object A { const val BASE = "/right"; const val ROUTE = BASE + "/m" }` the * operand `BASE` is rewritten to `A.BASE`. Collecting every declaration before @@ -570,20 +653,21 @@ function declaredPackage(root: Parser.SyntaxNode): string { * object happened to be walked last, so moving `object B` above `object A` * changed the emitted route for source that had not changed at all. * - * KNOWN LIMIT, unchanged by the above: a companion member's bare key is - * file-wide, so two companions in one file whose members share a name still - * resolve last-wins for an UNQUALIFIED reference. Kotlin scopes that name to the - * enclosing class, which this map cannot express — the fold is entered with a - * file key and a name, and nothing tells it which class body the annotation sat - * in. Sibling INITIALIZERS are unaffected (they go through the scope chain - * above); only a bare reference from a route annotation can land on the wrong - * companion, and only when two companions in the same file collide. + * A TOP-LEVEL initializer has an EMPTY scope chain, so its bare operands are + * left bare and resolve at file level. That is now correct and was not before: + * with a file-wide companion key, `const val ROUTE = BASE + "/m"` beside a + * companion `BASE` folded through the companion — measured `/comp/m` where + * Kotlin serves `/top/m`. (An earlier revision of this comment claimed sibling + * initializers could not be affected because they "go through the scope chain"; + * an empty chain is exactly the case that claim missed.) * * A non-foldable rebind (`X = compute()`) DROPS X to unresolvable rather than - * leaving a stale literal — and drops a same-named import with it, but only when - * the declaration is bare-visible, since only then does it shadow the import for - * unqualified references. An `object` member of the same name shadows nothing - * and must leave the import alone. + * leaving a stale literal — and drops a same-named import with it whenever the + * declaration shadows that import ANYWHERE (top level, or a companion inside its + * class). The import map has no scopes, so a companion's shadow is applied + * file-wide: the conservative direction, costing a route rather than publishing + * the imported value at a reference the compiler binds to the unfoldable member. + * An `object` member shadows nothing and must leave the import alone. */ export function extractKotlinModuleConstants(tree: Parser.Tree): KotlinModuleConstants { const literals = new Map(); @@ -601,13 +685,14 @@ export function extractKotlinModuleConstants(tree: Parser.Tree): KotlinModuleCon if (!isWildcard && identifier) { const segments = identifier.namedChildren .filter((c) => c.type === 'simple_identifier') - .map((c) => c.text); + .map((c) => unquoteKotlinIdentifier(c.text)); if (segments.length >= 2) { const spec = segments.join('.'); const originalName = segments[segments.length - 1]; - const alias = node.children + const aliasNode = node.children .find((c) => c.type === 'import_alias') - ?.namedChildren.find((c) => c.type === 'type_identifier')?.text; + ?.namedChildren.find((c) => c.type === 'type_identifier'); + const alias = aliasNode ? unquoteKotlinIdentifier(aliasNode.text) : undefined; // `module` is the specifier AS WRITTEN, complete. Kotlin does not mark // member imports, so the fold — not the extractor — decides whether the // trailing segment is a declaration or one of its members. @@ -631,7 +716,8 @@ export function extractKotlinModuleConstants(tree: Parser.Tree): KotlinModuleCon body: Parser.SyntaxNode, declaringType: string | null, scopes: readonly string[], - bareVisible: boolean, + fileLevelName: boolean, + shadowsImport: boolean, ): void => { for (const member of body.children ?? []) { if (member.type !== 'property_declaration') continue; @@ -639,7 +725,7 @@ export function extractKotlinModuleConstants(tree: Parser.Tree): KotlinModuleCon const declaration = member.children.find((c) => c.type === 'variable_declaration'); const nameNode = declaration?.namedChildren.find((c) => c.type === 'simple_identifier'); if (!nameNode) continue; - const name = nameNode.text; + const name = unquoteKotlinIdentifier(nameNode.text); if (declaringType !== null) { let members = membersByScope.get(declaringType); if (!members) membersByScope.set(declaringType, (members = new Set())); @@ -652,7 +738,8 @@ export function extractKotlinModuleConstants(tree: Parser.Tree): KotlinModuleCon name, qualified: declaringType === null ? null : `${declaringType}.${name}`, scopes, - bareVisible, + fileLevelName, + shadowsImport, operands: parseKotlinConstOperands(initializerOf(member)), }); } @@ -661,6 +748,12 @@ export function extractKotlinModuleConstants(tree: Parser.Tree): KotlinModuleCon const bodyOf = (node: Parser.SyntaxNode): Parser.SyntaxNode | undefined => node.children.find((c) => c.type === 'class_body'); + /** The declared name of an `object_declaration` / `class_declaration`. */ + const typeNameOf = (node: Parser.SyntaxNode): string | null => { + const ident = node.children.find((c) => c.type === 'type_identifier'); + return ident ? unquoteKotlinIdentifier(ident.text) : null; + }; + const walkDeclarations = ( node: Parser.SyntaxNode, enclosingType: string | null, @@ -668,13 +761,13 @@ export function extractKotlinModuleConstants(tree: Parser.Tree): KotlinModuleCon ): void => { for (const child of node.children ?? []) { if (child.type === 'object_declaration') { - const name = child.children.find((c) => c.type === 'type_identifier')?.text ?? null; + const name = typeNameOf(child); const body = bodyOf(child); if (!body) continue; // Members are reachable only as `A.NAME`; inside the body, `NAME` alone // means this object's member and nothing else, hence the pushed scope. const inner = name === null ? scopes : [name, ...scopes]; - collectProperties(body, name, inner, false); + collectProperties(body, name, inner, false, false); walkDeclarations(body, name, inner); continue; } @@ -683,16 +776,18 @@ export function extractKotlinModuleConstants(tree: Parser.Tree): KotlinModuleCon if (!body) continue; // Referenced through the enclosing class (`Holder.NAME`), never through // `Companion` — so the qualified alias is keyed on `enclosingType`. The - // simple name IS bound, throughout that class body. + // simple name is bound inside that class body only, which is a SCOPE and + // not a file-level key: it is reached from the reference site by + // `qualifyKotlinRefInEnclosingTypes`, through this same `Holder.NAME`. const inner = enclosingType === null ? scopes : [enclosingType, ...scopes]; - collectProperties(body, enclosingType, inner, true); + collectProperties(body, enclosingType, inner, false, true); walkDeclarations(body, enclosingType, inner); continue; } if (child.type === 'class_declaration') { // A class/interface body's own `val`s are per-instance or abstract, so // only its nested objects and companion contribute constants. - const name = child.children.find((c) => c.type === 'type_identifier')?.text ?? null; + const name = typeNameOf(child); const body = bodyOf(child); if (body) walkDeclarations(body, name, scopes); continue; @@ -701,13 +796,13 @@ export function extractKotlinModuleConstants(tree: Parser.Tree): KotlinModuleCon } }; - collectProperties(tree.rootNode, null, [], true); + collectProperties(tree.rootNode, null, [], true, true); walkDeclarations(tree.rootNode, null, []); // Pass 2b: rewrite each initializer's unqualified operands against the scope - // chain that encloses it, then record. Top level last-wins over nothing; - // top-level declarations are recorded first and a companion's bare key after, - // which is the order Kotlin resolves them in inside the class body. + // chain that encloses it, then record. Only a top-level declaration writes a + // bare key, so nothing here can collide across scopes; a companion's + // unqualified binding is applied at the reference site instead. const qualifyRef = (refName: string, scopes: readonly string[]): string => { if (refName.includes('.')) return refName; // already carries its owner for (const scope of scopes) { @@ -718,7 +813,7 @@ export function extractKotlinModuleConstants(tree: Parser.Tree): KotlinModuleCon for (const decl of declarations) { const keys: string[] = []; - if (decl.bareVisible) keys.push(decl.name); + if (decl.fileLevelName) keys.push(decl.name); if (decl.qualified !== null) keys.push(decl.qualified); if (decl.operands === null) { @@ -726,8 +821,7 @@ export function extractKotlinModuleConstants(tree: Parser.Tree): KotlinModuleCon literals.delete(key); exprs.delete(key); } - // Only a bare-visible declaration shadows a same-named import. - if (decl.bareVisible) imports.delete(decl.name); + if (decl.shadowsImport) imports.delete(decl.name); continue; } @@ -944,10 +1038,53 @@ function foldOperands( return out; } +/** + * Rewrite one BARE reference to the enclosing type that binds it, or leave it + * bare when none does — the reference-site twin of the `qualifyRef` that + * {@link extractKotlinModuleConstants} applies to sibling initializers. + * + * `enclosingTypes` is the chain of type declarations the reference sits inside, + * INNERMOST FIRST (`['Inner', 'Outer']`). A companion member is keyed + * `.` and is bound unqualified exactly within that class + * body — including its nested types, which is why the whole chain is walked and + * not just the innermost link. An `object`'s own members are in scope inside its + * body under the same `.` key, so the same walk covers both. + * + * Innermost-first, and BEFORE the file-level maps the fold consults next, is + * Kotlin's own order: a companion member shadows a same-named top-level + * declaration and a same-named import throughout its class. Outside that class + * the bare name never means the companion at all, which is precisely what an + * empty chain expresses. + */ +export function qualifyKotlinRefInEnclosingTypes( + fileKey: string, + name: string, + repo: RepoConstants, + enclosingTypes: readonly string[], +): string { + if (name.includes('.')) return name; // already carries its owner + const mc = repo.get(fileKey); + if (!mc) return name; + for (const type of enclosingTypes) { + const key = `${type}.${name}`; + if (mc.literals.has(key) || mc.exprs.has(key)) return key; + } + return name; +} + /** * Fold an inline operand list (e.g. `ApiPaths.BASE + "/orders"`) against * `fileKey`, or null when any piece is unresolvable (skip floor). * + * `enclosingTypes` is the chain of type declarations the REFERENCE sits inside + * (innermost first), and it is applied to the entry operands only — everything + * deeper is either already qualified by + * {@link extractKotlinModuleConstants} against its own declaring scope, or lives + * in another file where this chain means nothing. Passing it empty answers + * "what does this name mean at file level", which is the right question for a + * reference outside any type and the only one a caller without position + * information can honestly ask. + * * An empty result is a SUCCESS, not a skip. `const val ROOT = ""` folds to `""`, * which `joinPath` then resolves against the class-level prefix exactly as it * resolves the literal `@GetMapping("")` — both mean "the prefix itself", the @@ -963,6 +1100,18 @@ export function foldKotlinOperands( fileKey: string, operands: readonly Operand[], repo: RepoConstants, + enclosingTypes: readonly string[] = [], ): string | null { - return foldOperands(fileKey, operands, newFoldState(repo), 0); + const scoped = + enclosingTypes.length === 0 + ? operands + : operands.map((op) => + op.kind === 'ref' + ? { + kind: 'ref' as const, + name: qualifyKotlinRefInEnclosingTypes(fileKey, op.name, repo, enclosingTypes), + } + : op, + ); + return foldOperands(fileKey, scoped, newFoldState(repo), 0); } diff --git a/gitnexus/test/unit/group/kotlin-const-route-fold.test.ts b/gitnexus/test/unit/group/kotlin-const-route-fold.test.ts index 1536b27ee..b50652625 100644 --- a/gitnexus/test/unit/group/kotlin-const-route-fold.test.ts +++ b/gitnexus/test/unit/group/kotlin-const-route-fold.test.ts @@ -319,6 +319,45 @@ class OrderController { ).toEqual(['GET /api/v1/orders']); }); + it('does not treat an EMPTY class path array as an unresolvable prefix', () => { + // `@RequestMapping(arrayOf())` designates NO prefix — Spring maps the class + // at the application root — so `/literal` really is served at `/literal`. + // Treating it as an unresolvable prefix suppressed every route under the + // class, the literal one included, which no constant fold was ever involved + // in. The arithmetic behind that: an empty array has no elements, and "no + // element is a literal" is trivially true of an empty set, so the class read + // as unresolvable. "No prefix" is not "an unresolvable prefix". + expect(providers(controllerWithPrefix('arrayOf()'))).toEqual([ + 'GET /api/v1/orders', + 'GET /literal', + ]); + }); + + it('still suppresses a NON-empty array whose only element is a constant', () => { + // The control for the test above, and the reason the empty case needs its + // own arm rather than a blanket "arrays never suppress": here the array DOES + // designate a prefix, and that prefix is unknowable. + expect(providers(controllerWithPrefix('arrayOf(ApiPaths.BASE)'))).toEqual([]); + }); + + it('does not treat an EMPTY @FeignClient(path) array as an unresolvable path', () => { + // Same distinction on the consumer side, where the governing-prefix guard is + // its own set: `path = arrayOf()` adds no prefix, so the remote URL is + // exactly the method's own path and the consumer is knowable. + expect( + consumers({ + [CLIENT]: `package com.example.app.client + +@FeignClient(name = "orders", path = arrayOf()) +interface OrderClient { + @GetMapping("/orders") + fun listOrders(): String +} +`, + }), + ).toEqual(['GET /orders']); + }); + it('still applies a LITERAL class prefix to a folded method path', () => { expect( providers({ @@ -759,6 +798,141 @@ class OrderController { ).toEqual([]); }); + it('resolves a bare reference by the class it sits in, not by declaration order', () => { + // A companion member is bound unqualified inside its enclosing class BODY + // and nowhere else. Recorded under a file-level bare key it landed in the + // same namespace as top-level declarations and, because companions are + // walked last, won every unqualified reference in the file — so the + // reference in `OrderController` below, which is not `Holder`'s body, + // published `/companion` where the application serves `/top`. + // + // Both classes are asserted from ONE file: a fixture with only the wrong + // one would also pass on an implementation that simply dropped companions, + // and a fixture with only the right one would pass on the old file-wide key. + expect( + providers({ + [CONTROLLER]: `package com.example.app.web + +const val ORDERS = "/top" + +class Holder { + companion object { + const val ORDERS = "/companion" + } + + @GetMapping(ORDERS) + fun inside() {} +} + +@RestController +class OrderController { + @GetMapping(ORDERS) + fun outside() {} +} +`, + }), + ).toEqual(['GET /companion', 'GET /top']); + }); + + it('gives two colliding companions each their own class', () => { + // Kotlin scopes each companion's members to its own class, so the two + // references below mean different constants even though they are spelled + // identically. One file-level namespace could only answer last-wins: both + // read `/h2`, and swapping the two classes flipped both to `/h1`. + expect( + providers({ + [CONTROLLER]: `package com.example.app.web + +@RestController +class FirstController { + companion object { + const val ORDERS = "/h1" + } + + @GetMapping(ORDERS) + fun list() {} +} + +@RestController +class SecondController { + companion object { + const val ORDERS = "/h2" + } + + @GetMapping(ORDERS) + fun list() {} +} +`, + }), + ).toEqual(['GET /h1', 'GET /h2']); + }); + + it('folds a top-level initializer at file level even when a companion shares the name', () => { + // `ROUTE`'s initializer is TOP-LEVEL, so its scope chain is empty and its + // operand `BASE` means the top-level `BASE`. With a file-wide companion key + // the empty chain left the operand bare and the companion answered it, + // publishing `/comp/m` where the application serves `/top/m`. + expect( + providers({ + [CONTROLLER]: `package com.example.app.web + +const val BASE = "/top" +const val ROUTE = BASE + "/m" + +class Holder { + companion object { + const val BASE = "/comp" + } +} + +@RestController +class OrderController { + @GetMapping(ROUTE) + fun list() {} +} +`, + }), + ).toEqual(['GET /top/m']); + }); + + it('folds through a package whose segment is backtick-quoted on one side only', () => { + // `` package com.example.app.`api` `` and `package com.example.app.api` are + // the same package to the compiler — the quotes are lexical syntax, not part + // of the name. Comparing the two verbatim rejected the one real candidate + // and dropped the route. Both directions are asserted because either side + // can carry the quotes. + const controller = (spec: string): string => `package com.example.app.web + +import ${spec} + +@RestController +class OrderController { + @GetMapping(ApiPaths.ORDERS) + fun list() {} +} +`; + const quotedDeclaration = `package com.example.app.\`api\` + +object ApiPaths { + const val ORDERS = "/api/v1/orders" +} +`; + // Declaration quoted, import plain. + expect( + providers({ + [CONSTS]: quotedDeclaration, + [CONTROLLER]: controller('com.example.app.api.ApiPaths'), + }), + ).toEqual(['GET /api/v1/orders']); + // Import quoted, declaration plain. + expect( + providers({ + [CONSTS]: CONSTS_SRC, + [CONTROLLER]: controller('com.example.app.`api`.ApiPaths'), + }), + ).toEqual(['GET /api/v1/orders']); + }); + it('leaves literal routes unchanged and emits each exactly once', () => { expect( providers({ diff --git a/gitnexus/test/unit/kotlin-route-const-resolver.test.ts b/gitnexus/test/unit/kotlin-route-const-resolver.test.ts index 5cf2cbf32..8bb170eec 100644 --- a/gitnexus/test/unit/kotlin-route-const-resolver.test.ts +++ b/gitnexus/test/unit/kotlin-route-const-resolver.test.ts @@ -592,9 +592,13 @@ const val ORDERS = "/local" expect(resolveKotlinConstant(CONTROLLER_KEY, 'ORDERS', repo)).toBe('/local'); }); - it('keeps a companion member visible under its bare name', () => { + it('keeps a companion member visible under its bare name INSIDE its class', () => { // The other control: a companion's members ARE in scope unqualified - // throughout the enclosing class, which is where route annotations sit. + // throughout the enclosing class, which is where route annotations sit — + // but ONLY there. The binding is reached from the reference site through + // the enclosing type chain, not from a file-level key, so all three + // spellings below are asserted together: the same name answers one way + // inside `OrderApi` and does not answer at all outside it. const key = 'src/main/kotlin/com/example/app/web/OrderApi.kt'; const repo = repoOf({ [key]: `package com.example.app.web @@ -606,10 +610,153 @@ class OrderApi { } `, }); - expect(resolveKotlinConstant(key, 'ORDERS', repo)).toBe('/companion/orders'); + const bare = [{ kind: 'ref', name: 'ORDERS' } as const]; + expect(foldKotlinOperands(key, bare, repo, ['OrderApi'])).toBe('/companion/orders'); + expect(foldKotlinOperands(key, bare, repo)).toBeNull(); expect(resolveKotlinConstant(key, 'OrderApi.ORDERS', repo)).toBe('/companion/orders'); }); + it('does not let a companion outrank a top-level constant outside its class', () => { + // Recording the companion's simple name at FILE level put it in the same + // namespace as the top-level declaration, and companions are recorded + // last, so the companion won every unqualified reference in the file — + // including from a class that is not its own. Kotlin binds the top-level + // `ORDERS` there. Both scopes are asserted from one file; either alone + // passes on an implementation that gets the other wrong. + const key = 'src/main/kotlin/com/example/app/web/Routes.kt'; + const repo = repoOf({ + [key]: `package com.example.app.web + +const val ORDERS = "/top" + +class Holder { + companion object { + const val ORDERS = "/companion" + } +} + +class OrderController +`, + }); + const bare = [{ kind: 'ref', name: 'ORDERS' } as const]; + expect(foldKotlinOperands(key, bare, repo, ['OrderController'])).toBe('/top'); + expect(foldKotlinOperands(key, bare, repo, ['Holder'])).toBe('/companion'); + expect(foldKotlinOperands(key, bare, repo)).toBe('/top'); + }); + + it('scopes each of two colliding companions to its own class', () => { + // Kotlin scopes the member name to its enclosing class, so the same + // spelling means a different constant in each body. One file-level + // namespace could only answer last-wins — both reads returned `/h2`, and + // reordering the two classes flipped both to `/h1`. Both orders are + // asserted, because either alone passes on a last-wins implementation. + const holders = (first: 'A' | 'B'): string => { + const a = `class HolderA { + companion object { + const val ORDERS = "/h1" + } +}`; + const b = `class HolderB { + companion object { + const val ORDERS = "/h2" + } +}`; + return `package com.example.app.web\n\n${first === 'A' ? `${a}\n\n${b}` : `${b}\n\n${a}`}\n`; + }; + const key = 'src/main/kotlin/com/example/app/web/Holders.kt'; + const bare = [{ kind: 'ref', name: 'ORDERS' } as const]; + for (const first of ['A', 'B'] as const) { + const repo = repoOf({ [key]: holders(first) }); + expect(foldKotlinOperands(key, bare, repo, ['HolderA']), `${first} first`).toBe('/h1'); + expect(foldKotlinOperands(key, bare, repo, ['HolderB']), `${first} first`).toBe('/h2'); + } + }); + + it('folds a TOP-LEVEL initializer at file level despite a same-named companion', () => { + // A top-level initializer has an EMPTY scope chain, so `qualifyRef` leaves + // its operand bare and the file-level maps answer it. A file-wide + // companion key WAS one of those maps, so `ROUTE` folded to `/comp/m` + // where Kotlin serves `/top/m` — the case an earlier note in the resolver + // claimed could not arise because sibling initializers "go through the + // scope chain". An empty chain is exactly what that missed. + const key = 'src/main/kotlin/com/example/app/web/Routes.kt'; + const repo = repoOf({ + [key]: `package com.example.app.web + +const val BASE = "/top" +const val ROUTE = BASE + "/m" + +class Holder { + companion object { + const val BASE = "/comp" + } +} +`, + }); + expect(resolveKotlinConstant(key, 'ROUTE', repo)).toBe('/top/m'); + // The companion's own binding is intact, reached the way Kotlin reaches it. + expect(resolveKotlinConstant(key, 'Holder.BASE', repo)).toBe('/comp'); + }); + + it('lets a single-name import win over a companion outside that companion class', () => { + // The fold consults literals before imports, so a file-level companion key + // outranked a genuine `import …Paths.ORDERS` everywhere in the file. The + // import is what Kotlin binds outside `Holder`; inside `Holder`, the + // companion shadows it. + const repo = repoOf({ + 'src/main/kotlin/com/example/app/api/Paths.kt': `package com.example.app.api + +object Paths { + const val ORDERS = "/imported" +} +`, + [CONTROLLER_KEY]: `package com.example.app.web + +import com.example.app.api.Paths.ORDERS + +class Holder { + companion object { + const val ORDERS = "/companion" + } +} +`, + }); + const bare = [{ kind: 'ref', name: 'ORDERS' } as const]; + expect(foldKotlinOperands(CONTROLLER_KEY, bare, repo)).toBe('/imported'); + expect(foldKotlinOperands(CONTROLLER_KEY, bare, repo, ['Other'])).toBe('/imported'); + expect(foldKotlinOperands(CONTROLLER_KEY, bare, repo, ['Holder'])).toBe('/companion'); + }); + + it('reaches an enclosing class companion from a NESTED type', () => { + // Kotlin keeps the companion's members in scope through the nested types + // of its class, so the whole enclosing chain is walked, innermost first — + // and the inner link still wins where both declare the name. + const key = 'src/main/kotlin/com/example/app/web/Nested.kt'; + const repo = repoOf({ + [key]: `package com.example.app.web + +class Outer { + companion object { + const val ORDERS = "/outer" + const val ONLY_OUTER = "/only-outer" + } + + class Inner { + companion object { + const val ORDERS = "/inner" + } + } +} +`, + }); + expect( + foldKotlinOperands(key, [{ kind: 'ref', name: 'ORDERS' }], repo, ['Inner', 'Outer']), + ).toBe('/inner'); + expect( + foldKotlinOperands(key, [{ kind: 'ref', name: 'ONLY_OUTER' }], repo, ['Inner', 'Outer']), + ).toBe('/only-outer'); + }); + it('resolves a nested object member through the enclosing object', () => { // `Inner`'s initializer names `P`, which `Inner` does not declare and // `Outer` does; the scope chain is walked innermost-first, so it means @@ -860,6 +1007,99 @@ import com.example.api.ApiPaths ); expect(resolveKotlinConstant(CONTROLLER_KEY, 'ApiPaths.ORDERS', repo)).toBeNull(); }); + + it('matches a package whose segment is backtick-quoted on one side only', () => { + // `` package com.example.`api` `` and `package com.example.api` name the + // SAME package: the quotes are lexical syntax, not part of the name. The + // declared package was recorded with the backticks and the import required + // an exact string match, so the one real candidate was rejected and the + // route lost. All three spellings are asserted, because either side can + // carry the quotes — a declaration may quote a segment an import spells + // plainly, and an import may quote one the declaration does not. + const declaration = (pkg: string): string => `package ${pkg} + +object ApiPaths { + const val ORDERS = "/right" +} +`; + const importing = (spec: string): string => `package com.example.app.web + +import ${spec} +`; + const CONSTS = 'src/main/kotlin/com/example/api/ApiPaths.kt'; + for (const [declared, spec] of [ + ['com.example.`api`', 'com.example.api.ApiPaths'], + ['com.example.api', 'com.example.`api`.ApiPaths'], + ['com.example.`api`', 'com.example.`api`.ApiPaths'], + ] as const) { + const repo = repoOf({ + [CONSTS]: declaration(declared), + [CONTROLLER_KEY]: importing(spec), + }); + expect( + resolveKotlinConstant(CONTROLLER_KEY, 'ApiPaths.ORDERS', repo), + `${declared} <- ${spec}`, + ).toBe('/right'); + } + }); + + it('still refuses a package that merely resembles the quoted one', () => { + // The control for the test above: unquoting compares NAMES, it does not + // widen the match. `com.example.other` is a different package however + // either side spells it, so the fold floors to skip rather than reaching + // for the only file it can see. + const repo = repoOf({ + 'src/main/kotlin/com/example/other/ApiPaths.kt': `package com.example.\`other\` + +object ApiPaths { + const val ORDERS = "/wrong" +} +`, + [CONTROLLER_KEY]: `package com.example.app.web + +import com.example.api.ApiPaths +`, + }); + expect(resolveKotlinConstant(CONTROLLER_KEY, 'ApiPaths.ORDERS', repo)).toBeNull(); + }); + + it('folds through a KEYWORD package segment, which Kotlin can only spell quoted', () => { + // `package com.example.fun` does not compile — the segment must be + // `` `fun` `` on both sides. Unquoting must not break the case that only + // works BECAUSE it is quoted, so this is the control for the pair above. + const repo = repoOf({ + 'src/main/kotlin/com/example/fun/ApiPaths.kt': `package com.example.\`fun\` + +object ApiPaths { + const val ORDERS = "/right" +} +`, + [CONTROLLER_KEY]: `package com.example.app.web + +import com.example.\`fun\`.ApiPaths +`, + }); + expect(resolveKotlinConstant(CONTROLLER_KEY, 'ApiPaths.ORDERS', repo)).toBe('/right'); + }); + + it('matches a backtick-quoted declaration name and reference', () => { + // Quoting reaches declarations too, and the two sides need not agree: + // `` object `ApiPaths` `` is keyed `ApiPaths.ORDERS`, and a reference + // written `` `ApiPaths`.`ORDERS` `` parses to that same name. + const key = 'src/main/kotlin/com/example/api/ApiPaths.kt'; + const repo = repoOf({ + [key]: `package com.example.api + +object \`ApiPaths\` { + const val \`ORDERS\` = "/right" +} +`, + }); + expect(resolveKotlinConstant(key, 'ApiPaths.ORDERS', repo)).toBe('/right'); + expect(parseKotlinConstOperands(firstInitializer('val X = `ApiPaths`.`ORDERS`\n'))).toEqual([ + { kind: 'ref', name: 'ApiPaths.ORDERS' }, + ]); + }); }); describe('the fold is bounded in output, depth and time', () => {