feat(group): extract OpenFeign @RequestLine consumer contracts#1904
Conversation
Adds Java HTTP plugin support for the native OpenFeign annotation
`@RequestLine("METHOD /path")`. Previously only `@FeignClient` interfaces
using Spring MVC method annotations (`@GetMapping` etc.) were detected;
the native annotation form — required by Feign Builder users and
non-Spring Feign deployments — was silently ignored.
Implementation:
- New `FEIGN_REQUEST_LINE_PATTERNS` covers both positional and named-arg
(`value =`) forms.
- New `parseRequestLine()` parses the verb+path string and drops any
query string (consistent with how RestTemplate/WebClient consumers
handle inline literal URLs).
- The enclosing interface MUST carry `@FeignClient`; otherwise the
detection is dropped to avoid false positives from same-named
annotations in unrelated libraries.
- Reuses the existing `feignPrefixByInterfaceId` map so
`@FeignClient(path=)` and `@RequestMapping` interface prefixes apply
uniformly across both Spring MVC and `@RequestLine` methods.
- Confidence 0.75 — slightly higher than the 0.7 used for Spring MVC
annotations because the verb is a string-literal value, not inferred
from the annotation name (less ambiguous).
Six new unit tests cover: basic two-method extraction; `@FeignClient(path=)`
prefix joining; query-string stripping; rejection of `@RequestLine` on
non-Feign interfaces; mixing with `@GetMapping` on the same interface;
named-argument form (`value = "..."`).
Verification: `npx tsc --noEmit`, full `test/unit/group` (31 files / 563
tests), `http-route-extractor.test.ts` (83/83 incl. 6 new), `prettier
--check` and `eslint` on touched files all pass.
|
Someone is attempting to deploy a commit to the NexusCore Team on Vercel. A member of the Team first needs to authorize it. |
| // @line → the literal `@RequestLine` | ||
| // @value → the request-line string literal (e.g. `"GET /users/{id}"`) | ||
| // @method → the enclosing method node, used for enclosing-interface lookup | ||
| const FEIGN_REQUEST_LINE_PATTERNS = compilePatterns({ |
There was a problem hiding this comment.
I think, we could optimise all of the annotations into one compilePatterns. It seems to me that we do this a couple of times that we don't need to. Could you please merge all of the annotation match queries into one? 🙏
There was a problem hiding this comment.
Done in f10854d3 — both the positional and named-argument (value =) forms now share a single tree-sitter query via [(...) (...)] alternation, so compilePatterns builds one query and runCompiledPatterns issues one matches() pass.
There was a problem hiding this comment.
Can you please give a more generic name to this please? 🙏
CI Report✅ All checks passed Pipeline Status
Test Results
✅ All 10182 tests passed 7 test(s) skipped — expand for details
Code CoverageTests
📋 View full run · Generated by CI |
…e query Per @magyargergo's review on PR abhigyanpatwari#1904 — uses tree-sitter alternation `[(...) (...)]` so the positional and named-argument forms of the `@RequestLine` annotation are matched by a single compiled query and invoked through one `runCompiledPatterns` pass instead of two.
|
Sorry I really meant it for all kinds of annotations that ends up as a request mapper |
|
@magyargergo Thanks for clarifying! I gave the full alternation merge a try and ran into a snag worth flagging. The Spring // identical input + grammar; only the predicate differs
#eq? alternation → 1 match ✅
#match? alternation → 0 matches ❌That's why the Two ways to land your request:
I'd lean toward (b) — it gets the consolidation win without the binding caveat. Happy to take the (a) route if you'd prefer the pure-alternation form, or do (b) and revisit (a) once we bump tree-sitter. Let me know which way you'd like to go 🙏 |
| // @line → the literal `@RequestLine` | ||
| // @value → the request-line string literal (e.g. `"GET /users/{id}"`) | ||
| // @method → the enclosing method node, used for enclosing-interface lookup | ||
| const FEIGN_REQUEST_LINE_PATTERNS = compilePatterns({ |
There was a problem hiding this comment.
Can you please give a more generic name to this please? 🙏
…ant names Per review feedback on abhigyanpatwari#1904 — renames the four route-mapper pattern constants to framework-agnostic names (the per-constant comments already document which framework each targets): SPRING_TYPE_PREFIX_PATTERNS -> TYPE_PREFIX_PATTERNS FEIGN_REQUEST_LINE_PATTERNS -> REQUEST_LINE_PATTERNS FEIGN_INTERFACE_PREFIX_PATTERNS -> INTERFACE_PREFIX_PATTERNS SPRING_METHOD_ROUTE_PATTERNS -> METHOD_ROUTE_PATTERNS
|
@henry201605 To summarise what I'm looking for on the consolidation feedback: Goal: one How it should work:
The performance win is fewer tree-sitter passes; the behaviour should stay the same. Happy to discuss if anything in the current query shape makes a particular branch hard to capture cleanly. |
|
@henry201605 Don't worry about this! I'll get it out today with the changes I want to see. |
|
@magyargergo Actually, I'd already taken a stab at the consolidation before seeing this — happy to push it for you to review, or just share the approach so you can take it from here, whatever's least disruptive. One thing worth flagging that I hit: collapsing the route-mapper annotations into a single alternation with How I worked around it: make the single query purely structural (class / interface / method × positional / named, zero predicates) and do all the annotation-name / key filtering in JS while iterating the matches — which lines up with your steps 2 & 3 (capture-tag + branch in the loop), just with the filtering in JS instead of query predicates. That collapses the four annotation pattern blocks ( Scope note: the method-invocation consumers ( I can push it to this branch if useful, or leave it entirely to you — your call. 🙏 |
Merge the four annotation pattern bundles (Spring @RequestMapping type prefix, @FeignClient(path) prefix, @(Get|Post|Put|Delete|Patch)Mapping method routes and native @RequestLine) into a single JAVA_ROUTE_ANNOTATION_PATTERNS query, read by scanRouteAnnotations() in exactly one matches() pass per file. Variants are tagged by branch-local captures and discriminated in JS (METHOD_ANNOTATION_TO_HTTP, isRouteMemberKey), per review feedback. This drops the per-file annotation passes from 4->1 in scan() and 2->1 in collectSpringTypes(), and removes the interface-@RequestMapping / @FeignClient prefix redundancy. Verb and path/value key filtering stay in JS rather than in-query: under the pinned tree-sitter 0.21.1 binding a top-level [...] alternation compiles to one pattern whose text predicates share a single bucket keyed by capture name. A #match? against a capture absent from the matched branch evaluates FALSE and silently drops every sibling-branch match, whereas #eq? against an absent capture is vacuously true. So only fixed annotation names use in-query #eq? (on branch-local captures); the variable verb name and member key carry no in-query predicate. Behaviour is unchanged for all compilable Java; existing http-route tests (93) and the full group suite remain green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… in loop Collapse JAVA_ROUTE_ANNOTATION_PATTERNS from 9 annotation-name-pinned branches to 6 generic structural branches (class/interface/method x positional/named) that capture the annotation name (@ann), declaration (@node), argument (@value) and member key (@key) generically. The query now carries NO #eq?/#match? predicates at all; scanRouteAnnotations reads @ann.text and @node.type in its for-loop to decide what each match means (RequestMapping prefix, FeignClient(path) prefix, @(Get|...)Mapping route, or @RequestLine), ignoring unrecognised annotations. This makes the query framework-agnostic and extensible — adding a new route annotation is a change to the loop and the lookup maps, not the query — and removes the last tree-sitter-0.21.1 shared-predicate-bucket footgun, since a predicate-free alternation cannot drop sibling branches. Behaviour is byte-identical: 93 targeted http-route tests and the full 569-test group suite stay green; tsc and prettier clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…larify invariants Code-review follow-up to the route-annotation query consolidation. No behaviour change to the extractor: - Add two regression tests for branches the generic predicate-free query made reachable in scanRouteAnnotations: (1) a @RequestLine whose named argument is not `value` must be dropped (the in-query `#eq? @key "value"` guard now lives in JS); (2) @FeignClient(path) must win over @RequestMapping even when @RequestMapping is the first annotation in source order, covering the deferred interfaceRequestMappingPrefixes apply (the existing precedence test only covered @FeignClient-first). - Document two invariants flagged in review: why prefixByTypeId and feignPrefixByInterfaceId intentionally diverge for the same interface node (Spring provider vs OpenFeign consumer prefix), and that the query's single-string-argument shape excludes array-valued annotations. http-route-extractor + multi-verb suites: 95/95 (was 93); tsc + prettier clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…@FeignClient) (#1917) * fix(group): recognize OpenFeign @RequestLine on plain interfaces (no @FeignClient) PR #1904 gated @RequestLine consumer extraction on the enclosing interface also carrying @FeignClient. That guard is wrong: @RequestLine is a core feign.* annotation used with Feign.builder(), while @FeignClient is the Spring Cloud variant that uses Spring MVC annotations (@GetMapping etc.) — the two are effectively mutually exclusive. Requiring @FeignClient therefore excluded the annotation's primary, canonical usage, so the feature recognized nothing on real core-Feign client interfaces. Fix: drop the @FeignClient requirement for @RequestLine. The match still requires an enclosing interface (Feign proxies are always interfaces), and the `RequestLine` annotation name is itself a strong, framework-specific signal, so false-positive risk stays low. A @FeignClient(path=...) prefix is still applied when present. The @(Get|Post|...)Mapping consumer path keeps its @FeignClient requirement: those annotations are generic Spring MVC and need the Feign context to be disambiguated from provider routes. Verification (real-world, not just synthetic fixtures): - A real client-jar consumer (BigModeClientService.java: a plain interface with 12 @RequestLine methods, no @FeignClient) now yields 12 openfeign consumer contracts; it yielded 0 before this change. - End-to-end `group sync` over that consumer repo + its FastAPI provider repo (with zero hand-written links) produces 12 exact cross-links (confidence 1.0), Java @RequestLine consumer → Python route provider. - The prior test that asserted the wrong behavior ("ignores @RequestLine on interfaces without @FeignClient") is reversed into a realistic core-Feign fixture. - Full test/unit/group suite (579) green; tsc and prettier clean. * test(group): add negative cases for relaxed @RequestLine matcher Per review on #1917 — guard the no-@FeignClient relaxation with explicit negative tests: malformed @RequestLine values (no verb / no leading-slash path / unknown verb) yield no contract, and @RequestLine on a concrete class method (not an interface) is not emitted as a consumer. --------- Co-authored-by: henry <zhangwei2017@unipus.cn> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Adds Java HTTP plugin support for the native OpenFeign annotation
@RequestLine("METHOD /path"). Previously only@FeignClientinterfaces using Spring MVC method annotations (@GetMappingetc.) were detected; the native annotation form — required by Feign Builder users and non-Spring Feign deployments — was silently ignored.Why
OpenFeign’s
@RequestLineis the canonical annotation for the framework (https://github.com/OpenFeign/feign#interface-annotations) and is the only option for clients built without Spring Cloud OpenFeign. Real-world Java microservices that mix Spring Cloud and plain Feign Builder clients (e.g. internalclient-jarlibraries shared across non-Spring services) fall back to manifest-link workarounds today because GitNexus produces no consumer contract for these methods.What changed
FEIGN_REQUEST_LINE_PATTERNS— tree-sitter query covering both positional and named-arg (value =) forms.parseRequestLine()— splits the verb + path string and drops any query portion, matching howRestTemplate/WebClientinline URL consumers are normalized.@FeignClient; otherwise the detection is dropped to avoid false positives from same-named annotations in unrelated libraries.feignPrefixByInterfaceIdmap so@FeignClient(path=)and@RequestMappinginterface prefixes apply uniformly across both Spring MVC and@RequestLinemethods.0.75— slightly higher than the0.7used for Spring MVC annotations because the verb is a string-literal value (less ambiguous than annotation-name inference).Tests
Six new unit tests under
extracts OpenFeign clientsblock:POST /ai/summarizeandGET /ai/healthextracted withframework: openfeign,confidence: 0.75@FeignClient(path = "/api")prefix joining/api/orders/{param}for both GET and DELETEGET /search?q={query}&limit={limit}→http::GET::/search@FeignClientannotation@RequestLine+@GetMappingon the same interface@RequestLine(value = "...")Verification
npx tsc --noEmit✅test/unit/group/http-route-extractor.test.ts— 83/83 (77 existing + 6 new) ✅test/unit/group/— 31 files / 563 tests ✅npx prettier --checkon touched files ✅npx eslinton touched files — 0 errors, 0 warnings introduced ✅