diff --git a/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts b/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts index 651dc27e0..20e4cccb3 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts @@ -173,6 +173,31 @@ const FOLDABLE_PATH_EXPRESSIONS: ReadonlySet = new Set([ 'additive_expression', ]); +/** + * Repo-relative path in the POSIX form the Kotlin constant map is keyed by. + * + * The orchestrator's file list comes from glob v13, which has no `posix: true` + * option and joins with the platform separator, so on Windows `prepareRepo` + * receives `src\main\kotlin\com\example\ApiPaths.kt` and `scan` receives the + * same for `fileRel`. `resolveKotlinImport` turns an import specifier into + * `com/example/ApiPaths.kt` and asks whether a key ENDS WITH it — a test no + * backslashed key can pass. Left unnormalized, every cross-file constant fold + * returns null on Windows and on Windows only: the pre-pass still runs, the + * context is still built, and the feature is simply, silently absent. The unit + * fixtures build POSIX keys by hand, so CI cannot see it. + * + * Normalizing at this boundary — write side (the map keys below) and read side + * (`fileRel`) — is the same fix `node.ts` (`normalizeRel`) and `python.ts` + * (`fileShortKey` / `fileLongKey`) already apply for the same reason, and it is + * the only coherent place: the resolver returns the key it matched, so + * normalizing inside it would hand back a value that misses in a map nobody + * normalized. `readFile` still receives the ORIGINAL `rel`, since the filesystem + * wants the platform's own spelling. + */ +function normalizeRel(rel: string): string { + return rel.replace(/\\/g, '/').replace(/^\.\//, ''); +} + /** * The path expression carried by one route-annotation argument, or null when the * argument does not designate a path. @@ -1258,7 +1283,8 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin { if (!tree) continue; const mc = extractKotlinModuleConstants(tree); if (mc.literals.size > 0 || mc.exprs.size > 0 || mc.imports.size > 0) { - constants.set(rel, mc); + // POSIX key (see `normalizeRel`); `readFile` above got the raw `rel`. + constants.set(normalizeRel(rel), mc); } } catch { // Per-file resilience: one unreadable/oversized/ill-formed file must @@ -1272,6 +1298,11 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin { const out: HttpDetection[] = []; const kotlinCtx = repoContext as { constants: RepoConstants } | undefined; + // Read side of the POSIX keying (see `normalizeRel`): the map `prepareRepo` + // built is keyed by normalized path, so every lookup and every fold entry + // point below uses `fileKey`, never the raw `fileRel`. + const fileKey = fileRel === undefined ? undefined : normalizeRel(fileRel); + // Lazy per-file constants view. `prepareRepo` only indexes constant- // DEFINING files, so an importing controller is absent from that map. // When a route actually references a constant, extract THIS file's import @@ -1282,13 +1313,13 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin { const getFoldConstants = (): RepoConstants | undefined => { if (foldConstants !== undefined) return foldConstants; foldConstants = kotlinCtx?.constants; - if (!kotlinCtx?.constants || !fileRel) return foldConstants; - if (kotlinCtx.constants.has(fileRel)) return foldConstants; + if (!kotlinCtx?.constants || !fileKey) return foldConstants; + if (kotlinCtx.constants.has(fileKey)) return foldConstants; try { const mc = extractKotlinModuleConstants(tree); if (mc.imports.size > 0) { const merged = new Map(kotlinCtx.constants); - merged.set(fileRel, mc); + merged.set(fileKey, mc); foldConstants = merged; } } catch { @@ -1377,12 +1408,12 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin { if (!expr || !FOLDABLE_PATH_EXPRESSIONS.has(expr.type)) continue; // No repo context (context-less fallback scanning) means no constant map // and therefore no honest answer — skip rather than guess a path. - if (!fileRel) continue; + if (!fileKey) continue; const constants = getFoldConstants(); if (!constants) continue; const operands = parseKotlinConstOperands(expr); if (operands === null) continue; - const rawPath = foldKotlinOperands(fileRel, operands, constants); + const rawPath = foldKotlinOperands(fileKey, operands, constants); 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 9918006d7..39fd2d219 100644 --- a/gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts +++ b/gitnexus/src/core/ingestion/route-extractors/kotlin-const-resolver.ts @@ -1,13 +1,31 @@ /** * Kotlin binding for the language-agnostic constant resolver (#2391 core). * - * Supplies the two Kotlin-specific pieces the shared fold in - * `constant-resolver.ts` needs — {@link resolveKotlinImport} (import specifier → - * file, honoring JVM package rules) and {@link extractKotlinModuleConstants} - * (tree → {@link ModuleConstants}) — plus a pre-bound - * {@link resolveKotlinConstant} wrapper so callers stay language-oblivious. The - * reusable fold, the cycle guard, and the depth cap all live in the agnostic - * core. + * Supplies the two Kotlin-specific pieces — {@link resolveKotlinImport} (import + * specifier → file, honoring JVM package rules) and + * {@link extractKotlinModuleConstants} (tree → {@link ModuleConstants}) — plus + * the folding entry points {@link resolveKotlinConstant} and + * {@link foldKotlinOperands}, so callers stay language-oblivious. + * + * WHAT IS ACTUALLY SHARED WITH THE AGNOSTIC CORE. One value — + * {@link MAX_FOLD_LENGTH} — and five types. The core's own `resolveConstant` / + * `resolveOperands` are NOT called: the fold state machine below (cycle guard, + * success memo, depth caps, operand concatenation — roughly 200 of this file's + * lines) is a local fork, close enough to `java-const-resolver.ts`'s already + * forked copy that the two read as the same code with the language name + * swapped. + * + * That fork is a consequence, not an oversight. The core keys its maps by + * SIMPLE name, and a Kotlin operand can be a QUALIFIED reference at any + * position (`X = ApiPaths.Y + "/tail"`); handed to the core, `ApiPaths.Y` misses + * every map and floors the whole chain to null — see {@link computeKotlinFold}, + * which resolves operands through the qualified-aware walk for exactly this + * reason. The import chase is Kotlin-specific too: a member import is spelled + * identically to a type import, so {@link resolveImportedName} has to try both + * readings, and the core exposes no hook for that. Java forked first on the same + * grounds. Teaching the core qualified names, and retiring both copies against + * it, is the standing follow-up; until then the honest description of this file + * is "a second fork", not "a binding over a shared fold". * * Kotlin shares the JVM package/import model with Java, so this binding mirrors * `java-const-resolver.ts` in structure, naming and skip-floor discipline. The @@ -54,6 +72,17 @@ * 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. * + * 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` + * normalizes separators on both the write side (the `prepareRepo` map keys) and + * the read side (`scan`'s `fileRel`). It has to, because the orchestrator's file + * list comes from glob v13, which has no `posix: true` and joins with the + * platform separator — so on Windows the keys arrive backslashed and every + * `/.kt` test in {@link resolveKotlinImport} would miss, silently + * disabling cross-file folding on that platform alone. Normalizing INSIDE this + * module instead cannot work: the resolver returns the key it matched, and a + * normalized return value would then miss in a map that was never normalized. + * * WHERE THIS IS WIRED. Java reaches its binding from BOTH layers: the group * extractor (`group/extractors/http-patterns/java.ts`) and the ingestion * provider (`languages/java.ts`, via `extractModuleConstants` + @@ -89,6 +118,30 @@ export type { /** Source extensions a Kotlin declaration can live in. */ const KOTLIN_EXTENSIONS = ['.kt', '.kts'] as const; +/** + * Recursion ceiling for {@link parseKotlinConstOperands}, counted in `+` links. + * + * This bounds SYNTAX depth, not resolution: `A + B + C` nests one + * `additive_expression` per link, so the cap is really "how long a concatenation + * may one initializer be". Deliberately loose — generated route tables do + * concatenate a dozen fragments, and overrunning costs a skipped route, so the + * cap is a guard against pathological input rather than a statement about + * reasonable code. + */ +const MAX_OPERAND_PARSE_DEPTH = 64; + +/** + * Recursion ceiling for the fold, counted in REFERENCE hops (`A = B`, `B = C`). + * + * Larger than the agnostic core's own `MAX_RESOLVE_DEPTH` (8), which is + * module-private in `constant-resolver.ts` and therefore cannot simply be + * reused, and equal to the value the Java binding spells inline. It backstops + * the cycle guard, which terminates loops but not a long acyclic chain; the + * memo makes reaching it cheap. Both caps floor to null, i.e. to a skipped + * route. + */ +const MAX_FOLD_DEPTH = 32; + /** * Cheap content gate: can this Kotlin file DEFINE a string constant that a route * annotation might reference? @@ -97,8 +150,9 @@ const KOTLIN_EXTENSIONS = ['.kt', '.kts'] as const; * {@link extractKotlinModuleConstants} about which files carry constants — the * defect class the Java binding's shared `isJavaConstantFile` exists to prevent. * - * Arms, both deliberately WIDER than the extractor (a gate may over-admit — it - * only costs a parse — but must never reject a file the extractor accepts): + * Arms, both intended to be WIDER than the extractor (a gate may over-admit — it + * only costs a parse — while rejecting a file the extractor would accept costs a + * fact): * - `const val NAME [: T] =`. `const` is legal only at a file's top level or in * an `object`/`companion object`, i.e. exactly the carriers the extractor * harvests, so this arm needs no scope check. @@ -108,6 +162,23 @@ const KOTLIN_EXTENSIONS = ['.kt', '.kts'] as const; * whose only `val`s are function locals from costing a parse. It still admits * a top-level `val` in a file that happens to declare an object elsewhere, * which is the harmless direction. + * + * KNOWN GAP — the "never rejects what the extractor accepts" property does NOT + * hold, and claiming it did was wrong. {@link extractKotlinModuleConstants} + * calls `collectProperties(tree.rootNode, null)`, so it harvests a TOP-LEVEL + * non-`const` `val`; a file whose only constant has that shape and declares no + * `object` fails both arms above (`const val` absent, `object` absent) and is + * never parsed. + * + * The direction is safe — the constant is simply missing from the map, so a + * reference to it floors to skip, never to a wrong path — but the cost is not + * nil. Measured on both sides of this change: with the declaration in the SAME + * file as the route it still folds, because `scan` re-extracts that file's tree + * on demand and bypasses the gate; with the declaration in its OWN file the + * route is silently dropped. Closing the gap means admitting every file that + * contains any `val … =`, function locals included — very nearly the whole + * repository, in a pass whose entire purpose is to avoid parsing it. That trade + * has not been measured, so the gap is recorded here rather than papered over. */ const CONST_VAL_RE = /\bconst\s+val\s+\w+\s*(?::[^=\n{}()]{0,60})?=/; const OBJECT_DECL_RE = /\bobject\b/; @@ -272,7 +343,7 @@ export function parseKotlinConstOperands( depth = 0, ): Operand[] | null { if (!node) return null; - if (depth > 64) return null; + if (depth > MAX_OPERAND_PARSE_DEPTH) return null; if (node.type === 'string_literal') { const value = stringLiteralValue(node); return value === null ? null : [{ kind: 'literal', value }]; @@ -523,7 +594,7 @@ function resolveWithState( state: KotlinFoldState, depth: number, ): string | null { - if (depth > 32) return null; + if (depth > MAX_FOLD_DEPTH) return null; const guard = `${fileKey}::${name}`; const memoized = state.memo.get(guard); if (memoized !== undefined) return memoized; @@ -667,12 +738,22 @@ function foldOperands( /** * Fold an inline operand list (e.g. `ApiPaths.BASE + "/orders"`) against * `fileKey`, or null when any piece is unresolvable (skip floor). + * + * 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 + * Spring idiom for a collection root. Collapsing it into `null` would make a + * resolved-empty path indistinguishable from an unresolvable one — the skip + * floor is reserved for "could not fold", and nothing else in the resolver + * conflates the two: {@link resolveKotlinConstant} returns `''` for an empty + * constant, and `resolveOperands` in the shared core returns its fold + * unfiltered. Matches `foldJavaOperands`, so the two JVM bindings do not + * diverge on the same input. */ export function foldKotlinOperands( fileKey: string, operands: readonly Operand[], repo: RepoConstants, ): string | null { - const out = foldOperands(fileKey, operands, newFoldState(repo), 0); - return out === '' ? null : out; + return foldOperands(fileKey, operands, 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 6dca14ac9..ba876cc9e 100644 --- a/gitnexus/test/unit/group/kotlin-const-route-fold.test.ts +++ b/gitnexus/test/unit/group/kotlin-const-route-fold.test.ts @@ -31,6 +31,12 @@ * an interface whose `@RequestMapping` is a constant, and an unresolvable * `path` is fatal on its own; * • an unresolvable constant emits nothing rather than a guessed path; + * • a cross-file fold survives BACKSLASHED repository keys — the shape glob + * v13 hands the orchestrator on Windows, and the one every other fixture + * here misses by writing POSIX string literals; + * • a constant that folds to `""` publishes the class prefix, exactly as the + * literal `@GetMapping("")` beside it does — an empty fold is a success, + * not the skip floor; * • without a repo context the plugin emits nothing (the documented skip * floor, and the branch the 1-argument guards cannot reach); * • literal routes are untouched and are not emitted twice. @@ -532,6 +538,74 @@ interface OrderClient { ).toEqual([]); }); + it('folds across files when repository keys use Windows separators', () => { + // The orchestrator's file list comes from glob v13, which has no + // `posix: true` and joins with the platform separator, so on Windows both + // `prepareRepo({files})` and `scan(tree, ctx, rel)` see + // `src\main\kotlin\…`. `resolveKotlinImport` asks whether a key ends with + // `com/example/app/api/ApiPaths.kt` — a test no backslashed key can pass — + // so EVERY cross-file fold returned null on Windows and on Windows only: + // the pre-pass still ran and the context was still built, the feature was + // just silently absent. Every other fixture in this file is a POSIX string + // literal, which is exactly why CI stayed green. + // + // The keys are backslashed HERE rather than derived from `path.sep`, so the + // regression is pinned on every runner instead of only on the Windows + // matrix — the plugin reads keys, not the host OS, so simulating the keys + // simulates the whole bug. + const winKey = (rel: string): string => rel.replace(/\//g, '\\'); + expect( + providers({ + [winKey(CONSTS)]: CONSTS_SRC, + [winKey(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('treats a constant that folds to "" as the class prefix itself', () => { + // `const val ROOT = ""` is Spring's idiom for "the collection root", and + // `joinPath` resolves it against the class prefix exactly as it resolves the + // literal `@PostMapping("")` beside it. The fold used to collapse `''` into + // the skip floor, so the two annotations below — the same path, written two + // ways — disagreed: the literal published `POST /api`, the constant + // published nothing. Asserting BOTH in one class is the point; a test on the + // constant alone would pass against any chosen convention rather than + // pinning the two spellings together. + expect( + providers({ + [CONSTS]: `package com.example.app.api + +object ApiPaths { + const val ROOT = "" +} +`, + [CONTROLLER]: `package com.example.app.web + +import com.example.app.api.ApiPaths + +@RestController +@RequestMapping("/api") +class OrderController { + @GetMapping(ApiPaths.ROOT) + fun list() {} + + @PostMapping("") + fun create() {} +} +`, + }), + ).toEqual(['GET /api/', 'POST /api/']); + }); + 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 47b2f6dca..fc8dbc6f0 100644 --- a/gitnexus/test/unit/kotlin-route-const-resolver.test.ts +++ b/gitnexus/test/unit/kotlin-route-const-resolver.test.ts @@ -296,6 +296,28 @@ import com.example.app.api.* expect(foldKotlinOperands(CONSTS_KEY, [{ kind: 'ref', name: 'MISSING' }], repo)).toBeNull(); }); + it('folds to the empty string as a SUCCESS, not a skip', () => { + // The counterpart of the test above, and the distinction it depends on: + // `null` means "could not fold", `''` means "folded, and the answer is + // empty". `const val ROOT = ""` is Spring's spelling for "the class prefix + // itself", so collapsing it into null loses a route the literal + // `@GetMapping("")` publishes from the same class. `resolveKotlinConstant` + // already returned `''` here; `foldKotlinOperands` did not, which made the + // two entry points disagree about the same constant. + const key = 'src/main/kotlin/com/example/app/api/Root.kt'; + const repo = repoOf({ + [key]: `package com.example.app.api + +object ApiPaths { + const val ROOT = "" +} +`, + }); + expect(resolveKotlinConstant(key, 'ApiPaths.ROOT', repo)).toBe(''); + expect(foldKotlinOperands(key, [{ kind: 'ref', name: 'ApiPaths.ROOT' }], repo)).toBe(''); + expect(foldKotlinOperands(key, [{ kind: 'literal', value: '' }], repo)).toBe(''); + }); + it('terminates on a self-referential constant', () => { const key = 'src/main/kotlin/com/example/app/api/Cycle.kt'; const repo = repoOf({