From 4453310eef21f1e22ad18ab387a3da005595f51c Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Tue, 8 Sep 2026 20:20:48 -0500 Subject: [PATCH] fix(spring): index multi-path mappings and same-file constant paths (#1461) (#1811) Co-authored-by: Colby McHenry --- CHANGELOG.md | 1 + __tests__/frameworks.test.ts | 112 +++++++++++++++++++++++++++++ src/resolution/frameworks/java.ts | 114 ++++++++++++++++++------------ 3 files changed, 180 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89a4d36..6f9c6b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,6 +135,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- Spring mappings now include every declared path combination and resolve constants declared in the same file, while unresolved paths no longer appear as false root routes. (#1461) - `codegraph callers`, `codegraph callees` and `codegraph impact` now resolve qualified names, group results and JSON edges by definition, and accept `--file` to narrow ambiguous names; thanks @ferrine. (#1512, #1656) - `codegraph callers`, `codegraph callees` and `codegraph impact` (CLI and MCP) now report missing names with did-you-mean suggestions instead of another symbol's results, and exact matches with no callers stay empty; thanks @uvmplus. (#1473, #1481) diff --git a/__tests__/frameworks.test.ts b/__tests__/frameworks.test.ts index a1d7fb3..e467ab6 100644 --- a/__tests__/frameworks.test.ts +++ b/__tests__/frameworks.test.ts @@ -837,6 +837,118 @@ describe('railsResolver.extract', () => { import { springResolver } from '../src/resolution/frameworks/java'; describe('springResolver.extract', () => { + it.each([ + ['UserController.java', '{"/a", "/b"}', '@GetMapping({"/x", "/y"})', 'public String handle() { return "ok"; }'], + ['UserController.java', 'path = {"/a", "/b"}', '@RequestMapping(value = {"/x", "/y"}, method = RequestMethod.GET)', 'public String handle() { return "ok"; }'], + ['UserController.kt', 'value = ["/a", "/b"]', '@GetMapping(path = ["/x", "/y"])', 'fun handle(): String = "ok"'], + ])('indexes every class/method path pair in %s with %s and %s (#1461)', (filePath, base, mapping, handler) => { + const src = `@RestController +@RequestMapping(${base}) +public class UserController { + ${mapping} + ${handler} +}`; + const { nodes, references } = springResolver.extract!(filePath, src); + expect(nodes.map(n => n.name)).toEqual(['GET /a/x', 'GET /a/y', 'GET /b/x', 'GET /b/y']); + expect(new Set(nodes.map(n => n.id)).size).toBe(4); + expect(references.map(r => [r.fromNodeId, r.referenceName])).toEqual(nodes.map(n => [n.id, 'handle'])); + }); + + it.each(['ErrorHandler.PATH', 'PATH', 'value = ErrorHandler.PATH', 'path = PATH'])( + 'resolves a same-file constant prefix in @RequestMapping(%s) (#1461)', (args) => { + const src = `@Controller +@RequestMapping(${args}) +public class ErrorHandler { + public static final String PATH = "/error"; + @RequestMapping(method = {RequestMethod.GET}) + public String handle() { return "err"; } +}`; + const { nodes, references } = springResolver.extract!('ErrorHandler.java', src); + expect(nodes.map(n => n.name)).toEqual(['GET /error']); + expect(references.map(r => [r.fromNodeId, r.referenceName])).toEqual([[nodes[0].id, 'handle']]); + }, + ); + + it('keeps literals and resolved constants in path arrays, including URI variables (#1461)', () => { + const src = `@RequestMapping({"/api", "/{tenant}/api"}) +public class ItemController { + public static final String ITEMS = "/items"; + @GetMapping(path = {ITEMS, "/items/{id}", External.MISSING}, produces = "application/json") + public String get() { return "ok"; } +}`; + const { nodes, references } = springResolver.extract!('ItemController.java', src); + expect(nodes.map(n => n.name)).toEqual([ + 'GET /api/items', 'GET /api/items/{id}', 'GET /{tenant}/api/items', 'GET /{tenant}/api/items/{id}', + ]); + expect(references.map(r => r.referenceName)).toEqual(['get', 'get', 'get', 'get']); + }); + + it.each([ + ['value = "/ok", produces = "application/json"', '/base/ok'], + ['consumes = {"application/json", "text/plain"}, path = "/ok", produces = "application/json"', '/base/ok'], + ['produces = "application/json", consumes = "text/plain"', '/base'], + ])('only treats path arguments as paths: %s (#1461)', (args, expected) => { + const src = `@RequestMapping("/base") +public class UserController { + @GetMapping(${args}) + public String handle() { return "ok"; } +}`; + const { nodes } = springResolver.extract!('UserController.java', src); + expect(nodes.map(n => n.name)).toEqual([`GET ${expected}`]); + }); + + it.each([ + ['External.MISSING', '@GetMapping'], + ['value = MISSING, produces = "application/json"', '@GetMapping("/ok")'], + ['"/base"', '@GetMapping(External.MISSING)'], + ['"/base"', '@GetMapping(path = MISSING, produces = "application/json")'], + ['"/base"', '@RequestMapping(value = MISSING, method = RequestMethod.GET)'], + ])('omits unresolved paths: class %s, method %s (#1461)', (base, mapping) => { + const src = `@RequestMapping(${base}) +public class UserController { + // public static final String MISSING = "/comment"; + ${mapping} + public String handle() { return "ok"; } +}`; + expect(springResolver.extract!('UserController.java', src)).toEqual({ nodes: [], references: [] }); + }); + + it.each([ + ['@GetMapping', 'GET'], + ['@GetMapping()', 'GET'], + ['@RequestMapping(method = RequestMethod.GET)', 'GET'], + ['@RequestMapping(method = {RequestMethod.GET})', 'GET'], + ['@RequestMapping', 'ANY'], + ])('inherits the class prefix for %s without emitting a class route (#1461)', (mapping, verb) => { + const src = `@RequestMapping("/base") +public class UserController { + ${mapping} + public String handle() { return "ok"; } +}`; + const { nodes, references } = springResolver.extract!('UserController.java', src); + expect(nodes.map(n => n.name)).toEqual([`${verb} /base`]); + expect(references.map(r => r.referenceName)).toEqual(['handle']); + }); + + it('preserves annotation and reference line numbers after multiline Javadocs (#1461)', () => { + const src = `/** + * Controller documentation. + */ +@RequestMapping("/base") +public class UserController { + /** + * Handler documentation with @GetMapping("/fake"). + */ + @GetMapping({"/x", "/y"}) + public String handle() { return "ok"; } +}`; + const { nodes, references } = springResolver.extract!('UserController.java', src); + expect(nodes.map(n => [n.name, n.startLine, n.endLine])).toEqual([ + ['GET /base/x', 9, 9], ['GET /base/y', 9, 9], + ]); + expect(references.map(r => [r.referenceName, r.line])).toEqual([['handle', 9], ['handle', 9]]); + }); + it('extracts route with @GetMapping and next method', () => { const src = ` @GetMapping("/users") diff --git a/src/resolution/frameworks/java.ts b/src/resolution/frameworks/java.ts index 916bfbc..ff31c84 100644 --- a/src/resolution/frameworks/java.ts +++ b/src/resolution/frameworks/java.ts @@ -211,14 +211,17 @@ export const springResolver: FrameworkResolver = { const now = Date.now(); const lang: 'java' | 'kotlin' = filePath.endsWith('.kt') ? 'kotlin' : 'java'; const safe = stripCommentsForRegex(content, 'java'); + const consts = new Map(); + for (const m of safe.matchAll(/\bstatic\s+final\s+String\s+(\w+)\s*=\s*"([^"]*)"\s*;/g)) { + consts.set(m[1]!, m[2]!); + } // Class-level @RequestMapping prefix (an @RequestMapping whose tail leads to a // `class`). Joined onto each method's path — and, crucially, NOT treated as a // route itself (the old regex did, creating one bogus class route and missing // every BARE method mapping like `@PostMapping` with the path on the class). - let classPrefix = ''; const cls = /@RequestMapping\s*\(([^)]*)\)\s*(?:@[\w.]+(?:\([^)]*\))?\s*)*(?:public\s+|final\s+|abstract\s+|open\s+|data\s+|sealed\s+)*class\b/.exec(safe); - if (cls) classPrefix = parseMappingPath(cls[1]!); + const classPrefixes = cls ? parseMappingPaths(cls[1]!, consts) : ['']; const VERB: Record = { GetMapping: 'GET', PostMapping: 'POST', PutMapping: 'PUT', PatchMapping: 'PATCH', DeleteMapping: 'DELETE', @@ -228,38 +231,39 @@ export const springResolver: FrameworkResolver = { let match: RegExpExecArray | null; while ((match = mappingRegex.exec(safe)) !== null) { const method = VERB[match[1]!]!; - const sub = parseMappingPath((match[2] || '').replace(/^\(|\)$/g, '')); - const routePath = joinPath(classPrefix, sub); + const paths = parseMappingPaths((match[2] || '').replace(/^\(|\)$/g, ''), consts); const line = safe.slice(0, match.index).split('\n').length; - const routeNode: Node = { - id: `route:${filePath}:${line}:${method}:${routePath}`, - kind: 'route', - name: `${method} ${routePath}`, - qualifiedName: `${filePath}::route:${routePath}`, - filePath, - startLine: line, - endLine: line, - startColumn: 0, - endColumn: match[0].length, - language: lang, - updatedAt: now, - }; - nodes.push(routeNode); - // Method it decorates: first declared method after (skip stacked annotations; // Java puts the return type before the name). Bounded so we don't grab a far one. const tail = safe.slice(match.index + match[0].length, match.index + match[0].length + 600); const methodMatch = tail.match(/\bfun\s+(\w+)\s*\(|\b(?:public|private|protected)\s+[^;{=]*?\s+(\w+)\s*\(/); - if (methodMatch) { - references.push({ - fromNodeId: routeNode.id, - referenceName: (methodMatch[1] ?? methodMatch[2])!, - referenceKind: 'references', - line, - column: 0, + for (const routePath of classPrefixes.flatMap(prefix => paths.map(sub => joinPath(prefix, sub)))) { + const routeNode: Node = { + id: `route:${filePath}:${line}:${method}:${routePath}`, + kind: 'route', + name: `${method} ${routePath}`, + qualifiedName: `${filePath}::route:${routePath}`, filePath, + startLine: line, + endLine: line, + startColumn: 0, + endColumn: match[0].length, language: lang, - }); + updatedAt: now, + }; + nodes.push(routeNode); + + if (methodMatch) { + references.push({ + fromNodeId: routeNode.id, + referenceName: (methodMatch[1] ?? methodMatch[2])!, + referenceKind: 'references', + line, + column: 0, + filePath, + language: lang, + }); + } } } @@ -273,24 +277,26 @@ export const springResolver: FrameworkResolver = { if (/^\s*(?:@[\w.]+(?:\([^)]*\))?\s*)*(?:public\s+|final\s+|abstract\s+|open\s+|data\s+|sealed\s+)*class\b/.test(after)) continue; // class-level prefix const methodMatch = after.match(/\bfun\s+(\w+)\s*\(|\b(?:public|private|protected)\s+[^;{=]*?\s+(\w+)\s*\(/); if (!methodMatch) continue; - const verbM = args.match(/method\s*=\s*(?:RequestMethod\.)?(\w+)/); + const verbM = args.match(/method\s*=\s*[{\[]?\s*(?:RequestMethod\.)?(\w+)/); const method = verbM ? verbM[1]!.toUpperCase() : 'ANY'; - const routePath = joinPath(classPrefix, parseMappingPath(args)); + const paths = parseMappingPaths(args, consts); const line = safe.slice(0, match.index).split('\n').length; - const routeNode: Node = { - id: `route:${filePath}:${line}:${method}:${routePath}`, - kind: 'route', - name: `${method} ${routePath}`, - qualifiedName: `${filePath}::route:${routePath}`, - filePath, startLine: line, endLine: line, startColumn: 0, endColumn: match[0].length, language: lang, updatedAt: now, - }; - nodes.push(routeNode); - references.push({ - fromNodeId: routeNode.id, - referenceName: (methodMatch[1] ?? methodMatch[2])!, - referenceKind: 'references', - line, column: 0, filePath, language: lang, - }); + for (const routePath of classPrefixes.flatMap(prefix => paths.map(sub => joinPath(prefix, sub)))) { + const routeNode: Node = { + id: `route:${filePath}:${line}:${method}:${routePath}`, + kind: 'route', + name: `${method} ${routePath}`, + qualifiedName: `${filePath}::route:${routePath}`, + filePath, startLine: line, endLine: line, startColumn: 0, endColumn: match[0].length, language: lang, updatedAt: now, + }; + nodes.push(routeNode); + references.push({ + fromNodeId: routeNode.id, + referenceName: (methodMatch[1] ?? methodMatch[2])!, + referenceKind: 'references', + line, column: 0, filePath, language: lang, + }); + } } // @Value("${key}") and @ConfigurationProperties(prefix="...") — bind @@ -512,10 +518,24 @@ const COMPONENT_DIRS = ['/component/', '/components/', '/config/']; const CLASS_KINDS = new Set(['class']); const SERVICE_KINDS = new Set(['class', 'interface']); -/** Path string from a mapping's args (`"/x"`, `value = "/x"`, `path = "/x"`); '' if bare. */ -function parseMappingPath(args: string): string { - const m = args.match(/["']([^"']*)["']/); - return m ? m[1]! : ''; +/** All declared paths; [''] for an omitted path, [] for an unresolved one. */ +function parseMappingPaths(args: string, consts: Map): string[] { + const paths: string[] = []; + let hasPaths = false; + // Keep Java/Kotlin arrays and quoted URI variables together when separating + // attributes. Strings in produces/consumes/etc. are never mapping paths. + const argRe = /(?:^|,)\s*(?:(\w+)\s*=\s*)?(\{(?:[^"'}]|"[^"]*"|'[^']*')*\}|\[(?:[^"'\]]|"[^"]*"|'[^']*')*\]|"[^"]*"|'[^']*'|[^,]+)/g; + for (const arg of args.matchAll(argRe)) { + if (arg[1] && arg[1] !== 'value' && arg[1] !== 'path') continue; + hasPaths = true; + const scope = arg[2]!.trim().replace(/^[{\[]|[}\]]$/g, '').trim(); + if (!scope) paths.push(''); // An explicitly empty path array also inherits the prefix. + for (const value of scope.matchAll(/(?:^|,)\s*(?:"([^"]*)"|'([^']*)'|([\w.]+))\s*(?=,|$)/g)) { + const path = value[1] ?? value[2] ?? consts.get(value[3]!.split('.').pop()!); + if (path !== undefined) paths.push(path); + } + } + return hasPaths ? [...new Set(paths)] : ['']; } /** Join a class-level prefix and a method sub-path into one normalized `/path`. */