fix(spring): index multi-path mappings and same-file constant paths (#1461) (#1811)

Co-authored-by: Colby McHenry <colbymchenry@users.noreply.github.com>

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)
 

__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")

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<string, string>();
+    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<string, string> = {
       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, string>): 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`. */