mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
fix(routes): address the open review findings on Java constant route folding
Answers every reproduced finding still open on #2980, plus the defects an
adversarial pass found in the first round of those fixes. The wrong-path group
each turned a *missing* fact into a *wrong* one, which is what this module's
skip-or-correct contract exists to prevent.
Wrong-path fixes
* Escapes were deleted from constant values. tree-sitter-java splits a
`string_literal` around its `escape_sequence` children, so joining
`string_fragment`s alone folded `"/user/{id:\\d+}"` — the standard Spring
path-variable constraint — to `/user/{id:d+}`, and a pure-escape literal to
the empty string. Worse, the LITERAL path keeps escapes verbatim, so one Java
route had two irreconcilable spellings. `stringLiteralValue` now reuses
`unquoteSpringLiteral`, the helper that literal path already uses. Java text
blocks are excluded: that helper's `"""` arm would hand back the raw block,
newline and incidental indentation included, so they keep the old skip.
* A constant-valued class prefix produced a truncated route. The new
`@value_expr` query branches were `method_declaration`-only, so
`@RequestMapping(ApiPaths.BASE)` left the prefix empty and the method route
was emitted unprefixed — a path the application does not serve, where the base
emitted nothing at all. Both subsystems now detect such a class and suppress
its method routes, the rule `classesWithArrayPrefix` already encodes for the
array form. The suppression covers ingestion's separate no-argument-mapping
loop too, without which a bare `@GetMapping` under a constant prefix still
shipped an empty-path Route while the group emitted nothing.
* A shadowed static import survived a non-foldable rebind. The rebind-drop
deleted `literals`/`exprs` but not `imports`, so a name both static-imported
and locally redeclared resolved through the stale import to the imported
value instead of skipping (#2393's Python defect, reproduced for Java).
* `resolveJavaImport` guessed where its own docstring promised null. The
nearest-shared-directory tie-break is gone: javac resolves duplicate FQNs by
classpath order, so proximity can return a src/test fixture copy.
Parity and coverage fixes
* One constant-file gate, exported as `isJavaConstantFile` and used by both the
ingestion provider and the group `prepareRepo` pre-pass. The two spellings
disagreed on a constant INTERFACE — implicitly `public static final`, so it
carries neither keyword — which the group admitted and ingestion rejected, so
the group published a contract while the graph got no Route node. It is also
modifier-order agnostic now, and its interface arm requires a String
assignment so a javadoc mentioning "interface" no longer costs a parse.
* Import ambiguity is measured over constant-DEFINING files on both sides.
Ingestion's harvest gate also admits import-only files, so handing
`resolveJavaImport` every repo key let a duplicate FQN that defines nothing
make ingestion alone floor to skip — reopening the same parity break in the
same losing direction.
* Python's constant harvest is unconditional again. The gate added here
required NAME immediately followed by `=`, so it dropped `API: str = "/api"`,
`API: Final[str] = "/api"` and every composed constant whose RHS starts with
an identifier — routes that already resolve on main. The worker now treats a
missing heuristic as "harvest" rather than "skip".
* Enum and record declarations were traversed but never collected, so a
`static final String` declared in one was absent from the map. The walk still
descends the whole body, so a type nested in an enum-constant body is kept.
* Constants composed across files through a qualified ref never resolved:
operands found inside an initializer went to the agnostic core, which only
knows bare names, so `X = BConsts.Y + "/tail"` floored to null even
acyclically. The Java binding now folds its own expressions — and carries the
core's guards with them: a `visited` stack popped on unwind, a memo of
successes, and `MAX_FOLD_LENGTH`. Without the memo a shared-descendant DAG
re-folds each child per reference; because a chain of empty strings never
accumulates output, the length cap could not stop it, and one route over a
31-line constants file took 11 s at 28 levels on the main thread.
* Dropped the dead `com.java.lang.` type normalization.
Cache
* `SCHEMA_BUMP` 70 -> 72. Leaving it at 70 was justified by "the ledger already
sits at 70, whose capture set post-dates and includes this harvest" — it does
not: 70 was cut by fe3d7e56b for #2417/#2891, an ancestor of this base. With
package.json untouched, `PARSE_CACHE_VERSION` was byte-identical across the
merge, so every same-version warm cache replayed pre-feature captures and the
feature was inert. 72 rather than 71 because open PR #3017 already claims 71
with an identical pin test — the ledger's rule is the next value above every
in-flight claim, not above origin/main.
Tests
* Regression cover for each fix above, including a gate-level test (the gate
itself had none), an import-ambiguity test, a text-block test, and a 30-level
shared-descendant DAG that fails by timeout if the memo is ever removed.
* New `group/java-const-route-parity.test.ts` drives `prepareRepo` + a
three-argument `scan`. Every existing Spring parity guard calls `scan(tree)`
with ONE argument, and the plugin drops constant-valued routes without a repo
context — so those guards were structurally blind to this whole feature.
* The pipeline e2e now proves the warm run is a REPLAY (`usedWorkerPool` false)
instead of only comparing route sets. It was not one: the test never persisted
the durable ParsedFile store, so the "warm" run reparsed through the workers
and would have passed with the cache round-trip completely broken.
* Its dist freshness gate covers every source the pipeline loads, not just
parse-worker.ts, and prints the loud message the docblock promised.
* The self-import cycle fixture now actually self-imports, so it reaches the
qualified-ref recursion and its depth cap.
* Removed the dead `WIN_POST_MAPPING` fixture and the claim behind it: Spring
alias recognition is an exact-name map on this base, so `@WinPostMapping`
extracts zero routes no matter how its value folds (#2883 is still open).
Fixtures now use annotations this branch actually recognises.
This commit is contained in:
Gergo Magyar
parent
a1ea58ef61
commit
43a0ff2902
14 changed files with 1051 additions and 182 deletions
|
|
@ -31,6 +31,7 @@ import {
|
|||
import {
|
||||
extractJavaModuleConstants,
|
||||
foldJavaOperands,
|
||||
isJavaConstantFile,
|
||||
parseJavaConstOperands,
|
||||
type RepoConstants,
|
||||
} from '../../../ingestion/route-extractors/java-const-resolver.js';
|
||||
|
|
@ -171,6 +172,19 @@ const JAVA_ROUTE_ANNOTATION_PATTERNS = compilePatterns({
|
|||
key: (identifier) @key
|
||||
value: [(string_literal) @value (element_value_array_initializer (string_literal) @value)]))))
|
||||
name: (identifier) @member) @node
|
||||
(class_declaration
|
||||
(modifiers
|
||||
(annotation
|
||||
name: [(identifier) (scoped_identifier)] @ann
|
||||
arguments: (annotation_argument_list [(identifier) @value_expr (field_access) @value_expr (binary_expression) @value_expr])))) @node
|
||||
(class_declaration
|
||||
(modifiers
|
||||
(annotation
|
||||
name: [(identifier) (scoped_identifier)] @ann
|
||||
arguments: (annotation_argument_list
|
||||
(element_value_pair
|
||||
key: (identifier) @key
|
||||
value: [(identifier) @value_expr (field_access) @value_expr (binary_expression) @value_expr]))))) @node
|
||||
(method_declaration
|
||||
(modifiers
|
||||
(annotation
|
||||
|
|
@ -511,6 +525,16 @@ interface RouteAnnotationScan {
|
|||
feignPrefixByInterfaceId: Map<number, string[]>;
|
||||
/** Spring HTTP Interface `@HttpExchange(url|value)` type-level prefixes per class/interface node id. */
|
||||
httpExchangePrefixByTypeId: Map<number, string[]>;
|
||||
/**
|
||||
* Class node ids whose `@RequestMapping` prefix is a constant reference or
|
||||
* concat rather than a literal. Folding a TYPE-level prefix would need the
|
||||
* repo constant map inside `scanRouteAnnotations`, which has no access to it,
|
||||
* so `scan()` suppresses every method route under such a class instead of
|
||||
* emitting it with the prefix silently dropped (a wrong path, not a missing
|
||||
* one). Ingestion's `extractSpringRoutes` applies the identical rule — R4
|
||||
* parity.
|
||||
*/
|
||||
typesWithUnfoldablePrefix: Set<number>;
|
||||
/** Resolved Spring shortcut/`@RequestMapping` routes — paths × verbs yield one entry each. */
|
||||
methodRoutes: MethodRouteAnnotation[];
|
||||
/** One entry per OpenFeign `@RequestLine` whose value parses to a verb + path. */
|
||||
|
|
@ -538,6 +562,7 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan {
|
|||
// feeds the OpenFeign *consumer* path in scan(). An interface carrying both
|
||||
// `@RequestMapping` and `@FeignClient(path)` lands a different value in each.
|
||||
const prefixByTypeId = new Map<number, string[]>();
|
||||
const typesWithUnfoldablePrefix = new Set<number>();
|
||||
const feignPrefixByInterfaceId = new Map<number, string[]>();
|
||||
const httpExchangePrefixByTypeId = new Map<number, string[]>();
|
||||
const methodRoutes: MethodRouteAnnotation[] = [];
|
||||
|
|
@ -649,6 +674,11 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan {
|
|||
// — on an interface — an OpenFeign `@FeignClient(path = "...")` prefix.
|
||||
if (ann === 'RequestMapping') {
|
||||
if (!isRouteMemberKey(keyNode)) continue;
|
||||
if (!valueNode) {
|
||||
// Constant-valued class prefix — see `typesWithUnfoldablePrefix`.
|
||||
typesWithUnfoldablePrefix.add(node.id);
|
||||
continue;
|
||||
}
|
||||
const prefix = unquoteLiteral(valueNode.text);
|
||||
if (prefix !== null) {
|
||||
pushPrefix(prefixByTypeId, node.id, prefix);
|
||||
|
|
@ -659,13 +689,13 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan {
|
|||
} else if (ann === 'FeignClient' && node.type === 'interface_declaration') {
|
||||
// Feign's `name`/`value` identify a service, not a path — only `path` is a prefix.
|
||||
if (!keyNode || keyNode.text !== 'path') continue;
|
||||
const prefix = unquoteLiteral(valueNode.text);
|
||||
const prefix = valueNode ? unquoteLiteral(valueNode.text) : null;
|
||||
if (prefix !== null) pushPrefix(feignPrefixByInterfaceId, node.id, prefix);
|
||||
} else if (ann === 'HttpExchange') {
|
||||
// Spring HTTP Interface type-level prefix: the path lives in `url`/`value`
|
||||
// (or positionally). Applies to its `@(Get|...)Exchange` consumer methods.
|
||||
if (keyNode && keyNode.text !== 'url' && keyNode.text !== 'value') continue;
|
||||
const prefix = unquoteLiteral(valueNode.text);
|
||||
const prefix = valueNode ? unquoteLiteral(valueNode.text) : null;
|
||||
if (prefix !== null) pushPrefix(httpExchangePrefixByTypeId, node.id, prefix);
|
||||
}
|
||||
}
|
||||
|
|
@ -715,6 +745,7 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan {
|
|||
|
||||
return {
|
||||
prefixByTypeId,
|
||||
typesWithUnfoldablePrefix,
|
||||
feignPrefixByInterfaceId,
|
||||
httpExchangePrefixByTypeId,
|
||||
methodRoutes: constrainedMethodRoutes,
|
||||
|
|
@ -760,9 +791,14 @@ function collectImplementedInterfaces(typeNode: Parser.SyntaxNode): string[] {
|
|||
}
|
||||
|
||||
function collectSpringTypes(filePath: string, tree: Parser.Tree): SharedSpringType[] {
|
||||
const { prefixByTypeId, methodRoutes } = scanRouteAnnotations(tree);
|
||||
const { prefixByTypeId, typesWithUnfoldablePrefix, methodRoutes } = scanRouteAnnotations(tree);
|
||||
const routesByMethodId = new Map<number, Array<{ method: string; path: string }>>();
|
||||
for (const route of methodRoutes) {
|
||||
// Constant-valued class prefix: no single prefix string exists here, so the
|
||||
// inheritance view would publish this route unprefixed. Skip — same rule as
|
||||
// scan() and as ingestion (R4 parity).
|
||||
const owner = findEnclosingClass(route.methodNode);
|
||||
if (owner && typesWithUnfoldablePrefix.has(owner.id)) continue;
|
||||
// A constant-referencing route still carries `rawPath: ''` here — folding
|
||||
// happens in scan() against the repo constant map, which this
|
||||
// inheritance-view collector has no access to. Emitting it as an empty
|
||||
|
|
@ -870,7 +906,12 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = {
|
|||
// A gate that also matched `import ...;` would parse the entire
|
||||
// repository here (tens of thousands of files) just to build import
|
||||
// tables the fold can derive per-file on demand.
|
||||
if (!src || !/static\s+final\s+String\s|interface\s+[A-Z]\w*\s*\{/.test(src)) {
|
||||
//
|
||||
// The predicate is the SHARED one the ingestion provider uses, so the
|
||||
// two subsystems agree on which files define constants. Its previous
|
||||
// local spelling missed `final static String` and lowercase interface
|
||||
// names, and admitted an interface that ingestion's gate rejected.
|
||||
if (!src || !isJavaConstantFile(src)) {
|
||||
continue;
|
||||
}
|
||||
const tree = args.parseSource(args.parser, src);
|
||||
|
|
@ -898,6 +939,7 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = {
|
|||
// `@RequestLine`s — from a single `matches()` pass over the tree.
|
||||
const {
|
||||
prefixByTypeId,
|
||||
typesWithUnfoldablePrefix,
|
||||
feignPrefixByInterfaceId,
|
||||
httpExchangePrefixByTypeId,
|
||||
methodRoutes,
|
||||
|
|
@ -936,6 +978,12 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = {
|
|||
};
|
||||
|
||||
for (const route of methodRoutes) {
|
||||
// A constant-valued CLASS prefix cannot be folded here, so every method
|
||||
// route under such a class is suppressed rather than emitted at a wrong
|
||||
// (unprefixed) path — the same rule `classesWithArrayPrefix` already
|
||||
// encodes for the array form, and the same rule ingestion applies.
|
||||
const owner = findEnclosingClass(route.methodNode);
|
||||
if (owner && typesWithUnfoldablePrefix.has(owner.id)) continue;
|
||||
// Non-literal route path: fold the operand list against the repo-wide
|
||||
// constant map. Skip (never a guessed path) when the fold fails or the
|
||||
// repo context is absent (context-less fallback scanning).
|
||||
|
|
|
|||
|
|
@ -347,11 +347,11 @@ interface LanguageProviderConfig {
|
|||
* parse phase can resolve non-literal decorator route paths cross-file.
|
||||
*
|
||||
* The worker calls this when BOTH hold:
|
||||
* - the file is cheap to harvest (provider-declared `moduleConstantHeuristic`
|
||||
* matched — syntax-driven, e.g. a `static final String` field or a
|
||||
* constants-bearing import; NEVER a class-name pattern like `*Constants`,
|
||||
* which silently drops route constants living in classes named e.g.
|
||||
* `ApiPaths`/`Routes`), and
|
||||
* - the provider declares no `moduleConstantHeuristic`, or the one it
|
||||
* declares matched — syntax-driven, e.g. a `static final String` field or
|
||||
* a constants-bearing import; NEVER a class-name pattern like
|
||||
* `*Constants`, which silently drops route constants living in classes
|
||||
* named e.g. `ApiPaths`/`Routes`, and
|
||||
* - the extraction yields something resolvable (a literal, an expression, or
|
||||
* an import binding), keeping the aggregate bounded on large repos.
|
||||
*
|
||||
|
|
@ -366,6 +366,12 @@ interface LanguageProviderConfig {
|
|||
* repos: files that cannot contribute (no constant-bearing syntax) are not
|
||||
* walked. Must be syntax-driven (field/import shape), not identifier
|
||||
* pattern-matching on class names.
|
||||
*
|
||||
* Default: undefined — harvest EVERY file of this language. A gate is opt-in
|
||||
* because getting it wrong silently drops routes that already resolve, and a
|
||||
* missed gate only costs time. Declare one only where the cost bites (Java's
|
||||
* Maven monorepos) and only after checking it against every shape
|
||||
* {@link extractModuleConstants} accepts.
|
||||
*/
|
||||
readonly moduleConstantHeuristic?: (content: string) => boolean;
|
||||
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ import { extractSpringRoutes, extractSpringTypes } from '../route-extractors/spr
|
|||
import {
|
||||
extractJavaModuleConstants,
|
||||
foldJavaOperands,
|
||||
isJavaConstantFile,
|
||||
} from '../route-extractors/java-const-resolver.js';
|
||||
import { javaExportChecker } from '../export-detection.js';
|
||||
import { createImportResolver } from '../import-resolvers/resolver-factory.js';
|
||||
|
|
@ -222,17 +223,24 @@ export const javaProvider = defineLanguage({
|
|||
extractRouteInheritanceTypes: extractSpringTypes,
|
||||
|
||||
// ── #2980: constant harvest + qualified-ref fold for non-literal mapping
|
||||
// paths (`@WinPostMapping(ApiPaths.SAVE_V1)`) — kept behind provider hooks so
|
||||
// paths (`@PostMapping(ApiPaths.SAVE_V1)`) — kept behind provider hooks so
|
||||
// the shared ingestion layers stay language-agnostic. The heuristic is
|
||||
// SYNTAX-driven (field/import shape), never a class-name pattern: constant
|
||||
// classes are routinely named `ApiPaths`/`Routes`/`Paths`, which a
|
||||
// `*Constants`-style gate would silently drop (review round-2 High finding).
|
||||
extractModuleConstants: extractJavaModuleConstants,
|
||||
// One gate, shared with the group side's `prepareRepo` pre-pass so the two
|
||||
// subsystems cannot disagree about which files define constants (see
|
||||
// JAVA_CONSTANT_FILE_RE — the previous divergence dropped constant
|
||||
// INTERFACES on this side only, which cost the graph its Route nodes while
|
||||
// the group still published the contract).
|
||||
moduleConstantHeuristic: (content) =>
|
||||
/\bstatic\s+final\s+String\s/.test(content) ||
|
||||
isJavaConstantFile(content) ||
|
||||
// `import com.winning.opt.common.ApiPaths;` — ANY class import can bind a
|
||||
// constant ref (`ApiPaths.X` at an annotation site), so gate on the
|
||||
// general import shape, not on the imported name.
|
||||
// general import shape, not on the imported name. Ingestion-only: this
|
||||
// side needs the importing controller's own import table, which the group
|
||||
// side instead derives lazily from the tree it already holds.
|
||||
/\bimport\s+(?:static\s+)?[\w.]+\s*;/.test(content),
|
||||
foldRoutePathOperands: foldJavaOperands,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -163,9 +163,13 @@ export const pythonProvider = defineLanguage({
|
|||
// ── #2391 constant harvest, provider-hook form (#2980): module-level string
|
||||
// constants + from-imports for non-literal decorator route paths. Bare-name
|
||||
// refs fold through the shared resolver (no foldRoutePathOperands needed).
|
||||
// No `moduleConstantHeuristic`: Python harvests unconditionally, exactly as
|
||||
// #2391 shipped it. A content gate was tried here and removed on review — it
|
||||
// required `NAME` immediately followed by `=`, so it silently dropped the two
|
||||
// idiomatic typed-FastAPI shapes (`API: str = "/api"`,
|
||||
// `API: Final[str] = "/api"`) and every composed constant whose RHS starts
|
||||
// with an identifier (`USERS = BASE + "/users"`), i.e. it REGRESSED routes
|
||||
// that already resolve on main. The worker treats a missing heuristic as
|
||||
// default-open; only Java opts into a gate, where the cost actually bites.
|
||||
extractModuleConstants: extractPythonModuleConstants,
|
||||
moduleConstantHeuristic: (content) =>
|
||||
// Module-level assignment (`X = "/path"`, possibly typed/annotated) or a
|
||||
// from-import that can bind a constant name at a decorator site.
|
||||
/^[^\S\n]*(?:[A-Za-z_]\w*\s*=\s*['"]|from\s+[\w.]+\s+import\s)/m.test(content),
|
||||
});
|
||||
|
|
|
|||
|
|
@ -33,7 +33,7 @@ const MAX_RESOLVE_DEPTH = 8;
|
|||
* whose true value is genuinely huge — building it risks a `RangeError`/heap OOM,
|
||||
* so we floor to `null` (skip) instead (#2393). The depth cap bounds recursion but
|
||||
* NOT output size, which grows multiplicatively; this bounds the output. */
|
||||
const MAX_FOLD_LENGTH = 8192;
|
||||
export const MAX_FOLD_LENGTH = 8192;
|
||||
|
||||
/**
|
||||
* One term of a constant's right-hand side. A `+`-concatenation
|
||||
|
|
|
|||
|
|
@ -19,10 +19,16 @@
|
|||
* }
|
||||
*
|
||||
* Reference shapes at annotation sites this binding resolves:
|
||||
* @WinPostMapping(ApiPathConstants.DIAGNOSIS_SAVE_V1) // qualified
|
||||
* @WinPostMapping(com.winning.opt.X.ApiPathConstants.Y) // FQN-qualified
|
||||
* @WinPostMapping(DIAGNOSIS_SAVE_V1) // static-imported
|
||||
* @WinPostMapping(API_CIS_V1 + "summary/save") // inline concat
|
||||
* @PostMapping(ApiPathConstants.DIAGNOSIS_SAVE_V1) // qualified
|
||||
* @PostMapping(com.winning.opt.X.ApiPathConstants.Y) // FQN-qualified
|
||||
* @PostMapping(DIAGNOSIS_SAVE_V1) // static-imported
|
||||
* @PostMapping(API_CIS_V1 + "summary/save") // inline concat
|
||||
*
|
||||
* Which ANNOTATIONS count as routes is a separate question this module has no
|
||||
* say in: `spring-shared.ts` holds an exact-name map, so a vendor alias like
|
||||
* `@WinPostMapping` yields no route on this base regardless of how its value
|
||||
* folds (#2883). Folding and alias recognition compose; neither implies the
|
||||
* other.
|
||||
*
|
||||
* Import shapes consumed:
|
||||
* import com.winning.opt.diagnosis.api.constants.ApiPathConstants;
|
||||
|
|
@ -35,8 +41,9 @@
|
|||
*/
|
||||
|
||||
import type Parser from 'tree-sitter';
|
||||
import { unquoteSpringLiteral } from './spring-shared.js';
|
||||
import {
|
||||
resolveConstant as foldConstant,
|
||||
MAX_FOLD_LENGTH,
|
||||
type ImportBinding,
|
||||
type ImportResolver,
|
||||
type ModuleConstants,
|
||||
|
|
@ -51,6 +58,43 @@ export type {
|
|||
RepoConstants,
|
||||
} from './constant-resolver.js';
|
||||
|
||||
/**
|
||||
* Cheap content gate: can this Java file DEFINE a string constant that a route
|
||||
* annotation might reference?
|
||||
*
|
||||
* Exported so BOTH sides of the pipeline use the same predicate and cannot
|
||||
* disagree about which files carry constants — the ingestion provider
|
||||
* (`languages/java.ts`, as `moduleConstantHeuristic`) and the group extractor's
|
||||
* `prepareRepo` pre-pass (`group/extractors/http-patterns/java.ts`). They used
|
||||
* to spell it differently, and the two spellings disagreed on a constant
|
||||
* INTERFACE: the group admitted it and published a provider contract at the
|
||||
* folded path, while ingestion rejected the file and emitted no Route node for
|
||||
* it — an R4 parity break in the losing direction, since ingestion is the side
|
||||
* that drives the graph and `api_impact`.
|
||||
*
|
||||
* Arms:
|
||||
* - `static final String` / `final static String` — Java lets modifiers appear
|
||||
* in any order, and `public final static String X = "/a";` is legal.
|
||||
* - any `interface` declaration — interface fields are implicitly
|
||||
* `public static final` (JLS 9.3), so a pure constant interface carries
|
||||
* neither keyword and no import. `@interface` (annotation type) matches too,
|
||||
* costing one parse that yields nothing.
|
||||
*/
|
||||
const EXPLICIT_STRING_CONSTANT_RE = /\b(?:static\s+final|final\s+static)\s+String\s/;
|
||||
const INTERFACE_DECL_RE = /\binterface\s+\w/;
|
||||
const STRING_ASSIGNMENT_RE = /\bString\s+\w+\s*=/;
|
||||
|
||||
export function isJavaConstantFile(source: string): boolean {
|
||||
if (EXPLICIT_STRING_CONSTANT_RE.test(source)) return true;
|
||||
// The interface arm is a bare word match, so on its own it admits any file
|
||||
// whose PROSE mentions "interface " — and every admitted file costs the group
|
||||
// side a full extra parse. Requiring a String assignment as well keeps every
|
||||
// shape `extractJavaModuleConstants` accepts in an interface body (bare
|
||||
// `String`, `java.lang.String`, no space before `=`, multi-declarator) while
|
||||
// dropping the comment-only matches.
|
||||
return INTERFACE_DECL_RE.test(source) && STRING_ASSIGNMENT_RE.test(source);
|
||||
}
|
||||
|
||||
/**
|
||||
* The Java {@link ImportResolver}: map a fully-qualified import specifier to
|
||||
* the unique file key it refers to, or null when it cannot be pinned to
|
||||
|
|
@ -61,14 +105,18 @@ export type {
|
|||
* file-path-keyed and Maven multi-module trees repeat package roots across
|
||||
* modules (`winning-opt-a/.../api/constants/ApiPathConstants.java` and
|
||||
* `winning-opt-b/.../api/constants/ApiPathConstants.java`), suffix matching
|
||||
* must stay UNIQUE-suffix: an import whose class name matches N files in N
|
||||
* different modules cannot be pinned by package alone — unless exactly one of
|
||||
* them ALSO matches the full package path. We therefore rank candidates:
|
||||
* 1. exact full-suffix match (`<pkg-path>/<Class>.java` as a path suffix)
|
||||
* 2. class-name-only suffix (`**/<Class>.java`) when exactly one exists
|
||||
* and return null when both attempts are ambiguous.
|
||||
* stays UNIQUE-suffix: an import whose full package+class path matches N files
|
||||
* in N different modules cannot be pinned, so it returns null — the skip floor
|
||||
* this module promises, never a wrong path.
|
||||
*
|
||||
* A nearest-shared-directory tie-break was tried here and removed on review:
|
||||
* javac resolves duplicate FQNs by CLASSPATH ORDER, not directory proximity, so
|
||||
* a `src/test` fixture copy or a module that merely sits 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.
|
||||
*/
|
||||
export const resolveJavaImport: ImportResolver = (importingFileKey, moduleSpec, repoKeys) => {
|
||||
export const resolveJavaImport: ImportResolver = (_importingFileKey, moduleSpec, repoKeys) => {
|
||||
// A static import `a.b.C.CONST` names the class as all-but-last segment;
|
||||
// a plain import `a.b.C` names the class as last segment. Both resolve to
|
||||
// a file ending `a/b/C.java`; treating the whole spec as a path and
|
||||
|
|
@ -76,61 +124,42 @@ export const resolveJavaImport: ImportResolver = (importingFileKey, moduleSpec,
|
|||
const asPath = moduleSpec.replace(/\./g, '/');
|
||||
const classFile = `${asPath}.java`;
|
||||
|
||||
// 1. Exact package-path suffix match.
|
||||
// Exact package-path suffix match, unique or nothing.
|
||||
let hit: string | null = null;
|
||||
let ambiguity = false;
|
||||
for (const key of repoKeys) {
|
||||
if (key === classFile || key.endsWith(`/${classFile}`)) {
|
||||
if (hit !== null) {
|
||||
ambiguity = true;
|
||||
break;
|
||||
}
|
||||
if (hit !== null) return null; // 2+ modules carry this FQN — unresolvable
|
||||
hit = key;
|
||||
}
|
||||
}
|
||||
if (!ambiguity) return hit;
|
||||
|
||||
// Ambiguous full-path match (same package+class in 2+ modules is legal in
|
||||
// separated-source monorepos but pathological for route constants). Try
|
||||
// disambiguating by proximity to the importing file: prefer the candidate
|
||||
// sharing the longest leading directory prefix with the importer. This
|
||||
// mirrors how Maven/Gradle resolve classpath collisions in practice (nearest
|
||||
// module wins) without ever guessing across unrelated trees.
|
||||
const candidates: string[] = [];
|
||||
for (const key of repoKeys) {
|
||||
if (key === classFile || key.endsWith(`/${classFile}`)) candidates.push(key);
|
||||
}
|
||||
if (candidates.length > 1) {
|
||||
const importerDirs = importingFileKey.split('/').slice(0, -1);
|
||||
let best: string | null = null;
|
||||
let bestDepth = -1;
|
||||
let tie = false;
|
||||
for (const c of candidates) {
|
||||
const cDirs = c.split('/');
|
||||
let d = 0;
|
||||
while (d < importerDirs.length && d < cDirs.length && importerDirs[d] === cDirs[d]) d++;
|
||||
if (d > bestDepth) {
|
||||
bestDepth = d;
|
||||
best = c;
|
||||
tie = false;
|
||||
} else if (d === bestDepth) {
|
||||
tie = true;
|
||||
}
|
||||
}
|
||||
if (best !== null && !tie) return best;
|
||||
}
|
||||
return null;
|
||||
return hit;
|
||||
};
|
||||
|
||||
/** Is `node` a Java string literal (`"..."`) with its unquoted value? */
|
||||
/**
|
||||
* Is `node` a Java string literal (`"..."`), and if so what value does the
|
||||
* route layer give it?
|
||||
*
|
||||
* tree-sitter-java splits a `string_literal` AROUND its `escape_sequence`
|
||||
* children, so joining `string_fragment`s alone silently DELETES every escape:
|
||||
* `"/user/{id:\\d+}"` — the standard Spring path-variable regex constraint —
|
||||
* folded to `/user/{id:d+}`, and a pure-escape literal (`"\\t"`) folded to the
|
||||
* empty string. Slicing the quotes off the raw text keeps the source spelling,
|
||||
* which is precisely what the LITERAL path does
|
||||
* ({@link unquoteSpringLiteral}) — so `@GetMapping(ApiPaths.USER_REGEX)` and
|
||||
* `@GetMapping("/user/{id:\\d+}")` now emit the same path for the same Java
|
||||
* source instead of two spellings the graph cannot reconcile. Same
|
||||
* `string_fragment`-join trap as the NestJS one in #3017.
|
||||
*/
|
||||
function stringLiteralValue(node: Parser.SyntaxNode): string | null {
|
||||
if (node.type !== 'string_literal') return null;
|
||||
const parts = node.children.filter((c) => c.type === 'string_fragment');
|
||||
if (parts.length === 0) {
|
||||
// Empty literal `""` has no string_fragment child.
|
||||
return '';
|
||||
}
|
||||
return parts.map((c) => c.text).join('');
|
||||
// A Java text block is also a `string_literal` here, and `unquoteSpringLiteral`
|
||||
// has a `"""` arm that would hand back the raw block — leading newline and
|
||||
// incidental indentation included, both of which Java strips. Nothing
|
||||
// downstream normalizes that, so it would publish a Route at a path like
|
||||
// "\n /api/v1/x\n ". The old fragment-join returned '' here, which
|
||||
// floored to skip; keep that floor rather than trade it for a wrong path.
|
||||
if (node.text.startsWith('"""')) return null;
|
||||
return unquoteSpringLiteral(node.text);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -276,7 +305,7 @@ export function extractJavaModuleConstants(tree: Parser.Tree): ModuleConstants {
|
|||
// Type must be String (java.lang.String is implicit-imported).
|
||||
const typeNode = member.childForFieldName('type');
|
||||
if (!typeNode) continue;
|
||||
const typeText = typeNode.text.replace(/^com\.java\.lang\./, '');
|
||||
const typeText = typeNode.text;
|
||||
if (typeText !== 'String' && typeText !== 'java.lang.String') continue;
|
||||
|
||||
const declarators = member.children.filter((c) => c.type === 'variable_declarator');
|
||||
|
|
@ -298,6 +327,23 @@ export function extractJavaModuleConstants(tree: Parser.Tree): ModuleConstants {
|
|||
if (operands === null) {
|
||||
literals.delete(name);
|
||||
exprs.delete(name);
|
||||
// …and the static IMPORT of the same simple name. A local
|
||||
// `static final String` shadows `import static a.b.C.PATH` inside
|
||||
// that class (JLS 6.4.1), so the correct answer for a non-foldable
|
||||
// rebind is "unresolvable" — leaving the import alive makes the fold
|
||||
// fall through it (computeFold: literals → exprs → imports) and
|
||||
// return the IMPORTED value, i.e. a wrong path where the skip floor
|
||||
// is owed. #2393's Python defect, reproduced for Java.
|
||||
//
|
||||
// The delete is file-scoped because these maps are (see the header:
|
||||
// nested types flatten into one file-level namespace). So a SIBLING
|
||||
// top-level class in the same file that legitimately uses the import
|
||||
// loses it too and floors to skip, where javac would resolve it.
|
||||
// That direction is the acceptable one — a missing route, not a wrong
|
||||
// one — and the shape (two top-level classes, one shadowing a static
|
||||
// import with a non-foldable initializer) is vanishingly rare next to
|
||||
// the wrong-value it prevents.
|
||||
imports.delete(name);
|
||||
if (qname) {
|
||||
literals.delete(qname);
|
||||
exprs.delete(qname);
|
||||
|
|
@ -331,28 +377,45 @@ export function extractJavaModuleConstants(tree: Parser.Tree): ModuleConstants {
|
|||
|
||||
const walkTypes = (node: Parser.SyntaxNode, insideInterface: boolean): void => {
|
||||
for (const child of node.children ?? []) {
|
||||
const isClass = child.type === 'class_declaration';
|
||||
const isInterface = child.type === 'interface_declaration';
|
||||
if (isClass || isInterface) {
|
||||
const nameNode = child.childForFieldName('name');
|
||||
const className = nameNode?.text ?? null;
|
||||
const body = child.children.find(
|
||||
(c) => c.type === 'class_body' || c.type === 'interface_body',
|
||||
);
|
||||
// Recompute implicit interface semantics at each type boundary: a
|
||||
// class nested in an interface is a normal class whose fields need
|
||||
// explicit `static final` (JLS 9.5 — only the interface's own fields
|
||||
// are implicitly public static final). Propagating the outer
|
||||
// `insideInterface` flag in would harvest mutable nested fields as
|
||||
// constants and let a same-name nested field shadow a real interface
|
||||
// constant with a stale value.
|
||||
if (body && className) collectFieldConstants(body, isInterface, className);
|
||||
if (body) walkTypes(body, isInterface);
|
||||
} else if (child.type === 'enum_declaration' || child.type === 'record_declaration') {
|
||||
walkTypes(child, insideInterface);
|
||||
} else {
|
||||
// Enums and records are ordinary type declarations for constant
|
||||
// purposes — their fields need an explicit `static final` (JLS 8.9/8.10),
|
||||
// unlike an interface's implicitly-constant ones. They used to be only
|
||||
// RECURSED into, never collected, so a `static final String` declared
|
||||
// directly in an enum or record was silently absent from the map.
|
||||
const isTypeDecl =
|
||||
isInterface ||
|
||||
child.type === 'class_declaration' ||
|
||||
child.type === 'enum_declaration' ||
|
||||
child.type === 'record_declaration';
|
||||
if (!isTypeDecl) {
|
||||
walkTypes(child, insideInterface);
|
||||
continue;
|
||||
}
|
||||
const className = child.childForFieldName('name')?.text ?? null;
|
||||
const body = child.children.find(
|
||||
(c) => c.type === 'class_body' || c.type === 'interface_body' || c.type === 'enum_body',
|
||||
);
|
||||
if (!body) continue;
|
||||
// An enum's members hang one level deeper, under `enum_body_declarations`
|
||||
// (the `enum_body` itself holds only the enum constants).
|
||||
const memberBody = body.children.find((c) => c.type === 'enum_body_declarations') ?? body;
|
||||
// Recompute implicit interface semantics at each type boundary: a
|
||||
// class nested in an interface is a normal class whose fields need
|
||||
// explicit `static final` (JLS 9.5 — only the interface's own fields
|
||||
// are implicitly public static final). Propagating the outer
|
||||
// `insideInterface` flag in would harvest mutable nested fields as
|
||||
// constants and let a same-name nested field shadow a real interface
|
||||
// constant with a stale value.
|
||||
if (className) collectFieldConstants(memberBody, isInterface, className);
|
||||
// Recurse over the WHOLE body, not just `memberBody`: an enum's constants
|
||||
// are siblings of `enum_body_declarations`, so narrowing here dropped any
|
||||
// type nested inside an enum-constant body whenever the enum also had
|
||||
// member declarations. For a class/interface/record the two are the same
|
||||
// node; for an enum `body` is a strict superset, and the extra visit to
|
||||
// `enum_body_declarations` collects nothing twice (collectFieldConstants
|
||||
// is still called on `memberBody` alone).
|
||||
walkTypes(body, isInterface);
|
||||
}
|
||||
};
|
||||
walkTypes(tree.rootNode, false);
|
||||
|
|
@ -360,6 +423,43 @@ export function extractJavaModuleConstants(tree: Parser.Tree): ModuleConstants {
|
|||
return { literals, exprs, imports: imports as Map<string, ImportBinding> };
|
||||
}
|
||||
|
||||
/**
|
||||
* Per-fold state. Mirrors the guards the agnostic core carries in `foldName`,
|
||||
* which this binding stopped delegating to once it had to resolve qualified
|
||||
* operands itself:
|
||||
*
|
||||
* - `memo` caches SUCCESSES only and is never popped. Without it a
|
||||
* shared-descendant DAG (`X_k = X_{k+1} + X_{k+1}`) re-folds each child once
|
||||
* per reference — O(2^depth) — and {@link MAX_FOLD_LENGTH} cannot save it,
|
||||
* because a chain whose intermediate values are the empty string never
|
||||
* accumulates any output. Measured before this state existed: one route over
|
||||
* a 31-line constants file took 2.7 s at 26 levels and 11 s at 28, on the
|
||||
* main thread, per file. A `null` may be transient (a name that cycles on one
|
||||
* branch can resolve on another), so caching it would be unsound.
|
||||
* - `visited` is the ACTIVE resolution stack, popped on unwind, so diamonds
|
||||
* fold instead of false-cycling while true cycles still terminate.
|
||||
* - `constantKeys` is the candidate set import ambiguity is measured over:
|
||||
* files that actually DEFINE a constant. Handing `resolveJavaImport` every
|
||||
* repo key made the two subsystems disagree — ingestion's map also holds
|
||||
* import-only files (its gate has an import arm), so a duplicate FQN that
|
||||
* defines nothing was invisible to the group and made ingestion alone floor
|
||||
* to skip. Hoisting it also stops rebuilding the set on every qualified ref.
|
||||
*/
|
||||
interface JavaFoldState {
|
||||
readonly repo: RepoConstants;
|
||||
readonly constantKeys: ReadonlySet<string>;
|
||||
readonly visited: Set<string>;
|
||||
readonly memo: Map<string, string>;
|
||||
}
|
||||
|
||||
function newFoldState(repo: RepoConstants): JavaFoldState {
|
||||
const constantKeys = new Set<string>();
|
||||
for (const [key, mc] of repo) {
|
||||
if (mc.literals.size > 0 || mc.exprs.size > 0) constantKeys.add(key);
|
||||
}
|
||||
return { repo, constantKeys, visited: new Set(), memo: new Map() };
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a single Java constant referenced in `fileKey` to its literal string
|
||||
* value, folding `+` concatenation and following import chains via
|
||||
|
|
@ -375,28 +475,54 @@ export function resolveJavaConstant(
|
|||
repo: RepoConstants,
|
||||
depth = 0,
|
||||
): string | null {
|
||||
// Cycle guard for the Java-qualified walk (self/mutual imports). The shared
|
||||
// fold's visited-stack only guards the bare-name path below; qualified refs
|
||||
// recurse through resolveJavaConstant directly, so bound the recursion here.
|
||||
return resolveWithState(fileKey, name, newFoldState(repo), depth);
|
||||
}
|
||||
|
||||
function resolveWithState(
|
||||
fileKey: string,
|
||||
name: string,
|
||||
state: JavaFoldState,
|
||||
depth: number,
|
||||
): string | null {
|
||||
if (depth > 32) return null;
|
||||
// Qualified ref (`ApiPathConstants.FIELD`): the fold layer keys imports and
|
||||
// constants by their IN-FILE name, so a dotted name never hits directly.
|
||||
// Split head.tail: resolve the head through the importing file's class
|
||||
// import, then look the tail up in the target file — first as the
|
||||
// class-qualified alias `Head.TAIL` (what extractJavaModuleConstants
|
||||
// records), then as a bare `TAIL` (same-file nested/interface constant).
|
||||
const guard = `${fileKey}::${name}`;
|
||||
const memoized = state.memo.get(guard);
|
||||
if (memoized !== undefined) return memoized;
|
||||
if (state.visited.has(guard)) return null; // cycle: `name` is on the active stack
|
||||
state.visited.add(guard);
|
||||
try {
|
||||
const result = computeJavaFold(fileKey, name, state, depth);
|
||||
if (result !== null) state.memo.set(guard, result);
|
||||
return result;
|
||||
} finally {
|
||||
state.visited.delete(guard);
|
||||
}
|
||||
}
|
||||
|
||||
function computeJavaFold(
|
||||
fileKey: string,
|
||||
name: string,
|
||||
state: JavaFoldState,
|
||||
depth: number,
|
||||
): string | null {
|
||||
const { repo, constantKeys } = state;
|
||||
// Qualified ref (`ApiPathConstants.FIELD`): constants and imports are keyed by
|
||||
// their IN-FILE name, so a dotted name never hits directly. Split head.tail:
|
||||
// resolve the head through the importing file's class import, then look the
|
||||
// tail up in the target file — first as the class-qualified alias `Head.TAIL`
|
||||
// (what extractJavaModuleConstants records), then as a bare `TAIL` (same-file
|
||||
// nested/interface constant).
|
||||
const dot = name.indexOf('.');
|
||||
if (dot > 0) {
|
||||
const head = name.slice(0, dot);
|
||||
const tail = name.slice(dot + 1);
|
||||
const importing = repo.get(fileKey);
|
||||
const imp = importing?.imports.get(head);
|
||||
const imp = repo.get(fileKey)?.imports.get(head);
|
||||
if (imp) {
|
||||
const targetFile = resolveJavaImport(fileKey, imp.module, new Set(repo.keys()));
|
||||
const targetFile = resolveJavaImport(fileKey, imp.module, constantKeys);
|
||||
if (targetFile !== null) {
|
||||
const qualified = resolveJavaConstant(targetFile, `${head}.${tail}`, repo, depth + 1);
|
||||
const qualified = resolveWithState(targetFile, `${head}.${tail}`, state, depth + 1);
|
||||
if (qualified !== null) return qualified;
|
||||
const bare = resolveJavaConstant(targetFile, tail, repo, depth + 1);
|
||||
const bare = resolveWithState(targetFile, tail, state, depth + 1);
|
||||
if (bare !== null) return bare;
|
||||
}
|
||||
return null;
|
||||
|
|
@ -406,41 +532,83 @@ export function resolveJavaConstant(
|
|||
const parts = name.split('.');
|
||||
for (let cut = parts.length - 2; cut >= 1; cut--) {
|
||||
const fqn = parts.slice(0, cut + 1).join('.');
|
||||
const targetFile = resolveJavaImport(fileKey, fqn, new Set(repo.keys()));
|
||||
const targetFile = resolveJavaImport(fileKey, fqn, constantKeys);
|
||||
if (targetFile !== null) {
|
||||
const field = parts.slice(cut + 1).join('.');
|
||||
const declaring = parts[cut];
|
||||
const qualified = resolveJavaConstant(targetFile, `${declaring}.${field}`, repo, depth + 1);
|
||||
const qualified = resolveWithState(targetFile, `${declaring}.${field}`, state, depth + 1);
|
||||
if (qualified !== null) return qualified;
|
||||
return resolveJavaConstant(targetFile, field, repo, depth + 1);
|
||||
return resolveWithState(targetFile, field, state, depth + 1);
|
||||
}
|
||||
}
|
||||
// No import bound the head and no FQN prefix resolved — fall through. A
|
||||
// dotted name is ALSO a valid key in this file's own maps:
|
||||
// `extractJavaModuleConstants` records every constant under
|
||||
// `<DeclaringClass>.<FIELD>` as well as its simple name, so a same-file
|
||||
// qualified reference (`ApiPaths.X` inside ApiPaths.java) resolves below.
|
||||
}
|
||||
return foldConstant(fileKey, name, repo, resolveJavaImport);
|
||||
|
||||
// Name lookup: literals, then same-file expressions, then the import chase.
|
||||
// Reached for a bare name and for a dotted name that named no import.
|
||||
// Expressions are folded HERE rather than handed to the agnostic core because
|
||||
// an operand of a Java initializer may itself be a QUALIFIED ref
|
||||
// (`X = BConsts.Y + "/tail"`) and the core only knows bare names: it looks
|
||||
// `BConsts.Y` up in maps keyed by simple name, misses, and floors the whole
|
||||
// chain to null. Recursing through this function gives every operand the same
|
||||
// qualified treatment the entry-point name got.
|
||||
const mc = repo.get(fileKey);
|
||||
if (!mc) return null;
|
||||
const literal = mc.literals.get(name);
|
||||
if (literal !== undefined) return literal;
|
||||
const expr = mc.exprs.get(name);
|
||||
if (expr !== undefined) return foldOperands(fileKey, expr, state, depth + 1);
|
||||
const imp = mc.imports.get(name);
|
||||
if (imp !== undefined) {
|
||||
const targetFile = resolveJavaImport(fileKey, imp.module, constantKeys);
|
||||
if (targetFile === null) return null;
|
||||
return resolveWithState(targetFile, imp.originalName, state, depth + 1);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Concatenate an operand list, resolving each `ref` through the qualified-aware
|
||||
* walk so `Class.CONST` works at every position, not just at the entry point.
|
||||
*
|
||||
* Bounded by {@link MAX_FOLD_LENGTH}: the depth cap bounds RECURSION but not
|
||||
* OUTPUT, which grows multiplicatively (`X = A + A; A = B + B; …`), so a
|
||||
* pathological chain would build a gigabyte-scale string before any cap fired.
|
||||
* Overrun floors to null (#2393).
|
||||
*/
|
||||
function foldOperands(
|
||||
fileKey: string,
|
||||
operands: readonly Operand[],
|
||||
state: JavaFoldState,
|
||||
depth: number,
|
||||
): string | null {
|
||||
let out = '';
|
||||
for (const op of operands) {
|
||||
if (op.kind === 'literal') {
|
||||
out += op.value;
|
||||
} else {
|
||||
const piece = resolveWithState(fileKey, op.name, state, depth);
|
||||
if (piece === null) return null;
|
||||
out += piece;
|
||||
}
|
||||
if (out.length > MAX_FOLD_LENGTH) return null;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fold an inline operand list (e.g. `API_CIS_V1 + "summary/save"`) against
|
||||
* `fileKey`. Unlike the Python binding, refs are resolved through
|
||||
* {@link resolveJavaConstant} first — the agnostic fold has no notion of
|
||||
* Java's `Class.CONST` qualified names (its import indirection only covers
|
||||
* bare names), so each `ref` operand is resolved individually and the pieces
|
||||
* are concatenated here.
|
||||
* `fileKey`, or null when any piece is unresolvable (skip floor).
|
||||
*/
|
||||
export function foldJavaOperands(
|
||||
fileKey: string,
|
||||
operands: readonly Operand[],
|
||||
repo: RepoConstants,
|
||||
): string | null {
|
||||
let out = '';
|
||||
for (const op of operands) {
|
||||
if (op.kind === 'literal') {
|
||||
out += op.value;
|
||||
continue;
|
||||
}
|
||||
const piece = resolveJavaConstant(fileKey, op.name, repo);
|
||||
if (piece === null) return null;
|
||||
out += piece;
|
||||
}
|
||||
const out = foldOperands(fileKey, operands, newFoldState(repo), 0);
|
||||
return out === '' ? null : out;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -54,6 +54,13 @@ import { parseJavaConstOperands } from './java-const-resolver.js';
|
|||
* suppresses that class's method-level array routes rather than emit them with a
|
||||
* dropped prefix (a wrong route). Full class-array cross-product support is left
|
||||
* to a follow-up (#2280).
|
||||
*
|
||||
* The class-level `@value_expr` branches exist for the same reason: a
|
||||
* CONSTANT-valued class prefix (`@RequestMapping(ApiPaths.BASE)`) cannot be
|
||||
* folded here — the repo-wide constant map only exists in the parse phase — so
|
||||
* they only DETECT it, and Phase 2 suppresses every method route under such a
|
||||
* class. Without them the prefix was invisible and the method route was emitted
|
||||
* unprefixed, i.e. at a path the application does not serve.
|
||||
*/
|
||||
const ROUTE_ANNOTATION_QUERY = new Parser.Query(
|
||||
Java,
|
||||
|
|
@ -91,6 +98,24 @@ const ROUTE_ANNOTATION_QUERY = new Parser.Query(
|
|||
key: (identifier) @key
|
||||
value: [(string_literal) @value
|
||||
(element_value_array_initializer (string_literal) @value)]))))) @node
|
||||
(class_declaration
|
||||
(modifiers
|
||||
(annotation
|
||||
name: [(identifier) (scoped_identifier)] @ann
|
||||
arguments: (annotation_argument_list
|
||||
[(identifier) @value_expr
|
||||
(field_access) @value_expr
|
||||
(binary_expression) @value_expr])))) @node
|
||||
(class_declaration
|
||||
(modifiers
|
||||
(annotation
|
||||
name: [(identifier) (scoped_identifier)] @ann
|
||||
arguments: (annotation_argument_list
|
||||
(element_value_pair
|
||||
key: (identifier) @key
|
||||
value: [(identifier) @value_expr
|
||||
(field_access) @value_expr
|
||||
(binary_expression) @value_expr]))))) @node
|
||||
(method_declaration
|
||||
(modifiers
|
||||
(annotation
|
||||
|
|
@ -141,6 +166,11 @@ export function extractSpringRoutes(
|
|||
// class-array cross-product support is out of scope here.
|
||||
const prefixByClassId = new Map<number, string>();
|
||||
const classesWithArrayPrefix = new Set<number>();
|
||||
// Classes whose `@RequestMapping` prefix is a constant reference or concat.
|
||||
// Same treatment as the array form, for the same reason: no single prefix
|
||||
// string is knowable at extraction time, so emitting the methods below would
|
||||
// publish them at a WRONG (unprefixed) path rather than not at all.
|
||||
const classesWithUnfoldablePrefix = new Set<number>();
|
||||
const classHttpMethodsById = new Map<number, readonly string[]>();
|
||||
for (const match of TYPE_DECLARATION_QUERY.matches(tree.rootNode)) {
|
||||
const typeNode = match.captures.find((capture) => capture.name === 'type')?.node;
|
||||
|
|
@ -158,11 +188,16 @@ export function extractSpringRoutes(
|
|||
const node = caps['node'];
|
||||
const valueNode = caps['value'];
|
||||
const keyNode = caps['key'];
|
||||
if (!annNode || !node || !valueNode) continue;
|
||||
const valueExprNode = caps['value_expr'];
|
||||
if (!annNode || !node || (!valueNode && !valueExprNode)) continue;
|
||||
|
||||
const capturedAnnotationName = annNode.text.split('.').pop() ?? annNode.text;
|
||||
if (node.type === 'class_declaration' && capturedAnnotationName === 'RequestMapping') {
|
||||
if (!isRouteMemberKey(keyNode)) continue;
|
||||
if (!valueNode) {
|
||||
classesWithUnfoldablePrefix.add(node.id);
|
||||
continue;
|
||||
}
|
||||
if (valueNode.parent?.type === 'element_value_array_initializer') {
|
||||
classesWithArrayPrefix.add(node.id);
|
||||
continue;
|
||||
|
|
@ -237,6 +272,16 @@ export function extractSpringRoutes(
|
|||
if (isArrayElement && enclosingClass && classesWithArrayPrefix.has(enclosingClass.id)) {
|
||||
continue;
|
||||
}
|
||||
// Same rule for a CONSTANT-valued class prefix (`@RequestMapping(ApiPaths.BASE)`),
|
||||
// and for every method route under it — not just array-form ones. The prefix
|
||||
// needs the repo-wide constant map, which does not exist at extraction time,
|
||||
// so the prefix would simply be dropped and the route emitted at a path the
|
||||
// application never serves. On base such a route was not emitted at all;
|
||||
// turning a missing fact into a wrong one is the failure this module's skip
|
||||
// floor exists to prevent. Folding class prefixes cross-file is a follow-up.
|
||||
if (enclosingClass && classesWithUnfoldablePrefix.has(enclosingClass.id)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const classPrefix = enclosingClass ? (prefixByClassId.get(enclosingClass.id) ?? '') : '';
|
||||
// `node` is the annotated `method_declaration`; its name field is the
|
||||
|
|
@ -279,6 +324,13 @@ export function extractSpringRoutes(
|
|||
for (const match of TYPE_DECLARATION_QUERY.matches(tree.rootNode)) {
|
||||
const typeNode = match.captures.find((capture) => capture.name === 'type')?.node;
|
||||
if (typeNode?.type !== 'class_declaration') continue;
|
||||
// A no-argument `@GetMapping` IS the class prefix, so a class prefix that
|
||||
// cannot be folded here leaves nothing to emit — the route would ship with
|
||||
// `routePath: ''` and no prefix, i.e. an empty-path Route. The Phase 2 loop
|
||||
// above already suppresses these classes; this loop needs the same guard, or
|
||||
// the suppression is one-sided and the group side (which routes both shapes
|
||||
// through `methodRoutes`) disagrees with ingestion.
|
||||
if (classesWithUnfoldablePrefix.has(typeNode.id)) continue;
|
||||
const classPrefix = prefixByClassId.get(typeNode.id) ?? '';
|
||||
const classMethods = classHttpMethodsById.get(typeNode.id) ?? ['*'];
|
||||
for (const methodNode of directMethods(typeNode)) {
|
||||
|
|
|
|||
|
|
@ -2969,7 +2969,12 @@ const processFileGroup = (
|
|||
// paths cross-file. Cost-gated by the provider's syntax-driven heuristic;
|
||||
// only files that carry something resolvable (a constant definition or an
|
||||
// import binding) are emitted, keeping the aggregate bounded on large repos.
|
||||
if (provider.extractModuleConstants && provider.moduleConstantHeuristic?.(parseContent)) {
|
||||
// A provider that declares no heuristic harvests unconditionally (`?.`
|
||||
// would have read `undefined` as "skip" and disabled the hook outright).
|
||||
if (
|
||||
provider.extractModuleConstants &&
|
||||
(!provider.moduleConstantHeuristic || provider.moduleConstantHeuristic(parseContent))
|
||||
) {
|
||||
const constants = provider.extractModuleConstants(tree);
|
||||
if (constants.literals.size > 0 || constants.exprs.size > 0 || constants.imports.size > 0) {
|
||||
(result.moduleConstants ??= []).push({ filePath: file.path, constants });
|
||||
|
|
|
|||
|
|
@ -539,14 +539,37 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j
|
|||
// then adds Spring non-HTTP handler side-channel facts (#2417 / #2891), so Java
|
||||
// and Kotlin caches persist scheduled, event, messaging, and managed-job facts.
|
||||
//
|
||||
// This PR originally carried its own bump (47 -> 48) for the Java constant-route
|
||||
// capture set change (route-extractors/java-const-resolver.ts + the spring.ts
|
||||
// operand branch + the parse-worker Java constant harvest): a warm pre-feature
|
||||
// cache replays `moduleConstants=0` captures verbatim and silently drops every
|
||||
// constant-based Spring route on unchanged files. After rebasing onto current
|
||||
// main the ledger already sits at 70, whose capture set post-dates and includes
|
||||
// this harvest — v70 invalidates those caches, so no additional bump is needed.
|
||||
const SCHEMA_BUMP = 70;
|
||||
// 70 -> 71 adds the Java constant-route capture set (#2980):
|
||||
// `route-extractors/java-const-resolver.ts`, the `spring.ts` operand branch,
|
||||
// and the parse-worker's provider-driven constant harvest. A warm pre-feature
|
||||
// cache replays those files' worker results with `moduleConstants` absent and
|
||||
// `routePathOperands` unset, so every constant-based Spring route on an
|
||||
// unchanged file is silently dropped — the feature is inert until something
|
||||
// else invalidates the cache.
|
||||
//
|
||||
// This branch briefly reasoned that no bump was needed because the ledger
|
||||
// "already sits at 70, whose capture set post-dates and includes this harvest".
|
||||
// It does not: v70 was cut by fe3d7e56b for Spring non-HTTP handler facts
|
||||
// (#2417 / #2891), an ancestor of this PR's base, and it cannot include a
|
||||
// harvest that does not exist on main. Because
|
||||
// `PARSE_CACHE_VERSION = ${SCHEMA_BUMP}+${GITNEXUS_PKG_VERSION}` and
|
||||
// package.json is untouched here, leaving 70 makes the key BYTE-IDENTICAL
|
||||
// before and after this merge — precisely the inert-feature trap the v33/v34
|
||||
// notes above warn about. Exposure is bounded by the package version (a
|
||||
// released upgrade invalidates anyway), but same-version warm caches — dev
|
||||
// builds, CI caches, anyone who indexed with an unreleased build — replay the
|
||||
// stale captures.
|
||||
//
|
||||
// 72, not 71: open PR #3017 (`fix/nest-decorator-routes`, NestJS decorator route
|
||||
// indexing) already claims 71, with an identical pin test. Re-checking
|
||||
// origin/main alone would not catch that — main is 70 and stays 70 until one of
|
||||
// the two merges, at which point the second lands a byte-identical
|
||||
// PARSE_CACHE_VERSION and is inert. This is exactly the rule the ledger states
|
||||
// and the v37/v38 clash it was written for: the next free value above every
|
||||
// IN-FLIGHT claim, not above origin/main. Every open PR touching gitnexus/ was
|
||||
// scanned; #3017 is the only other claimant.
|
||||
// RE-CHECK AGAINST origin/main AND OPEN PRs IMMEDIATELY BEFORE MERGING.
|
||||
const SCHEMA_BUMP = 72;
|
||||
const GITNEXUS_PKG_VERSION = (() => {
|
||||
try {
|
||||
// package.json sits at gitnexus/package.json — two levels up from
|
||||
|
|
|
|||
213
gitnexus/test/unit/group/java-const-route-parity.test.ts
Normal file
213
gitnexus/test/unit/group/java-const-route-parity.test.ts
Normal file
|
|
@ -0,0 +1,213 @@
|
|||
/**
|
||||
* Group ↔ ingestion parity for Java constant-valued Spring route paths (#2980).
|
||||
*
|
||||
* Drives `JAVA_HTTP_PLUGIN.prepareRepo` + `scan(tree, ctx, rel)` with all three
|
||||
* arguments and compares the result against what `extractSpringRoutes` + the
|
||||
* Java operand fold produce on the ingestion side. The existing Spring parity
|
||||
* guards call `scan(tree)` with ONE argument, which makes them structurally
|
||||
* blind here: without a repo context the plugin drops every constant-valued
|
||||
* route, so no fixture they carry can exercise this feature.
|
||||
*
|
||||
* Asserted:
|
||||
* • a constant-valued mapping resolves to the SAME path on both sides;
|
||||
* • a CONSTANT class prefix suppresses the method route on both sides — the
|
||||
* prefix cannot be folded at extraction time, and emitting the route
|
||||
* unprefixed would publish a path the application does not serve;
|
||||
* • without a repo context the group side emits nothing (the documented skip
|
||||
* floor, and the branch that makes the 1-arg guards blind);
|
||||
* • literal routes are untouched.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import Parser from 'tree-sitter';
|
||||
import Java from 'tree-sitter-java';
|
||||
import { JAVA_HTTP_PLUGIN } from '../../../src/core/group/extractors/http-patterns/java.js';
|
||||
import type { HttpDetection } from '../../../src/core/group/extractors/http-patterns/types.js';
|
||||
import { extractSpringRoutes } from '../../../src/core/ingestion/route-extractors/spring.js';
|
||||
import { javaProvider } from '../../../src/core/ingestion/languages/java.js';
|
||||
import {
|
||||
extractJavaModuleConstants,
|
||||
foldJavaOperands,
|
||||
type RepoConstants,
|
||||
} from '../../../src/core/ingestion/route-extractors/java-const-resolver.js';
|
||||
|
||||
const parser = new Parser();
|
||||
const parseSource = (p: Parser, src: string): Parser.Tree => {
|
||||
p.setLanguage(Java);
|
||||
return p.parse(src);
|
||||
};
|
||||
const parse = (src: string): Parser.Tree => parseSource(parser, src);
|
||||
|
||||
/** Group side: prepareRepo + a 3-argument scan over every .java file. */
|
||||
function groupProviders(files: Record<string, string>): string[] {
|
||||
const ctx = JAVA_HTTP_PLUGIN.prepareRepo?.({
|
||||
files: Object.keys(files),
|
||||
parser: new Parser(),
|
||||
readFile: (rel: string) => files[rel] ?? null,
|
||||
parseSource,
|
||||
});
|
||||
const out: string[] = [];
|
||||
for (const rel of Object.keys(files)) {
|
||||
const detections: HttpDetection[] = JAVA_HTTP_PLUGIN.scan(parse(files[rel]), ctx, rel);
|
||||
for (const d of detections) {
|
||||
if (d.role === 'provider') out.push(`${d.method} ${d.path}`);
|
||||
}
|
||||
}
|
||||
return out.sort();
|
||||
}
|
||||
|
||||
/** Ingestion side: extract routes, then fold operands against the same map. */
|
||||
function ingestionRoutes(files: Record<string, string>): string[] {
|
||||
const repo: RepoConstants = new Map();
|
||||
for (const [rel, src] of Object.entries(files)) {
|
||||
repo.set(rel, extractJavaModuleConstants(parse(src)));
|
||||
}
|
||||
const out: string[] = [];
|
||||
for (const [rel, src] of Object.entries(files)) {
|
||||
for (const route of extractSpringRoutes(parse(src), rel, 0)) {
|
||||
const path = route.routePathOperands
|
||||
? foldJavaOperands(rel, route.routePathOperands, repo)
|
||||
: route.routePath;
|
||||
if (path === null) continue;
|
||||
out.push(`${route.httpMethod} ${`${route.prefix ?? ''}${path}`.replace(/\/{2,}/g, '/')}`);
|
||||
}
|
||||
}
|
||||
return out.sort();
|
||||
}
|
||||
|
||||
const CONSTS = 'src/main/java/com/example/ApiPaths.java';
|
||||
const CTL = 'src/main/java/com/example/OrderController.java';
|
||||
|
||||
const CONSTS_SRC = `package com.example;
|
||||
public class ApiPaths {
|
||||
public static final String BASE = "/api/v1";
|
||||
public static final String ORDERS = "/api/v1/orders";
|
||||
}`;
|
||||
|
||||
describe('Java constant-valued routes: group ↔ ingestion parity (#2980)', () => {
|
||||
it('resolves a constant-valued mapping to the same path on both sides', () => {
|
||||
const files = {
|
||||
[CONSTS]: CONSTS_SRC,
|
||||
[CTL]: `package com.example;
|
||||
import com.example.ApiPaths;
|
||||
public class OrderController {
|
||||
@GetMapping(ApiPaths.ORDERS)
|
||||
public void list() {}
|
||||
}`,
|
||||
};
|
||||
expect(groupProviders(files)).toEqual(['GET /api/v1/orders']);
|
||||
expect(ingestionRoutes(files)).toEqual(groupProviders(files));
|
||||
});
|
||||
|
||||
it('suppresses the method route under a CONSTANT class prefix on both sides', () => {
|
||||
// The class prefix needs the repo-wide constant map, which does not exist
|
||||
// at extraction time on either side. Emitting the method route would drop
|
||||
// the prefix and publish `GET /api/v1/orders`-without-its-base — a path the
|
||||
// application never serves. On base such a route was not emitted at all, so
|
||||
// shipping it unprefixed would turn a missing fact into a wrong one.
|
||||
const files = {
|
||||
[CONSTS]: CONSTS_SRC,
|
||||
[CTL]: `package com.example;
|
||||
import com.example.ApiPaths;
|
||||
@RequestMapping(ApiPaths.BASE)
|
||||
public class OrderController {
|
||||
@GetMapping(ApiPaths.ORDERS)
|
||||
public void list() {}
|
||||
|
||||
@GetMapping("/literal")
|
||||
public void literal() {}
|
||||
}`,
|
||||
};
|
||||
expect(groupProviders(files)).toEqual([]);
|
||||
expect(ingestionRoutes(files)).toEqual([]);
|
||||
});
|
||||
|
||||
it('still applies a LITERAL class prefix', () => {
|
||||
const files = {
|
||||
[CONSTS]: CONSTS_SRC,
|
||||
[CTL]: `package com.example;
|
||||
import com.example.ApiPaths;
|
||||
@RequestMapping("/api/v1")
|
||||
public class OrderController {
|
||||
@GetMapping("/orders")
|
||||
public void list() {}
|
||||
}`,
|
||||
};
|
||||
expect(groupProviders(files)).toEqual(['GET /api/v1/orders']);
|
||||
expect(ingestionRoutes(files)).toEqual(['GET /api/v1/orders']);
|
||||
});
|
||||
|
||||
it('emits nothing for a constant route when scanned without a repo context', () => {
|
||||
// This is the branch that makes the 1-argument parity guards blind to the
|
||||
// whole feature; pin it so it is not silently dead in the suite.
|
||||
const src = `package com.example;
|
||||
import com.example.ApiPaths;
|
||||
public class OrderController {
|
||||
@GetMapping(ApiPaths.ORDERS)
|
||||
public void list() {}
|
||||
}`;
|
||||
const detections = JAVA_HTTP_PLUGIN.scan(parse(src));
|
||||
expect(detections.filter((d) => d.role === 'provider')).toEqual([]);
|
||||
});
|
||||
|
||||
it('suppresses a NO-ARGUMENT mapping under a constant class prefix on both sides', () => {
|
||||
// A bare `@GetMapping` IS the class prefix, so an unfoldable class prefix
|
||||
// leaves nothing to emit. Ingestion routes these through a separate loop
|
||||
// from the path-carrying ones, and that loop needs the same guard — without
|
||||
// it ingestion emitted an empty-path Route where the group emitted nothing.
|
||||
const files = {
|
||||
[CONSTS]: CONSTS_SRC,
|
||||
[CTL]: `package com.example;
|
||||
import com.example.ApiPaths;
|
||||
@RequestMapping(ApiPaths.BASE)
|
||||
public class OrderController {
|
||||
@GetMapping public void list() {}
|
||||
@PostMapping public void create() {}
|
||||
}`,
|
||||
};
|
||||
expect(ingestionRoutes(files)).toEqual([]);
|
||||
expect(groupProviders(files)).toEqual([]);
|
||||
});
|
||||
|
||||
it('measures import ambiguity over the same candidate set on both sides', () => {
|
||||
// Ingestion's harvest gate also admits import-only files, so its repo map is
|
||||
// a superset of the group's. When ambiguity was measured over every key, a
|
||||
// duplicate FQN belonging to a class that defines NOTHING was invisible to
|
||||
// the group and made ingestion alone floor to skip — reopening the very
|
||||
// parity break this feature exists to close. Both sides now measure over
|
||||
// constant-DEFINING files only.
|
||||
const files = {
|
||||
'svc-a/src/main/java/com/x/ApiPaths.java': `package com.x;
|
||||
public class ApiPaths { public static final String ORDERS = "/api/v1/orders"; }`,
|
||||
// Same FQN, different module, defines no constant — must not create ambiguity.
|
||||
'svc-b/src/main/java/com/x/ApiPaths.java': `package com.x;
|
||||
import java.util.List;
|
||||
public class ApiPaths {}`,
|
||||
'svc-a/src/main/java/com/x/web/OrderController.java': `package com.x.web;
|
||||
import com.x.ApiPaths;
|
||||
public class OrderController {
|
||||
@GetMapping(ApiPaths.ORDERS)
|
||||
public void list() {}
|
||||
}`,
|
||||
};
|
||||
// Guard the premise: the two maps really are different sizes.
|
||||
const ingestionKeys = Object.entries(files).filter(([, src]) =>
|
||||
javaProvider.moduleConstantHeuristic?.(src),
|
||||
).length;
|
||||
expect(ingestionKeys).toBe(3);
|
||||
expect(groupProviders(files)).toEqual(['GET /api/v1/orders']);
|
||||
expect(ingestionRoutes(files)).toEqual(['GET /api/v1/orders']);
|
||||
});
|
||||
|
||||
it('leaves literal routes unchanged with no constant map at all', () => {
|
||||
const files = {
|
||||
[CTL]: `package com.example;
|
||||
public class OrderController {
|
||||
@PostMapping("/api/v1/orders")
|
||||
public void create() {}
|
||||
}`,
|
||||
};
|
||||
expect(groupProviders(files)).toEqual(['POST /api/v1/orders']);
|
||||
expect(ingestionRoutes(files)).toEqual(['POST /api/v1/orders']);
|
||||
});
|
||||
});
|
||||
|
|
@ -221,14 +221,23 @@ describe('PARSE_CACHE_VERSION', () => {
|
|||
// Version 69 added #2969's JS/TS data-route-table decoratorRoutes. Version 70
|
||||
// adds Spring non-HTTP handler side-channel facts (#2417 / #2891), so it is
|
||||
// the next free value after both cache payload changes.
|
||||
it('pins SCHEMA_BUMP to 70 so concurrent bumps cannot silently collide (#2766)', () => {
|
||||
expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(70);
|
||||
// Moved 70 -> 71 for #2980's Java constant-route capture set (moduleConstants
|
||||
// + routePathOperands). This branch first argued no bump was needed because
|
||||
// "the ledger already sits at 70, whose capture set post-dates and includes
|
||||
// this harvest" — it does not: 70 was cut by fe3d7e56b for #2417/#2891, an
|
||||
// ancestor of this PR's base. Leaving it made PARSE_CACHE_VERSION byte-
|
||||
// identical across the merge, so every same-package-version warm cache
|
||||
// replayed pre-feature captures and the feature was inert. 71 is the next
|
||||
// free value above every claim at this merge — origin/main is 70 and open
|
||||
// PR #3017 already claims 71, so 71 would have collided.
|
||||
it('pins SCHEMA_BUMP to 72 so concurrent bumps cannot silently collide (#2766)', () => {
|
||||
expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(72);
|
||||
// The PREVIOUS version must fail the reuse gate, not merely differ from the
|
||||
// current one — a hardcoded number outside the conflict hunk rebases cleanly
|
||||
// while being wrong, which is exactly how the 37/38 exact clashes landed.
|
||||
// Every nearby historical or in-flight value is rejected, including 69,
|
||||
// which carried the route-table payload before this merge.
|
||||
for (const taken of [59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69]) {
|
||||
for (const taken of [59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71]) {
|
||||
expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).not.toBe(taken);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -19,16 +19,18 @@
|
|||
* *Constants (the High bug)
|
||||
* src/main/java/com/example/UserController.java — @RequestMapping prefix +
|
||||
* @PostMapping(ApiPaths.X) +
|
||||
* FQN form + inline concat +
|
||||
* static import
|
||||
* FQN form + concat over a
|
||||
* static-imported bare ref
|
||||
*
|
||||
* Assertions (both runs):
|
||||
* - the emitted Route node carries the FOLDED literal path, not the expr;
|
||||
* - ALL THREE non-literal shapes survive (qualified, FQN-qualified, concat);
|
||||
* - a phantom `POST ` / empty path never appears (skip floor);
|
||||
* - the warm run (parse-cache replay, no worker spawn) yields the IDENTICAL
|
||||
* route set — the harvest result survives the structured-clone cache round
|
||||
* trip (ModuleConstants uses Map, exercised through mapReplacer/mapReviver).
|
||||
* - the warm run yields the IDENTICAL route set AND is a genuine replay
|
||||
* (`usedWorkerPool === false`) — the harvest result survives the
|
||||
* structured-clone cache round trip (ModuleConstants uses Map, exercised
|
||||
* through mapReplacer/mapReviver). Asserting the route set alone would pass
|
||||
* on a cache MISS that silently reparsed.
|
||||
*
|
||||
* Rebuild gate: this test requires dist/ to be current; when dist/ is stale
|
||||
* (older than src/) it self-skips with a loud message rather than silently
|
||||
|
|
@ -42,13 +44,43 @@ import path from 'node:path';
|
|||
import { createKnowledgeGraph } from '../../src/core/graph/graph.js';
|
||||
import { runChunkedParseAndResolve } from '../../src/core/ingestion/pipeline-phases/parse-impl.js';
|
||||
import { PARSE_CACHE_VERSION, type ParseCache } from '../../src/storage/parse-cache.js';
|
||||
import {
|
||||
getDurableParsedFileDir,
|
||||
pruneAndSaveDurableParsedFileStore,
|
||||
} from '../../src/storage/parsedfile-store.js';
|
||||
|
||||
// ── dist freshness gate ───────────────────────────────────────────────────
|
||||
// The worker is one emitted file among many: TypeScript emits every module in
|
||||
// this feature separately, so comparing dist/parse-worker.js against
|
||||
// src/parse-worker.ts alone passes while the resolver, the Spring extractor or
|
||||
// the provider behind it are stale — and the test then asserts against the
|
||||
// PREVIOUS build's harvest. Gate on the newest mtime across every source this
|
||||
// pipeline actually loads.
|
||||
const repoRoot = path.resolve(__dirname, '..', '..');
|
||||
const distWorker = path.join(repoRoot, 'dist', 'core', 'ingestion', 'workers', 'parse-worker.js');
|
||||
const srcWorker = path.join(repoRoot, 'src', 'core', 'ingestion', 'workers', 'parse-worker.ts');
|
||||
const distStale =
|
||||
!fs.existsSync(distWorker) || fs.statSync(distWorker).mtimeMs < fs.statSync(srcWorker).mtimeMs;
|
||||
const GATED_SOURCES = [
|
||||
'core/ingestion/workers/parse-worker.ts',
|
||||
'core/ingestion/route-extractors/java-const-resolver.ts',
|
||||
'core/ingestion/route-extractors/constant-resolver.ts',
|
||||
'core/ingestion/route-extractors/spring.ts',
|
||||
'core/ingestion/languages/java.ts',
|
||||
'core/ingestion/languages/python.ts',
|
||||
'core/ingestion/language-provider.ts',
|
||||
'core/ingestion/pipeline-phases/parse-impl.ts',
|
||||
];
|
||||
const newestSourceMs = Math.max(
|
||||
...GATED_SOURCES.map((rel) => fs.statSync(path.join(repoRoot, 'src', rel)).mtimeMs),
|
||||
);
|
||||
const distStale = !fs.existsSync(distWorker) || fs.statSync(distWorker).mtimeMs < newestSourceMs;
|
||||
|
||||
if (distStale) {
|
||||
// `describe.skip` prints only vitest's ordinary skip marker, so without this
|
||||
// the docblock's promised "loud message" did not exist and a stale/absent
|
||||
// dist/ looked like a passing run.
|
||||
console.warn(
|
||||
'[#2980 e2e] SKIPPED: dist/ is missing or older than src/ — run `npm run build` to exercise the real pipeline.',
|
||||
);
|
||||
}
|
||||
|
||||
const maybeDescribe = distStale ? describe.skip : describe;
|
||||
|
||||
|
|
@ -65,6 +97,7 @@ public class ApiPaths {
|
|||
const USER_CONTROLLER = `package com.example;
|
||||
|
||||
import com.example.common.ApiPaths;
|
||||
import static com.example.common.ApiPaths.V1;
|
||||
import org.springframework.web.bind.annotation.RequestMapping;
|
||||
import org.springframework.web.bind.annotation.PostMapping;
|
||||
import org.springframework.web.bind.annotation.GetMapping;
|
||||
|
|
@ -81,9 +114,11 @@ public class UserController {
|
|||
@GetMapping(com.example.common.ApiPaths.ORDERS)
|
||||
public void list() {}
|
||||
|
||||
// Inline concat with a static-import-style bare ref (same-file constant
|
||||
// through the composed-operand fold).
|
||||
@PostMapping(com.example.common.ApiPaths.V1 + "/orders")
|
||||
// Inline concat with a STATIC-IMPORTED bare ref — the shape this fixture
|
||||
// used to only claim: it spelled the operand as the full FQN chain, which
|
||||
// just re-tested the FQN branch above, so bare-name resolution through the
|
||||
// import table had no coverage anywhere in the suite.
|
||||
@PostMapping(V1 + "/orders")
|
||||
public void createOrders() {}
|
||||
}
|
||||
`;
|
||||
|
|
@ -158,6 +193,7 @@ maybeDescribe('#2980 provider-hook constant harvest — real pipeline (cold + wa
|
|||
};
|
||||
|
||||
const result = await runPipeline(cache, files);
|
||||
expect(result.usedWorkerPool).toBe(true);
|
||||
const routes = foldedRoutesOf(result);
|
||||
|
||||
// All three non-literal shapes resolve to folded literals. (The class-level
|
||||
|
|
@ -186,16 +222,27 @@ maybeDescribe('#2980 provider-hook constant harvest — real pipeline (cold + wa
|
|||
onDiskKeys: new Set(),
|
||||
};
|
||||
|
||||
// Run #1 populates the cache; persist it like run-analyze does.
|
||||
// Run #1 populates the cache; persist it like run-analyze does — BOTH the
|
||||
// chunk shards and the durable ParsedFile store. `slimParseWorkerResultsForCache`
|
||||
// blanks `parsedFiles` before writing a shard, so a warm run without the
|
||||
// durable store cannot replay the chunk and silently falls back to the
|
||||
// workers — which is what this test used to do while still passing.
|
||||
const run1 = await runPipeline(cache, files);
|
||||
const { saveParseCache, pruneCache } = await import('../../src/storage/parse-cache.js');
|
||||
pruneCache(cache, cache.usedKeys);
|
||||
await saveParseCache(storageDir, cache);
|
||||
const savedKeys = await saveParseCache(storageDir, cache);
|
||||
expect(savedKeys.length).toBeGreaterThan(0);
|
||||
await pruneAndSaveDurableParsedFileStore(
|
||||
getDurableParsedFileDir(storageDir),
|
||||
PARSE_CACHE_VERSION,
|
||||
new Set(savedKeys),
|
||||
);
|
||||
|
||||
// Run #2 — warm: every chunk is a cache HIT, no worker spawn, the cached
|
||||
// ParseWorkerResult (moduleConstants included) is replayed from disk.
|
||||
const { loadParseCache } = await import('../../src/storage/parse-cache.js');
|
||||
const warm = await loadParseCache(storageDir);
|
||||
expect(warm.onDiskKeys).toEqual(new Set(savedKeys));
|
||||
const run2 = await runPipeline(warm, files);
|
||||
|
||||
const cold = foldedRoutesOf(run1)
|
||||
|
|
@ -206,5 +253,12 @@ maybeDescribe('#2980 provider-hook constant harvest — real pipeline (cold + wa
|
|||
.sort();
|
||||
expect(hot).toEqual(cold);
|
||||
expect(hot.length).toBeGreaterThan(0);
|
||||
// Without this the test proves nothing about the cache: `loadParseCache`
|
||||
// returns an EMPTY cache on any failure (missing file, corrupt JSON,
|
||||
// version mismatch) and never throws, so a broken Map round-trip through
|
||||
// mapReplacer/mapReviver — the exact regression this test exists for —
|
||||
// would silently reparse through the workers and produce the same routes.
|
||||
expect(run1.usedWorkerPool).toBe(true);
|
||||
expect(run2.usedWorkerPool).toBe(false);
|
||||
}, 120_000);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -6,19 +6,28 @@
|
|||
* fixtures missed the dominant real-world spelling — 1198 constant-ref
|
||||
* routes vs 2 literals in the real repo).
|
||||
*
|
||||
* Real shapes covered (counts from the live repo):
|
||||
* - `@WinPostMapping(ApiPathConstants.DIAGNOSIS_SAVE_V1)` — qualified ref,
|
||||
* 1063 occurrences
|
||||
* - `@WinPostMapping(value = ApiPathConstants.X)` / `(path = X)` — named
|
||||
* Value shapes covered, spelled with the Spring annotations this branch
|
||||
* actually recognises (`@PostMapping` & co., bare or fully qualified):
|
||||
* - `@PostMapping(ApiPathConstants.DIAGNOSIS_SAVE_V1)` — qualified ref, the
|
||||
* dominant real-world spelling (1063 occurrences in the source corpus)
|
||||
* - `@PostMapping(value = ApiPathConstants.X)` / `(path = X)` — named
|
||||
* argument, 414+ occurrences
|
||||
* - `@WinPostMapping(API_CIS_GET_TREATMENT_ORDER_V1)` — static-imported bare
|
||||
* - `@PostMapping(API_CIS_GET_TREATMENT_ORDER_V1)` — static-imported bare
|
||||
* name, 79 files
|
||||
* - `public static final String API = OTHER + "suffix"` — composed constant
|
||||
* - interface constants (implicitly static final)
|
||||
* - same-package simple-name collision handled by unique-suffix import
|
||||
* resolution across Maven modules
|
||||
* - escaped characters survive folding identically to the literal path
|
||||
* - same-package simple-name collision floors to skip across Maven modules
|
||||
* - FQN-qualified annotation value (4 occurrences)
|
||||
* - unresolvable references floor to skip (never a phantom path)
|
||||
*
|
||||
* NOT covered, deliberately: the vendor alias `@WinPostMapping`. The corpus is
|
||||
* dominated by it, but Spring alias recognition is an EXACT-NAME map
|
||||
* (`spring-shared.ts`) on this base — there is no `*Mapping`-suffix rule, #2883
|
||||
* is still open — so `@PostMapping(...)` extracts zero routes here no matter
|
||||
* how the constant folds. A fixture written in that spelling would be dead
|
||||
* (one was, and CodeQL flagged it). Constant folding and alias recognition are
|
||||
* independent: when #2883 lands, every shape below works unchanged for aliases.
|
||||
*/
|
||||
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
|
@ -26,11 +35,15 @@ import Parser from 'tree-sitter';
|
|||
import Java from 'tree-sitter-java';
|
||||
import {
|
||||
extractJavaModuleConstants,
|
||||
foldJavaOperands,
|
||||
isJavaConstantFile,
|
||||
parseJavaConstOperands,
|
||||
resolveJavaConstant,
|
||||
resolveJavaImport,
|
||||
type RepoConstants,
|
||||
} from '../../src/core/ingestion/route-extractors/java-const-resolver.js';
|
||||
import { javaProvider } from '../../src/core/ingestion/languages/java.js';
|
||||
import { unquoteSpringLiteral } from '../../src/core/ingestion/route-extractors/spring-shared.js';
|
||||
|
||||
const parser = new Parser();
|
||||
parser.setLanguage(Java);
|
||||
|
|
@ -79,13 +92,13 @@ import com.winning.opt.diagnosis.api.constants.ApiPathConstants;
|
|||
|
||||
public class DiagnosisController {
|
||||
|
||||
@WinPostMapping(ApiPathConstants.DIAGNOSIS_SAVE_V1)
|
||||
@PostMapping(ApiPathConstants.DIAGNOSIS_SAVE_V1)
|
||||
public String save() { return "{}"; }
|
||||
|
||||
@WinPostMapping(value = ApiPathConstants.DIAGNOSIS_SAVE_V2)
|
||||
@PostMapping(value = ApiPathConstants.DIAGNOSIS_SAVE_V2)
|
||||
public String saveV2() { return "{}"; }
|
||||
|
||||
@WinPostMapping(path = ApiPathConstants.API_CIS_SAVE_SUMMARY)
|
||||
@PostMapping(path = ApiPathConstants.API_CIS_SAVE_SUMMARY)
|
||||
public String saveSummary() { return "{}"; }
|
||||
}`;
|
||||
|
||||
|
|
@ -95,7 +108,7 @@ import static com.winning.opt.diagnosis.api.constants.ApiPathConstants.DIAGNOSIS
|
|||
|
||||
public class CisController {
|
||||
|
||||
@WinPostMapping(DIAGNOSIS_SAVE_V1)
|
||||
@PostMapping(DIAGNOSIS_SAVE_V1)
|
||||
public String save() { return "{}"; }
|
||||
}`;
|
||||
|
||||
|
|
@ -105,16 +118,6 @@ public interface LabApiPath {
|
|||
String LAB_QUERY_V1 = "/api/v1/labtest/query";
|
||||
}`;
|
||||
|
||||
const WIN_POST_MAPPING = `package com.winning.opt.annotations;
|
||||
|
||||
public @interface WinPostMapping {
|
||||
String value() default "";
|
||||
String path() default "";
|
||||
}`;
|
||||
|
||||
// Fake minimal annotation so fixtures parse — the alias layer treats any
|
||||
// *Mapping-suffixed annotation as a route annotation.
|
||||
|
||||
describe('extractJavaModuleConstants', () => {
|
||||
it('collects static final String literals with class-qualified aliases', () => {
|
||||
const mc = extractJavaModuleConstants(parse(CONSTANTS_FILE));
|
||||
|
|
@ -312,11 +315,11 @@ public class DemoController {
|
|||
public String save() { return "ok"; }
|
||||
}`);
|
||||
const routes = extractSpringRoutes(tree, 'DemoController.java', 0);
|
||||
assert.strictEqual(routes.length, 1);
|
||||
assert.strictEqual(routes[0].httpMethod, 'POST');
|
||||
assert.strictEqual(routes[0].routePathExpr, 'ApiPathConstants.DIAGNOSIS_SAVE_V1');
|
||||
assert.ok(routes[0].routePathOperands && routes[0].routePathOperands.length > 0);
|
||||
assert.strictEqual(routes[0].routePath, '');
|
||||
expect(routes.length).toBe(1);
|
||||
expect(routes[0].httpMethod).toBe('POST');
|
||||
expect(routes[0].routePathExpr).toBe('ApiPathConstants.DIAGNOSIS_SAVE_V1');
|
||||
expect(routes[0].routePathOperands && routes[0].routePathOperands.length > 0).toBeTruthy();
|
||||
expect(routes[0].routePath).toBe('');
|
||||
});
|
||||
|
||||
it('keeps literal routes unchanged', async () => {
|
||||
|
|
@ -329,9 +332,9 @@ public class DemoController {
|
|||
public String save() { return "ok"; }
|
||||
}`);
|
||||
const routes = extractSpringRoutes(tree, 'DemoController.java', 0);
|
||||
assert.strictEqual(routes.length, 1);
|
||||
assert.strictEqual(routes[0].routePath, '/literal/path');
|
||||
assert.strictEqual(routes[0].routePathExpr, undefined);
|
||||
expect(routes.length).toBe(1);
|
||||
expect(routes[0].routePath).toBe('/literal/path');
|
||||
expect(routes[0].routePathExpr).toBe(undefined);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -339,6 +342,7 @@ describe('qualified-ref recursion cycle guard (maintainer point 5)', () => {
|
|||
it('self-import: qualified self-reference terminates with null, not a stack overflow', () => {
|
||||
const repo = repoOf({
|
||||
'src/main/java/com/example/SelfConsts.java': `package com.example;
|
||||
import com.example.SelfConsts;
|
||||
public class SelfConsts {
|
||||
public static final String X = SelfConsts.X + "/x";
|
||||
}`,
|
||||
|
|
@ -509,3 +513,252 @@ public class Ctl {
|
|||
).toBe('/api/v1/users');
|
||||
});
|
||||
});
|
||||
|
||||
describe('escaped characters survive folding (review P1)', () => {
|
||||
// tree-sitter-java splits a string_literal AROUND its escape_sequence
|
||||
// children, so a string_fragment-only join silently deleted every escape:
|
||||
// the standard Spring path-variable constraint `{id:\\d+}` folded to
|
||||
// `{id:d+}` and a pure-escape literal folded to ''. Worse, the LITERAL path
|
||||
// keeps escapes verbatim, so one Java route had two spellings.
|
||||
const cases = [
|
||||
['"/user/{id:\\d+}"', '/user/{id:\\d+}'],
|
||||
['"/a\\tb"', '/a\\tb'],
|
||||
['"/a\\u002Fb"', '/a\\u002Fb'],
|
||||
['"\\t"', '\\t'],
|
||||
['""', ''],
|
||||
['"/plain"', '/plain'],
|
||||
] as const;
|
||||
|
||||
it.each(cases)('keeps %s intact through the constant path', (literal, expected) => {
|
||||
const mc = extractJavaModuleConstants(
|
||||
parse(`public class C { public static final String X = ${literal}; }`),
|
||||
);
|
||||
expect(mc.literals.get('X')).toBe(expected);
|
||||
});
|
||||
|
||||
it.each(cases)('agrees with the literal path for %s', (literal, expected) => {
|
||||
// The constant path and `unquoteSpringLiteral` (what a literal-valued
|
||||
// @GetMapping goes through) must produce the SAME string, or the graph
|
||||
// carries two irreconcilable spellings of one route.
|
||||
expect(unquoteSpringLiteral(literal)).toBe(expected);
|
||||
});
|
||||
});
|
||||
|
||||
describe('a non-foldable rebind drops the static import too (review P1)', () => {
|
||||
it('returns null rather than the shadowed imported value', () => {
|
||||
const repo = repoOf({
|
||||
'src/main/java/com/x/Base.java': `package com.x;
|
||||
public class Base { public static final String PATH = "/WRONG-imported"; }`,
|
||||
'src/main/java/com/y/C.java': `package com.y;
|
||||
import static com.x.Base.PATH;
|
||||
public class C { public static final String PATH = compute(); }`,
|
||||
});
|
||||
// A local `static final` shadows a static import of the same simple name
|
||||
// inside that class (JLS 6.4.1), so the only correct answer is
|
||||
// "unresolvable". Leaving the import alive made the fold fall through to
|
||||
// it and return the imported literal — a wrong path where the skip floor
|
||||
// is owed (#2393's Python defect, reproduced for Java).
|
||||
expect(repo.get('src/main/java/com/y/C.java')!.imports.has('PATH')).toBe(false);
|
||||
expect(
|
||||
foldJavaOperands('src/main/java/com/y/C.java', [{ kind: 'ref', name: 'PATH' }], repo),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('the drop is file-scoped: a sibling class floors to skip, never to a wrong value', () => {
|
||||
// These maps are file-level by design (nested types flatten into one
|
||||
// namespace), so dropping the import costs a sibling class that
|
||||
// legitimately uses it. javac would answer `/imported/b` here; we answer
|
||||
// null. Pinned deliberately — the alternative direction is a wrong path.
|
||||
const repo = repoOf({
|
||||
'src/main/java/com/x/Base.java': `package com.x;
|
||||
public class Base { public static final String PATH = "/imported"; }`,
|
||||
'src/main/java/com/y/Two.java': `package com.y;
|
||||
import static com.x.Base.PATH;
|
||||
class A { public static final String PATH = compute(); }
|
||||
class B { public static final String USE = PATH + "/b"; }`,
|
||||
});
|
||||
expect(
|
||||
foldJavaOperands('src/main/java/com/y/Two.java', [{ kind: 'ref', name: 'B.USE' }], repo),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('a FOLDABLE rebind still wins over the import', () => {
|
||||
const repo = repoOf({
|
||||
'src/main/java/com/x/Base.java': `package com.x;
|
||||
public class Base { public static final String PATH = "/imported"; }`,
|
||||
'src/main/java/com/y/C.java': `package com.y;
|
||||
import static com.x.Base.PATH;
|
||||
public class C { public static final String PATH = "/local"; }`,
|
||||
});
|
||||
expect(
|
||||
foldJavaOperands('src/main/java/com/y/C.java', [{ kind: 'ref', name: 'PATH' }], repo),
|
||||
).toBe('/local');
|
||||
});
|
||||
});
|
||||
|
||||
describe('isJavaConstantFile — one gate, both subsystems (review P1)', () => {
|
||||
// The ingestion provider and the group extractor's prepareRepo pre-pass used
|
||||
// to spell this gate differently. A constant INTERFACE passed the group's and
|
||||
// failed ingestion's, so the group published a provider contract while the
|
||||
// graph got no Route node — an R4 parity break in the losing direction.
|
||||
const shapes = [
|
||||
[
|
||||
'constant interface (implicitly static final, no import)',
|
||||
`package com.x;
|
||||
public interface ApiPathConstants { String SAVE = "/api/v1/save"; }`,
|
||||
],
|
||||
[
|
||||
'lowercase interface name',
|
||||
`package com.x;
|
||||
public interface apiPaths { String SAVE = "/api/v1/save"; }`,
|
||||
],
|
||||
[
|
||||
'reversed modifier order',
|
||||
`package com.x;
|
||||
public class P { public final static String SAVE = "/api/v1/save"; }`,
|
||||
],
|
||||
[
|
||||
'conventional order',
|
||||
`package com.x;
|
||||
public class P { public static final String SAVE = "/api/v1/save"; }`,
|
||||
],
|
||||
] as const;
|
||||
|
||||
it.each(shapes)('admits %s on BOTH sides', (_name, src) => {
|
||||
expect(isJavaConstantFile(src)).toBe(true);
|
||||
// The provider hook is what the parse worker actually calls — drive it,
|
||||
// not just the regex, so the gate itself is covered and not only the
|
||||
// extractor behind it.
|
||||
expect(javaProvider.moduleConstantHeuristic?.(src)).toBe(true);
|
||||
expect(extractJavaModuleConstants(parse(src)).literals.get('SAVE')).toBe('/api/v1/save');
|
||||
});
|
||||
|
||||
it('still skips a file with no constant-bearing syntax', () => {
|
||||
const src = `package com.x;
|
||||
public class P { void run() { System.out.println("/not-a-constant"); } }`;
|
||||
expect(isJavaConstantFile(src)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveJavaImport honours the documented skip floor (review P2)', () => {
|
||||
it('returns null when the same package+class exists in two modules', () => {
|
||||
// A nearest-shared-directory tie-break used to pick one. javac resolves
|
||||
// duplicate FQNs by classpath order, so proximity can hand back a
|
||||
// src/test fixture copy — a silently wrong literal in a resolver whose
|
||||
// contract is skip-or-correct.
|
||||
const keys = new Set([
|
||||
'svc-order/src/main/java/com/x/ApiPaths.java',
|
||||
'svc-user/src/main/java/com/x/ApiPaths.java',
|
||||
]);
|
||||
expect(
|
||||
resolveJavaImport(
|
||||
'svc-order/src/main/java/com/x/web/OrderController.java',
|
||||
'com.x.ApiPaths',
|
||||
keys,
|
||||
),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('still resolves a unique full-suffix match', () => {
|
||||
const keys = new Set([
|
||||
'svc-order/src/main/java/com/x/ApiPaths.java',
|
||||
'svc-user/src/main/java/com/y/ApiPaths.java',
|
||||
]);
|
||||
expect(
|
||||
resolveJavaImport(
|
||||
'svc-order/src/main/java/com/x/web/OrderController.java',
|
||||
'com.x.ApiPaths',
|
||||
keys,
|
||||
),
|
||||
).toBe('svc-order/src/main/java/com/x/ApiPaths.java');
|
||||
});
|
||||
});
|
||||
|
||||
describe('enum and record constants are collected', () => {
|
||||
it.each([
|
||||
['enum', 'public enum E { A, B; public static final String P = "/e"; }', 'E'],
|
||||
['record', 'public record R(int x) { public static final String P = "/r"; }', 'R'],
|
||||
])('harvests a static final String declared in a %s', (_kind, src, owner) => {
|
||||
const mc = extractJavaModuleConstants(parse(src));
|
||||
expect(mc.literals.get('P')).toBe(src.includes('enum') ? '/e' : '/r');
|
||||
expect(mc.literals.get(`${owner}.P`)).toBe(src.includes('enum') ? '/e' : '/r');
|
||||
});
|
||||
|
||||
it('does not harvest a non-static field of a record', () => {
|
||||
const mc = extractJavaModuleConstants(parse('public record R(int x) { String p = "/r"; }'));
|
||||
expect(mc.literals.has('p')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('constants composed across files through a qualified ref', () => {
|
||||
it('folds `X = BConsts.Y + "/tail"` across the import', () => {
|
||||
// Operands found INSIDE an initializer used to go straight to the agnostic
|
||||
// fold, which only knows bare names — so a qualified operand missed and
|
||||
// floored the whole chain to null, even acyclically.
|
||||
const repo = repoOf({
|
||||
'src/com/example/AConsts.java': `package com.example;
|
||||
import com.example.BConsts;
|
||||
public class AConsts { public static final String X = BConsts.Y + "/tail"; }`,
|
||||
'src/com/example/BConsts.java': `package com.example;
|
||||
public class BConsts { public static final String Y = "/y"; }`,
|
||||
});
|
||||
expect(resolveJavaConstant('src/com/example/AConsts.java', 'X', repo)).toBe('/y/tail');
|
||||
expect(
|
||||
foldJavaOperands('src/com/example/AConsts.java', [{ kind: 'ref', name: 'AConsts.X' }], repo),
|
||||
).toBe('/y/tail');
|
||||
});
|
||||
|
||||
it('a missing link in the chain still floors to null', () => {
|
||||
const repo = repoOf({
|
||||
'src/com/example/AConsts.java': `package com.example;
|
||||
import com.example.BConsts;
|
||||
public class AConsts { public static final String X = BConsts.MISSING + "/tail"; }`,
|
||||
'src/com/example/BConsts.java': `package com.example;
|
||||
public class BConsts { public static final String Y = "/y"; }`,
|
||||
});
|
||||
expect(resolveJavaConstant('src/com/example/AConsts.java', 'X', repo)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('the fold is bounded in time as well as depth', () => {
|
||||
it('folds a 30-level shared-descendant DAG instead of exploring 2^30 paths', () => {
|
||||
// `X_k = X_{k+1} + X_{k+1}` re-folds each child once per reference without a
|
||||
// memo — O(2^depth). MAX_FOLD_LENGTH cannot save it here because every
|
||||
// intermediate value is the EMPTY string, so nothing ever accumulates.
|
||||
// Un-memoized this took 2.7 s at 26 levels and 11 s at 28, on the main
|
||||
// thread, for one route. The assertion is the explicit timeout below: a
|
||||
// regression does not fail this test slowly, it fails it.
|
||||
const lines = ['public static final String X30 = "";'];
|
||||
for (let i = 29; i >= 0; i--) {
|
||||
lines.push(`public static final String X${i} = X${i + 1} + X${i + 1};`);
|
||||
}
|
||||
const repo = repoOf({ 'C.java': `public class C {\n${lines.join('\n')}\n}` });
|
||||
expect(resolveJavaConstant('C.java', 'X0', repo)).toBe('');
|
||||
}, 5_000);
|
||||
|
||||
it('still caps a chain that genuinely produces a huge string', () => {
|
||||
const lines = ['public static final String X30 = "a";'];
|
||||
for (let i = 29; i >= 0; i--) {
|
||||
lines.push(`public static final String X${i} = X${i + 1} + X${i + 1};`);
|
||||
}
|
||||
const repo = repoOf({ 'C.java': `public class C {\n${lines.join('\n')}\n}` });
|
||||
expect(resolveJavaConstant('C.java', 'X0', repo)).toBeNull();
|
||||
}, 5_000);
|
||||
});
|
||||
|
||||
describe('text blocks keep the skip floor', () => {
|
||||
it('does not fold a text-block constant into a path with newlines and indentation', () => {
|
||||
// `unquoteSpringLiteral` has a `"""` arm that slices 3/-3, which would hand
|
||||
// back the raw block — leading newline and incidental indentation included,
|
||||
// both of which Java strips — and nothing downstream normalizes it. The old
|
||||
// fragment-join returned '' here, i.e. a skip; keep the skip.
|
||||
const src = [
|
||||
'public class C {',
|
||||
' public static final String X = """',
|
||||
' /api/v1/tb',
|
||||
' """;',
|
||||
'}',
|
||||
].join('\n');
|
||||
expect(extractJavaModuleConstants(parse(src)).literals.has('X')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ import {
|
|||
type ImportBinding,
|
||||
type RepoConstants,
|
||||
} from '../../src/core/ingestion/route-extractors/python-const-resolver.js';
|
||||
import { pythonProvider } from '../../src/core/ingestion/languages/python.js';
|
||||
|
||||
const lit = (value: string): Operand => ({ kind: 'literal', value });
|
||||
const ref = (name: string): Operand => ({ kind: 'ref', name });
|
||||
|
|
@ -377,3 +378,28 @@ describe('extractPythonModuleConstants — source-order snapshot (#2393)', () =>
|
|||
expect(resolveConstant('m.py', 'C', r)).toBe('/a/b/c');
|
||||
});
|
||||
});
|
||||
|
||||
describe('the Python provider harvests unconditionally (#2980 review P2)', () => {
|
||||
// A cheap content gate was added on the provider here and removed on review.
|
||||
// It required NAME immediately followed by `=`, so it silently dropped the
|
||||
// idiomatic typed-FastAPI shapes and every composed constant whose RHS starts
|
||||
// with an identifier — i.e. it REGRESSED routes that already resolve on main.
|
||||
// The parse worker treats a missing heuristic as "harvest"; pin that here so
|
||||
// the gate cannot come back without a decision.
|
||||
it('declares no moduleConstantHeuristic', () => {
|
||||
expect(pythonProvider.moduleConstantHeuristic).toBeUndefined();
|
||||
});
|
||||
|
||||
it.each([
|
||||
['plain', 'API = "/api/v1"\nUSERS = API + "/users"\n'],
|
||||
['PEP 526 annotated', 'API: str = "/api/v1"\nUSERS: str = API + "/users"\n'],
|
||||
[
|
||||
'Final-annotated',
|
||||
'from typing import Final\nAPI: Final[str] = "/api/v1"\nUSERS: Final[str] = API + "/users"\n',
|
||||
],
|
||||
['composed, identifier RHS', 'API = _base()\nUSERS = API + "/users"\n'],
|
||||
])('still reaches the extractor for the %s shape', (_name, src) => {
|
||||
const mc = extract(src);
|
||||
expect(mc.literals.size + mc.exprs.size + mc.imports.size).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue