fix(group): handle named annotation args in Java Spring route extraction (#1834)

* fix(group): handle named annotation args in Java Spring route extraction

The Java HTTP plugin only matched positional `@RequestMapping("/path")`
syntax for class-level prefixes and method-level routes. Named argument
forms (`path = "/path"` and `value = "/path"`) produce an
`element_value_pair` AST node that the tree-sitter queries did not cover,
causing the class prefix to be lost and named-arg method routes to be
missed entirely during cross-repo contract extraction.

Add a second pattern to both SPRING_CLASS_PREFIX_PATTERNS and
SPRING_METHOD_ROUTE_PATTERNS matching the element_value_pair structure.

* fix(group): constrain Spring named-arg query to path/value keys + add regression tests

Address Claude review on PR #1834. The named-argument patterns added
in 8b6fa6e used `value: (string_literal)` (a tree-sitter field
selector for the right-hand side of element_value_pair), which matched
ANY annotation member with a string value — not just `path`/`value`.

Concrete fallout (without this fix):
  @GetMapping(produces = "application/json") → bogus http::GET::/application/json
  @GetMapping(name = "listUsers", value = "/users") → extra http::GET::/listUsers
  @RequestMapping(headers = "X-Foo=bar", path = "/api") → class prefix
    could be set to "X-Foo=bar" because prefixByClassId.set runs per
    match in document order, so the LAST element_value_pair wins.

The sibling topic-patterns/java.ts already demonstrates the correct
shape: constrain the `key:` field to the route member names.

This commit:
  - Adds `key: (identifier) @key (#match? @key "^(path|value)$")` to
    both SPRING_CLASS_PREFIX_PATTERNS and SPRING_METHOD_ROUTE_PATTERNS
    named-arg queries.
  - Adds 9 regression tests under
    `provider extraction — source-scan fallback (Strategy B)`:
      * @RequestMapping(path = "/api/v3") class prefix
      * @RequestMapping(value = "/orders") class prefix
      * @GetMapping(value = "/users") method route
      * @PostMapping(path = "/users") method route
      * mixed: class named-arg + method positional
      * mixed: class positional + method named-arg
      * @GetMapping(produces = "application/json") → no provider emitted
      * @GetMapping(name = "listUsers", value = "/users") → exactly one
        provider with path "/users", no /listUsers route
      * @RequestMapping(path = "/api", name = "myApi") → prefix is /api,
        not myApi (verifies the class-prefix overwrite scenario)

Tests: 42/42 pass in http-route-extractor.test.ts;
       522/522 pass under test/unit/group;
       npx tsc --noEmit clean.

* test(group): add @GetMapping(path = ...) case to match review checklist verbatim

Claude review on PR #1834 explicitly asked for the method-level
`@GetMapping(path = "/users")` case. The previous commit covered it
indirectly by exercising path= on @PostMapping (the Spring method
annotations share the same query, so any verb proves the path= field
is matched). Add a dedicated GET+path= test so the reviewer's
checklist is satisfied 1:1, and keep the POST+path= case as a bonus
verb-coverage test.

Tests: 43/43 pass in http-route-extractor.test.ts.

---------

Co-authored-by: henry <zhangwei2017@unipus.cn>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>

gitnexus/src/core/group/extractors/http-patterns/java.ts

@@ -30,6 +30,19 @@ const METHOD_ANNOTATION_TO_HTTP: Record<string, string> = {
 };
 
 // ─── Provider: Spring class-level @RequestMapping prefix ──────────────
+// Two patterns are needed because the AST shape differs depending on
+// whether the annotation uses a positional argument or a named one:
+//   @RequestMapping("/api")          → (annotation_argument_list (string_literal))
+//   @RequestMapping(path = "/api")   → (annotation_argument_list (element_value_pair key:(identifier) value:(string_literal)))
+//   @RequestMapping(value = "/api")  → same as above
+//
+// The named-argument pattern MUST constrain the `key` field to the route
+// member names (`path`/`value`); without it, the query also captures
+// non-route attributes such as `produces`, `consumes`, `headers`, `name`,
+// `params` (their right-hand string literals would be mis-extracted as
+// route prefixes — e.g. `produces = "application/json"` would corrupt
+// every method route under that controller). The sibling
+// `topic-patterns/java.ts` uses the same `key:` constraint approach.
 const SPRING_CLASS_PREFIX_PATTERNS = compilePatterns({
   name: 'java-spring-class-prefix',
   language: Java,
@@ -44,10 +57,27 @@ const SPRING_CLASS_PREFIX_PATTERNS = compilePatterns({
               arguments: (annotation_argument_list (string_literal) @prefix)))) @class
       `,
     },
+    {
+      meta: {},
+      query: `
+        (class_declaration
+          (modifiers
+            (annotation
+              name: (identifier) @ann (#eq? @ann "RequestMapping")
+              arguments: (annotation_argument_list
+                (element_value_pair
+                  key: (identifier) @key (#match? @key "^(path|value)$")
+                  value: (string_literal) @prefix))))) @class
+      `,
+    },
   ],
 } satisfies LanguagePatterns<Record<string, never>>);
 
 // ─── Provider: Spring @(Get|Post|...)Mapping method annotations ───────
+// Same dual-pattern approach: positional vs named argument. The named
+// pattern restricts the annotation member name to `path`/`value` to
+// avoid capturing unrelated string-valued attributes
+// (`produces`, `consumes`, `headers`, `name`, `params`, ...).
 const SPRING_METHOD_ROUTE_PATTERNS = compilePatterns({
   name: 'java-spring-method-route',
   language: Java,
@@ -63,6 +93,20 @@ const SPRING_METHOD_ROUTE_PATTERNS = compilePatterns({
           name: (identifier) @method_name) @method
       `,
     },
+    {
+      meta: {},
+      query: `
+        (method_declaration
+          (modifiers
+            (annotation
+              name: (identifier) @ann (#match? @ann "^(Get|Post|Put|Delete|Patch)Mapping$")
+              arguments: (annotation_argument_list
+                (element_value_pair
+                  key: (identifier) @key (#match? @key "^(path|value)$")
+                  value: (string_literal) @path))))
+          name: (identifier) @method_name) @method
+      `,
+    },
   ],
 } satisfies LanguagePatterns<Record<string, never>>);
 

gitnexus/test/unit/group/http-route-extractor.test.ts

@@ -217,6 +217,302 @@ public class UserController {
       expect(getByIdRoute).toBeDefined();
     });
 
+    // ─── #1834 — Spring named annotation arguments ──────────────────
+    // Spring annotations accept both positional shorthand
+    // (`@GetMapping("/users")`) and named arguments
+    // (`@GetMapping(value = "/users")` or `@GetMapping(path = "/users")`).
+    // The two AST shapes produced by tree-sitter-java differ:
+    //   @GetMapping("/users")          → annotation_argument_list > string_literal
+    //   @GetMapping(value = "/users")  → annotation_argument_list > element_value_pair
+    // The named-arg pattern in `http-patterns/java.ts` MUST constrain
+    // the `key` field to `path`/`value`; without that constraint the
+    // query also captures other string-valued attributes such as
+    // `produces`, `consumes`, `headers`, `name`, `params` (see PR #1834
+    // review). The tests below pin both the positive cases and the
+    // negative anti-regression cases.
+    it('extracts Spring class-level @RequestMapping(path = "/api")', async () => {
+      const dir = path.join(tmpDir, 'spring-class-named-path');
+      fs.mkdirSync(path.join(dir, 'src/controller'), { recursive: true });
+      fs.writeFileSync(
+        path.join(dir, 'src/controller/UserController.java'),
+        `
+package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+@RequestMapping(path = "/api/v3")
+public class UserController {
+    @GetMapping("/users")
+    public List<User> list() { return service.findAll(); }
+}
+`,
+      );
+
+      const contracts = await extractor.extract(null, dir, makeRepo(dir));
+      const providers = contracts.filter((c) => c.role === 'provider');
+
+      const route = providers.find((c) => c.contractId === 'http::GET::/api/v3/users');
+      expect(route).toBeDefined();
+      expect(route!.meta.path).toBe('/api/v3/users');
+    });
+
+    it('extracts Spring class-level @RequestMapping(value = "/api")', async () => {
+      const dir = path.join(tmpDir, 'spring-class-named-value');
+      fs.mkdirSync(path.join(dir, 'src/controller'), { recursive: true });
+      fs.writeFileSync(
+        path.join(dir, 'src/controller/OrderController.java'),
+        `
+package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+@RequestMapping(value = "/orders")
+public class OrderController {
+    @GetMapping("/list")
+    public List<Order> list() { return service.findAll(); }
+}
+`,
+      );
+
+      const contracts = await extractor.extract(null, dir, makeRepo(dir));
+      const providers = contracts.filter((c) => c.role === 'provider');
+
+      const route = providers.find((c) => c.contractId === 'http::GET::/orders/list');
+      expect(route).toBeDefined();
+    });
+
+    it('extracts Spring method-level @GetMapping(value = "/users") (named value)', async () => {
+      const dir = path.join(tmpDir, 'spring-method-named-value');
+      fs.mkdirSync(path.join(dir, 'src/controller'), { recursive: true });
+      fs.writeFileSync(
+        path.join(dir, 'src/controller/UserController.java'),
+        `
+package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+public class UserController {
+    @GetMapping(value = "/users")
+    public List<User> list() { return service.findAll(); }
+}
+`,
+      );
+
+      const contracts = await extractor.extract(null, dir, makeRepo(dir));
+      const providers = contracts.filter((c) => c.role === 'provider');
+
+      const route = providers.find((c) => c.contractId === 'http::GET::/users');
+      expect(route).toBeDefined();
+      expect(route!.symbolName).toBe('list');
+    });
+
+    it('extracts Spring method-level @GetMapping(path = "/users") (named path)', async () => {
+      const dir = path.join(tmpDir, 'spring-method-named-path-get');
+      fs.mkdirSync(path.join(dir, 'src/controller'), { recursive: true });
+      fs.writeFileSync(
+        path.join(dir, 'src/controller/UserController.java'),
+        `
+package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+public class UserController {
+    @GetMapping(path = "/users")
+    public List<User> list() { return service.findAll(); }
+}
+`,
+      );
+
+      const contracts = await extractor.extract(null, dir, makeRepo(dir));
+      const providers = contracts.filter((c) => c.role === 'provider');
+
+      const route = providers.find((c) => c.contractId === 'http::GET::/users');
+      expect(route).toBeDefined();
+      expect(route!.symbolName).toBe('list');
+    });
+
+    it('extracts Spring method-level @PostMapping(path = "/users") (named path)', async () => {
+      const dir = path.join(tmpDir, 'spring-method-named-path-post');
+      fs.mkdirSync(path.join(dir, 'src/controller'), { recursive: true });
+      fs.writeFileSync(
+        path.join(dir, 'src/controller/UserController.java'),
+        `
+package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+public class UserController {
+    @PostMapping(path = "/users")
+    public User create(@RequestBody User user) { return service.save(user); }
+}
+`,
+      );
+
+      const contracts = await extractor.extract(null, dir, makeRepo(dir));
+      const providers = contracts.filter((c) => c.role === 'provider');
+
+      const route = providers.find((c) => c.contractId === 'http::POST::/users');
+      expect(route).toBeDefined();
+      expect(route!.symbolName).toBe('create');
+    });
+
+    it('combines class named-arg prefix with method positional path', async () => {
+      const dir = path.join(tmpDir, 'spring-mixed-class-named-method-pos');
+      fs.mkdirSync(path.join(dir, 'src/controller'), { recursive: true });
+      fs.writeFileSync(
+        path.join(dir, 'src/controller/UserController.java'),
+        `
+package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+@RequestMapping(path = "/api")
+public class UserController {
+    @GetMapping("/users")
+    public List<User> list() { return service.findAll(); }
+}
+`,
+      );
+
+      const contracts = await extractor.extract(null, dir, makeRepo(dir));
+      const providers = contracts.filter((c) => c.role === 'provider');
+
+      const route = providers.find((c) => c.contractId === 'http::GET::/api/users');
+      expect(route).toBeDefined();
+    });
+
+    it('combines class positional prefix with method named-arg path', async () => {
+      const dir = path.join(tmpDir, 'spring-mixed-class-pos-method-named');
+      fs.mkdirSync(path.join(dir, 'src/controller'), { recursive: true });
+      fs.writeFileSync(
+        path.join(dir, 'src/controller/UserController.java'),
+        `
+package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+@RequestMapping("/api")
+public class UserController {
+    @GetMapping(value = "/users")
+    public List<User> list() { return service.findAll(); }
+}
+`,
+      );
+
+      const contracts = await extractor.extract(null, dir, makeRepo(dir));
+      const providers = contracts.filter((c) => c.role === 'provider');
+
+      const route = providers.find((c) => c.contractId === 'http::GET::/api/users');
+      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.
+      const dir = path.join(tmpDir, 'spring-produces-only');
+      fs.mkdirSync(path.join(dir, 'src/controller'), { recursive: true });
+      fs.writeFileSync(
+        path.join(dir, 'src/controller/MisleadingController.java'),
+        `
+package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+public class MisleadingController {
+    @GetMapping(produces = "application/json")
+    public List<User> list() { return service.findAll(); }
+}
+`,
+      );
+
+      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.
+      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.
+      const fromThisFile = providers.filter((c) =>
+        c.symbolRef.filePath.endsWith('MisleadingController.java'),
+      );
+      expect(fromThisFile).toHaveLength(0);
+    });
+
+    it('emits exactly one provider for @GetMapping(name = "...", value = "/users")', async () => {
+      // Anti-regression: without the `key:` constraint, the named-arg
+      // query would capture both string literals and emit two
+      // contracts (`/listUsers` + `/users`). With the constraint, only
+      // `/users` is emitted.
+      const dir = path.join(tmpDir, 'spring-name-and-value');
+      fs.mkdirSync(path.join(dir, 'src/controller'), { recursive: true });
+      fs.writeFileSync(
+        path.join(dir, 'src/controller/UserController.java'),
+        `
+package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+public class UserController {
+    @GetMapping(name = "listUsers", value = "/users")
+    public List<User> list() { return service.findAll(); }
+}
+`,
+      );
+
+      const contracts = await extractor.extract(null, dir, makeRepo(dir));
+      const providers = contracts.filter((c) => c.role === 'provider');
+
+      const usersRoute = providers.find((c) => c.contractId === 'http::GET::/users');
+      expect(usersRoute).toBeDefined();
+      expect(usersRoute!.symbolName).toBe('list');
+
+      // The non-route `name` attribute must NOT produce a route.
+      expect(providers.find((c) => c.contractId === 'http::GET::/listUsers')).toBeUndefined();
+
+      const fromThisFile = providers.filter((c) =>
+        c.symbolRef.filePath.endsWith('UserController.java'),
+      );
+      expect(fromThisFile).toHaveLength(1);
+    });
+
+    it('uses `path` (not non-route key) as class prefix when both appear', async () => {
+      // Anti-regression: without the `key:` constraint, the LAST
+      // element_value_pair in the annotation wins because
+      // prefixByClassId.set is called per match, in document order. So
+      // `@RequestMapping(path = "/api", name = "myApi")` would mistakenly
+      // set the prefix to `myApi`. With the constraint, only the
+      // `path`/`value` pair is captured and the prefix stays `/api`.
+      const dir = path.join(tmpDir, 'spring-class-prefix-last-wins');
+      fs.mkdirSync(path.join(dir, 'src/controller'), { recursive: true });
+      fs.writeFileSync(
+        path.join(dir, 'src/controller/UserController.java'),
+        `
+package com.example;
+import org.springframework.web.bind.annotation.*;
+
+@RestController
+@RequestMapping(path = "/api", name = "myApi")
+public class UserController {
+    @GetMapping("/users")
+    public List<User> list() { return service.findAll(); }
+}
+`,
+      );
+
+      const contracts = await extractor.extract(null, dir, makeRepo(dir));
+      const providers = contracts.filter((c) => c.role === 'provider');
+
+      const route = providers.find((c) => c.contractId === 'http::GET::/api/users');
+      expect(route).toBeDefined();
+
+      // Must NOT have used `myApi` as the class prefix.
+      expect(providers.find((c) => c.contractId === 'http::GET::/myApi/users')).toBeUndefined();
+    });
+
     it('extracts Express router.get patterns', async () => {
       const dir = path.join(tmpDir, 'express');
       fs.mkdirSync(path.join(dir, 'src/routes'), { recursive: true });