refactor(group): let the resolver own Kotlin enclosing-type qualification

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 <cursoragent@cursor.com>
This commit is contained in:
Gergo Magyar 2026-08-28 11:05:47 +00:00
parent b9a4d7edff
commit 699702653c
2 changed files with 47 additions and 97 deletions

View file

@ -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 `<Owner>.<NAME>`. 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[] = [];

View file

@ -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
* `<EnclosingType>.<NAME>` 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 `<EnclosingType>.<NAME>` 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);
}