diff --git a/gitnexus/src/core/group/extractors/http-patterns/java.ts b/gitnexus/src/core/group/extractors/http-patterns/java.ts index 0c83eaab4..4eba0c1f1 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/java.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/java.ts @@ -7,7 +7,8 @@ import { type LanguagePatterns, } from '../tree-sitter-scanner.js'; import { - METHOD_ANNOTATION_TO_HTTP, + springAnnotationHttpMethods, + intersectSpringHttpMethods, isRouteMemberKey, findEnclosingClass, joinPath, @@ -44,7 +45,7 @@ import type { /** * Java HTTP plugin. Handles: - * - Spring `@RequestMapping` class prefixes + `@(Get|Post|...)Mapping` method annotations + * - Spring `@RequestMapping` class prefixes + shortcut/`@RequestMapping` method annotations * - Spring `RestTemplate.getForObject/...`, `exchange(...)` * - Spring `WebClient.method(HttpMethod.X, ...)`, `WebClient.get().uri(...)` * - OkHttp `new Request.Builder().url("...")` @@ -408,6 +409,35 @@ function simpleName(text: string): string { return text.split('.').pop() ?? text; } +function declarationAnnotations(node: Parser.SyntaxNode): Parser.SyntaxNode[] { + const modifiers = node.namedChildren.find((child) => child.type === 'modifiers'); + if (!modifiers) return []; + return modifiers.namedChildren.filter( + (child) => child.type === 'annotation' || child.type === 'marker_annotation', + ); +} + +function annotationHasRouteMember(annotation: Parser.SyntaxNode): boolean { + const args = annotation.childForFieldName('arguments'); + if (!args) return false; + for (const child of args.namedChildren) { + if (child.type !== 'element_value_pair') return true; + const key = child.childForFieldName('key'); + if (isRouteMemberKey(key ?? undefined)) return true; + } + return false; +} + +function typeRequestMethods(typeNode: Parser.SyntaxNode): readonly string[] { + const mappings = declarationAnnotations(typeNode).filter( + (annotation) => + simpleName(annotation.childForFieldName('name')?.text ?? '') === 'RequestMapping', + ); + if (mappings.length === 0) return ['*']; + if (mappings.length !== 1) return []; + return springAnnotationHttpMethods('RequestMapping', mappings[0].text); +} + function hasAnnotation(node: Parser.SyntaxNode, names: string | readonly string[]): boolean { const modifiers = node.namedChildren.find((child) => child.type === 'modifiers'); if (!modifiers) return false; @@ -437,6 +467,8 @@ interface MethodRouteAnnotation { methodName: string | null; httpMethod: string; rawPath: string; + /** OpenFeign's single effective verb; null means its contract is invalid/ambiguous. */ + feignHttpMethod?: string | null; } interface RequestLineAnnotation { @@ -452,7 +484,7 @@ interface RouteAnnotationScan { feignPrefixByInterfaceId: Map; /** Spring HTTP Interface `@HttpExchange(url|value)` type-level prefixes per class/interface node id. */ httpExchangePrefixByTypeId: Map; - /** One entry per resolved Spring `@(Get|...)Mapping` route — a method with N mappings yields N entries. */ + /** 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. */ requestLines: RequestLineAnnotation[]; @@ -484,6 +516,7 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan { const methodRoutes: MethodRouteAnnotation[] = []; const requestLines: RequestLineAnnotation[] = []; const exchangeRoutes: MethodRouteAnnotation[] = []; + const httpMethodsByAnnotationId = new Map(); // Interface `@RequestMapping` prefixes rank below `@FeignClient(path)`; // collect them and apply only after the FeignClient pass below. const interfaceRequestMappingPrefixes: Array<{ id: number; prefix: string }> = []; @@ -505,18 +538,29 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan { const keyNode = captures.key; // undefined for the positional shape if (node.type === 'method_declaration') { - // Method-level: a Spring `@(Get|...)Mapping` route, or native `@RequestLine`. - const httpMethod = METHOD_ANNOTATION_TO_HTTP[ann]; - if (httpMethod) { + // Method-level: a Spring shortcut/`@RequestMapping` route, or native `@RequestLine`. + const annotationNode = annNode.parent; + if (!annotationNode) continue; + let httpMethods = httpMethodsByAnnotationId.get(annotationNode.id); + if (!httpMethods) { + httpMethods = springAnnotationHttpMethods(ann, annotationNode.text); + httpMethodsByAnnotationId.set(annotationNode.id, httpMethods); + } + if (httpMethods.length > 0) { + const feignHttpMethod = + httpMethods.length === 1 ? (httpMethods[0] === '*' ? 'GET' : httpMethods[0]) : null; if (!isRouteMemberKey(keyNode)) continue; const rawPath = unquoteLiteral(valueNode.text); if (rawPath !== null) { - methodRoutes.push({ - methodNode: node, - methodName: captures.member?.text ?? null, - httpMethod, - rawPath, - }); + for (const httpMethod of httpMethods) { + methodRoutes.push({ + methodNode: node, + methodName: captures.member?.text ?? null, + httpMethod, + rawPath, + feignHttpMethod, + }); + } } } else if (ann === 'RequestLine') { // Feign packs verb + path in one literal; its only named argument is `value`. @@ -573,6 +617,43 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan { } } + const classHttpMethodsByTypeId = new Map(); + for (const match of runCompiledPatterns(SPRING_TYPE_DECLARATION_PATTERNS, tree)) { + const typeNode = match.captures.type; + if (!typeNode) continue; + const classMethods = typeRequestMethods(typeNode); + classHttpMethodsByTypeId.set(typeNode.id, classMethods); + for (const methodNode of collectDirectMethods(typeNode)) { + for (const annotationNode of declarationAnnotations(methodNode)) { + if (annotationHasRouteMember(annotationNode)) continue; + const ann = simpleName(annotationNode.childForFieldName('name')?.text ?? ''); + const httpMethods = springAnnotationHttpMethods(ann, annotationNode.text); + if (httpMethods.length === 0) continue; + const feignHttpMethod = + httpMethods.length === 1 ? (httpMethods[0] === '*' ? 'GET' : httpMethods[0]) : null; + for (const httpMethod of httpMethods) { + methodRoutes.push({ + methodNode, + methodName: getNodeName(methodNode), + httpMethod, + rawPath: '', + feignHttpMethod, + }); + } + } + } + } + + const constrainedMethodRoutes = methodRoutes.flatMap((route) => { + const typeNode = + findEnclosingInterface(route.methodNode) ?? findEnclosingClass(route.methodNode); + const classMethods = typeNode ? (classHttpMethodsByTypeId.get(typeNode.id) ?? ['*']) : ['*']; + return intersectSpringHttpMethods(classMethods, [route.httpMethod]).map((httpMethod) => ({ + ...route, + httpMethod, + })); + }); + // `@RequestMapping` on a Feign interface is the fallback prefix, but only when // the interface has no `@FeignClient(path)` of its own (path wins). for (const { id, prefix } of interfaceRequestMappingPrefixes) { @@ -583,7 +664,7 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan { prefixByTypeId, feignPrefixByInterfaceId, httpExchangePrefixByTypeId, - methodRoutes, + methodRoutes: constrainedMethodRoutes, requestLines, exchangeRoutes, }; @@ -705,7 +786,7 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = { // ─── Spring providers + OpenFeign consumers (one query pass) ──── // `scanRouteAnnotations` resolves every route-defining annotation — - // class/interface prefixes, method `@(Get|...)Mapping`s and native + // class/interface prefixes, method shortcut/`@RequestMapping`s and native // `@RequestLine`s — from a single `matches()` pass over the tree. const { prefixByTypeId, @@ -724,12 +805,13 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = { for (const route of methodRoutes) { const enclosingInterface = findEnclosingInterface(route.methodNode); if (enclosingInterface && hasAnnotation(enclosingInterface, 'FeignClient')) { + if (!route.feignHttpMethod) continue; const prefixes = feignPrefixByInterfaceId.get(enclosingInterface.id) ?? ['']; for (const prefix of prefixes) { out.push({ role: 'consumer', framework: OPENFEIGN_FRAMEWORK, - method: route.httpMethod, + method: route.feignHttpMethod, path: joinPath(prefix, route.rawPath), name: route.methodName, line: route.methodNode.startPosition.row + 1, diff --git a/gitnexus/src/core/ingestion/route-extractors/spring-shared.ts b/gitnexus/src/core/ingestion/route-extractors/spring-shared.ts index b8a6d4bc7..d417d5ff4 100644 --- a/gitnexus/src/core/ingestion/route-extractors/spring-shared.ts +++ b/gitnexus/src/core/ingestion/route-extractors/spring-shared.ts @@ -17,6 +17,7 @@ */ import type Parser from 'tree-sitter'; +import { parseSpringAnnotationArguments } from '../frameworks/spring/annotation-arguments.js'; /** * Spring shortcut method-annotation → HTTP verb. @@ -34,6 +35,91 @@ export const METHOD_ANNOTATION_TO_HTTP: Record = { PatchMapping: 'PATCH', }; +/** + * Parse one `RequestMethod.X` literal or a Java annotation array of literals. + * An empty array is valid and means Spring's unrestricted/default method set. + * Runtime expressions fail closed instead of producing a guessed route. + */ +function parseRequestMethodValues(value: string): readonly string[] | null { + let trimmed = ''; + for (let index = 0; index < value.length; index += 1) { + const char = value[index]; + const next = value[index + 1]; + if (/\s/.test(char)) continue; + if (char === '/' && next === '*') { + const commentEnd = value.indexOf('*/', index + 2); + if (commentEnd < 0) return null; + index = commentEnd + 1; + continue; + } + if (char === '/' && next === '/') { + const lineEnd = value.slice(index + 2).search(/[\r\n]/); + if (lineEnd < 0) return null; + index += lineEnd + 1; + continue; + } + trimmed += char; + } + const hasOpeningBrace = trimmed.startsWith('{'); + const hasClosingBrace = trimmed.endsWith('}'); + if (hasOpeningBrace !== hasClosingBrace) return null; + const body = hasOpeningBrace ? trimmed.slice(1, -1).trim() : trimmed; + if (body.length === 0) return []; + if (!hasOpeningBrace && body.includes(',')) return null; + + const methods: string[] = []; + const parts = body.split(','); + if (hasOpeningBrace && parts[parts.length - 1].trim() === '') parts.pop(); + for (const rawPart of parts) { + const part = rawPart.trim(); + const match = + /^(?:(?:[A-Za-z_$][A-Za-z0-9_$]*\.)*RequestMethod\.)?(GET|HEAD|POST|PUT|PATCH|DELETE|OPTIONS|TRACE)$/.exec( + part, + ); + if (!match) return null; + if (!methods.includes(match[1])) methods.push(match[1]); + } + return methods; +} + +/** + * Resolve a method-level Spring mapping annotation to its HTTP method(s). + * + * Shortcut annotations have one implicit verb. `@RequestMapping` may declare + * one or more static `RequestMethod.X` values; when its `method` member is + * absent or an empty array, `'*'` preserves Spring's method-agnostic semantics. + * A present but non-static method expression yields no methods (fail closed). + */ +export function springAnnotationHttpMethods( + annotationName: string, + annotationText: string, +): readonly string[] { + const shortcut = METHOD_ANNOTATION_TO_HTTP[annotationName]; + if (shortcut) return [shortcut]; + if (annotationName !== 'RequestMapping') return []; + + const args = parseSpringAnnotationArguments(annotationText); + if (args === null) return []; + const methodArgs = args.filter((arg) => arg.name === 'method'); + if (methodArgs.length === 0) return ['*']; + if (methodArgs.length !== 1) return []; + + const methods = parseRequestMethodValues(methodArgs[0].value); + if (methods === null) return []; + return methods.length > 0 ? methods : ['*']; +} + +/** Intersect class- and method-level Spring mapping constraints. */ +export function intersectSpringHttpMethods( + classMethods: readonly string[], + methodMethods: readonly string[], +): readonly string[] { + if (classMethods.length === 0 || methodMethods.length === 0) return []; + if (classMethods.includes('*')) return methodMethods; + if (methodMethods.includes('*')) return classMethods; + return methodMethods.filter((method) => classMethods.includes(method)); +} + /** * A named annotation argument contributes a route only when its member key is * `path` or `value`; a positional argument (no key node) always qualifies. diff --git a/gitnexus/src/core/ingestion/route-extractors/spring.ts b/gitnexus/src/core/ingestion/route-extractors/spring.ts index 4933c4816..5c0b0637b 100644 --- a/gitnexus/src/core/ingestion/route-extractors/spring.ts +++ b/gitnexus/src/core/ingestion/route-extractors/spring.ts @@ -23,7 +23,8 @@ import Parser from 'tree-sitter'; import Java from 'tree-sitter-java'; import type { ExtractedDecoratorRoute } from '../workers/parse-worker.js'; import { - METHOD_ANNOTATION_TO_HTTP, + intersectSpringHttpMethods, + springAnnotationHttpMethods, isRouteMemberKey, findEnclosingType, unquoteSpringLiteral, @@ -60,14 +61,14 @@ const ROUTE_ANNOTATION_QUERY = new Parser.Query( (class_declaration (modifiers (annotation - name: (identifier) @ann + name: [(identifier) (scoped_identifier)] @ann arguments: (annotation_argument_list [(string_literal) @value (element_value_array_initializer (string_literal) @value)])))) @node (class_declaration (modifiers (annotation - name: (identifier) @ann + name: [(identifier) (scoped_identifier)] @ann arguments: (annotation_argument_list (element_value_pair key: (identifier) @key @@ -76,14 +77,14 @@ const ROUTE_ANNOTATION_QUERY = new Parser.Query( (method_declaration (modifiers (annotation - name: (identifier) @ann + name: [(identifier) (scoped_identifier)] @ann arguments: (annotation_argument_list [(string_literal) @value (element_value_array_initializer (string_literal) @value)])))) @node (method_declaration (modifiers (annotation - name: (identifier) @ann + name: [(identifier) (scoped_identifier)] @ann arguments: (annotation_argument_list (element_value_pair key: (identifier) @key @@ -121,6 +122,13 @@ export function extractSpringRoutes( // class-array cross-product support is out of scope here. const prefixByClassId = new Map(); const classesWithArrayPrefix = 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; + if (typeNode?.type === 'class_declaration') { + classHttpMethodsById.set(typeNode.id, typeRequestMethods(typeNode)); + } + } for (const match of matches) { const caps: Record = {}; @@ -133,7 +141,8 @@ export function extractSpringRoutes( const keyNode = caps['key']; if (!annNode || !node || !valueNode) continue; - if (node.type === 'class_declaration' && annNode.text === 'RequestMapping') { + const capturedAnnotationName = annNode.text.split('.').pop() ?? annNode.text; + if (node.type === 'class_declaration' && capturedAnnotationName === 'RequestMapping') { if (!isRouteMemberKey(keyNode)) continue; if (valueNode.parent?.type === 'element_value_array_initializer') { classesWithArrayPrefix.add(node.id); @@ -146,6 +155,7 @@ export function extractSpringRoutes( // Phase 2: collect method-level routes and resolve their class prefix const routes: ExtractedDecoratorRoute[] = []; + const httpMethodsByAnnotationId = new Map(); for (const match of matches) { const caps: Record = {}; @@ -160,9 +170,15 @@ export function extractSpringRoutes( if (node.type !== 'method_declaration') continue; - const ann = annNode.text; - const httpMethod = METHOD_ANNOTATION_TO_HTTP[ann]; - if (!httpMethod) continue; // skip @RequestMapping on methods (ambiguous verb) + const ann = annNode.text.split('.').pop() ?? annNode.text; + const annotationNode = annNode.parent; + if (!annotationNode) continue; + let methodMethods = httpMethodsByAnnotationId.get(annotationNode.id); + if (!methodMethods) { + methodMethods = springAnnotationHttpMethods(ann, annotationNode.text); + httpMethodsByAnnotationId.set(annotationNode.id, methodMethods); + } + if (methodMethods.length === 0) continue; if (!isRouteMemberKey(keyNode)) continue; const routePath = unquoteSpringLiteral(valueNode.text); @@ -177,6 +193,11 @@ export function extractSpringRoutes( if (enclosingType?.kind === 'interface') continue; const enclosingClass = enclosingType?.kind === 'class' ? enclosingType.node : null; + const httpMethods = intersectSpringHttpMethods( + enclosingClass ? (classHttpMethodsById.get(enclosingClass.id) ?? ['*']) : ['*'], + methodMethods, + ); + if (httpMethods.length === 0) continue; // Suppress a method-level *array-form* route nested under a class-level // array-form @RequestMapping. The class prefix is one of several values that // cannot be resolved to a single string here, so emitting the route would @@ -195,15 +216,47 @@ export function extractSpringRoutes( // handler method name (resolved to a symbol UID later by the routes phase). const handlerName = node.childForFieldName('name')?.text; - routes.push({ - filePath, - routePath, - httpMethod, - decoratorName: ann, - lineNumber: annNode.startPosition.row + lineOffset, - ...(classPrefix ? { prefix: classPrefix } : {}), - ...(handlerName ? { handlerName } : {}), - }); + for (const httpMethod of httpMethods) { + routes.push({ + filePath, + routePath, + httpMethod, + decoratorName: ann, + lineNumber: annNode.startPosition.row + lineOffset, + ...(classPrefix ? { prefix: classPrefix } : {}), + ...(handlerName ? { handlerName } : {}), + }); + } + } + + // Mapping annotations without a path bind to the enclosing class prefix. + 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; + const classPrefix = prefixByClassId.get(typeNode.id) ?? ''; + const classMethods = classHttpMethodsById.get(typeNode.id) ?? ['*']; + for (const methodNode of directMethods(typeNode)) { + const handlerName = methodNode.childForFieldName('name')?.text; + if (!handlerName) continue; + for (const annotationNode of declarationAnnotations(methodNode)) { + const ann = annotationName(annotationNode) ?? ''; + if (annotationHasRouteMember(annotationNode)) continue; + const methodMethods = springAnnotationHttpMethods(ann, annotationNode.text); + const httpMethods = intersectSpringHttpMethods(classMethods, methodMethods); + if (httpMethods.length === 0) continue; + for (const httpMethod of httpMethods) { + routes.push({ + filePath, + routePath: '', + httpMethod, + decoratorName: ann, + lineNumber: annotationNode.startPosition.row + lineOffset, + ...(classPrefix ? { prefix: classPrefix } : {}), + handlerName, + }); + } + } + } } return routes; @@ -272,6 +325,34 @@ function annotationRoutePaths(ann: Parser.SyntaxNode): string[] { return out; } +/** Whether an annotation explicitly supplies a positional or path/value member. */ +function annotationHasRouteMember(ann: Parser.SyntaxNode): boolean { + const args = ann.childForFieldName('arguments'); + if (!args) return false; + for (const child of args.namedChildren) { + if (child.type !== 'element_value_pair') return true; + const key = child.childForFieldName('key'); + if (isRouteMemberKey(key ?? undefined)) return true; + } + return false; +} + +/** Static class/interface-level RequestMapping method constraint, or wildcard by default. */ +function typeRequestMethods(typeNode: Parser.SyntaxNode): readonly string[] { + const mappings = declarationAnnotations(typeNode).filter( + (ann) => annotationName(ann) === 'RequestMapping', + ); + if (mappings.length === 0) return ['*']; + if (mappings.length !== 1) return []; + return springAnnotationHttpMethods('RequestMapping', mappings[0].text); +} + +function annotationRoutePathsOrDefault(ann: Parser.SyntaxNode): string[] { + const paths = annotationRoutePaths(ann); + if (paths.length > 0) return paths; + return annotationHasRouteMember(ann) ? [] : ['']; +} + /** Class-level `@RequestMapping` prefixes for a type (array-aware; may be []). */ function typeClassPrefixes(typeNode: Parser.SyntaxNode): string[] { const prefixes: string[] = []; @@ -343,6 +424,7 @@ export function extractSpringTypes(tree: Parser.Tree, filePath: string): SharedS const annNames = declarationAnnotations(typeNode).map(annotationName); const isController = kind === 'class' && (annNames.includes('RestController') || annNames.includes('Controller')); + const classMethods = typeRequestMethods(typeNode); const methods = directMethods(typeNode) .map((methodNode) => { @@ -350,9 +432,12 @@ export function extractSpringTypes(tree: Parser.Tree, filePath: string): SharedS if (!methodName) return null; const routes: Array<{ method: string; path: string }> = []; for (const ann of declarationAnnotations(methodNode)) { - const verb = METHOD_ANNOTATION_TO_HTTP[annotationName(ann) ?? '']; - if (!verb) continue; - for (const path of annotationRoutePaths(ann)) routes.push({ method: verb, path }); + const methodMethods = springAnnotationHttpMethods(annotationName(ann) ?? '', ann.text); + const verbs = intersectSpringHttpMethods(classMethods, methodMethods); + for (const verb of verbs) { + for (const path of annotationRoutePathsOrDefault(ann)) + routes.push({ method: verb, path }); + } } return { name: methodName, routes }; }) diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 34f571dd4..08874fb8c 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -267,7 +267,23 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j // store would have replayed pre-fix ParsedFiles verbatim for one of them. // RE-CHECK AGAINST origin/main IMMEDIATELY BEFORE MERGING. // -// 46 -> 47 for the round-2 capture work: object literals behind an +// 46 -> 47: method-level Spring `@RequestMapping` now emits wildcard routes +// and one route per static `RequestMethod.X` value. These decorator routes live +// in ParseWorkerResult and are replayed verbatim on warm cache hits, so keeping +// the previous version would make the fix a no-op for every unchanged Java file. +// PR #2856 claims 46, so this branch owns 47. Verified against upstream/main at +// 021ac3037 (still 45). RE-CHECK BEFORE MERGE. +// +// ── The FIFTH clash, and the first one the ledger's own convention prevented ── +// #2857 above merged while this branch sat waiting, and it did the right thing: +// it read this PR's claim on 46 and took 47 instead of colliding. That left the +// clash one step further up — 46 was safe, but THIS branch's own 47 (below) was +// not, and neither was anything after it. Every entry from here down has been +// renumbered +1 at merge time. Nothing about the capture sets changed; only the +// numbers did, which is the whole point of re-checking at merge rather than at +// review. Ledger entries 11 through 15. +// +// 47 -> 48 for the round-2 capture work: object literals behind an // identity-preserving wrapper (`const X = Object.freeze({ ... })`) now mint // `@definition.property` for their keys. Parse-time like every entry above, and // this one was ALSO observed as a false negative first: `analyze --force` @@ -278,7 +294,7 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j // for this bump. // RE-CHECK AGAINST origin/main IMMEDIATELY BEFORE MERGING. // -// 47 -> 48 for the TypeScript object-literal captures (R3-3): named +// 48 -> 49 for the TypeScript object-literal captures (R3-3): named // object-literal keys and the identity-wrapper form now mint `@definition.property` // in TYPESCRIPT_QUERIES, as they already did for JavaScript. Parse-time, so a // warm cache would replay ParsedFiles carrying none of those matches and the @@ -289,39 +305,39 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j // clashes. It is NOT on this branch, so until that one merges the re-check // below is still manual. // RE-CHECK AGAINST origin/main IMMEDIATELY BEFORE MERGING. -// 48 -> 49 for the return-shape and shorthand captures (R3-4): keys of an +// 49 -> 50 for the return-shape and shorthand captures (R3-4): keys of an // anonymous literal in return position, and shorthand keys in both that and the // variable-bound form. Parse-time again. // // The v34 hazard, and this branch has already tripped it: a build stamped 48 -// was installed and used to analyze two repos BEFORE these captures existed, so -// caches stamped 48 exist that carry none of them. Within one PR the version +// (now 49) was installed and used to analyze two repos BEFORE these captures +// existed, so caches stamped 48 exist that carry none of them. Within one PR the version // only has to differ from main's, but an INTERMEDIATE build of the same series // is a different capture set wearing the same number — which is exactly what // the note above records for 33/34. // RE-CHECK AGAINST origin/main IMMEDIATELY BEFORE MERGING. -// 49 -> 50 is NOT needed for R3-5: that pass is scope-resolution, not +// 50 -> 51 is NOT needed for R3-5: that pass is scope-resolution, not // parse-time capture, so a warm cache replays ParsedFiles that already carry // everything it reads. Recorded because the reflex on this branch has been to // bump, and a bump nobody needs still forces every user a full re-parse. // -// 49 -> 50 IS needed for dispatch-guard routes (R3-7): the JS/TS providers now +// 50 -> 51 IS needed for dispatch-guard routes (R3-7): the JS/TS providers now // implement `extractDecoratorRoutes`, and decorator routes are worker output // carried in the parse cache. A warm cache replays a worker result whose // `decoratorRoutes` predates the extractor entirely, so every hand-rolled route // stays invisible and `route_map` keeps answering empty — the exact symptom the // change exists to fix, wearing the mask of "the extractor does not work". // -// 50 -> 51 for the same-file constant folding that followed it. The v34 hazard -// again, and this branch has now tripped it TWICE: a build stamped 50 was used -// to analyze before folding existed, so caches stamped 50 carry the unfolded +// 51 -> 52 for the same-file constant folding that followed it. The v34 hazard +// again, and this branch has now tripped it TWICE: a build stamped 50 (now 51) +// was used to analyze before folding existed, so those caches carry the unfolded // route set. Caught by measuring — the post-folding run came back suspiciously // fast and would have reported the pre-folding number, which is precisely how // "an intermediate build of the same series is a different capture set wearing // the same number" shows up in practice. Within one PR the version only has to // differ from main's; against a cache YOU wrote, it has to differ from itself. // RE-CHECK AGAINST origin/main IMMEDIATELY BEFORE MERGING. -const SCHEMA_BUMP = 51; +const SCHEMA_BUMP = 52; const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/test/fixtures/spring-route-app/src/main/java/com/example/controller/LegacyController.java b/gitnexus/test/fixtures/spring-route-app/src/main/java/com/example/controller/LegacyController.java new file mode 100644 index 000000000..5b9e8fccb --- /dev/null +++ b/gitnexus/test/fixtures/spring-route-app/src/main/java/com/example/controller/LegacyController.java @@ -0,0 +1,24 @@ +package com.example.controller; + +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RestController; + +@RestController +@RequestMapping("/api/legacy") +public class LegacyController { + @RequestMapping("/all") + public String allMethods() { + return "all"; + } + + @RequestMapping(value = "/save", method = RequestMethod.POST) + public String save() { + return "saved"; + } + + @RequestMapping(path = "/inspect", method = {RequestMethod.GET, RequestMethod.HEAD}) + public String inspect() { + return "inspected"; + } +} diff --git a/gitnexus/test/integration/spring-route-pipeline.test.ts b/gitnexus/test/integration/spring-route-pipeline.test.ts index 018ead80f..e6fd739af 100644 --- a/gitnexus/test/integration/spring-route-pipeline.test.ts +++ b/gitnexus/test/integration/spring-route-pipeline.test.ts @@ -87,6 +87,48 @@ describe('Spring @RequestMapping route ingestion pipeline', () => { expect(names).toContain('/api/admin/settings'); }); + it('emits wildcard and explicit method-level @RequestMapping routes', () => { + const routes = new Set(); + const handlerNames = new Set(); + const handledLegacyRoutes = new Set(); + result.graph.forEachNode((node) => { + if (node.label !== 'Route') return; + routes.add(`${String(node.properties.method)} ${String(node.properties.name)}`); + if (node.properties.handlerSymbolId) { + const handler = result.graph.getNode(String(node.properties.handlerSymbolId)); + if (handler) handlerNames.add(String(handler.properties.name)); + } + }); + result.graph.forEachRelationship((relationship) => { + if (relationship.type !== 'HANDLES_ROUTE') return; + const file = result.graph.getNode(relationship.sourceId); + const route = result.graph.getNode(relationship.targetId); + if ( + file?.label === 'File' && + String(file.properties.name).includes('LegacyController.java') && + route?.label === 'Route' + ) { + handledLegacyRoutes.add( + `${String(route.properties.method)} ${String(route.properties.name)}`, + ); + } + }); + + const expectedRoutes = [ + '* /api/legacy/all', + 'POST /api/legacy/save', + 'GET /api/legacy/inspect', + 'HEAD /api/legacy/inspect', + ]; + for (const route of expectedRoutes) { + expect(routes).toContain(route); + expect(handledLegacyRoutes).toContain(route); + } + for (const handlerName of ['allMethods', 'save', 'inspect']) { + expect(handlerNames).toContain(handlerName); + } + }); + it('emits HANDLES_ROUTE edges linking Route nodes to their handler files', () => { const handlesRouteEdges: Array<{ routeName: string; filePath: string }> = []; result.graph.forEachRelationship((r) => { diff --git a/gitnexus/test/unit/group/http-route-extractor.test.ts b/gitnexus/test/unit/group/http-route-extractor.test.ts index 9dda77ffa..b56b08d32 100644 --- a/gitnexus/test/unit/group/http-route-extractor.test.ts +++ b/gitnexus/test/unit/group/http-route-extractor.test.ts @@ -1605,10 +1605,8 @@ public class UserController { expect(route).toBeDefined(); }); - it('does NOT emit a provider for @GetMapping(produces = ...) without path/value', async () => { - // Anti-regression: without the `key:` constraint, the named-arg - // query would capture `produces = "application/json"` and emit - // a bogus `http::GET::/application/json` contract. + it('emits a root provider for pathless @GetMapping without leaking produces', async () => { + // A pathless mapping is valid, but produces metadata must never become a path. const dir = path.join(tmpDir, 'spring-produces-only'); fs.mkdirSync(path.join(dir, 'src/controller'), { recursive: true }); fs.writeFileSync( @@ -1628,17 +1626,16 @@ public class MisleadingController { const contracts = await extractor.extract(null, dir, makeRepo(dir)); const providers = contracts.filter((c) => c.role === 'provider'); - // No GET provider should be emitted for this method — the only - // string literal in the annotation is a non-route attribute. + // The non-route string must not become an /application/json provider. expect( providers.find((c) => c.contractId === 'http::GET::/application/json'), ).toBeUndefined(); - // And the controller has no other route, so providers list for - // this file should be empty. + // The mapping itself still contributes one pathless root provider. const fromThisFile = providers.filter((c) => c.symbolRef.filePath.endsWith('MisleadingController.java'), ); - expect(fromThisFile).toHaveLength(0); + expect(fromThisFile).toHaveLength(1); + expect(fromThisFile[0].contractId).toBe('http::GET::/'); }); it('emits exactly one provider for @GetMapping(name = "...", value = "/users")', async () => { diff --git a/gitnexus/test/unit/group/spring-route-parity.test.ts b/gitnexus/test/unit/group/spring-route-parity.test.ts index 6ecab61ae..003eb0ad2 100644 --- a/gitnexus/test/unit/group/spring-route-parity.test.ts +++ b/gitnexus/test/unit/group/spring-route-parity.test.ts @@ -61,6 +61,14 @@ function groupProviders(src: string): Set { ); } +function groupConsumers(src: string): Set { + return new Set( + JAVA_HTTP_PLUGIN.scan(parse(src)) + .filter((d) => d.role === 'consumer' && d.framework === 'openfeign') + .map((d) => canon(d.method, d.path)), + ); +} + describe('Spring route extractor parity — ingestion spring.ts vs group java.ts', () => { it('agree on bare, named-arg, and array-form method routes under a class prefix', () => { const src = `package com.example; @@ -124,6 +132,109 @@ public class NamedArrayController { expect(ingestionProviders(src)).toEqual(group); }); + it('agree on method-level @RequestMapping verbs and wildcard mappings', () => { + const src = `package com.example; +import org.springframework.web.bind.annotation.*; + +@RestController +@RequestMapping("/api") +public class LegacyController { + @RequestMapping("/bare") public Object bare() { return null; } + @RequestMapping(value = "/named") public Object named() { return null; } + @RequestMapping(value = "/save", method = RequestMethod.POST) + public Object save() { return null; } + @RequestMapping(path = "/multi", method = {RequestMethod.GET, RequestMethod.HEAD}) + public Object multi() { return null; } + @RequestMapping(path = {"/one", "/two"}, method = {RequestMethod.GET, RequestMethod.POST}) + public Object crossProduct() { return null; } + @RequestMapping(path = "/empty", method = {}) + public Object empty() { return null; } + @RequestMapping(path = "/spaced", method = RequestMethod . PUT) + public Object spaced() { return null; } + @RequestMapping(path = "/commented", method = { + RequestMethod.GET, // read + /* write */ RequestMethod.POST, + }) + public Object commented() { return null; } + @org.springframework.web.bind.annotation.RequestMapping( + path = "/fqn", + method = org.springframework.web.bind.annotation.RequestMethod.DELETE + ) + public Object fqn() { return null; } +} +`; + const expected = new Set([ + '* /api/bare', + '* /api/named', + 'POST /api/save', + 'GET /api/multi', + 'HEAD /api/multi', + 'GET /api/one', + 'POST /api/one', + 'GET /api/two', + 'POST /api/two', + '* /api/empty', + 'PUT /api/spaced', + 'GET /api/commented', + 'POST /api/commented', + 'DELETE /api/fqn', + ]); + + expect(groupProviders(src)).toEqual(expected); + expect(ingestionProviders(src)).toEqual(expected); + }); + + it('intersects class verbs and emits pathless method mappings at the class prefix', () => { + const src = `package com.example; +import org.springframework.web.bind.annotation.*; + +@RestController +@RequestMapping(path = "/api", method = RequestMethod.GET) +public class ConstrainedController { + @RequestMapping("/wild") public Object wildcard() { return null; } + @RequestMapping(method = RequestMethod.GET) public Object root() { return null; } + @GetMapping public Object shortcut() { return null; } + @PostMapping("/blocked") public Object blocked() { return null; } +} +`; + + const expected = new Set(['GET /api/wild', 'GET /api']); + expect(groupProviders(src)).toEqual(expected); + expect(ingestionProviders(src)).toEqual(expected); + }); + + it('uses GET for verb-less Feign RequestMapping and rejects multi-method contracts', () => { + const src = `package com.example; +import org.springframework.cloud.openfeign.FeignClient; +import org.springframework.web.bind.annotation.*; + +@FeignClient(name = "orders") +@RequestMapping("/api") +public interface OrdersClient { + @RequestMapping("/default") Object defaultCall(); + @RequestMapping(path = "/multi", method = {RequestMethod.GET, RequestMethod.POST}) + Object ambiguous(); +} +`; + + expect(groupConsumers(src)).toEqual(new Set(['GET /api/default'])); + }); + it('fails closed when @RequestMapping method is not statically resolvable', () => { + const src = `package com.example; +import org.springframework.web.bind.annotation.*; + +@RestController +public class DynamicController { + private static final RequestMethod VERB = RequestMethod.POST; + @RequestMapping(path = "/dynamic", method = VERB) + public Object dynamic() { return null; } +} +`; + + expect(groupProviders(src)).toEqual(new Set()); + expect(ingestionProviders(src)).toEqual(new Set()); + }); + it('do not leak non-route arrays (consumes/produces) as routes — array analogue', () => { // The scalar `produces` anti-regression already exists in the route tests; // this is its array form. `consumes`/`produces` arrays must never surface as @@ -141,13 +252,12 @@ public class ContentTypeController { `; const ingestion = ingestionProviders(src); const group = groupProviders(src); - // Only the explicit path leaks through; the consumes/produces arrays do not, - // and the path-less @PostMapping contributes nothing. - expect(group).toEqual(new Set(['GET /v'])); + // Non-route arrays never become paths; the pathless POST binds to root. + expect(group).toEqual(new Set(['GET /v', 'POST /'])); expect(ingestion).toEqual(group); - // Pure-consumes controller (no path anywhere) → EMPTY provider set on both - // sides: a consumes/produces array must never be misread as a route path. + // Pure consumes/produces members still describe pathless root mappings; + // their media-type values must never be misread as route paths. const consumesOnly = `package com.example; import org.springframework.web.bind.annotation.*; @@ -159,8 +269,8 @@ public class ConsumesOnlyController { public Object b() { return null; } } `; - expect(groupProviders(consumesOnly)).toEqual(new Set()); - expect(ingestionProviders(consumesOnly)).toEqual(new Set()); + expect(groupProviders(consumesOnly)).toEqual(new Set(['POST /', 'PUT /'])); + expect(ingestionProviders(consumesOnly)).toEqual(new Set(['POST /', 'PUT /'])); }); it('pins the deliberate class-array divergence: ingestion suppresses, group emits cross-product (#2280)', () => { diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts index 761ee3c0e..7a3939c14 100644 --- a/gitnexus/test/unit/incremental-parse-cache.test.ts +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -141,15 +141,24 @@ describe('PARSE_CACHE_VERSION', () => { // a row. Same lesson as the note above — the pin cannot detect the tie, since // both sides asserted `toBe(45)` and that passes while main is already 45. // Only the merge-time diff against origin/main surfaces it. - // Moved 49 -> 50 for dispatch-guard routes (R3-7): the JS/TS providers now + // + // Moved 46 -> 47 for method-level Spring `@RequestMapping` routes (#2857): + // cached ParseWorkerResults otherwise replay the pre-fix empty route set. + // That PR read this branch's claim on 46 and took 47 rather than colliding — + // the FIFTH clash, and the first the ledger's convention actually prevented. + // It only moved the collision up one step, though: this branch's own 47 and + // everything above it had to be renumbered +1 at merge time. Capture sets + // unchanged; only the numbers moved. + // + // Moved 50 -> 51 for dispatch-guard routes (R3-7): the JS/TS providers now // implement `extractDecoratorRoutes`, and decorator routes are worker output - // carried in the cache. A v49 warm cache replays a worker result whose + // carried in the cache. A v50 warm cache replays a worker result whose // `decoratorRoutes` predates the extractor, so `route_map` keeps answering // empty — the exact symptom the change fixes, disguised as "it does not work". - // Moved 50 -> 51 for the same-file constant folding that followed, because a - // build stamped 50 had already been used to analyze without it. - it('pins SCHEMA_BUMP to 51 so concurrent bumps cannot silently collide (#2766)', () => { - expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(51); + // Moved 51 -> 52 for the same-file constant folding that followed, because a + // build stamped 50 (now 51) had already been used to analyze without it. + it('pins SCHEMA_BUMP to 52 so concurrent bumps cannot silently collide (#2766)', () => { + expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(52); }); it('embeds the gitnexus package version (so upgrades invalidate the cache)', () => { diff --git a/gitnexus/test/unit/spring-annotation-arguments.test.ts b/gitnexus/test/unit/spring-annotation-arguments.test.ts index 7e6107e59..faaf9997e 100644 --- a/gitnexus/test/unit/spring-annotation-arguments.test.ts +++ b/gitnexus/test/unit/spring-annotation-arguments.test.ts @@ -15,6 +15,10 @@ import { springResourceDefaultName, springResourceInjectionMatch, } from '../../src/core/ingestion/frameworks/spring/resource-injection.js'; +import { + intersectSpringHttpMethods, + springAnnotationHttpMethods, +} from '../../src/core/ingestion/route-extractors/spring-shared.js'; describe('Spring annotation static arguments', () => { it('parses Java and Kotlin named arrays without splitting nested values', () => { @@ -80,6 +84,77 @@ describe('Spring annotation static arguments', () => { }); }); +describe('Spring request mapping methods', () => { + it('resolves shortcut, wildcard, scalar, and array method declarations', () => { + expect(springAnnotationHttpMethods('GetMapping', '@GetMapping("/x")')).toEqual(['GET']); + expect(springAnnotationHttpMethods('RequestMapping', '@RequestMapping("/x")')).toEqual(['*']); + expect( + springAnnotationHttpMethods( + 'RequestMapping', + '@RequestMapping(path = "/x", method = RequestMethod.POST)', + ), + ).toEqual(['POST']); + expect( + springAnnotationHttpMethods( + 'RequestMapping', + '@RequestMapping(path = "/x", method = {RequestMethod.GET, RequestMethod.HEAD})', + ), + ).toEqual(['GET', 'HEAD']); + expect( + springAnnotationHttpMethods('RequestMapping', '@RequestMapping(path = "/x", method = {})'), + ).toEqual(['*']); + }); + + it('accepts a terminal array comma and intersects class/method constraints', () => { + expect( + springAnnotationHttpMethods( + 'RequestMapping', + '@RequestMapping(method = {RequestMethod.GET, RequestMethod.HEAD,})', + ), + ).toEqual(['GET', 'HEAD']); + expect(intersectSpringHttpMethods(['GET'], ['*'])).toEqual(['GET']); + expect(intersectSpringHttpMethods(['*'], ['POST'])).toEqual(['POST']); + expect(intersectSpringHttpMethods(['GET', 'HEAD'], ['HEAD', 'POST'])).toEqual(['HEAD']); + expect(intersectSpringHttpMethods(['GET'], ['POST'])).toEqual([]); + }); + + it('accepts Java whitespace and comments around static RequestMethod values', () => { + expect( + springAnnotationHttpMethods( + 'RequestMapping', + '@RequestMapping(path = "/x", method = RequestMethod . GET)', + ), + ).toEqual(['GET']); + expect( + springAnnotationHttpMethods( + 'RequestMapping', + `@RequestMapping(method = { + RequestMethod.GET, // read + /* write */ RequestMethod.POST, + })`, + ), + ).toEqual(['GET', 'POST']); + }); + + it('fails closed for runtime, malformed, and duplicate method members', () => { + expect( + springAnnotationHttpMethods('RequestMapping', '@RequestMapping(path = "/x", method = VERB)'), + ).toEqual([]); + expect( + springAnnotationHttpMethods( + 'RequestMapping', + '@RequestMapping(path = "/x", method = RequestMethod.GET, method = RequestMethod.POST)', + ), + ).toEqual([]); + expect( + springAnnotationHttpMethods( + 'RequestMapping', + '@RequestMapping(path = "/x", method = RequestMethod./* unterminated)', + ), + ).toEqual([]); + }); +}); + describe('Spring Bean factory metadata', () => { it('uses the method default and recognizes Java/Kotlin aliases', () => { expect(springBeanNames('@Bean', 'gateway')).toEqual({ diff --git a/gitnexus/test/unit/spring-interface-inheritance.test.ts b/gitnexus/test/unit/spring-interface-inheritance.test.ts index 8d2fca64f..6a5c16f99 100644 --- a/gitnexus/test/unit/spring-interface-inheritance.test.ts +++ b/gitnexus/test/unit/spring-interface-inheritance.test.ts @@ -224,6 +224,37 @@ public class C implements Api { expect(ingestionInheritedKeys(files)).toEqual(group); }); + it('agrees on inherited @RequestMapping verbs and wildcard mappings', () => { + const files = [ + { + path: 'LegacyApi.java', + src: `package com.example; +import org.springframework.web.bind.annotation.*; +@RequestMapping("/api") +public interface LegacyApi { + @RequestMapping(path = "/save", method = RequestMethod.POST) Object save(); + @RequestMapping("/all") Object all(); +} +`, + }, + { + path: 'LegacyController.java', + src: `package com.example; +import org.springframework.web.bind.annotation.*; +@RestController +public class LegacyController implements LegacyApi { + public Object save() { return null; } + public Object all() { return null; } +} +`, + }, + ]; + + const expected = new Set(['POST /api/save', '* /api/all']); + expect(groupInheritedKeys(files)).toEqual(expected); + expect(ingestionInheritedKeys(files)).toEqual(expected); + }); + it('agrees on fully-qualified annotation names', () => { // Both sides normalise an FQN annotation to its trailing segment, so an // interface using `@org.springframework...GetMapping` still resolves.