diff --git a/gitnexus/package-lock.json b/gitnexus/package-lock.json index ba9670bdc..6d4e37d12 100644 --- a/gitnexus/package-lock.json +++ b/gitnexus/package-lock.json @@ -1618,9 +1618,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1638,9 +1635,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -1658,9 +1652,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1678,9 +1669,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1698,9 +1686,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1718,9 +1703,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3810,9 +3792,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -3834,9 +3813,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -3858,9 +3834,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -3882,9 +3855,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ diff --git a/gitnexus/src/core/group/extractors/http-patterns/java.ts b/gitnexus/src/core/group/extractors/http-patterns/java.ts index 4eba0c1f1..081b71805 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/java.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/java.ts @@ -28,6 +28,13 @@ import { REQUEST_LINE_CONFIDENCE, EXCHANGE_CONFIDENCE, } from './spring-consumer-shared.js'; +import { + extractJavaModuleConstants, + foldJavaOperands, + isJavaConstantFile, + parseJavaConstOperands, + type RepoConstants, +} from '../../../ingestion/route-extractors/java-const-resolver.js'; import { extractStaticPathExpression, inferOkHttpMethod, @@ -165,6 +172,34 @@ 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 + name: [(identifier) (scoped_identifier)] @ann + arguments: (annotation_argument_list [(identifier) @value_expr (field_access) @value_expr (binary_expression) @value_expr]))) + name: (identifier) @member) @node + (method_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])))) + name: (identifier) @member) @node ] `, }, @@ -469,6 +504,12 @@ interface MethodRouteAnnotation { rawPath: string; /** OpenFeign's single effective verb; null means its contract is invalid/ambiguous. */ feignHttpMethod?: string | null; + /** + * Non-literal path operands (constant ref or `+`-concat), captured when the + * annotation value is not a string literal. Resolved against the repo-wide + * Java constant map in scan(); a failed fold drops the route (skip floor). + */ + pathOperands?: readonly import('../../../ingestion/route-extractors/constant-resolver.js').Operand[]; } interface RequestLineAnnotation { @@ -484,6 +525,16 @@ interface RouteAnnotationScan { feignPrefixByInterfaceId: Map; /** Spring HTTP Interface `@HttpExchange(url|value)` type-level prefixes per class/interface node id. */ httpExchangePrefixByTypeId: Map; + /** + * 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; /** 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. */ @@ -511,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(); + const typesWithUnfoldablePrefix = new Set(); const feignPrefixByInterfaceId = new Map(); const httpExchangePrefixByTypeId = new Map(); const methodRoutes: MethodRouteAnnotation[] = []; @@ -527,7 +579,10 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan { const annNode = captures.ann; const node = captures.node; const valueNode = captures.value; - if (!annNode || !node || !valueNode) continue; + // A non-literal annotation value (constant ref / `+`-concat) is captured + // as @value_expr instead of @value — one of the two must be present. + const valueExprNode = captures.value_expr; + if (!annNode || !node || (!valueNode && !valueExprNode)) continue; // Discrimination is on the trailing segment only (`simpleName`), so a // non-Spring annotation whose last segment collides with a route annotation // (e.g. `@com.evil.GetMapping("/x")`) is treated as a route. This is the @@ -550,7 +605,7 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan { const feignHttpMethod = httpMethods.length === 1 ? (httpMethods[0] === '*' ? 'GET' : httpMethods[0]) : null; if (!isRouteMemberKey(keyNode)) continue; - const rawPath = unquoteLiteral(valueNode.text); + const rawPath = valueNode ? unquoteLiteral(valueNode.text) : null; if (rawPath !== null) { for (const httpMethod of httpMethods) { methodRoutes.push({ @@ -561,10 +616,33 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan { feignHttpMethod, }); } + } else { + // Non-literal path (a constant reference or `+`-concatenation). + // Defer to scan(): the fold needs the repo-wide constant map built + // by prepareRepo. Capture the operand list now; resolution happens + // in scan() against JavaRepoContext, and an unresolvable operand + // list leaves the route skipped (KTD5 skip floor). + const operands = parseJavaConstOperands(valueExprNode); + if (operands !== null) { + for (const httpMethod of httpMethods) { + methodRoutes.push({ + methodNode: node, + methodName: captures.member?.text ?? null, + httpMethod, + rawPath: '', + feignHttpMethod, + pathOperands: operands, + }); + } + } } } else if (ann === 'RequestLine') { // Feign packs verb + path in one literal; its only named argument is `value`. if (keyNode && keyNode.text !== 'value') continue; + // A constant-valued `@RequestLine` arrives as @value_expr, not @value — + // `valueNode` is undefined in that shape. Skip rather than dereference + // (constant folding for Feign verb+path literals is out of scope here). + if (!valueNode) continue; const raw = unquoteLiteral(valueNode.text); const parsed = raw !== null ? parseRequestLine(raw) : null; if (parsed) { @@ -579,7 +657,7 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan { // `url` or `value` attribute (or positionally); other attributes // (`accept`, `contentType`, …) are not routes. if (keyNode && keyNode.text !== 'url' && keyNode.text !== 'value') continue; - const rawPath = unquoteLiteral(valueNode.text); + const rawPath = valueNode ? unquoteLiteral(valueNode.text) : null; if (rawPath !== null) { exchangeRoutes.push({ methodNode: node, @@ -596,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); @@ -606,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); } } @@ -662,6 +745,7 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan { return { prefixByTypeId, + typesWithUnfoldablePrefix, feignPrefixByInterfaceId, httpExchangePrefixByTypeId, methodRoutes: constrainedMethodRoutes, @@ -707,9 +791,20 @@ 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>(); 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 + // path would publish `POST /`-shaped noise into the shared type view; + // skip instead (ingestion keeps the same skip floor — R4 parity). + if (route.pathOperands) continue; const routes = routesByMethodId.get(route.methodNode.id) ?? []; routes.push({ method: route.httpMethod, path: route.rawPath }); routesByMethodId.set(route.methodNode.id, routes); @@ -781,8 +876,62 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = { content, ); }, - scan(tree) { + prepareRepo(args) { + // Build the repo-wide Java string-constant map once per extract() run + // (mirrors the Python binding's cost-gated pre-pass). A cheap content + // gate keeps literal-only repos at zero parses: only files containing a + // `static final String` declaration are parsed for constants. + try { + // The orchestrator hands over a bare Parser (no language set yet); + // bind Java explicitly — Python's prepareRepo does the same — otherwise + // parseSourceSafe spins to its 15 s budget per file. + args.parser.setLanguage(Java); + } catch { + // fall through: a parser that rejects binding cannot produce a constant + // map; per-file try/catch below then skips everything harmlessly. + } + const constants = new Map< + string, + import('../../../ingestion/route-extractors/constant-resolver.js').ModuleConstants + >(); + for (const rel of args.files) { + if (!rel.endsWith('.java')) continue; + try { + const src = args.readFile(rel); + // Cheap content gate: only constant-DEFINITION candidates get parsed + // here (~hundreds of files). Import-only files (every controller) + // are deliberately NOT parsed in this pass — scan() lazily extracts + // the importing file's own import table from the tree it already + // holds when a constant-referencing route actually needs the fold. + // 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. + // + // 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); + if (!tree) continue; + const mc = extractJavaModuleConstants(tree); + if (mc.literals.size > 0 || mc.exprs.size > 0 || mc.imports.size > 0) { + constants.set(rel, mc); + } + } catch { + // Per-file resilience: one unreadable/oversized/ill-formed file must + // not forfeit the whole repo's constant map (a missing constants + // class only degrades refs that pointed at it). + continue; + } + } + return { constants }; + }, + scan(tree, repoContext, fileRel) { const out: HttpDetection[] = []; + const javaCtx = repoContext as { constants: RepoConstants } | undefined; // ─── Spring providers + OpenFeign consumers (one query pass) ──── // `scanRouteAnnotations` resolves every route-defining annotation — @@ -790,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, @@ -802,7 +952,48 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = { // class is a Spring *provider*. A mapping on a non-Feign interface has no // enclosing class and is dropped here — interface→controller inheritance is // handled by `scanProject`. + // Lazy per-file constants view. prepareRepo only indexes constant- + // DEFINING files (cheap gate); an importing controller is absent from + // that map. When a route actually references a constant, extract THIS + // file's import table from the tree scan() already holds (zero extra + // parses) and overlay it for the fold. Files whose routes are all + // literal — the overwhelming majority — never pay this cost. + let foldConstants: RepoConstants | undefined; + const getFoldConstants = (): RepoConstants | undefined => { + if (foldConstants !== undefined) return foldConstants; + foldConstants = javaCtx?.constants; + if (!javaCtx?.constants || !fileRel) return foldConstants; + if (javaCtx.constants.has(fileRel)) return foldConstants; + try { + const mc = extractJavaModuleConstants(tree); + if (mc.imports.size > 0) { + const merged = new Map(javaCtx.constants); + merged.set(fileRel, mc); + foldConstants = merged; + } + } catch { + // fold falls back to the repo-wide map (imports stay unresolved) + } + return foldConstants; + }; + 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). + if (route.pathOperands && javaCtx && fileRel) { + const resolved = foldJavaOperands(fileRel, route.pathOperands, getFoldConstants()!); + if (resolved === null) continue; + route.rawPath = resolved; + } else if (route.pathOperands) { + continue; + } const enclosingInterface = findEnclosingInterface(route.methodNode); if (enclosingInterface && hasAnnotation(enclosingInterface, 'FeignClient')) { if (!route.feignHttpMethod) continue; diff --git a/gitnexus/src/core/ingestion/language-provider.ts b/gitnexus/src/core/ingestion/language-provider.ts index ec9006450..71802100d 100644 --- a/gitnexus/src/core/ingestion/language-provider.ts +++ b/gitnexus/src/core/ingestion/language-provider.ts @@ -39,6 +39,11 @@ import type { CfgVisitor } from './cfg/types.js'; import type { NodeLabel } from 'gitnexus-shared'; import type { ExtractedRoute } from './route-extractors/laravel.js'; import type { SharedSpringType } from './route-extractors/spring-shared.js'; +import type { + ModuleConstants, + Operand, + RepoConstants, +} from './route-extractors/constant-resolver.js'; import type Parser from 'tree-sitter'; import type { ExtractedDecoratorRoute } from './workers/parse-worker.js'; @@ -64,6 +69,25 @@ export interface AstFrameworkPatternConfig { * Required fields must be explicitly set; optional fields have defaults * applied by defineLanguage(). */ +/** + * Should the parse worker run {@link LanguageProviderConfig.extractModuleConstants} + * on this file? + * + * Exported so the DECISION is testable without booting a worker. It encodes the + * one rule that is easy to get backwards: a provider that declares no + * `moduleConstantHeuristic` harvests unconditionally. Writing the gate as + * `provider.moduleConstantHeuristic?.(content)` reads `undefined` as "skip" and + * silently disables the hook for every provider without a heuristic — which is + * exactly how Python's already-shipped harvest was turned off (#2391/#2980). + */ +export function shouldHarvestModuleConstants( + provider: Pick, + content: string, +): boolean { + if (!provider.extractModuleConstants) return false; + return !provider.moduleConstantHeuristic || provider.moduleConstantHeuristic(content); +} + interface LanguageProviderConfig { // ── Identity ────────────────────────────────────────────────────── readonly id: SupportedLanguages; @@ -336,6 +360,58 @@ interface LanguageProviderConfig { filePath: string, ) => SharedSpringType[]; + /** + * Harvest this file's module-level string constants (#2391 core, #2980 Java + * parity) into the language-agnostic {@link ModuleConstants} shape, so the + * parse phase can resolve non-literal decorator route paths cross-file. + * + * The worker calls this when BOTH hold: + * - 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. + * + * Default: undefined (no constant harvest; non-literal route paths of this + * language floor to skip). + */ + readonly extractModuleConstants?: (tree: Parser.Tree) => ModuleConstants; + + /** + * Cheap content heuristic deciding whether the worker should run + * {@link extractModuleConstants} on a file. Guards the harvest cost on huge + * 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; + + /** + * Fold one file's non-literal route-path operand list + * (`routePathExpr`/`routePathOperands` of an `ExtractedDecoratorRoute`) + * against the repo-wide, file-path-keyed constant map, or null when it cannot + * be fully folded (skip floor — never a phantom path). Languages whose + * qualified refs resolve through class imports (`Outer.CONST`, + * `com.example.ApiPaths.USERS`) need this hook because the shared fold has no + * notion of qualified names; Python's bare-name refs use the shared default. + * + * Default: undefined (the parse phase falls back to the shared + * language-agnostic operand fold). + */ + readonly foldRoutePathOperands?: ( + filePath: string, + operands: readonly Operand[], + repo: RepoConstants, + ) => string | null; + // ── Noise filtering ──────────────────────────────────────────────── /** Built-in/stdlib names that should be filtered from the call graph for this language. * Default: undefined (no language-specific filtering). */ diff --git a/gitnexus/src/core/ingestion/languages/java.ts b/gitnexus/src/core/ingestion/languages/java.ts index 317a853ac..0fddb6657 100644 --- a/gitnexus/src/core/ingestion/languages/java.ts +++ b/gitnexus/src/core/ingestion/languages/java.ts @@ -15,6 +15,11 @@ import type { AstFrameworkPatternConfig } from '../language-provider.js'; import { createLeadingDocDescriptionExtractor } from '../utils/ast-helpers.js'; import { javaTypeConfig } from '../type-extractors/jvm.js'; import { extractSpringRoutes, extractSpringTypes } from '../route-extractors/spring.js'; +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'; import { javaImportConfig } from '../import-resolvers/configs/jvm.js'; @@ -216,4 +221,26 @@ export const javaProvider = defineLanguage({ // ── Route extraction ── extractDecoratorRoutes: extractSpringRoutes, extractRouteInheritanceTypes: extractSpringTypes, + + // ── #2980: constant harvest + qualified-ref fold for non-literal mapping + // 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) => + 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. 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, }); diff --git a/gitnexus/src/core/ingestion/languages/python.ts b/gitnexus/src/core/ingestion/languages/python.ts index f9c345b4b..ca40a4566 100644 --- a/gitnexus/src/core/ingestion/languages/python.ts +++ b/gitnexus/src/core/ingestion/languages/python.ts @@ -44,6 +44,7 @@ import { } from './python/index.js'; import { extractDjangoRoutes } from '../route-extractors/django.js'; import { discoverDjangoRootUrls } from '../route-extractors/django-root-discovery.js'; +import { extractPythonModuleConstants } from '../route-extractors/python-const-resolver.js'; const BUILT_INS: ReadonlySet = new Set([ 'print', @@ -158,4 +159,17 @@ export const pythonProvider = defineLanguage({ receiverBinding: pythonReceiverBinding, arityCompatibility: pythonArityCompatibility, resolveImportTarget: resolvePythonImportTarget, + + // ── #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, }); diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts index 2a3d21612..0f4ba1b2e 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts @@ -60,7 +60,7 @@ import { createParserForLanguage, } from '../../tree-sitter/parser-loader.js'; import { parseSourceSafe } from '../../tree-sitter/safe-parse.js'; -import { getProvider, providers } from '../languages/index.js'; +import { getProvider, getProviderForFile, providers } from '../languages/index.js'; import { SCOPE_RESOLVERS } from '../scope-resolution/pipeline/registry.js'; import { DATA_ROUTE_TABLE_SOURCE } from '../route-extractors/data-route-table.js'; import type Parser from 'tree-sitter'; @@ -1303,8 +1303,15 @@ export async function runChunkedParseAndResolve( resolvedRoutes.push(dr); continue; } + // Provider-driven fold (#2980): languages with qualified-ref semantics + // (Java `ApiPaths.X` / `com.example.ApiPaths.X`) fold through their + // provider hook; everything else uses the shared language-agnostic + // operand fold. No language names in the shared layer. + const fold = getProviderForFile(dr.filePath)?.foldRoutePathOperands; const value = dr.routePathOperands - ? resolveOperands(dr.filePath, dr.routePathOperands, repoConstants) + ? fold + ? fold(dr.filePath, dr.routePathOperands, repoConstants) + : resolveOperands(dr.filePath, dr.routePathOperands, repoConstants) : null; if (value === null) { skipped++; diff --git a/gitnexus/src/core/ingestion/route-extractors/constant-resolver.ts b/gitnexus/src/core/ingestion/route-extractors/constant-resolver.ts index 509d1de99..c96406d87 100644 --- a/gitnexus/src/core/ingestion/route-extractors/constant-resolver.ts +++ b/gitnexus/src/core/ingestion/route-extractors/constant-resolver.ts @@ -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 diff --git a/gitnexus/src/core/ingestion/route-extractors/java-const-resolver.ts b/gitnexus/src/core/ingestion/route-extractors/java-const-resolver.ts new file mode 100644 index 000000000..0ca5e71e2 --- /dev/null +++ b/gitnexus/src/core/ingestion/route-extractors/java-const-resolver.ts @@ -0,0 +1,630 @@ +/** + * Java binding for the language-agnostic constant resolver (#2391 core). + * + * Supplies the two Java-specific pieces the shared fold in + * `constant-resolver.ts` needs — {@link resolveJavaImport} (import-specifier → + * file, honoring JVM package/classpath rules) and + * {@link extractJavaModuleConstants} (tree → {@link ModuleConstants}) — plus a + * pre-bound {@link resolveJavaConstant} wrapper so callers stay + * language-oblivious. The reusable fold, the cycle guard, and the depth cap + * all live in the agnostic core. + * + * Java constant shape (one per type declaration; nested classes flatten into + * the same file-level namespace, mirroring how `Outer.CONST` and a top-level + * `CONST` are indistinguishable at the fold layer): + * + * public class ApiPathConstants { + * public static final String DIAGNOSIS_SAVE_V1 = "/api/v1/diagnosis/add"; + * public static final String API_CIS_SAVE_SUMMARY = API_CIS_V1 + "summary/save"; + * } + * + * Reference shapes at annotation sites this binding resolves: + * @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; + * import static com.winning.opt.diagnosis.api.constants.ApiPathConstants.API_CIS_V1; + * + * Keying (KTD4 parity with the Python binding): the repo map is keyed by + * unique POSIX file path. A Java import `com.a.b.CONSTS` resolves to the file + * whose path ends with `com/a/b/CONSTS.java`; when 2+ files share that suffix + * the import is ambiguous and returns null (skip floor), never a wrong path. + */ + +import type Parser from 'tree-sitter'; +import { unquoteSpringLiteral } from './spring-shared.js'; +import { + MAX_FOLD_LENGTH, + type ImportBinding, + type ImportResolver, + type ModuleConstants, + type Operand, + type RepoConstants, +} from './constant-resolver.js'; + +export type { + ImportBinding, + ModuleConstants, + Operand, + 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: + * - a `static` … `String NAME =` declaration, with the modifier run matched as + * a span so every legal order works (`static public final String`, + * `public final static String`) and so `java.lang.String` — which the + * extractor accepts — is admitted too. + * - an `interface` declaration carrying a String assignment — interface fields + * are implicitly `public static final` (JLS 9.3), so a pure constant + * interface has neither keyword and no import. The assignment conjunct keeps + * a file whose PROSE merely mentions "interface " from costing a parse. + */ +// `static` … `String NAME =` on one declaration. The modifier run is matched as +// a span rather than as the adjacent pair `static final`, because the extractor +// scans modifiers INDEPENDENTLY (`isStaticFinal`) and Java lets them appear in +// any order — `static public final String`, `public final static String` — and +// because the type may be written out as `java.lang.String`, which the +// extractor also accepts. A gate narrower than the extractor it feeds is the +// same defect class as the ingestion/group divergence this predicate exists to +// prevent, just one layer down. +// +// The span excludes `;{}()` so it cannot jump a statement or block boundary: a +// local `String s = "x"` inside `static void f() { … }` is not matched, because +// reaching it from `static` crosses `(`, `)` and `{`. `final` is not required +// even though the extractor requires it — the gate may be wider than the +// extractor, never narrower. +const STATIC_STRING_CONSTANT_RE = /\bstatic\b[^;{}()]{0,80}\bString\s+\w+\s*=/; +const INTERFACE_DECL_RE = /\binterface\s+\w/; +const STRING_ASSIGNMENT_RE = /\bString\s+\w+\s*=/; + +export function isJavaConstantFile(source: string): boolean { + if (STATIC_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 + * exactly one file. + * + * `com.winning.opt.X.ApiPathConstants` → the file key ending in + * `com/winning/opt/X/ApiPathConstants.java`. Because the repo map is + * 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 + * 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) => { + // 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 + // trimming the last segment when the direct hit fails covers both shapes. + const asPath = moduleSpec.replace(/\./g, '/'); + const classFile = `${asPath}.java`; + + // Exact package-path suffix match, unique or nothing. + let hit: string | null = null; + for (const key of repoKeys) { + if (key === classFile || key.endsWith(`/${classFile}`)) { + if (hit !== null) return null; // 2+ modules carry this FQN — unresolvable + hit = key; + } + } + return hit; +}; + +/** + * 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; + // 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); +} + +/** + * Flatten a qualified-name expression (`ApiPaths`, `com.example.ApiPaths`) to + * its dotted text, or null when any segment is not a plain identifier (calls, + * `this`, array access, generics — not a static constant shape). + */ +function flattenQualifiedIdentifier(node: Parser.SyntaxNode): string | null { + if (node.type === 'identifier') return node.text; + if (node.type === 'field_access') { + const object = node.childForFieldName('object'); + const field = node.childForFieldName('field'); + if (object && field) { + const head = flattenQualifiedIdentifier(object); + return head === null ? null : `${head}.${field.text}`; + } + } + return null; +} + +/** + * Parse a Java constant initializer into an operand list, or null when it is + * not a foldable string expression. Handles a bare string literal, a bare + * identifier (`X = Y`), qualified/static-import-free references + * (`X = CONSTS.Y` — recorded as ONE ref named `CONSTS.Y`), and + * left-associative `+` chains of the three. Everything else — numbers, calls, + * ternaries, method refs, `String.format`, enum constants — returns null, + * which makes the constant unresolvable (→ skip floor), never a wrong value. + */ +export function parseJavaConstOperands( + node: Parser.SyntaxNode | null | undefined, + depth = 0, +): Operand[] | null { + if (!node) return null; + if (depth > 64) return null; + if (node.type === 'string_literal') { + const value = stringLiteralValue(node); + return value === null ? null : [{ kind: 'literal', value }]; + } + if (node.type === 'identifier') { + return [{ kind: 'ref', name: node.text }]; + } + // `CONSTS.FIELD` — field_access in tree-sitter-java for expressions. The + // object side may itself be a chain (`com.example.ApiPaths` parses as + // nested field_access), so flatten recursively: every segment must be a + // plain identifier/keyword to qualify (a call `f().X`, `this.X`, or an + // array access object side is not a constant shape → null, skip floor). + if (node.type === 'field_access') { + const object = node.childForFieldName('object'); + const field = node.childForFieldName('field'); + if (object && field) { + const objectName = flattenQualifiedIdentifier(object); + if (objectName !== null) return [{ kind: 'ref', name: `${objectName}.${field.text}` }]; + } + return null; + } + if (node.type === 'binary_expression') { + const isPlus = (node.children ?? []).some((c) => c.type === '+'); + if (!isPlus) return null; + const left = parseJavaConstOperands(node.childForFieldName('left'), depth + 1); + const right = parseJavaConstOperands(node.childForFieldName('right'), depth + 1); + if (left === null || right === null) return null; + return [...left, ...right]; + } + return null; +} + +/** + * Extract the file-level string constants and import bindings of one parsed + * Java file into the {@link ModuleConstants} shape the resolver consumes. + * + * Constants: every `static final String NAME = …` field of every type + * declaration in the file (nested classes included — their simple names + * would collide at the fold layer, but qualified refs carry the class name + * so nesting only matters for same-name fields, which flatten last-wins). + * Interface constants (`String NAME = "…"`) are implicitly static final and + * are collected too. + * + * References to OTHER constants via qualified names (`ApiPathConstants.X`) + * are stored as refs named `ApiPathConstants.X`; at the fold layer such a ref + * resolves through the import map (`ApiPathConstants` → module) followed by + * field lookup in the target file's OWN class-name-qualified namespace. To + * support that, constant names are ALSO recorded under + * `.` (both spellings share one entry). + * + * Last-wins in source order; a non-foldable rebind (`X = compute()`) drops X + * to unresolvable rather than keeping a stale literal. + */ +export function extractJavaModuleConstants(tree: Parser.Tree): ModuleConstants { + const literals = new Map(); + const exprs = new Map(); + const imports = new Map(); + + // Pass 1: imports (both shapes). + const walkImports = (node: Parser.SyntaxNode): void => { + if (node.type === 'import_declaration') { + // import a.b.C; | import static a.b.C; | import static a.b.C.F; + const isStatic = node.children.some((c) => c.type === 'static' && c.text === 'static'); + const scoped = node.children.find((c) => c.type === 'scoped_identifier'); + if (scoped) { + const text = scoped.text; + const lastDot = text.lastIndexOf('.'); + const fqn = text.slice(0, lastDot); + const name = text.slice(lastDot + 1); + if (isStatic) { + // import static a.b.C.F → local F from module a.b.C, original F. + imports.set(name, { module: fqn, originalName: name }); + } else { + // import a.b.C → module IS the class FQN; originalName is the class + // simple name. resolveJavaImport maps `a.b.C` → `a/b/C.java`. + imports.set(name, { module: text, originalName: name }); + } + } + } + for (const child of node.children ?? []) walkImports(child); + }; + walkImports(tree.rootNode); + + // Pass 2: constants. A field declaration is a constant when it is + // `static final` (explicit) or inside an interface (implicit). + const isStaticFinal = (modifiers: Parser.SyntaxNode | null | undefined): boolean => { + if (!modifiers) return false; + let sawStatic = false; + let sawFinal = false; + for (const m of modifiers.children ?? []) { + if (m.type === 'static') sawStatic = true; + if (m.type === 'final') sawFinal = true; + } + return sawStatic && sawFinal; + }; + + const collectFieldConstants = ( + classBody: Parser.SyntaxNode, + insideInterface: boolean, + declaringClass: string | null, + ): void => { + for (const member of classBody.children ?? []) { + // tree-sitter-java: interface fields are `constant_declaration`, class + // fields are `field_declaration`. Both carry `variable_declarator`s. + if (member.type !== 'field_declaration' && member.type !== 'constant_declaration') continue; + const mods = member.children.find((c) => c.type === 'modifiers'); + if (!insideInterface && !isStaticFinal(mods)) continue; + // Type must be String (java.lang.String is implicit-imported). + const typeNode = member.childForFieldName('type'); + if (!typeNode) continue; + const typeText = typeNode.text; + if (typeText !== 'String' && typeText !== 'java.lang.String') continue; + + const declarators = member.children.filter((c) => c.type === 'variable_declarator'); + for (const decl of declarators) { + const nameNode = decl.childForFieldName('name'); + const valueNode = decl.childForFieldName('value'); + if (!nameNode) continue; + const name = nameNode.text; + const operands = parseJavaConstOperands(valueNode); + // Same-name shadowing across nested types (legal Java, unlike + // same-class redeclaration): a later binding must REPLACE the earlier + // flattened simple-name entry — including dropping it to unresolvable + // when the new initializer is not foldable (`X = compute()`) — rather + // than leave the stale outer literal resolvable. Skip floor, mirroring + // Python #2391's rebind-drop. Qualified `Class.FIELD` aliases are + // per-type-keyed but same-named nested types can still collide, so + // they get the same replace/drop treatment. + const qname = declaringClass ? `${declaringClass}.${name}` : null; + 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); + } + continue; + } + const literalValue = + operands.length === 1 && operands[0].kind === 'literal' + ? (operands[0] as { value: string }).value + : null; + if (literalValue !== null) { + literals.set(name, literalValue); + exprs.delete(name); + } else { + exprs.set(name, operands); + literals.delete(name); + } + // Qualified alias: `CONSTS.X` refs (folded refs carry the class name). + if (qname) { + if (literalValue !== null) { + literals.set(qname, literalValue); + exprs.delete(qname); + } else { + exprs.set(qname, operands); + literals.delete(qname); + } + } + } + } + }; + + const walkTypes = (node: Parser.SyntaxNode, insideInterface: boolean): void => { + for (const child of node.children ?? []) { + const isInterface = child.type === 'interface_declaration'; + // 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); + + return { literals, exprs, imports: imports as Map }; +} + +/** + * 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; + readonly visited: Set; + readonly memo: Map; +} + +function newFoldState(repo: RepoConstants): JavaFoldState { + const constantKeys = new Set(); + 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 + * {@link resolveJavaImport}, or null when it cannot be fully folded. + * + * `name` may be simple (`DIAGNOSIS_SAVE_V1`, resolved via static import or + * same-file constant) or qualified (`ApiPathConstants.DIAGNOSIS_SAVE_V1`, + * resolved via the class import + the target file's qualified alias). + */ +export function resolveJavaConstant( + fileKey: string, + name: string, + repo: RepoConstants, + depth = 0, +): string | null { + return resolveWithState(fileKey, name, newFoldState(repo), depth); +} + +function resolveWithState( + fileKey: string, + name: string, + state: JavaFoldState, + depth: number, +): string | null { + if (depth > 32) return null; + 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 imp = repo.get(fileKey)?.imports.get(head); + if (imp) { + const targetFile = resolveJavaImport(fileKey, imp.module, constantKeys); + if (targetFile !== null) { + const qualified = resolveWithState(targetFile, `${head}.${tail}`, state, depth + 1); + if (qualified !== null) return qualified; + const bare = resolveWithState(targetFile, tail, state, depth + 1); + if (bare !== null) return bare; + } + return null; + } + // Un-imported qualified name (FQN form `com.a.b.C.FIELD`): try resolving + // the longest dotted prefix as a class import target. + 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, constantKeys); + if (targetFile !== null) { + const field = parts.slice(cut + 1).join('.'); + const declaring = parts[cut]; + const qualified = resolveWithState(targetFile, `${declaring}.${field}`, state, depth + 1); + if (qualified !== null) return qualified; + 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 + // `.` as well as its simple name, so a same-file + // qualified reference (`ApiPaths.X` inside ApiPaths.java) resolves below. + } + + // 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`, or null when any piece is unresolvable (skip floor). + */ +export function foldJavaOperands( + fileKey: string, + operands: readonly Operand[], + repo: RepoConstants, +): string | null { + const out = foldOperands(fileKey, operands, newFoldState(repo), 0); + return out === '' ? null : out; +} diff --git a/gitnexus/src/core/ingestion/route-extractors/spring.ts b/gitnexus/src/core/ingestion/route-extractors/spring.ts index 5c0b0637b..36828a1f5 100644 --- a/gitnexus/src/core/ingestion/route-extractors/spring.ts +++ b/gitnexus/src/core/ingestion/route-extractors/spring.ts @@ -30,6 +30,7 @@ import { unquoteSpringLiteral, type SharedSpringType, } from './spring-shared.js'; +import { parseJavaConstOperands } from './java-const-resolver.js'; /** * Single predicate-free tree-sitter query that captures all route annotations @@ -53,6 +54,13 @@ import { * 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, @@ -90,6 +98,42 @@ 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 + name: [(identifier) (scoped_identifier)] @ann + arguments: (annotation_argument_list + [(identifier) @value_expr + (field_access) @value_expr + (binary_expression) @value_expr])))) @node + (method_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 ] `, ); @@ -122,6 +166,11 @@ export function extractSpringRoutes( // class-array cross-product support is out of scope here. const prefixByClassId = new Map(); const classesWithArrayPrefix = new Set(); + // 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(); const classHttpMethodsById = new Map(); for (const match of TYPE_DECLARATION_QUERY.matches(tree.rootNode)) { const typeNode = match.captures.find((capture) => capture.name === 'type')?.node; @@ -139,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; @@ -166,7 +220,11 @@ export function extractSpringRoutes( const node = caps['node']; const valueNode = caps['value']; const keyNode = caps['key']; - if (!annNode || !node || !valueNode) continue; + // A constant-referencing value arrives as @value_expr, not @value — the + // match carries exactly one of the two. Require @value only when no + // @value_expr is present; the operand branch below folds the expression. + const valueExprCapture = match.captures.find((c) => c.name === 'value_expr')?.node ?? null; + if (!annNode || !node || (!valueNode && !valueExprCapture)) continue; if (node.type !== 'method_declaration') continue; @@ -181,8 +239,12 @@ export function extractSpringRoutes( if (methodMethods.length === 0) continue; if (!isRouteMemberKey(keyNode)) continue; - const routePath = unquoteSpringLiteral(valueNode.text); - if (routePath === null) continue; + // #2391-style non-literal path (constant ref or `+`-concat): emit with + // operands for cross-file folding in the parse phase. The match carries + // either @value (literal) or @value_expr (non-literal) — never both. + const valueExprNode = valueExprCapture; + const routePath = valueNode ? unquoteSpringLiteral(valueNode.text) : null; + if (routePath === null && !valueExprNode) continue; const enclosingType = findEnclosingType(node); // Interface-declared `@*Mapping`s are not concrete routes on their own — the @@ -206,10 +268,20 @@ export function extractSpringRoutes( // scan — safe under routeCoverage:'partial'. Full class-array cross-product // support is tracked in #2280. (Scalar method paths under an array class // prefix are left unchanged: that pre-existing divergence is out of scope.) - const isArrayElement = valueNode.parent?.type === 'element_value_array_initializer'; + const isArrayElement = valueNode?.parent?.type === 'element_value_array_initializer'; 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 @@ -217,6 +289,25 @@ export function extractSpringRoutes( const handlerName = node.childForFieldName('name')?.text; for (const httpMethod of httpMethods) { + if (routePath === null && valueExprNode) { + // Non-literal annotation value: parse operands now; the parse phase + // folds them against the repo-wide Java constant map (KTD5 skip floor + // on failure — never a phantom `POST /`). + const operands = parseJavaConstOperands(valueExprNode); + if (operands === null) continue; + routes.push({ + filePath, + routePath: '', + routePathExpr: valueExprNode.text, + routePathOperands: operands, + httpMethod, + decoratorName: ann, + lineNumber: annNode.startPosition.row + lineOffset, + ...(classPrefix ? { prefix: classPrefix } : {}), + ...(handlerName ? { handlerName } : {}), + }); + continue; + } routes.push({ filePath, routePath, @@ -233,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)) { diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index d9fb0fd67..b8417c7a6 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -141,6 +141,7 @@ import { templateConstraintsIdTag, } from '../utils/template-arguments.js'; import type { LanguageProvider } from '../language-provider.js'; +import { shouldHarvestModuleConstants } from '../language-provider.js'; import type { ParsedFile } from 'gitnexus-shared'; import { extractParsedFile, type ScopeCaptureSourceKind } from '../scope-extractor-bridge.js'; import { @@ -1421,7 +1422,6 @@ export function extractORMQueries( import { extractFastAPIRouterBindings } from '../route-extractors/fastapi-router-bindings.js'; import { - extractPythonModuleConstants, parseConstOperands, type ModuleConstants, type Operand, @@ -2963,11 +2963,18 @@ const processFileGroup = ( (result.routerModuleAliases ??= []), (result.routerConstructorPrefixes ??= []), ); - // #2391: harvest module-level string constants + from-imports so parse-impl - // can resolve non-literal decorator route paths cross-file. Only emit for - // files that carry something resolvable (a constant definition or an import - // binding) to keep the aggregate bounded on large repos. - const constants = extractPythonModuleConstants(tree); + } + + // #2391/#2980: harvest module-level string constants + import bindings via + // the provider hook so parse-impl can resolve non-literal decorator route + // 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. + // A provider that declares no heuristic harvests unconditionally — see + // `shouldHarvestModuleConstants`, which owns that rule so it can be tested + // without booting a worker. + if (provider.extractModuleConstants && shouldHarvestModuleConstants(provider, 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 }); } diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index f5c8be27f..6de50176a 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -538,7 +538,38 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j // cache would replay unchanged worker results without those routes. Version 70 // then adds Spring non-HTTP handler side-channel facts (#2417 / #2891), so Java // and Kotlin caches persist scheduled, event, messaging, and managed-job facts. -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 diff --git a/gitnexus/test/unit/group/java-const-route-parity.test.ts b/gitnexus/test/unit/group/java-const-route-parity.test.ts new file mode 100644 index 000000000..cfdf652a3 --- /dev/null +++ b/gitnexus/test/unit/group/java-const-route-parity.test.ts @@ -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[] { + 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[] { + 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']); + }); +}); diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts index 17595f15b..cd353e800 100644 --- a/gitnexus/test/unit/incremental-parse-cache.test.ts +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -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); } }); diff --git a/gitnexus/test/unit/java-route-const-pipeline-e2e.test.ts b/gitnexus/test/unit/java-route-const-pipeline-e2e.test.ts new file mode 100644 index 000000000..4370edc31 --- /dev/null +++ b/gitnexus/test/unit/java-route-const-pipeline-e2e.test.ts @@ -0,0 +1,264 @@ +/** + * #2980 review round-2: COLD and WARM pipeline e2e for the provider-hook + * constant harvest (`extractModuleConstants` / `moduleConstantHeuristic` / + * `foldRoutePathOperands`). + * + * The maintainer's blocking finding: unit tests only exercised worker-gated + * helpers — never the REAL pipeline. A controller referencing constants from + * a class NOT named `*Constants` (e.g. `ApiPaths`) was silently dropped: + * the old content gate `/import ... [\\w.]*Constants/` never matched, the + * constants file never entered the import map, the route resolved to null and + * got skipped. + * + * This file drives the REAL `runChunkedParseAndResolve` with the REAL compiled + * dist worker (vitest auto-falls back to dist/core/ingestion/workers/ + * parse-worker.js) over a fixture repo shaped like the reviewer's example: + * + * repo/ + * src/main/java/com/example/ApiPaths.java — constants class NOT named + * *Constants (the High bug) + * src/main/java/com/example/UserController.java — @RequestMapping prefix + + * @PostMapping(ApiPaths.X) + + * 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 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 + * asserting against the old binary. (CI builds before vitest, so it runs.) + */ +import { beforeEach, afterEach, describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +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 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; + +// ── fixture repo (reviewer's exact High-finding shape) ──────────────────── +const API_PATHS = `package com.example.common; + +public class ApiPaths { + public static final String USERS = "/api/v1/users"; + public static final String ORDERS = "/api/v1/orders"; + public static final String V1 = "/api/v1"; +} +`; + +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; + +@RequestMapping("/users") +public class UserController { + + // Qualified ref via a class NOT named *Constants (High finding): the old + // gate dropped the whole route because ApiPaths fails the name pattern. + @PostMapping(ApiPaths.USERS) + public void create() {} + + // FQN-qualified form (F3): multi-segment field_access chain. + @GetMapping(com.example.common.ApiPaths.ORDERS) + public void list() {} + + // 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() {} +} +`; + +let repoDir: string; +let storageDir: string; + +function writeFixture(): { path: string; size: number }[] { + const files: [string, string][] = [ + ['src/main/java/com/example/common/ApiPaths.java', API_PATHS], + ['src/main/java/com/example/UserController.java', USER_CONTROLLER], + ]; + const out: { path: string; size: number }[] = []; + for (const [rel, content] of files) { + const full = path.join(repoDir, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, content); + out.push({ path: rel, size: Buffer.byteLength(content) }); + } + return out; +} + +/** + * The parse phase does not emit Route nodes itself — it returns the folded + * `decoratorRoutes` (the routes phase emits them downstream). Asserting on the + * folded paths at THIS seam is exactly the regression the maintainer asked + * for: the worker's harvest → provider heuristic → parse-impl fold, with the + * real dist worker. + */ +type PipelineResult = Awaited>; + +function foldedRoutesOf(result: PipelineResult): Array<{ path: string; method: string }> { + return (result.allDecoratorRoutes ?? []) + .filter((r) => typeof r.routePath === 'string') + .map((r) => ({ path: r.routePath, method: r.httpMethod })); +} + +async function runPipeline( + cache: ParseCache, + files: { path: string; size: number }[], +): Promise { + const kg = createKnowledgeGraph(); + return await runChunkedParseAndResolve( + kg, + files, + files.map((f) => f.path), + files.length, + repoDir, + Date.now(), + () => {}, + { workerPoolSize: 1, parseCache: cache }, + ); +} + +maybeDescribe('#2980 provider-hook constant harvest — real pipeline (cold + warm)', () => { + beforeEach(() => { + repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gnx-2980-cold-')); + storageDir = path.join(repoDir, '.gitnexus'); + }); + afterEach(() => { + for (const d of [repoDir]) fs.rmSync(d, { recursive: true, force: true }); + }); + + it('cold run: folds qualified / FQN / concat paths from a non-*Constants class', async () => { + const files = writeFixture(); + const cache: ParseCache = { + version: PARSE_CACHE_VERSION, + entries: new Map(), + usedKeys: new Set(), + storagePath: storageDir, + onDiskKeys: new Set(), + }; + + 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 + // @RequestMapping("/users") prefix join happens in the downstream routes + // phase — at this seam we assert the method-level folded paths.) + const paths = routes.map((r) => r.path).sort(); + expect(paths).toContain('/api/v1/users'); // qualified ref via import + expect(paths).toContain('/api/v1/orders'); // FQN multi-segment chain + // The concat route folds to the same literal as the FQN route. + expect(paths.filter((p) => p === '/api/v1/orders').length).toBeGreaterThanOrEqual(2); + // Skip floor: no phantom empty/raw-expr paths. + for (const p of paths) { + expect(p.length).toBeGreaterThan(1); + expect(p).not.toContain('ApiPaths'); + expect(p).not.toContain('com.example'); + } + }, 120_000); + + it('warm run: parse-cache replay yields the identical folded route set', async () => { + const files = writeFixture(); + const cache: ParseCache = { + version: PARSE_CACHE_VERSION, + entries: new Map(), + usedKeys: new Set(), + storagePath: storageDir, + onDiskKeys: new Set(), + }; + + // 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); + 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) + .map((r) => `${r.method} ${r.path}`) + .sort(); + const hot = foldedRoutesOf(run2) + .map((r) => `${r.method} ${r.path}`) + .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); +}); diff --git a/gitnexus/test/unit/java-route-const-resolver.test.ts b/gitnexus/test/unit/java-route-const-resolver.test.ts new file mode 100644 index 000000000..0b628f52e --- /dev/null +++ b/gitnexus/test/unit/java-route-const-resolver.test.ts @@ -0,0 +1,794 @@ +/** + * Java route-path constant resolution (#2391 Java binding). + * + * Fixtures sampled from REAL Winning Health WiNEX-Outpatient source shapes + * (lesson from the vendor-alias PR #2883 review: hand-written textbook + * fixtures missed the dominant real-world spelling — 1198 constant-ref + * routes vs 2 literals in the real repo). + * + * 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 + * - `@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) + * - 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'; +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); + +function parse(src: string): Parser.Tree { + return parser.parse(src); +} + +/** Build a RepoConstants map from virtual files: { 'a/b/C.java': source }. */ +function repoOf(files: Record): RepoConstants { + const map = new Map(); + for (const [key, src] of Object.entries(files)) { + map.set(key, extractJavaModuleConstants(parse(src))); + } + return map; +} + +// ─── Real WiNEX shapes ──────────────────────────────────────────────────── + +const CONSTANTS_FILE = `package com.winning.opt.diagnosis.api.constants; + +import static com.winning.opt.common.constants.api.ApiPath.API_CIS_V1; + +public class ApiPathConstants { + + private ApiPathConstants() { + } + + public static final String DIAGNOSIS_SAVE_V1 = "/api/v1/app_record_cis_outpatient_diagnosis/encounter_diagnosis/add"; + + public static final String DIAGNOSIS_SAVE_V2 = "/api/v2/app_record_cis_outpatient_diagnosis/encounter_diagnosis/add"; + + public static final String API_CIS_SAVE_SUMMARY = API_CIS_V1 + "summary/save"; +}`; + +const COMMON_API_FILE = `package com.winning.opt.common.constants.api; + +public class ApiPath { + + public static final String API_CIS_V1 = "/api/v1/cis/"; +}`; + +const CONTROLLER_FILE = `package com.winning.opt.diagnosis.controller; + +import com.winning.opt.diagnosis.api.constants.ApiPathConstants; + +public class DiagnosisController { + + @PostMapping(ApiPathConstants.DIAGNOSIS_SAVE_V1) + public String save() { return "{}"; } + + @PostMapping(value = ApiPathConstants.DIAGNOSIS_SAVE_V2) + public String saveV2() { return "{}"; } + + @PostMapping(path = ApiPathConstants.API_CIS_SAVE_SUMMARY) + public String saveSummary() { return "{}"; } +}`; + +const STATIC_IMPORT_CONTROLLER = `package com.winning.opt.cis.controller; + +import static com.winning.opt.diagnosis.api.constants.ApiPathConstants.DIAGNOSIS_SAVE_V1; + +public class CisController { + + @PostMapping(DIAGNOSIS_SAVE_V1) + public String save() { return "{}"; } +}`; + +const INTERFACE_CONSTANTS_FILE = `package com.winning.opt.labtest.api.constants; + +public interface LabApiPath { + String LAB_QUERY_V1 = "/api/v1/labtest/query"; +}`; + +describe('extractJavaModuleConstants', () => { + it('collects static final String literals with class-qualified aliases', () => { + const mc = extractJavaModuleConstants(parse(CONSTANTS_FILE)); + expect(mc.literals.get('DIAGNOSIS_SAVE_V1')).toBe( + '/api/v1/app_record_cis_outpatient_diagnosis/encounter_diagnosis/add', + ); + expect(mc.literals.get('ApiPathConstants.DIAGNOSIS_SAVE_V1')).toBe( + '/api/v1/app_record_cis_outpatient_diagnosis/encounter_diagnosis/add', + ); + }); + + it('records composed constants as operand expressions', () => { + const mc = extractJavaModuleConstants(parse(CONSTANTS_FILE)); + const expr = mc.exprs.get('API_CIS_SAVE_SUMMARY'); + expect(expr).toEqual([ + { kind: 'ref', name: 'API_CIS_V1' }, + { kind: 'literal', value: 'summary/save' }, + ]); + }); + + it('records class and static imports', () => { + const mc = extractJavaModuleConstants(parse(CONTROLLER_FILE)); + expect(mc.imports.get('ApiPathConstants')).toEqual({ + module: 'com.winning.opt.diagnosis.api.constants.ApiPathConstants', + originalName: 'ApiPathConstants', + }); + const mcStatic = extractJavaModuleConstants(parse(STATIC_IMPORT_CONTROLLER)); + expect(mcStatic.imports.get('DIAGNOSIS_SAVE_V1')).toEqual({ + module: 'com.winning.opt.diagnosis.api.constants.ApiPathConstants', + originalName: 'DIAGNOSIS_SAVE_V1', + }); + }); + + it('collects interface constants (implicitly static final)', () => { + const mc = extractJavaModuleConstants(parse(INTERFACE_CONSTANTS_FILE)); + expect(mc.literals.get('LAB_QUERY_V1')).toBe('/api/v1/labtest/query'); + }); + + it('ignores non-static or non-String fields', () => { + const src = `package p; +public class C { + public static final int COUNT = 5; + public String instance = "x"; + static final String PRIVATE_OK = "/ok"; +}`; + const mc = extractJavaModuleConstants(parse(src)); + expect(mc.literals.has('COUNT')).toBe(false); + expect(mc.literals.has('instance')).toBe(false); + expect(mc.literals.get('PRIVATE_OK')).toBe('/ok'); + }); +}); + +describe('resolveJavaImport', () => { + const keys = new Set([ + 'winning-opt-diagnosis/src/main/java/com/winning/opt/diagnosis/api/constants/ApiPathConstants.java', + 'winning-opt-common/src/main/java/com/winning/opt/common/constants/api/ApiPath.java', + ]); + + it('resolves a package import to the unique path-suffix file', () => { + const hit = resolveJavaImport( + 'winning-opt-diagnosis/src/main/java/com/winning/opt/diagnosis/controller/DiagnosisController.java', + 'com.winning.opt.diagnosis.api.constants.ApiPathConstants', + keys, + ); + expect(hit).toBe( + 'winning-opt-diagnosis/src/main/java/com/winning/opt/diagnosis/api/constants/ApiPathConstants.java', + ); + }); + + it('resolves a static import (class.member → class file)', () => { + const hit = resolveJavaImport( + 'winning-opt-cis/src/main/java/com/winning/opt/cis/controller/CisController.java', + 'com.winning.opt.diagnosis.api.constants.ApiPathConstants', + keys, + ); + expect(hit).toBe( + 'winning-opt-diagnosis/src/main/java/com/winning/opt/diagnosis/api/constants/ApiPathConstants.java', + ); + }); + + it('returns null when the class does not exist in the repo map', () => { + const hit = resolveJavaImport('a/A.java', 'com.example.notthere.NoConst', keys); + expect(hit).toBeNull(); + }); +}); + +describe('resolveJavaConstant end-to-end (real repo shapes)', () => { + const repo = repoOf({ + 'winning-opt-diagnosis/src/main/java/com/winning/opt/diagnosis/api/constants/ApiPathConstants.java': + CONSTANTS_FILE, + 'winning-opt-common/src/main/java/com/winning/opt/common/constants/api/ApiPath.java': + COMMON_API_FILE, + }); + const controllerKey = + 'winning-opt-diagnosis/src/main/java/com/winning/opt/diagnosis/controller/DiagnosisController.java'; + + it('resolves qualified refs via the class import chain', () => { + // The controller imports ApiPathConstants; the ref name is qualified. + // Hand-rolled two-step: import resolves the class, qualified alias carries the field. + const mc = extractJavaModuleConstants(parse(CONTROLLER_FILE)); + const targetFile = resolveJavaImport( + controllerKey, + mc.imports.get('ApiPathConstants')!.module, + new Set(repo.keys()), + ); + expect(targetFile).toBeTruthy(); + const value = resolveJavaConstant(targetFile!, 'ApiPathConstants.DIAGNOSIS_SAVE_V1', repo); + expect(value).toBe('/api/v1/app_record_cis_outpatient_diagnosis/encounter_diagnosis/add'); + }); + + it('folds composed constants across files (static import + concat)', () => { + const mc = extractJavaModuleConstants(parse(CONSTANTS_FILE)); + const targetFile = resolveJavaImport( + 'winning-opt-diagnosis/src/main/java/com/winning/opt/diagnosis/api/constants/ApiPathConstants.java', + mc.imports.get('API_CIS_V1')!.module, + new Set(repo.keys()), + ); + expect(targetFile).toBe( + 'winning-opt-common/src/main/java/com/winning/opt/common/constants/api/ApiPath.java', + ); + const value = resolveJavaConstant( + 'winning-opt-diagnosis/src/main/java/com/winning/opt/diagnosis/api/constants/ApiPathConstants.java', + 'API_CIS_SAVE_SUMMARY', + repo, + ); + expect(value).toBe('/api/v1/cis/summary/save'); + }); + + it('floors to null on unresolvable names (skip, never guess)', () => { + expect(resolveJavaConstant(controllerKey, 'NOT_A_THING', repo)).toBeNull(); + }); +}); + +describe('parseJavaConstOperands', () => { + it('parses a bare identifier ref', () => { + const tree = parse(`package p; public class C { static final String X = Y; }`); + let valueNode: Parser.SyntaxNode | null = null; + const walk = (n: Parser.SyntaxNode): void => { + if (n.type === 'variable_declarator') { + const v = n.childForFieldName('value'); + if (v) valueNode = v; + } + for (const c of n.children ?? []) walk(c); + }; + walk(tree.rootNode); + expect(parseJavaConstOperands(valueNode)).toEqual([{ kind: 'ref', name: 'Y' }]); + }); + + it('parses left-associative + chains', () => { + const tree = parse(`package p; public class C { static final String X = A + "/b" + C; }`); + let valueNode: Parser.SyntaxNode | null = null; + const walk = (n: Parser.SyntaxNode): void => { + if (n.type === 'variable_declarator') { + const v = n.childForFieldName('value'); + if (v) valueNode = v; + } + for (const c of n.children ?? []) walk(c); + }; + walk(tree.rootNode); + expect(parseJavaConstOperands(valueNode)).toEqual([ + { kind: 'ref', name: 'A' }, + { kind: 'literal', value: '/b' }, + { kind: 'ref', name: 'C' }, + ]); + }); + + it('returns null for calls and non-string shapes', () => { + const tree = parse( + `package p; public class C { static final String X = String.format("%s", a); }`, + ); + let valueNode: Parser.SyntaxNode | null = null; + const walk = (n: Parser.SyntaxNode): void => { + if (n.type === 'variable_declarator') { + const v = n.childForFieldName('value'); + if (v) valueNode = v; + } + for (const c of n.children ?? []) walk(c); + }; + walk(tree.rootNode); + expect(parseJavaConstOperands(valueNode)).toBeNull(); + }); +}); + +// ── Ingestion extractor level: constant-referencing annotation values ── +// (regression for the review finding where the route loop's `!valueNode` +// guard dropped every @value_expr match before the operand branch ran) +describe('extractSpringRoutes constant value', () => { + it('emits routePathExpr + operands for @Mapping(CONSTS.X)', async () => { + const { extractSpringRoutes } = + await import('../../src/core/ingestion/route-extractors/spring.js'); + const tree = parser.parse(` +package com.winning.opt.demo; +public class DemoController { + @org.springframework.web.bind.annotation.PostMapping(ApiPathConstants.DIAGNOSIS_SAVE_V1) + public String save() { return "ok"; } +}`); + const routes = extractSpringRoutes(tree, 'DemoController.java', 0); + 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 () => { + const { extractSpringRoutes } = + await import('../../src/core/ingestion/route-extractors/spring.js'); + const tree = parser.parse(` +package com.winning.opt.demo; +public class DemoController { + @org.springframework.web.bind.annotation.PostMapping("/literal/path") + public String save() { return "ok"; } +}`); + const routes = extractSpringRoutes(tree, 'DemoController.java', 0); + expect(routes.length).toBe(1); + expect(routes[0].routePath).toBe('/literal/path'); + expect(routes[0].routePathExpr).toBe(undefined); + }); +}); + +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"; +}`, + }); + // In-file expr records the qualified ref `SelfConsts.X`; resolving it + // re-enters the same file via the (self) import head — must hit the depth + // cap, not the V8 stack. + expect( + resolveJavaConstant('src/main/java/com/example/SelfConsts.java', 'SelfConsts.X', repo), + ).toBeNull(); + }); + + it('mutual imports: A.X -> B.Y -> A.X terminates with null', () => { + const repo = repoOf({ + 'src/main/java/com/example/AConsts.java': `package com.example; +import com.example.BConsts; +public class AConsts { + public static final String X = BConsts.Y; +}`, + 'src/main/java/com/example/BConsts.java': `package com.example; +import com.example.AConsts; +public class BConsts { + public static final String Y = AConsts.X; +}`, + }); + expect( + resolveJavaConstant('src/main/java/com/example/AConsts.java', 'AConsts.X', repo), + ).toBeNull(); + }); +}); + +// ─── Review round 2 regressions (#2980) ─────────────────────────────────── + +describe('F4: class nested in an interface is NOT implicitly final', () => { + const SRC = `package p; +public interface Api { + String BASE = "/api"; + class Holder { + String mutable = "/mutable"; + static final String OK = "/ok"; + } + interface Inner { + String IMPLICIT = "/implicit"; + class Deep { + String alsoMutable = "/also"; + } + } +}`; + + it('harvests the interface own fields and explicit static final nested fields', () => { + const mc = extractJavaModuleConstants(parse(SRC)); + expect(mc.literals.get('BASE')).toBe('/api'); + expect(mc.literals.get('OK')).toBe('/ok'); + expect(mc.literals.get('Holder.OK')).toBe('/ok'); + }); + + it('does NOT harvest mutable fields of a class nested in an interface', () => { + const mc = extractJavaModuleConstants(parse(SRC)); + expect(mc.literals.has('mutable')).toBe(false); + expect(mc.literals.has('alsoMutable')).toBe(false); + expect(mc.literals.has('Holder.mutable')).toBe(false); + expect(mc.exprs.has('mutable')).toBe(false); + }); + + it('still harvests a class directly nested in an interface (own implicit semantics recomputed at each boundary)', () => { + const mc = extractJavaModuleConstants(parse(SRC)); + expect(mc.literals.get('IMPLICIT')).toBe('/implicit'); + expect(mc.literals.get('Inner.IMPLICIT')).toBe('/implicit'); + }); +}); + +describe('F5: same-name shadowing across nested types drops the stale entry', () => { + const SRC = `package p; +public class Outer { + public static final String PATH = "/v1"; + static class Inner { + // shadows Outer.PATH with a non-foldable initializer + public static final String PATH = compute(); + static String compute() { return "/v2"; } + } +}`; + + it('a non-foldable shadow must drop the outer literal, not keep it (skip floor)', () => { + const mc = extractJavaModuleConstants(parse(SRC)); + expect(mc.literals.has('PATH')).toBe(false); + expect(mc.exprs.has('PATH')).toBe(false); + }); + + it('qualified aliases survive per class (Outer.PATH resolvable, Inner.PATH not)', () => { + const mc = extractJavaModuleConstants(parse(SRC)); + expect(mc.literals.get('Outer.PATH')).toBe('/v1'); + expect(mc.literals.has('Inner.PATH')).toBe(false); + }); + + it('a foldable shadow REPLACES the outer value (last binding wins in source order)', () => { + const src = `package p; +public class Outer { + public static final String PATH = "/v1"; + static class Inner { + public static final String PATH = "/v2"; + } +}`; + const mc = extractJavaModuleConstants(parse(src)); + expect(mc.literals.get('PATH')).toBe('/v2'); + expect(mc.literals.get('Outer.PATH')).toBe('/v1'); + expect(mc.literals.get('Inner.PATH')).toBe('/v2'); + }); +}); + +describe('F3: multi-segment FQN annotation values and constant initializers', () => { + const constValueOf = (src: string): Parser.SyntaxNode => { + const cls = parse(src).rootNode.descendantsOfType('class_declaration')[0]!; + const body = cls.childForFieldName('body')!; + const field = body.children.find((c) => c.type === 'field_declaration')!; + const decl = field.children.find((c) => c.type === 'variable_declarator')!; + return decl.childForFieldName('value')!; + }; + + it('parses com.example.ApiPaths.USERS as ONE ref (nested field_access chain flattened)', () => { + const ops = parseJavaConstOperands( + constValueOf(`package p; +public class W { + public static final String X = com.example.ApiPaths.USERS; +}`), + ); + expect(ops).toEqual([{ kind: 'ref', name: 'com.example.ApiPaths.USERS' }]); + }); + + it('still rejects call/object-side chains: f().X, this.X, arr[0].X', () => { + expect( + parseJavaConstOperands( + constValueOf(`package p; +public class W { public static final String A = f().X; static Object f(){return null;} }`), + ), + ).toBeNull(); + expect( + parseJavaConstOperands( + constValueOf(`package p; +public class W { public static final String B = this.Y; String Y = "y"; }`), + ), + ).toBeNull(); + expect( + parseJavaConstOperands( + constValueOf(`package p; +public class W { public static final String C = arr[0].Z; }`), + ), + ).toBeNull(); + }); + + it('resolves an FQN-qualified annotation constant end-to-end (query → operands → fold)', () => { + const repo = repoOf({ + 'src/main/java/com/example/ApiPaths.java': `package com.example; +public class ApiPaths { + public static final String USERS = "/api/v1/users"; +}`, + 'src/main/java/com/example/Ctl.java': `package com.example; +import org.springframework.web.bind.annotation.PostMapping; +public class Ctl { + @PostMapping(com.example.ApiPaths.USERS) + public void list() {} +}`, + }); + // The whole FQN arrives as one ref operand (verified against the real + // tree-sitter-java parse shape); the resolver must follow it via the + // longest-prefix import fallback. + expect( + resolveJavaConstant('src/main/java/com/example/Ctl.java', 'com.example.ApiPaths.USERS', repo), + ).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"; }`, + ], + [ + 'modifiers interleaved', + `package com.x; +public class P { static public final String SAVE = "/api/v1/save"; }`, + ], + [ + 'fully-qualified java.lang.String', + `package com.x; +public class P { public static final java.lang.String SAVE = "/api/v1/save"; }`, + ], + [ + 'fully-qualified type in an interface', + `package com.x; +public interface P { java.lang.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.each([ + [ + 'no constant-bearing syntax', + `package com.x; +public class P { void run() { System.out.println("/not-a-constant"); } }`, + ], + [ + 'a local String inside a static method', + `package com.x; +public class P { static void run() { String s = "/local"; } }`, + ], + [ + 'prose that merely mentions an interface', + `/** interface EXTENDS (#1951). */ +public class A { void f() {} }`, + ], + ])('still skips %s', (_name, src) => { + expect(isJavaConstantFile(src)).toBe(false); + expect(extractJavaModuleConstants(parse(src)).literals.size).toBe(0); + }); +}); + +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); + }); +}); diff --git a/gitnexus/test/unit/python-const-resolver.test.ts b/gitnexus/test/unit/python-const-resolver.test.ts index 1f850567e..3a06eaebc 100644 --- a/gitnexus/test/unit/python-const-resolver.test.ts +++ b/gitnexus/test/unit/python-const-resolver.test.ts @@ -23,6 +23,8 @@ import { type ImportBinding, type RepoConstants, } from '../../src/core/ingestion/route-extractors/python-const-resolver.js'; +import { pythonProvider } from '../../src/core/ingestion/languages/python.js'; +import { shouldHarvestModuleConstants } from '../../src/core/ingestion/language-provider.js'; const lit = (value: string): Operand => ({ kind: 'literal', value }); const ref = (name: string): Operand => ({ kind: 'ref', name }); @@ -377,3 +379,46 @@ 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'], + ])('the worker GATE admits the %s shape, and the extractor harvests it', (_name, src) => { + // Drive the gate the worker actually evaluates, not just the extractor + // behind it. Asserting only on `extract(src)` would stay green if the worker + // went back to `provider.moduleConstantHeuristic?.(content)` — undefined read + // as "skip" — which is precisely the regression this pins. + expect(shouldHarvestModuleConstants(pythonProvider, src)).toBe(true); + const mc = extract(src); + expect(mc.literals.size + mc.exprs.size + mc.imports.size).toBeGreaterThan(0); + }); + + it('a provider with no extractModuleConstants is never harvested', () => { + expect(shouldHarvestModuleConstants({}, 'API = "/api"')).toBe(false); + }); + + it('a declared heuristic still gates the harvest', () => { + const provider = { + extractModuleConstants: pythonProvider.extractModuleConstants, + moduleConstantHeuristic: (content: string) => content.includes('ROUTES'), + }; + expect(shouldHarvestModuleConstants(provider, 'ROUTES = "/a"')).toBe(true); + expect(shouldHarvestModuleConstants(provider, 'OTHER = "/a"')).toBe(false); + }); +});