From 699702653c21b4203ee7d4743055f18c0e6b5482 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Fri, 28 Aug 2026 11:05:47 +0000 Subject: [PATCH] refactor(group): let the resolver own Kotlin enclosing-type qualification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The route caller gated kotlinEnclosingTypeNames on its own copy of foldKotlinOperands' bare-ref predicate. That gate could not change the result — qualifyKotlinRefInEnclosingTypes returns a dotted name unchanged — so it only spread one rule across two modules that can drift apart. Also corrects a trimmed comment that claimed a collection_literal never reaches classifyPathArgument, which the non-empty branch there disproves. Co-authored-by: Cursor --- .../group/extractors/http-patterns/kotlin.ts | 58 ++++--------- .../route-extractors/kotlin-const-resolver.ts | 86 +++++++------------ 2 files changed, 47 insertions(+), 97 deletions(-) diff --git a/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts b/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts index fbce99e19..13f916d66 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts @@ -281,31 +281,19 @@ function kotlinArrayOfElements(node: Parser.SyntaxNode): Parser.SyntaxNode[] | n /** * What a route-annotation path argument says about the prefix it designates. - * Three answers, and only one of them may suppress a route: + * Only `'unresolvable'` may suppress a route: * - * - `'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. + * - `'literal'` — at least one element is a plain literal, already harvested by + * the literal prefix patterns, so there is nothing to suppress. + * - `'none'` — no prefix. Empty `arrayOf()` is Spring's "map at the root". + * Kept distinct from `'unresolvable'` because conflating them suppressed even + * plain literal routes below such a class, which no constant fold was ever + * involved in. Reachable through `arrayOf()` only: tree-sitter-kotlin (fwcd) + * does not parse a class carrying `@RequestMapping([])` as a + * `class_declaration`, so an EMPTY `collection_literal` never reaches this + * function from an annotation — a non-empty one does, and is handled below. + * - `'unresolvable'` — a non-empty argument with no literal element + * (`ApiPaths.BASE`, `buildPath()`, a template). Served path is unknowable. */ type PathArgumentPrefix = 'literal' | 'none' | 'unresolvable'; @@ -323,23 +311,13 @@ function classifyPathArgument(expr: Parser.SyntaxNode): PathArgumentPrefix { } /** - * The type declarations enclosing `node`, INNERMOST FIRST, by declared name. + * 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. + * The scope a bare constant in a route annotation is resolved against; passed to + * `foldKotlinOperands`, which applies it. Collects `class_declaration` (including + * interfaces) and `object_declaration`. A `companion_object` adds no link of + * its own — members are keyed under the enclosing class one hop up. Skips + * unnamed types rather than guessing. */ function kotlinEnclosingTypeNames(node: Parser.SyntaxNode): string[] { const out: string[] = []; 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 364ef8f89..7ff867973 100644 --- a/gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts +++ b/gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts @@ -190,24 +190,12 @@ 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. + * Quotes are spelling, not part of the name. tree-sitter-kotlin keeps them in + * node text, so every identifier that becomes a map key or lookup is read + * through here. Applied per dot-separated segment — a quoted identifier cannot + * contain `.`. Both the declaration side ({@link declaredPackage}) and the + * import side ({@link resolveKotlinImport}) are normalized, because either may + * carry the quotes while the other spells the same name plainly. */ export function unquoteKotlinIdentifier(text: string): string { return text.length >= 2 && text.startsWith('`') && text.endsWith('`') ? text.slice(1, -1) : text; @@ -637,39 +625,20 @@ function declaredPackage(root: Parser.SyntaxNode): string { * have, and a fabricated key outranks the genuine `import com.example.api.Paths.ORDERS` * that {@link computeKotlinFold} consults only after literals and expressions. * - * 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. + * A COMPANION member gets no bare key either: it is bound unqualified inside + * its enclosing class body and nowhere else. {@link qualifyKotlinRefInEnclosingTypes} + * rewrites a bare name to `.` when an enclosing type + * declares it, so the companion wins inside its own class and loses everywhere + * else. * * 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 - * recording any is what makes that answer independent of declaration ORDER — - * flattening resolved such an operand through whichever same-named sibling - * 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. + * recording any keeps that independent of declaration order. * - * 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 TOP-LEVEL initializer has an EMPTY scope chain, so its bare operands stay + * bare and resolve at file level — they must not pick up a companion key. * * A non-foldable rebind (`X = compute()`) DROPS X to unresolvable rather than * leaving a stale literal — and drops a same-named import with it whenever the @@ -1066,7 +1035,7 @@ function foldOperands( * the bare name never means the companion at all, which is precisely what an * empty chain expresses. */ -export function qualifyKotlinRefInEnclosingTypes( +function qualifyKotlinRefInEnclosingTypes( fileKey: string, name: string, repo: RepoConstants, @@ -1112,16 +1081,19 @@ export function foldKotlinOperands( repo: RepoConstants, enclosingTypes: readonly string[] = [], ): string | null { - const scoped = - enclosingTypes.length === 0 - ? operands - : operands.map((op) => - op.kind === 'ref' - ? { - kind: 'ref' as const, - name: qualifyKotlinRefInEnclosingTypes(fileKey, op.name, repo, enclosingTypes), - } - : op, - ); + // Allocation gate only — the rule itself lives in the dotted-name early + // return of qualifyKotlinRefInEnclosingTypes, which this must not restate. + const needsQualify = + enclosingTypes.length > 0 && operands.some((op) => op.kind === 'ref' && !op.name.includes('.')); + const scoped = needsQualify + ? operands.map((op) => + op.kind === 'ref' + ? { + kind: 'ref' as const, + name: qualifyKotlinRefInEnclosingTypes(fileKey, op.name, repo, enclosingTypes), + } + : op, + ) + : operands; return foldOperands(fileKey, scoped, newFoldState(repo), 0); }