scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-10-10 03:27:59 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

fix(group): key Kotlin constants by visibility and resolve imports on the declared package

Browse source 
Two ways the Kotlin route fold could publish a path the application does not
serve. Both were inherited from the merged Java binding, which documents each as
accepted; the notes were wrong, not merely conservative, and both are left open
as a Java follow-up rather than changed here.

Simple-name flattening. Every `object`/companion member was recorded under BOTH
its qualified name `Owner.NAME` and its bare `NAME` in one file-level namespace,
so an initializer naming a sibling resolved through whichever object was walked
LAST:

  object A { const val BASE = "/right"; const val ROUTE = BASE + "/m" }
  object B { const val BASE = "/wrong" }
  @GetMapping(A.ROUTE)      // Kotlin serves /right/m; this emitted /wrong/m

Swapping the two objects flipped the answer back — the same source, merely
reordered, changed the emitted route. The bare key is also a binding Kotlin does
not have: `BASE` alone never names `A.BASE` from outside `object A`'s body, and
because the fold consults literals before imports, that fabricated key outranked
a genuine `import com.example.api.Paths.ORDERS` and published the local object's
value instead of the imported one.

Keys now follow Kotlin's own visibility. A member of a named `object` gets only
`Owner.NAME`; the simple name is recorded for a top-level `val` and for a
companion member, which really is in scope unqualified throughout its enclosing
class. Initializers resolve against their scope chain, innermost first, so `BASE`
inside `object A` means `A.BASE` — collecting every declaration before recording
any is what makes that independent of declaration order. An unfoldable object
member no longer drops a same-named import either, since it shadows nothing.

The known limit is now stated rather than argued away: a companion's bare key is
still file-wide, so two companions in one file whose members collide still
resolve last-wins for an unqualified reference. Kotlin scopes that to the
enclosing class and this map cannot express it — the fold is entered with a file
key and a name, and nothing says which class body the annotation sat in.
Initializers are unaffected; only a bare annotation reference can land wrong.

Import binding never read the `package` header. Both tiers picked candidates
purely from the path, so a file whose PATH ended with the imported FQN beat the
real declaration — and when the decoy declared the same constant the fold did not
skip, it invented a value. Measured: `object ApiPaths { const val ORDERS = "/right" }`
in `src/generated/Constants.kt` (`package com.example.api`) plus a decoy at
`src/x/com/example/api/ApiPaths.kt` (`package x.com.example.api`) emitted
`GET /wrong`. This falsifies the old docstring's safety argument, which only
covered a wrong file that LACKS the name. Two further triggers: a root-level
`package data` was impersonated by `com/example/data` on a path-suffix test,
while the real root-level file was invisible to the package-directory tier at
all; and a unique constant file under a test source tree folded into a
production route.

The declared `package` is now recorded per file and matched exactly. Candidates
that declare a different package are rejected rather than guessed at, an entry
with no recorded package is rejected too, and two files declaring the same
fully-qualified name resolve to nothing — a duplicated FQN names no single
declaration, so the test-source copy of a production constant is a skip, not a
guess about build configuration this layer cannot see. The file-name convention
survives only as a tie-break among candidates that already declare the right
package. `packageName` rides on a Kotlin-local `KotlinModuleConstants` rather
than widening the agnostic `ModuleConstants`, which Java, JS and Python share and
none of them needs it.

Measured with a differential probe over all 41 Kotlin fixtures, in both key
styles. Seven rows move, all of them from a wrong route:

  * sibling shadow, A first          /wrong/m   -> /right/m
  * bare key beats import            /wrong     -> /right
  * path-suffix decoy                /wrong     -> /right
  * root-package suffix match        /wrong     -> /right
  * root-package suffix only         /wrong     -> (skip; not in the repo)
  * test copy into production        /test-only -> (skip; FQN declared twice)
  * wrong file lacks the name        (skip)     -> /right

The last row is the one control that changes, and it changes from emitting
nothing to emitting the route Kotlin serves: its decoy declares a different
package, so the unconventionally named real file is now the sole candidate.
Every other row, all six remaining controls included, is byte-identical to
before, and POSIX and Windows keys still agree on every fixture.

Deliberately not done: `resolveKotlinImport` does not PREFER the candidate that
declares the sought name when several share the package — it only rejects when
two do. Preferring it would resolve more imports correctly (a package holding
`ApiPaths.kt` that declares something else and `Constants.kt` that declares
`ApiPaths`), but it is a separate skip-to-route improvement that would rewrite an
assertion this suite already pins, and the review round did not ask for it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Borozdenets Ilya 2026-08-28 10:59:58 +03:00
parent 7ccc344ec4
commit 6b5505c474
3 changed files with 856 additions and 143 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

493
gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts
View file

@ -39,15 +39,36 @@
* `const val ORDERS = "/orders"` carries no type node to check. The
* initializer decides: anything {@link parseKotlinConstOperands} cannot fold
* to a string (a number, a call, a template) drops the constant.
* 3. **File names are free.** `object ApiPaths` may live in `Constants.kt`, so
* a `<package>/<Name>.kt` lookup is a convention, not a rule — see
* {@link resolveKotlinImport}'s second tier.
* 3. **File names and directories are free.** `object ApiPaths` may live in
* `Constants.kt`, and a file's `package` need not match the directory it
* sits in, so a `<package>/<Name>.kt` PATH lookup is a convention and not a
* rule. The authority is each file's DECLARED `package`, which
* {@link extractKotlinModuleConstants} records and {@link resolveKotlinImport}
* requires an exact match on; the path is only a tie-break among files that
* already declare the right package.
* 4. **Member imports are unmarked.** Java spells them `import static a.b.C.F`;
* Kotlin writes `import a.b.C.F`, which is byte-identical to a type import
* 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.
*
* 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
* follow-up rather than fixed here:
*
* * `java-const-resolver.ts` flattens nested types into one file-level
* namespace and argues the collision away — "qualified refs carry the class
* name, so nesting only matters for same-name fields, which flatten
* last-wins". The argument does not hold: the collision is one level BELOW
* the qualification, in the initializer, so a fully qualified `A.ROUTE` whose
* initializer names a bare sibling `BASE` still resolves through whichever
* same-named sibling was walked last. {@link extractKotlinModuleConstants}
* keys by Kotlin's own visibility instead.
* * Java's import resolution can lean on the `<package>/<Name>.java` layout the
* 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.
*
* Constant shapes this binding harvests:
*
* const val TOP_LEVEL = "/api/v1" // file top level
@ -70,7 +91,10 @@
* Keying (parity with the Java and Python bindings): the repo map is keyed by
* unique POSIX file path, and an import that cannot be pinned to exactly one
* file returns null (skip floor), never a wrong path. A missing route is a
* missing fact; a wrongly folded one is a false edge in the graph.
* missing fact; a wrongly folded one is a false edge in the graph. "Exactly one
* file" is decided from the DECLARED package, not from the path: a path is a
* repository-layout accident that any decoy directory can imitate, whereas the
* `package` header is the declaration the compiler itself resolves against.
*
* POSIX keys are a PRECONDITION this module cannot check cheaply, so it is
* enforced at the one boundary that produces them: `http-patterns/kotlin.ts`
@ -102,7 +126,6 @@ import { unquoteSpringLiteral } from './spring-shared.js';
import {
MAX_FOLD_LENGTH,
type ImportBinding,
type ImportResolver,
type ModuleConstants,
type Operand,
type RepoConstants,
@ -115,6 +138,39 @@ export type {
RepoConstants,
} from './constant-resolver.js';
/**
* What {@link extractKotlinModuleConstants} returns: the agnostic
* {@link ModuleConstants} plus the one piece of per-file metadata JVM import
* resolution cannot be honest without — the file's declared `package`.
*
* Deliberately a KOTLIN-LOCAL widening rather than a field on the shared type.
* `ModuleConstants` is consumed by the Java, JS and Python bindings too, and
* none of them needs this: Python resolves imports from the module path, and
* Java's `package` is already pinned by the `<package>/<Name>.java` rule the
* language enforces. Adding a required field there would force three unrelated
* bindings to fill it in; adding an optional one would put a Kotlin-shaped hole
* in a type whose whole point is language neutrality.
*
* Read it through {@link declaredPackageOf}, never by field access: a
* {@link RepoConstants} is typed over the agnostic shape, so an entry that some
* other producer put there carries no package and must be REJECTED as a
* candidate rather than silently treated as the default package.
*/
export interface KotlinModuleConstants extends ModuleConstants {
/** The file's declared `package`, or `''` for the default package. */
readonly packageName: string;
}
/**
* The declared `package` of the file `mc` describes, or null when the entry did
* not come from {@link extractKotlinModuleConstants} and therefore cannot be
* matched against an import specifier.
*/
function declaredPackageOf(mc: ModuleConstants | undefined): string | null {
const declared = (mc as KotlinModuleConstants | undefined)?.packageName;
return typeof declared === 'string' ? declared : null;
}
/** Source extensions a Kotlin declaration can live in. */
const KOTLIN_EXTENSIONS = ['.kt', '.kts'] as const;
@ -198,71 +254,112 @@ function isFileNamedAfterDeclaration(key: string, asPath: string): boolean {
return false;
}
/** Is `key` a Kotlin file sitting DIRECTLY in the directory `<packageDir>`? */
function isInPackageDirectory(key: string, packageDir: string): boolean {
const slash = key.lastIndexOf('/');
if (slash < 0) return false;
const dir = key.slice(0, slash);
if (dir !== packageDir && !dir.endsWith(`/${packageDir}`)) return false;
const fileName = key.slice(slash + 1);
return KOTLIN_EXTENSIONS.some((ext) => fileName.endsWith(ext));
/**
* Does the file `mc` describes declare a top-level entity called `name` — an
* `object`/companion carrier whose members are keyed `name.<MEMBER>`, or a
* top-level constant keyed `name` outright?
*
* Used only to detect a DUPLICATED fully-qualified name, so both directions of
* imprecision are bounded. A false positive (the bare key belongs to a companion
* rather than a top-level `val`) can only add a skip; a false negative falls
* back to the path tie-breaks below, i.e. to the behavior this test refines.
*/
function declaresTopLevelName(mc: ModuleConstants, name: string): boolean {
const prefix = `${name}.`;
for (const map of [mc.literals, mc.exprs]) {
if (map.has(name)) return true;
for (const key of map.keys()) if (key.startsWith(prefix)) return true;
}
return false;
}
/**
* The Kotlin {@link ImportResolver}: map a fully-qualified import specifier to
* the unique file key it refers to, or null when it cannot be pinned to exactly
* one file.
* Map a fully-qualified import specifier to the unique file key it refers to, or
* null when it cannot be pinned to exactly one file.
*
* Two tiers, tried in order, each "unique or nothing":
* A specifier is split at its last dot into the package it names and the
* declaration inside it (`com.example.app.api` + `ApiPaths`). Resolution then
* runs in three steps, all of them "unique or nothing":
*
* 1. **File named after the declaration** — `com.example.app.api.ApiPaths` →
* the file ending `com/example/app/api/ApiPaths.kt`. This is the JVM
* convention Java can rely on outright, and it is what Kotlin projects
* overwhelmingly do.
* 2. **Package directory** — Kotlin does NOT require the file name to match the
* declaration (`object ApiPaths` may live in `Constants.kt`) or, strictly,
* the directory to match the package. When tier 1 finds nothing, fall back
* to the unique Kotlin file sitting directly in `com/example/app/api/`. The
* candidate set the fold passes in is the constant-DEFINING files only, so
* "unique file in this package" is a far tighter question than it sounds;
* when 2+ files in the package define constants, this returns null and the
* fold floors to skip.
* 0. **Declared package** — only files whose `package` header is EXACTLY the
* sought package can carry the declaration. 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
* at `src/x/com/example/api/ApiPaths.kt` declaring `package x.com.example.api`
* is not `com.example.api.ApiPaths` and must never be folded as it, and a
* path-suffix test also lets a root-level `package data` be impersonated by
* `…/com/example/data/`. An entry with no recorded package is rejected, not
* assumed to be the default package.
* 1. **Duplicated declaration** — when two of those files declare the sought
* name, the FQN itself is duplicated in the repository and names no single
* declaration. Return null. This is the general form of the same-FQN check
* step 2 could only make for files that happen to follow the file-name
* convention, and it is what stops a `src/test/…` copy of a production
* constant from being folded into a production route.
* 2. **File named after the declaration** — among the package-matching files,
* the one ending `com/example/app/api/ApiPaths.kt`. Kotlin does not require
* this (`object ApiPaths` may live in `Constants.kt`), so it is a tie-break
* among already-valid candidates, never evidence on its own.
* 3. **Sole file in the package** — when no name matches, the unique
* package-matching candidate. The set passed in is the constant-DEFINING
* files only, so this is a far tighter question than it sounds; with 2+
* candidates it returns null and the fold floors to skip.
*
* Tier 2 can hand back a file that does not declare the wanted name at all. That
* is safe by construction: the fold then looks the name up in that file's map,
* misses, and returns null. It cannot invent a value — the worst case is a
* skipped route.
* Steps 2 and 3 can still hand back a file that does not declare the wanted name
* (its package is right and it is the only candidate, but the name lives
* elsewhere or nowhere). That remains safe by construction: the fold looks the
* name up in that file's map, misses, and returns null.
*
* A "nearest shared directory" tie-break is deliberately NOT applied when a tier
* A "nearest shared directory" tie-break is deliberately NOT applied when a step
* has several candidates, for the reason the Java binding records: the JVM
* resolves duplicate FQNs by classpath order, not directory proximity, so a test
* fixture copy sitting closer in the tree can outrank the real dependency and
* yield a silently wrong literal. In a resolver whose whole contract is
* skip-or-correct, a plausible guess is the one answer that cannot be allowed.
*
* This can no longer be typed as the agnostic {@link ModuleConstants} consumer's
* `ImportResolver`, whose signature carries only file KEYS: deciding a candidate
* on its declared package needs the map those keys index. Nothing is lost — the
* core's own fold is not used here either (see the module header), and the
* alternative is a resolver that must guess from a path.
*/
export const resolveKotlinImport: ImportResolver = (_importingFileKey, moduleSpec, repoKeys) => {
export function resolveKotlinImport(
_importingFileKey: string,
moduleSpec: string,
candidateKeys: ReadonlySet<string>,
repo: RepoConstants,
): string | null {
const lastDot = moduleSpec.lastIndexOf('.');
const packageName = lastDot < 0 ? '' : moduleSpec.slice(0, lastDot);
const simpleName = lastDot < 0 ? moduleSpec : moduleSpec.slice(lastDot + 1);
// Step 0 + step 1 in one pass over the candidates.
const inPackage: string[] = [];
let declaring: string | null = null;
for (const key of candidateKeys) {
const mc = repo.get(key);
if (!mc || declaredPackageOf(mc) !== packageName) continue;
inPackage.push(key);
if (declaresTopLevelName(mc, simpleName)) {
if (declaring !== null) return null; // 2+ files declare this FQN
declaring = key;
}
}
if (inPackage.length === 0) return null;
if (inPackage.length === 1) return inPackage[0]; // steps 2 and 3 agree
// Step 2: the file-name convention, as a tie-break among valid candidates.
const asPath = moduleSpec.replace(/\./g, '/');
let hit: string | null = null;
for (const key of repoKeys) {
if (isFileNamedAfterDeclaration(key, asPath)) {
if (hit !== null) return null; // 2+ modules carry this FQN — unresolvable
hit = key;
}
let named: string | null = null;
for (const key of inPackage) {
if (!isFileNamedAfterDeclaration(key, asPath)) continue;
if (named !== null) return null; // 2+ files spell the convention
named = key;
}
if (hit !== null) return hit;
const lastSlash = asPath.lastIndexOf('/');
if (lastSlash <= 0) return null; // no package part to fall back to
const packageDir = asPath.slice(0, lastSlash);
for (const key of repoKeys) {
if (isInPackageDirectory(key, packageDir)) {
if (hit !== null) return null; // ambiguous package — unresolvable
hit = key;
}
}
return hit;
};
// Step 3 is "the sole candidate", already returned above.
return named;
}
/**
* Is `node` a Kotlin string literal, and if so what value does the route layer
@ -400,8 +497,51 @@ function initializerOf(property: Parser.SyntaxNode): Parser.SyntaxNode | null {
}
/**
* Extract the file-level string constants and import bindings of one parsed
* Kotlin file into the {@link ModuleConstants} shape the resolver consumes.
* One `val` declaration, captured before anything is written to the file's
* namespace so that the DECLARING SCOPE of every initializer is known regardless
* of the order the declarations appear in.
*/
interface KotlinConstDeclaration {
/** The declaration's simple name. */
readonly name: string;
/** `<DeclaringType>.<NAME>`, or null for a top-level declaration. */
readonly qualified: string | null;
/**
* The qualified-key prefixes in LEXICAL scope for this declaration's
* initializer, innermost first (`['Inner', 'Outer']`). Empty at file level.
*/
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.
*/
readonly bareVisible: boolean;
/** The parsed initializer, or null when it is not a foldable string. */
readonly operands: readonly Operand[] | null;
}
/**
* 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.
*/
function declaredPackage(root: Parser.SyntaxNode): string {
const header = root.children.find((c) => c.type === 'package_header');
const identifier = header?.children.find((c) => c.type === 'identifier');
if (!identifier) return '';
return identifier.namedChildren
.filter((c) => c.type === 'simple_identifier')
.map((c) => c.text)
.join('.');
}
/**
* Extract the declared package, file-level string constants and import bindings
* of one parsed Kotlin file into the {@link KotlinModuleConstants} shape the
* resolver consumes.
*
* Constants come from the three carriers Kotlin allows a caller to reach without
* an instance: file top level, `object` members, and `companion object` members.
@ -409,21 +549,43 @@ function initializerOf(property: Parser.SyntaxNode): Parser.SyntaxNode | null {
* NOT collected — the Kotlin analogue of Java's `static final` requirement. `var`
* is rejected outright.
*
* Every constant is recorded under its simple name AND, when it has a declaring
* type, under `<DeclaringType>.<NAME>` — the spelling a qualified reference uses.
* A companion member is keyed under the ENCLOSING CLASS (`Holder.NAME`), because
* that is how Kotlin source refers to it; `Companion` never appears in a
* reference. Nested types flatten into one file-level namespace (same as the
* Java binding), so same-named members of sibling objects collide on the simple
* name and resolve last-wins; their qualified spellings stay distinct.
* KEYS FOLLOW KOTLIN'S OWN VISIBILITY, not a flattened namespace. Every constant
* is recorded under `<DeclaringType>.<NAME>`, 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
* `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
* 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.
*
* 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 non-foldable rebind (`X = compute()`) DROPS X to unresolvable rather than
* leaving a stale literal — and drops a same-named import with it, since a local
* declaration shadows an import for unqualified references and the fold would
* otherwise fall through to the imported value, i.e. a wrong path where the skip
* floor is owed.
* 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.
*/
export function extractKotlinModuleConstants(tree: Parser.Tree): ModuleConstants {
export function extractKotlinModuleConstants(tree: Parser.Tree): KotlinModuleConstants {
const literals = new Map<string, string>();
const exprs = new Map<string, readonly Operand[]>();
const imports = new Map<string, ImportBinding>();
@ -458,23 +620,123 @@ export function extractKotlinModuleConstants(tree: Parser.Tree): ModuleConstants
};
walkImports(tree.rootNode);
// Pass 2: constants.
const record = (name: string, operands: readonly Operand[] | null, qualified: string | null) => {
if (operands === null) {
literals.delete(name);
exprs.delete(name);
imports.delete(name);
if (qualified) {
literals.delete(qualified);
exprs.delete(qualified);
// Pass 2a: collect every declaration, writing nothing yet. Which member each
// unqualified operand means depends on the whole file, so no key can be
// written — and no operand rewritten — until the last declaration is in.
const declarations: KotlinConstDeclaration[] = [];
/** Declaring scope → the simple names it declares, foldable or not. */
const membersByScope = new Map<string, Set<string>>();
const collectProperties = (
body: Parser.SyntaxNode,
declaringType: string | null,
scopes: readonly string[],
bareVisible: boolean,
): void => {
for (const member of body.children ?? []) {
if (member.type !== 'property_declaration') continue;
if (bindingKind(member) !== 'val') continue;
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;
if (declaringType !== null) {
let members = membersByScope.get(declaringType);
if (!members) membersByScope.set(declaringType, (members = new Set()));
// Recorded even when the initializer does not fold: a sibling reference
// to an unfoldable member must resolve to that member and then MISS,
// not fall through to a same-named constant at file level.
members.add(name);
}
return;
declarations.push({
name,
qualified: declaringType === null ? null : `${declaringType}.${name}`,
scopes,
bareVisible,
operands: parseKotlinConstOperands(initializerOf(member)),
});
}
};
const bodyOf = (node: Parser.SyntaxNode): Parser.SyntaxNode | undefined =>
node.children.find((c) => c.type === 'class_body');
const walkDeclarations = (
node: Parser.SyntaxNode,
enclosingType: string | null,
scopes: readonly string[],
): 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 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);
walkDeclarations(body, name, inner);
continue;
}
if (child.type === 'companion_object') {
const body = bodyOf(child);
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.
const inner = enclosingType === null ? scopes : [enclosingType, ...scopes];
collectProperties(body, enclosingType, inner, 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 body = bodyOf(child);
if (body) walkDeclarations(body, name, scopes);
continue;
}
walkDeclarations(child, enclosingType, scopes);
}
};
collectProperties(tree.rootNode, null, [], 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.
const qualifyRef = (refName: string, scopes: readonly string[]): string => {
if (refName.includes('.')) return refName; // already carries its owner
for (const scope of scopes) {
if (membersByScope.get(scope)?.has(refName)) return `${scope}.${refName}`;
}
return refName; // file level, or unresolvable — the fold decides
};
for (const decl of declarations) {
const keys: string[] = [];
if (decl.bareVisible) keys.push(decl.name);
if (decl.qualified !== null) keys.push(decl.qualified);
if (decl.operands === null) {
for (const key of keys) {
literals.delete(key);
exprs.delete(key);
}
// Only a bare-visible declaration shadows a same-named import.
if (decl.bareVisible) imports.delete(decl.name);
continue;
}
const operands = decl.operands.map((op) =>
op.kind === 'ref' ? { kind: 'ref' as const, name: qualifyRef(op.name, decl.scopes) } : op,
);
const literalValue =
operands.length === 1 && operands[0].kind === 'literal'
? (operands[0] as { value: string }).value
: null;
for (const key of qualified ? [name, qualified] : [name]) {
operands.length === 1 && operands[0].kind === 'literal' ? operands[0].value : null;
for (const key of keys) {
if (literalValue !== null) {
literals.set(key, literalValue);
exprs.delete(key);
@ -483,62 +745,9 @@ export function extractKotlinModuleConstants(tree: Parser.Tree): ModuleConstants
literals.delete(key);
}
}
};
}
const collectProperties = (body: Parser.SyntaxNode, declaringType: string | null): void => {
for (const member of body.children ?? []) {
if (member.type !== 'property_declaration') continue;
if (bindingKind(member) !== 'val') continue;
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;
record(
name,
parseKotlinConstOperands(initializerOf(member)),
declaringType ? `${declaringType}.${name}` : null,
);
}
};
const bodyOf = (node: Parser.SyntaxNode): Parser.SyntaxNode | undefined =>
node.children.find((c) => c.type === 'class_body');
const walkDeclarations = (node: Parser.SyntaxNode, enclosingType: string | null): 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 body = bodyOf(child);
if (!body) continue;
collectProperties(body, name);
walkDeclarations(body, name);
continue;
}
if (child.type === 'companion_object') {
const body = bodyOf(child);
if (!body) continue;
// Referenced through the enclosing class (`Holder.NAME`), never through
// `Companion` — so the qualified alias is keyed on `enclosingType`.
collectProperties(body, enclosingType);
walkDeclarations(body, enclosingType);
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 body = bodyOf(child);
if (body) walkDeclarations(body, name);
continue;
}
walkDeclarations(child, enclosingType);
}
};
collectProperties(tree.rootNode, null);
walkDeclarations(tree.rootNode, null);
return { literals, exprs, imports };
return { literals, exprs, imports, packageName: declaredPackage(tree.rootNode) };
}
/**
@ -628,7 +837,7 @@ function resolveImportedName(
): string | null {
// Reading A: the specifier names the declaration itself (a top-level
// `const val`, or a type whose file we then search).
const direct = resolveKotlinImport(fileKey, imp.module, state.constantKeys);
const direct = resolveKotlinImport(fileKey, imp.module, state.constantKeys, state.repo);
if (direct !== null) {
const value = resolveWithState(direct, imp.originalName, state, depth);
if (value !== null) return value;
@ -639,7 +848,7 @@ function resolveImportedName(
if (dot <= 0) return null;
const ownerSpec = imp.module.slice(0, dot);
const ownerName = ownerSpec.slice(ownerSpec.lastIndexOf('.') + 1);
const ownerFile = resolveKotlinImport(fileKey, ownerSpec, state.constantKeys);
const ownerFile = resolveKotlinImport(fileKey, ownerSpec, state.constantKeys, state.repo);
if (ownerFile === null) return null;
return resolveWithState(ownerFile, `${ownerName}.${imp.originalName}`, state, depth);
}
@ -666,7 +875,7 @@ function computeKotlinFold(
const tail = name.slice(dot + 1);
const imp = repo.get(fileKey)?.imports.get(head);
if (imp) {
const targetFile = resolveKotlinImport(fileKey, imp.module, constantKeys);
const targetFile = resolveKotlinImport(fileKey, imp.module, constantKeys, repo);
if (targetFile === null) return null;
// `originalName` un-aliases `import … .ApiPaths as Paths`, so the lookup
// uses the declaring type's real name.
@ -677,7 +886,7 @@ function computeKotlinFold(
const parts = name.split('.');
for (let cut = parts.length - 2; cut >= 1; cut--) {
const fqn = parts.slice(0, cut + 1).join('.');
const targetFile = resolveKotlinImport(fileKey, fqn, constantKeys);
const targetFile = resolveKotlinImport(fileKey, fqn, constantKeys, repo);
if (targetFile !== null) {
const declaring = parts[cut];
const member = parts.slice(cut + 1).join('.');

153
gitnexus/test/unit/group/kotlin-const-route-fold.test.ts
View file

@ -606,6 +606,159 @@ class OrderController {
).toEqual(['GET /api/', 'POST /api/']);
});
it('serves the same route whichever of two same-named objects is declared first', () => {
// `A.ROUTE = BASE + "/m"` means `A.BASE`. Recording every object member
// under its bare name too made that operand resolve through whichever
// same-named sibling was walked LAST, so reordering two objects — a change
// Kotlin does not even see — moved the published route from `/right/m` to
// `/wrong/m`. Both orders are asserted; either alone passes on a last-wins
// implementation.
const controllerWith = (objects: string): string => `package com.example.app.web
${objects}
@RestController
class OrderController {
@GetMapping(A.ROUTE)
fun get() {}
}
`;
const A = `object A {
const val BASE = "/right"
const val ROUTE = BASE + "/m"
}`;
const B = `object B {
const val BASE = "/wrong"
}`;
expect(providers({ [CONTROLLER]: controllerWith(`${A}\n\n${B}`) })).toEqual(['GET /right/m']);
expect(providers({ [CONTROLLER]: controllerWith(`${B}\n\n${A}`) })).toEqual(['GET /right/m']);
});
it('reads a bare route constant from the import, not from a local object member', () => {
// Bare `ORDERS` in this file is the IMPORT: `object Local` binds
// `Local.ORDERS` and nothing else. A bare key for the object member is a
// binding Kotlin does not have, and it outranked the import because the fold
// consults literals before imports — publishing a path the service does not
// serve.
expect(
providers({
[CONSTS]: `package com.example.app.api
object ApiPaths {
const val ORDERS = "/api/v1/orders"
}
`,
[CONTROLLER]: `package com.example.app.web
import com.example.app.api.ApiPaths.ORDERS
object Local {
const val ORDERS = "/local"
}
@RestController
class OrderController {
@GetMapping(ORDERS)
fun list() {}
}
`,
}),
).toEqual(['GET /api/v1/orders']);
});
it('keeps a companion constant readable under its bare name', () => {
// The control for the test above, and the reason object members and
// companion members are keyed differently: a companion's members ARE in
// scope unqualified throughout the enclosing class, which is precisely where
// route annotations sit.
expect(
providers({
[CONTROLLER]: `package com.example.app.web
@RestController
class OrderController {
companion object {
const val ORDERS = "/api/v1/orders"
}
@GetMapping(ORDERS)
fun list() {}
}
`,
}),
).toEqual(['GET /api/v1/orders']);
});
it('folds through the file that declares the package, not one whose path imitates it', () => {
// The decoy's PATH ends with the imported FQN, but it declares
// `package x.com.example.app.api` — a different declaration. Choosing the
// candidate by path let it win, and because it declares the same member the
// fold did not skip: it published `/wrong`. The declared `package` is the
// authority; the path is only a tie-break among files that already declare
// the right one.
expect(
providers({
'src/generated/Constants.kt': `package com.example.app.api
object ApiPaths {
const val ORDERS = "/api/v1/orders"
}
`,
'src/x/com/example/app/api/ApiPaths.kt': `package x.com.example.app.api
object ApiPaths {
const val ORDERS = "/wrong"
}
`,
[CONTROLLER]: `package com.example.app.web
import com.example.app.api.ApiPaths
@RestController
class OrderController {
@GetMapping(ApiPaths.ORDERS)
fun list() {}
}
`,
}),
).toEqual(['GET /api/v1/orders']);
});
it('emits nothing when a test-source copy duplicates a production constant', () => {
// Same package, same object, different value, and only the copy follows the
// `<package>/<Name>.kt` convention — so a file-name tie-break folded a
// test-only path into a production route. Two declarations of one
// fully-qualified name identify no single declaration, so the honest answer
// is no route: preferring the production source set would be a guess about
// build configuration this layer cannot see.
expect(
providers({
'src/main/kotlin/generated/RoutePaths.kt': `package com.example.app.api
object ApiPaths {
const val ORDERS = "/api/v1/orders"
}
`,
'src/test/kotlin/com/example/app/api/ApiPaths.kt': `package com.example.app.api
object ApiPaths {
const val ORDERS = "/test-only"
}
`,
[CONTROLLER]: `package com.example.app.web
import com.example.app.api.ApiPaths
@RestController
class OrderController {
@GetMapping(ApiPaths.ORDERS)
fun list() {}
}
`,
}),
).toEqual([]);
});
it('leaves literal routes unchanged and emits each exactly once', () => {
expect(
providers({

353
gitnexus/test/unit/kotlin-route-const-resolver.test.ts
View file

@ -34,6 +34,7 @@ import {
parseKotlinConstOperands,
resolveKotlinConstant,
resolveKotlinImport,
type ModuleConstants,
type RepoConstants,
} from '../../src/core/ingestion/route-extractors/kotlin-const-resolver.js';
import { unquoteSpringLiteral } from '../../src/core/ingestion/route-extractors/spring-shared.js';
@ -242,12 +243,15 @@ import com.example.app.api.ApiPaths
};
const repo = repoOf(files);
expect(resolveKotlinConstant(CONTROLLER_KEY, 'ApiPaths.ORDERS', repo)).toBeNull();
// Same verdict at the resolver layer the fold delegates to.
// Same verdict at the resolver layer the fold delegates to. It reads the
// candidates' DECLARED packages, so it takes the repo map as well as the
// key set.
expect(
resolveKotlinImport(
CONTROLLER_KEY,
'com.example.app.api.ApiPaths',
new Set(Object.keys(files)),
repo,
),
).toBeNull();
});
@ -511,6 +515,353 @@ import com.example.app.api.ApiPaths
});
});
describe('member names resolve in their declaring scope, not a flat namespace', () => {
/** Two objects declaring `BASE`; only `A` is referenced. Order is the axis. */
const siblingShadow = (first: 'A' | 'B'): string => {
const a = `object A {
const val BASE = "/right"
const val ROUTE = BASE + "/m"
}`;
const b = `object B {
const val BASE = "/wrong"
}`;
return `package com.example.app.api\n\n${first === 'A' ? `${a}\n\n${b}` : `${b}\n\n${a}`}\n`;
};
const SIBLING_KEY = 'src/main/kotlin/com/example/app/api/Siblings.kt';
it('answers a sibling initializer identically whichever object is declared first', () => {
// `A.ROUTE = BASE + "/m"` means `A.BASE`, so the answer is `/right/m` in
// both spellings. Recording every member under its BARE name too made the
// operand resolve through whichever object was walked last, so moving
// `object B` above `object A` changed the emitted route for source that
// had not changed — the same file, merely reordered, served a different
// path. Both orders are asserted because either one alone passes on a
// last-wins implementation.
for (const first of ['A', 'B'] as const) {
const repo = repoOf({ [SIBLING_KEY]: siblingShadow(first) });
expect(resolveKotlinConstant(SIBLING_KEY, 'A.ROUTE', repo), `${first} first`).toBe(
'/right/m',
);
expect(resolveKotlinConstant(SIBLING_KEY, 'B.BASE', repo), `${first} first`).toBe('/wrong');
}
});
it('does not bind an `object` member to its bare name, so an import still wins', () => {
// `object Local { const val ORDERS }` binds `Local.ORDERS` and nothing
// else — bare `ORDERS` in this file is the IMPORT. A bare key for the
// object member is a binding Kotlin does not have, and it outranks the
// import because the fold consults literals before imports.
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
object Local {
const val ORDERS = "/local-member"
}
`,
});
expect(resolveKotlinConstant(CONTROLLER_KEY, 'ORDERS', repo)).toBe('/imported');
// The qualified spelling still reaches the object member.
expect(resolveKotlinConstant(CONTROLLER_KEY, 'Local.ORDERS', repo)).toBe('/local-member');
});
it('keeps a top-level `const val` shadowing a same-named import', () => {
// The control for the test above: a top-level declaration IS the bare
// binding, so it must keep winning over the import.
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
const val ORDERS = "/local"
`,
});
expect(resolveKotlinConstant(CONTROLLER_KEY, 'ORDERS', repo)).toBe('/local');
});
it('keeps a companion member visible under its bare name', () => {
// The other control: a companion's members ARE in scope unqualified
// throughout the enclosing class, which is where route annotations sit.
const key = 'src/main/kotlin/com/example/app/web/OrderApi.kt';
const repo = repoOf({
[key]: `package com.example.app.web
class OrderApi {
companion object {
const val ORDERS = "/companion/orders"
}
}
`,
});
expect(resolveKotlinConstant(key, 'ORDERS', repo)).toBe('/companion/orders');
expect(resolveKotlinConstant(key, 'OrderApi.ORDERS', repo)).toBe('/companion/orders');
});
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
// `Outer.P` — not the same-named member of the unrelated `Other`.
const key = 'src/main/kotlin/com/example/app/api/Nested.kt';
const repo = repoOf({
[key]: `package com.example.app.api
object Other {
const val P = "/wrong"
}
object Outer {
const val P = "/right"
object Inner {
const val Q = P + "/q"
}
}
`,
});
expect(resolveKotlinConstant(key, 'Inner.Q', repo)).toBe('/right/q');
});
it('does not fall through to a file-level constant for an unfoldable sibling', () => {
// `A.R` names `A.BASE`, which does not fold. The answer is the skip floor,
// not the top-level `BASE` that happens to share the simple name.
const key = 'src/main/kotlin/com/example/app/api/Unfoldable.kt';
const repo = repoOf({
[key]: `package com.example.app.api
const val BASE = "/top-level"
object A {
val BASE = buildBase()
val R = BASE + "/r"
}
`,
});
expect(resolveKotlinConstant(key, 'A.R', repo)).toBeNull();
expect(resolveKotlinConstant(key, 'BASE', repo)).toBe('/top-level');
});
it('lets an unfoldable object member leave a same-named import alone', () => {
// A local declaration drops a same-named import only when it SHADOWS it.
// An object member shadows nothing, so dropping the import here would
// floor a reference the language resolves perfectly well.
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
object Local {
val ORDERS = buildOrders()
}
`,
});
expect(resolveKotlinConstant(CONTROLLER_KEY, 'ORDERS', repo)).toBe('/imported');
expect(resolveKotlinConstant(CONTROLLER_KEY, 'Local.ORDERS', repo)).toBeNull();
});
});
describe('imports resolve on the declared package, not on the path', () => {
it('folds through the file that DECLARES the package, not one whose path imitates it', () => {
// `src/x/com/example/api/ApiPaths.kt` ends with the imported FQN but
// declares `package x.com.example.api`, so it is a different declaration
// entirely. Selecting candidates by path made it beat the real file — and
// because the decoy declares the same member, the fold did not skip, it
// published `/wrong`.
const repo = repoOf({
'src/generated/Constants.kt': `package com.example.api
object ApiPaths {
const val ORDERS = "/right"
}
`,
'src/x/com/example/api/ApiPaths.kt': `package x.com.example.api
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)).toBe('/right');
});
it('does not let a deep directory impersonate a root-level package', () => {
// `package data` lives at the repository root, which the old
// package-DIRECTORY fallback could not see at all, while
// `src/main/kotlin/com/example/data/` matched `data` by path suffix. Both
// halves are gone: the declared package is the whole test.
const repo = repoOf({
'Constants.kt': `package data
object Constants {
const val ORDERS = "/right"
}
`,
'src/main/kotlin/com/example/data/AppPaths.kt': `package com.example.data
object Constants {
const val ORDERS = "/wrong"
}
`,
[CONTROLLER_KEY]: `package com.example.app.web
import data.Constants
`,
});
expect(resolveKotlinConstant(CONTROLLER_KEY, 'Constants.ORDERS', repo)).toBe('/right');
});
it('skips rather than guesses when no file declares the imported package', () => {
// The same import with the real declaration absent. A path-suffix match
// answered `/wrong` here; the honest answer is that the constant is not
// in this repository.
const repo = repoOf({
'src/main/kotlin/com/example/data/AppPaths.kt': `package com.example.data
object Constants {
const val ORDERS = "/wrong"
}
`,
[CONTROLLER_KEY]: `package com.example.app.web
import data.Constants
`,
});
expect(resolveKotlinConstant(CONTROLLER_KEY, 'Constants.ORDERS', repo)).toBeNull();
});
it('skips when two files declare the same fully-qualified name', () => {
// A test-source copy of a production constant: same package, same object,
// different value. Only the copy follows the `<package>/<Name>.kt`
// convention, so a file-name tie-break picked it and folded a test-only
// path into a production route. Two declarations of one FQN name no single
// declaration, whichever paths they sit at.
const repo = repoOf({
'src/main/kotlin/generated/RoutePaths.kt': `package com.example.api
object ApiPaths {
const val ORDERS = "/right"
}
`,
'src/test/kotlin/com/example/api/ApiPaths.kt': `package com.example.api
object ApiPaths {
const val ORDERS = "/test-only"
}
`,
[CONTROLLER_KEY]: `package com.example.app.web
import com.example.api.ApiPaths
`,
});
expect(resolveKotlinConstant(CONTROLLER_KEY, 'ApiPaths.ORDERS', repo)).toBeNull();
});
it('still prefers the conventionally named file among same-package candidates', () => {
// Two files declare `com.example.api`; only one declares `ApiPaths`, and
// it is also the one the file-name convention points at. The convention
// survives as a tie-break among candidates that already declare the right
// package — it is just no longer evidence on its own.
const repo = repoOf({
'src/main/kotlin/com/example/api/ApiPaths.kt': `package com.example.api
object ApiPaths {
const val ORDERS = "/right"
}
`,
'src/main/kotlin/com/example/api/Other.kt': `package com.example.api
object OtherPaths {
const val ITEMS = "/items"
}
`,
[CONTROLLER_KEY]: `package com.example.app.web
import com.example.api.ApiPaths
`,
});
expect(resolveKotlinConstant(CONTROLLER_KEY, 'ApiPaths.ORDERS', repo)).toBe('/right');
});
it('reaches the sole file of a package whose name matches nothing', () => {
// The decoy declares a DIFFERENT package, so it is not a candidate at all
// and the unconventionally named `Constants.kt` is the only one left.
// This used to be the one shape the old resolver's safety argument
// covered, and it covered it by emitting nothing.
const repo = repoOf({
'src/generated/Constants.kt': `package com.example.api
object ApiPaths {
const val ORDERS = "/right"
}
`,
'src/x/com/example/api/ApiPaths.kt': `package x.com.example.api
object ApiPaths {
const val OTHER = "/other"
}
`,
[CONTROLLER_KEY]: `package com.example.app.web
import com.example.api.ApiPaths
`,
});
expect(resolveKotlinConstant(CONTROLLER_KEY, 'ApiPaths.ORDERS', repo)).toBe('/right');
});
it('rejects a candidate carrying no recorded package', () => {
// `RepoConstants` is typed over the agnostic shape, so an entry some other
// producer put there has no `packageName`. Unknown is not "the default
// package": the candidate is rejected, and the fold floors to skip.
const key = 'src/main/kotlin/com/example/api/ApiPaths.kt';
const foreign = extractKotlinModuleConstants(
parse(`package com.example.api
object ApiPaths {
const val ORDERS = "/right"
}
`),
);
const repo = new Map<string, ModuleConstants>();
// Stripped to the agnostic shape: same maps, no `packageName`.
repo.set(key, {
literals: foreign.literals,
exprs: foreign.exprs,
imports: foreign.imports,
});
repo.set(
CONTROLLER_KEY,
extractKotlinModuleConstants(
parse(`package com.example.app.web
import com.example.api.ApiPaths
`),
),
);
expect(resolveKotlinConstant(CONTROLLER_KEY, 'ApiPaths.ORDERS', repo)).toBeNull();
});
});
describe('the fold is bounded in output, depth and time', () => {
/** `object Doubling { const val X<n> = <leaf>; val X<k> = X<k+1> + X<k+1> … }`. */
const doublingChain = (levels: number, leaf: string): string => {