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): suppress Kotlin routes only when the class prefix resolves to no literal

Browse source 
Class-prefix suppression decided "is this prefix unresolvable?" from a
three-element allow-list of node types (`simple_identifier`,
`navigation_expression`, `additive_expression`). An allow-list is safe for
FOLDING, where a forgotten shape yields no route, but it is the wrong shape
for SUPPRESSION, where a forgotten shape means "emit unprefixed" — a route
the application does not serve. `java.ts` gates on the ABSENCE of a literal
(`if (!valueNode)`) for exactly this reason.

The predicate is now inverted: a class is marked unless its `path`/`value`
argument is provably literal, recursing into `[…]` and `arrayOf(…)`
elements and refusing an interpolated `string_literal`. Measured against
the previous behavior, with `@PostMapping(ApiPaths.ORDERS)` under each
class prefix, on an app serving `/api/v1/orders`:

  * `[ApiPaths.BASE]`                   `POST /orders` -> dropped
  * `arrayOf(ApiPaths.BASE)`            `POST /orders` -> dropped
  * `value = [ApiPaths.BASE]`           `POST /orders` -> dropped
  * `buildPath()`                       `POST /orders` -> dropped
  * `if (USE_V2) "/api/v2" else …`      `POST /orders` -> dropped
  * `"${ApiPaths.BASE}"`   `POST /${ApiPaths.BASE}/orders` -> dropped

The last one published raw source text as a served path; refusing an
interpolated literal also fixes it for LITERAL method routes, which emitted
`/${ApiPaths.BASE}/list` before this branch existed.

Two regressions this suppression had introduced are repaired, both by
consulting the literal-prefix map that the pass above already built and
declining to mark a class that has an entry in it:

  * `@RequestMapping("/lit", ApiPaths.BASE)` + `@GetMapping("/list")` lost
    `GET /lit/list` entirely. Kotlin's vararg spelling leaves a resolvable
    arm behind, and suppression exists to avoid wrong routes, not to
    discard right ones.
  * `@FeignClient(path = "/api")` + `@RequestMapping(ApiPaths.BASE)` lost
    its consumer, though `path` outranks `@RequestMapping` when the URL is
    assembled and made the prefix perfectly knowable.

Two Feign emission paths never consulted the unfoldable set at all:

  * `@FeignClient(path = CONST)` was invisible to the analysis, which
    matches `@RequestMapping` only, so the client fell through to the
    no-prefix fallback and published `GET /orders` for a call the service
    makes to `/api/v1/orders`. Collected as its own set, kept separate
    because `path` outranks `@RequestMapping` in both directions.
  * The `@RequestLine` loop resolves through the identical "path wins"
    fallback chain but had no guard, so one interface could suppress its
    `@(Get|…)Mapping` route and publish its `@RequestLine` route under the
    very same unresolvable prefix. Both lanes now judge alike.

Note for reviewers: the `@RequestLine` guard is not a regression fix — that
lane emitted a wrong unprefixed consumer before this branch too. It moves a
wrong route to no route, on both sides of the change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Borozdenets Ilya 2026-08-28 09:36:07 +03:00
parent 6d5055db2f
commit 8fd58b490d
2 changed files with 450 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

267
gitnexus/src/core/group/extractors/http-patterns/kotlin.ts
View file

@ -55,9 +55,16 @@ import {
* is folded against a repo-wide Kotlin constant map built once per `extract()`
* run by `prepareRepo`, mirroring what the Java plugin does for the same shape
* in `java.ts`. An unresolvable fold skips the route (never a guessed path), and
* a CONSTANT class prefix suppresses every method route under that class — the
* rule `java.ts` applies too, because emitting those routes unprefixed would
* publish paths the application does not serve.
* a class prefix that resolves to NO literal at all suppresses every method
* route under that class — the rule `java.ts` applies too, because emitting
* those routes unprefixed would publish paths the application does not serve.
* A prefix that resolves only PARTLY (Kotlin's vararg spelling
* `@RequestMapping("/lit", ApiPaths.BASE)`) still publishes its resolvable arm:
* suppression exists to avoid wrong routes, not to discard right ones. On a
* `@FeignClient` the same rule is applied to whichever prefix GOVERNS, in the
* "path wins" order the URL is assembled in — `@FeignClient(path)` first, then
* the interface's `@RequestMapping` — and to both consumer lanes, `@(Get|...)Mapping`
* and `@RequestLine`.
*
* **Consumers** — four call-site patterns common in Kotlin
* Spring projects:
@ -149,9 +156,16 @@ const arrayOfArg = (cap: string): string => `(call_expression
(call_suffix (value_arguments (value_argument (string_literal) ${cap}))))`;
/**
* Expression node types a route path can be FOLDED from. A `string_literal` is
* deliberately absent: literal paths are already captured by the dedicated
* literal patterns, so admitting one here would emit the same route twice.
* Expression node types a METHOD route path can be FOLDED from. A
* `string_literal` is deliberately absent: literal paths are already captured by
* the dedicated literal patterns, so admitting one here would emit the same
* route twice.
*
* This is an allow-list on purpose, and only safe because it gates FOLDING: a
* shape missing from it yields no route, which is the skip floor. The
* unfoldable-CLASS-PREFIX analysis must not be written this way — there a shape
* missing from the list means "emit unprefixed", a wrong route — so it inverts
* the test instead (see `hasResolvableLiteralPathElement`).
*/
const FOLDABLE_PATH_EXPRESSIONS: ReadonlySet<string> = new Set([
'simple_identifier',
@ -178,6 +192,83 @@ function kotlinRouteArgumentExpression(arg: Parser.SyntaxNode): Parser.SyntaxNod
return arg.namedChild(1);
}
/**
* The `path = …` expression of one `@FeignClient` argument, or null.
*
* Deliberately narrower than {@link kotlinRouteArgumentExpression}: on a Feign
* client the positional argument and `value =` name a SERVICE, not a path, so
* only the explicit `path` key contributes a URL prefix. This mirrors the
* `#eq? @key "path"` guard the literal `@FeignClient` patterns use, and the
* `keyNode.text !== 'path'` guard `java.ts` applies to the same annotation.
*/
function kotlinFeignPathArgumentExpression(arg: Parser.SyntaxNode): Parser.SyntaxNode | null {
const first = arg.namedChild(0);
if (!first || first.type !== 'simple_identifier') return null;
if (!arg.children.some((c) => c.type === '=')) return null;
if (first.text !== 'path') return null;
return arg.namedChild(1);
}
/**
* Is `node` a string literal whose value is fully known at parse time — that is,
* a literal carrying no interpolation?
*
* tree-sitter-kotlin models `"$base/x"` and `"${base}/x"` as a `string_literal`
* whose named children INTERLEAVE `string_content` runs with interpolation nodes
* — `interpolation_identifier_start`/`interpolated_identifier` for the `$name`
* form, `interpolation_expression_start`/`interpolated_expression`/
* `interpolation_expression_end` for `${…}` — so the test has to be `every`, not
* `some`: `"pre${A.B}post"` carries `string_content` too. The route layer
* unquotes the RAW TEXT, so treating one as a literal publishes the source
* spelling — `/${ApiPaths.BASE}/orders` — as though the application served it.
* Escape sequences are NOT separate nodes in this grammar (`"/a\nb"` is one
* `string_content`), so this accepts exactly what it accepted before; a future
* grammar that split them would floor to "unknown" rather than to a de-escaped
* guess. Same test the constant resolver's `stringLiteralValue` applies, so a
* path is either literal on both sides or folded on neither.
*/
function isPlainStringLiteral(node: Parser.SyntaxNode): boolean {
if (node.type !== 'string_literal') return false;
return node.namedChildren.every((child) => child.type === 'string_content');
}
/**
* Element expressions of a Kotlin `arrayOf(...)` call, or null when `node` is
* not one. The JS mirror of the {@link arrayOfArg} query fragment, so the
* unfoldable-prefix analysis inspects exactly the elements the literal prefix
* patterns harvest.
*/
function kotlinArrayOfElements(node: Parser.SyntaxNode): Parser.SyntaxNode[] | null {
if (node.type !== 'call_expression') return null;
const callee = node.namedChild(0);
if (callee?.type !== 'simple_identifier' || callee.text !== 'arrayOf') return null;
const suffix = node.namedChildren.find((c) => c.type === 'call_suffix');
const args = suffix?.namedChildren.find((c) => c.type === 'value_arguments');
if (!args) return null;
return args.namedChildren
.filter((c) => c.type === 'value_argument')
.map((c) => c.namedChild(0))
.filter((c): c is Parser.SyntaxNode => c !== null);
}
/**
* Does this route-annotation path expression carry at least one element the
* literal prefix patterns can resolve to a real path?
*
* Accepts exactly the three shapes those patterns harvest — a bare literal, a
* `[…]` collection element, an `arrayOf(…)` element — and only when the literal
* is uninterpolated. Anything else (`ApiPaths.BASE`, `buildPath()`,
* `if (…) "/a" else "/b"`, a template) is NOT resolvable, which is the whole
* predicate the unfoldable-prefix analysis inverts.
*/
function hasResolvableLiteralPathElement(expr: Parser.SyntaxNode): boolean {
if (isPlainStringLiteral(expr)) return true;
if (expr.type === 'collection_literal') return expr.namedChildren.some(isPlainStringLiteral);
const elements = kotlinArrayOfElements(expr);
if (elements) return elements.some(isPlainStringLiteral);
return false;
}
// ─── Kotlin OkHttp builder verb-walk (parity with java-static-path.ts) ──
// Mirrors `inferOkHttpMethod`, adapted to the Kotlin grammar: a call `X.name(args)`
// is a `call_expression` whose callee is a `navigation_expression` (receiver +
@ -455,9 +546,11 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin {
// uses one `value_argument` node for both forms and 0.21.x has no negation to
// test the `=` token with.
//
// These deliberately match LITERAL arguments too (any `value_argument` does);
// the scan loops drop those via `FOLDABLE_PATH_EXPRESSIONS` so a literal route
// is emitted once, by the literal patterns.
// These deliberately match LITERAL arguments too (any `value_argument` does).
// The method-route loop drops those via `FOLDABLE_PATH_EXPRESSIONS` so a
// literal route is emitted once, by the literal patterns; the class-prefix
// collector instead KEEPS them and tests them for literalness, which is how a
// prefix that no literal pattern could resolve gets noticed at all.
const SPRING_CONST_CLASS_PREFIX_PATTERNS = compilePatterns({
name: 'kotlin-spring-const-class-prefix',
language,
@ -496,27 +589,94 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin {
],
} satisfies LanguagePatterns<Record<string, never>>);
const SPRING_CONST_FEIGN_PATH_PATTERNS = compilePatterns({
name: 'kotlin-spring-const-feign-path',
language,
patterns: [
{
meta: {},
query: `
(class_declaration
(modifiers
(annotation
(constructor_invocation
(user_type (type_identifier) @ann (#eq? @ann "FeignClient"))
(value_arguments (value_argument) @arg))))) @class
`,
},
],
} satisfies LanguagePatterns<Record<string, never>>);
/**
* Ids of classes whose `@RequestMapping` prefix is a CONSTANT reference rather
* than a literal.
* Ids of classes whose `@RequestMapping` prefix cannot be resolved to any
* literal, so no route under them can be published at a path the application
* actually serves.
*
* The prefix is not folded: it also feeds the cross-file interface-inheritance
* pass, which has no repo context, so folding it in `scan` alone would make
* the two views disagree. Every route under such a class is dropped instead —
* dropping the prefix would publish the method at a path the application never
* serves, turning a missing fact into a wrong one. Same rule `java.ts` applies
* The predicate is INVERTED rather than an allow-list of non-literal node
* types: a class is marked unless its `path`/`value` argument is provably
* literal (recursing into `[…]` and `arrayOf(…)` elements, and refusing an
* interpolated `string_literal`). An allow-list has to enumerate every
* non-literal spelling and silently passes the ones it forgot —
* `[ApiPaths.BASE]`, `arrayOf(ApiPaths.BASE)`, `buildPath()`,
* `if (…) "/a" else "/b"` — each of which then publishes its methods at their
* UNPREFIXED path, a route the application does not serve. `java.ts` gates on
* the ABSENCE of a literal (`if (!valueNode)`) for the same reason.
*
* `resolvedPrefixes` is the literal prefix map built by the pass ABOVE, and a
* class holding an entry there is deliberately NOT marked: Kotlin's vararg
* spelling `@RequestMapping("/lit", ApiPaths.BASE)` leaves a resolvable `/lit`
* behind, and suppressing it would drop a route that IS derivable — trading a
* wrong route for a missing one, which is not the bargain this suppression
* exists to make. The prefix set is then partial (the constant arm is absent)
* exactly as it was before constant folding existed.
*
* The prefix is never folded here: it also feeds the cross-file
* interface-inheritance pass, which has no repo context, so folding it in
* `scan` alone would make the two views disagree. Same rule `java.ts` applies
* (`typesWithUnfoldablePrefix`); folding class prefixes cross-file is a
* follow-up on both sides. Used by BOTH `scan` and the inheritance-view
* collector, so the two cannot drift apart.
* collector — with the prefix map each has already built — so the two cannot
* drift apart.
*/
const collectUnfoldablePrefixClassIds = (tree: Parser.Tree): Set<number> => {
const collectUnfoldablePrefixClassIds = (
tree: Parser.Tree,
resolvedPrefixes: ReadonlyMap<number, string[]>,
): Set<number> => {
const ids = new Set<number>();
for (const match of runCompiledPatterns(SPRING_CONST_CLASS_PREFIX_PATTERNS, tree)) {
const argNode = match.captures.arg;
const classNode = match.captures.class;
if (!argNode || !classNode) continue;
if ((resolvedPrefixes.get(classNode.id) ?? []).length > 0) continue;
const expr = kotlinRouteArgumentExpression(argNode);
if (!expr || !FOLDABLE_PATH_EXPRESSIONS.has(expr.type)) continue;
if (!expr || hasResolvableLiteralPathElement(expr)) continue;
ids.add(classNode.id);
}
return ids;
};
/**
* Ids of `@FeignClient` interfaces whose `path` argument is present but not
* resolvable to a literal.
*
* `collectUnfoldablePrefixClassIds` cannot see these: it matches
* `@RequestMapping` only, so `@FeignClient(path = ApiPaths.BASE)` fell through
* to the `['']` prefix fallback and published the consumer at its unprefixed
* path — a call the service never makes. Kept as its own set rather than
* merged into the `@RequestMapping` one because `path` OUTRANKS
* `@RequestMapping` on a Feign client: an unresolvable `path` is fatal
* whatever the `@RequestMapping` says, and a resolvable `path` rescues a route
* whose `@RequestMapping` is a constant. The consumer lanes therefore consult
* the two in that same "path wins" order.
*/
const collectFeignUnfoldablePathClassIds = (tree: Parser.Tree): Set<number> => {
const ids = new Set<number>();
for (const match of runCompiledPatterns(SPRING_CONST_FEIGN_PATH_PATTERNS, tree)) {
const argNode = match.captures.arg;
const classNode = match.captures.class;
if (!argNode || !classNode) continue;
const expr = kotlinFeignPathArgumentExpression(argNode);
if (!expr || hasResolvableLiteralPathElement(expr)) continue;
ids.add(classNode.id);
}
return ids;
@ -998,6 +1158,12 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin {
const prefixNode = match.captures.prefix;
const classNode = match.captures.class;
if (!prefixNode || !classNode) continue;
// An INTERPOLATED literal (`"${ApiPaths.BASE}"`) is not a path — unquoting
// its raw text would carry the source spelling into the shared type view
// as a served prefix. Refusing it here is also what lets the unfoldable
// analysis below mark such a class (it skips classes with a resolved
// prefix), so the two stay one decision rather than two.
if (!isPlainStringLiteral(prefixNode)) continue;
const prefix = unquoteLiteral(prefixNode.text);
if (prefix !== null) pushPrefix(prefixByClassId, classNode.id, prefix);
}
@ -1009,7 +1175,7 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin {
// noise into the shared type view, so it is left out — the same skip floor
// `java.ts`'s `collectSpringTypes` keeps.
const routesByMethodId = new Map<number, Array<{ method: string; path: string }>>();
const unfoldablePrefixClassIds = collectUnfoldablePrefixClassIds(tree);
const unfoldablePrefixClassIds = collectUnfoldablePrefixClassIds(tree, prefixByClassId);
for (const match of runCompiledPatterns(SPRING_METHOD_ROUTE_PATTERNS, tree)) {
const annNode = match.captures.ann;
const pathNode = match.captures.path;
@ -1137,11 +1303,16 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin {
const prefixNode = match.captures.prefix;
const classNode = match.captures.class;
if (!prefixNode || !classNode) continue;
// An INTERPOLATED literal (`"${ApiPaths.BASE}"`) is not a path — see
// `isPlainStringLiteral`. Refusing it here also lets the unfoldable
// analysis below mark such a class, since that skips classes whose
// prefix already resolved.
if (!isPlainStringLiteral(prefixNode)) continue;
const prefix = unquoteLiteral(prefixNode.text);
if (prefix !== null) pushPrefix(prefixByClassId, classNode.id, prefix);
}
const classesWithUnfoldablePrefix = collectUnfoldablePrefixClassIds(tree);
const classesWithUnfoldablePrefix = collectUnfoldablePrefixClassIds(tree, prefixByClassId);
// ─── OpenFeign client interfaces + HTTP Interface type prefixes ──
// In tree-sitter-kotlin an `interface` is a `class_declaration`, so a
@ -1155,11 +1326,12 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin {
if (!classNode) continue;
feignClassIds.add(classNode.id);
const prefixNode = match.captures.prefix;
if (prefixNode) {
if (prefixNode && isPlainStringLiteral(prefixNode)) {
const prefix = unquoteLiteral(prefixNode.text);
if (prefix !== null) pushPrefix(feignPrefixByClassId, classNode.id, prefix);
}
}
const feignClassesWithUnfoldablePath = collectFeignUnfoldablePathClassIds(tree);
const httpExchangePrefixByClassId = new Map<number, string[]>();
for (const match of runCompiledPatterns(SPRING_HTTP_EXCHANGE_CLASS_PATTERNS, tree)) {
const classNode = match.captures.class;
@ -1222,27 +1394,30 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin {
for (const { httpMethod, rawPath, nameNode, methodNode } of methodRoutes) {
const enclosingClass = findEnclosingClass(methodNode);
// A constant-valued class prefix cannot be resolved here, so every route
// under such a class is dropped rather than emitted at a wrong
// (unprefixed) path — the rule `java.ts` applies for Java.
//
// This reaches a @FeignClient INTERFACE too, because tree-sitter-kotlin
// models `interface` as a `class_declaration`, and it should: Spring
// Cloud prepends a type-level @RequestMapping to every method of the
// client, so an unfoldable prefix makes the remote URL unknowable
// whether or not @FeignClient(path) is also present. Java diverges here
// only by accident of its grammar — `findEnclosingClass` skips
// `interface_declaration`, so `java.ts` still emits such a consumer at
// its unprefixed path. Aligning Java is a change to Java's behavior and
// belongs in its own follow-up, not in the Kotlin binding.
if (enclosingClass && classesWithUnfoldablePrefix.has(enclosingClass.id)) continue;
// A @(Get|...)Mapping inside a @FeignClient interface is an OpenFeign
// consumer (a remote call), not a route this service serves.
if (enclosingClass && feignClassIds.has(enclosingClass.id)) {
// Whichever prefix GOVERNS must be resolvable, or the remote URL is
// unknowable and an unprefixed consumer would be a call this service
// never makes. Checked in the same "path wins" order the fallback
// below resolves in, so an unresolvable `@RequestMapping` does not
// suppress a client whose literal `@FeignClient(path)` outranks it,
// and an unresolvable `path` is fatal even when `@RequestMapping` is
// a literal.
//
// This reaches a Feign INTERFACE at all because tree-sitter-kotlin
// models `interface` as a `class_declaration`, and it should: Spring
// Cloud prepends the governing prefix to every method of the client.
// Java diverges only by accident of its grammar — `findEnclosingClass`
// skips `interface_declaration`, so `java.ts` still emits such a
// consumer at its unprefixed path. Aligning Java is a change to Java's
// behavior and belongs in its own follow-up, not in the Kotlin binding.
if (feignClassesWithUnfoldablePath.has(enclosingClass.id)) continue;
const feignPrefixes = feignPrefixByClassId.get(enclosingClass.id);
if (!feignPrefixes && classesWithUnfoldablePrefix.has(enclosingClass.id)) continue;
// @FeignClient(path) wins over @RequestMapping; a multi-element prefix
// yields one consumer per (prefix × this route).
const prefixes = feignPrefixByClassId.get(enclosingClass.id) ??
prefixByClassId.get(enclosingClass.id) ?? [''];
const prefixes = feignPrefixes ?? prefixByClassId.get(enclosingClass.id) ?? [''];
for (const prefix of prefixes) {
out.push({
role: 'consumer',
@ -1256,6 +1431,10 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin {
}
continue;
}
// An unresolvable class prefix leaves no path this service serves, so
// every route under such a class is dropped rather than emitted at a
// wrong (unprefixed) one — the rule `java.ts` applies for Java.
if (enclosingClass && classesWithUnfoldablePrefix.has(enclosingClass.id)) continue;
// A @(Get|...)Mapping on a (non-Feign) interface declares a route
// *contract*, not a route this service serves — the implementing
// @RestController is the provider, emitted via scanProject's interface
@ -1425,13 +1604,21 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin {
if (!parsed) continue;
const enclosingClass = findEnclosingClass(methodNode);
if (!enclosingClass || !isKotlinInterface(enclosingClass)) continue;
// The same governing-prefix resolvability guard the @(Get|...)Mapping-in-Feign
// lane applies, in the same "path wins" order — this loop resolves through
// the identical fallback chain, so an unresolvable governing prefix leaves
// the remote URL just as unknowable here. Without it a single interface
// could suppress its @(Get|...)Mapping routes and publish its @RequestLine
// routes under the very same unresolvable prefix.
if (feignClassesWithUnfoldablePath.has(enclosingClass.id)) continue;
const feignPrefixes = feignPrefixByClassId.get(enclosingClass.id);
if (!feignPrefixes && classesWithUnfoldablePrefix.has(enclosingClass.id)) continue;
// Mirror java.ts (which pre-merges the @RequestMapping fallback into
// feignPrefixByInterfaceId, "path wins"): @FeignClient(path) wins, else
// the interface's class-level @RequestMapping prefix, else none. Without
// the prefixByClassId fallback Kotlin dropped the class prefix that Java
// applies — the same fallback chain the @GetMapping-in-Feign path uses above.
const prefixes = feignPrefixByClassId.get(enclosingClass.id) ??
prefixByClassId.get(enclosingClass.id) ?? [''];
const prefixes = feignPrefixes ?? prefixByClassId.get(enclosingClass.id) ?? [''];
for (const prefix of prefixes) {
out.push({
role: 'consumer',

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

@ -10,14 +10,26 @@
* Asserted:
* • the four reference forms fold to the right provider contract — qualified
* access, fully-qualified name, single-name import, `+`-concatenation;
* • a CONSTANT class prefix suppresses every method route under that class,
* literal ones included (the prefix is not knowable here, and emitting the
* methods unprefixed would publish paths the application does not serve) —
* the rule `java.ts` already applies — in both the positional and the
* `value =` spelling, which take different branches of
* `kotlinRouteArgumentExpression`;
* • an OpenFeign consumer folds a constant method path, and is suppressed by a
* constant interface prefix for the same reason a provider is;
* • a class prefix that resolves to NO literal suppresses every method route
* under that class, literal ones included (the prefix is not knowable here,
* and emitting the methods unprefixed would publish paths the application
* does not serve) — the rule `java.ts` already applies. Pinned across every
* spelling that reaches the suppression, because the analysis inverts a
* literalness test rather than listing node types: a bare constant, both
* argument spellings, `[…]`, `arrayOf(…)`, a call, an `if`, and an
* interpolated string;
* • a prefix that resolves only PARTLY still publishes its resolvable arm —
* Kotlin's vararg `@RequestMapping("/lit", ApiPaths.BASE)` keeps `/lit`,
* because suppression exists to avoid wrong routes, not to discard right
* ones;
* • a `@RequestMapping` with no path argument at all is not a prefix and does
* not suppress anything;
* • an OpenFeign consumer folds a constant method path, and both consumer
* lanes (`@(Get|…)Mapping` and `@RequestLine`) are suppressed by an
* unresolvable governing prefix for the same reason a provider is —
* resolved in "path wins" order, so a literal `@FeignClient(path)` rescues
* 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;
* • without a repo context the plugin emits nothing (the documented skip
* floor, and the branch the 1-argument guards cannot reach);
@ -208,6 +220,99 @@ class OrderController {
).toEqual([]);
});
/**
* A controller carrying `prefix` as its class-level `@RequestMapping`, with
* one constant-valued and one literal route under it. `decls` holds any
* top-level declaration the prefix expression refers to.
*/
const controllerWithPrefix = (prefix: string, decls = ''): Record<string, string> => ({
[CONSTS]: CONSTS_SRC,
[CONTROLLER]: `package com.example.app.web
import com.example.app.api.ApiPaths
${decls}
@RestController
@RequestMapping(${prefix})
class OrderController {
@GetMapping(ApiPaths.ORDERS)
fun list() {}
@GetMapping("/literal")
fun literal() {}
}
`,
});
// Every prefix spelling that resolves to no literal, and so must suppress.
// This is a table rather than one representative case on purpose: the two
// tests above pin a BARE constant, which any node-type allow-list would also
// catch. These are the shapes such a list forgets — and forgetting one does
// not degrade to "no route", it publishes every method of the class at its
// UNPREFIXED path, which the application does not serve. The `if` and the
// interpolated string are the two that need no constant map at all to go
// wrong, and the `[…]` / `arrayOf(…)` pair matters because the literal
// prefix patterns DO reach inside both — so a naive "is it a literal
// container?" test would pass them straight through.
it.each([
['a collection literal holding a constant', '[ApiPaths.BASE]', ''],
['an arrayOf(…) holding a constant', 'arrayOf(ApiPaths.BASE)', ''],
['a named collection literal holding a constant', 'value = [ApiPaths.BASE]', ''],
['a function call', 'buildPath()', '\nfun buildPath(): String = ApiPaths.BASE\n'],
['an interpolated string', '"${ApiPaths.BASE}"', ''],
['an if expression', 'if (USE_V2) "/api/v2" else "/api/v1"', '\nconst val USE_V2 = false\n'],
])('suppresses every method route under a class prefix that is %s', (_label, prefix, decls) => {
expect(providers(controllerWithPrefix(prefix, decls))).toEqual([]);
});
it('keeps both routes when that same class prefix is a plain literal', () => {
// The control for the table above: same two methods, same helper, a prefix
// the extractor can resolve. Without it an empty result there would be
// indistinguishable from the fixture failing to produce routes at all.
expect(providers(controllerWithPrefix('"/api"'))).toEqual([
'GET /api/api/v1/orders',
'GET /api/literal',
]);
});
it('keeps the resolvable arm of a PARTLY resolvable class prefix', () => {
// Kotlin's vararg spelling. `/lit` is a real prefix the application really
// serves, so the routes under it are derivable and must survive; only the
// `ApiPaths.BASE` arm is missing from the result, exactly as it was before
// constant folding existed. Marking the class unfoldable here would trade a
// wrong route for a missing one, which is not the bargain suppression makes.
expect(providers(controllerWithPrefix('"/lit", ApiPaths.BASE'))).toEqual([
'GET /lit/api/v1/orders',
'GET /lit/literal',
]);
// Same shape spelled as one collection argument.
expect(providers(controllerWithPrefix('["/lit", ApiPaths.BASE]'))).toEqual([
'GET /lit/api/v1/orders',
'GET /lit/literal',
]);
});
it('does not treat a @RequestMapping without a path argument as a prefix', () => {
// `produces` is not a path, so this class has no prefix — not an
// unresolvable one. Suppressing here would drop routes that are correct and
// complete as written.
expect(
providers({
[CONSTS]: CONSTS_SRC,
[CONTROLLER]: `package com.example.app.web
import com.example.app.api.ApiPaths
@RestController
@RequestMapping(produces = [MediaType.APPLICATION_JSON_VALUE])
class OrderController {
@GetMapping(ApiPaths.ORDERS)
fun list() {}
}
`,
}),
).toEqual(['GET /api/v1/orders']);
});
it('still applies a LITERAL class prefix to a folded method path', () => {
expect(
providers({
@ -317,6 +422,116 @@ interface OrderClient {
).toEqual(['GET /api/v1/orders']);
});
it('drops a @FeignClient consumer whose `path` argument is a CONSTANT', () => {
// `path` is the Feign client's own prefix and is never a `@RequestMapping`,
// so the class-prefix analysis cannot see it. Left unchecked, this interface
// falls through to the no-prefix fallback and publishes a remote call to
// `/api/v1/orders` as a call to `/orders` — a consumer edge pointing at a
// route no service serves.
const files = {
[CONSTS]: CONSTS_SRC,
[CLIENT]: `package com.example.app.client
import com.example.app.api.ApiPaths
@FeignClient(name = "orders", path = ApiPaths.BASE)
interface OrderClient {
@GetMapping(ApiPaths.ORDERS)
fun list()
}
`,
};
expect(consumers(files)).toEqual([]);
// Control: the same interface with a LITERAL `path` is still detected.
expect(
consumers({
...files,
[CLIENT]: files[CLIENT].replace('path = ApiPaths.BASE', 'path = "/svc"'),
}),
).toEqual(['GET /svc/api/v1/orders']);
});
it('lets a literal @FeignClient(path) outrank a CONSTANT @RequestMapping', () => {
// `path` wins over `@RequestMapping` when the URL is assembled, so it has to
// win when resolvability is judged too — otherwise an interface whose real
// prefix is perfectly knowable loses its consumer to a `@RequestMapping`
// that never governed it.
expect(
consumers({
[CONSTS]: CONSTS_SRC,
[CLIENT]: `package com.example.app.client
import com.example.app.api.ApiPaths
@FeignClient(name = "orders", path = "/svc")
@RequestMapping(ApiPaths.BASE)
interface OrderClient {
@GetMapping("/orders")
fun list()
}
`,
}),
).toEqual(['GET /svc/orders']);
});
it('drops a @RequestLine consumer under an unresolvable interface prefix', () => {
// `@RequestLine` carries its own verb and path but is still prefixed by the
// interface, and it resolves through the same "path wins" fallback chain as
// the `@(Get|…)Mapping` lane — so an unresolvable governing prefix leaves
// the remote URL just as unknowable here.
const files = {
[CONSTS]: CONSTS_SRC,
[CLIENT]: `package com.example.app.client
import com.example.app.api.ApiPaths
@FeignClient(name = "orders")
@RequestMapping(ApiPaths.BASE)
interface OrderClient {
@RequestLine("GET /list")
fun list()
}
`,
};
expect(consumers(files)).toEqual([]);
// Control: a literal interface prefix still yields the prefixed consumer.
expect(
consumers({
...files,
[CLIENT]: files[CLIENT].replace(
'@RequestMapping(ApiPaths.BASE)',
'@RequestMapping("/lit")',
),
}),
).toEqual(['GET /lit/list']);
});
it('judges @RequestLine and @(Get|…)Mapping alike on ONE interface', () => {
// Both lanes read the same prefix through the same fallback chain, so they
// must reach the same verdict on it. A guard on only one of them lets the
// interface suppress one route and publish the other under the very same
// unresolvable prefix — a self-inconsistency visible in a single scan.
expect(
consumers({
[CONSTS]: CONSTS_SRC,
[CLIENT]: `package com.example.app.client
import com.example.app.api.ApiPaths
@FeignClient(name = "orders")
@RequestMapping(ApiPaths.BASE)
interface OrderClient {
@GetMapping(ApiPaths.ORDERS)
fun list()
@RequestLine("GET /list")
fun listLegacy()
}
`,
}),
).toEqual([]);
});
it('leaves literal routes unchanged and emits each exactly once', () => {
expect(
providers({