From da4a0c91c993dcb94f0c1783f7143441ee744594 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Mon, 22 Jun 2026 09:45:11 +0100 Subject: [PATCH] feat(group): expand Java and Kotlin HTTP consumer extraction (re #1888) (#2268) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(group): extract RestTemplate URI.create(...) static paths (Java) Widen the RestTemplate query @path captures to (_) and resolve URI.create("/x") arguments via a new extractStaticPathExpression helper. Variable-bound paths stay unresolved (no consumer). * feat(group): resolve RestTemplate UriComponentsBuilder chains (Java) Add appendPath + recursive extractUriComponentsBuilderPath (fromPath/ fromUriString/fromHttpUrl seeds, path/pathSegment append, build/query passthrough). Host-bearing seeds are normalized downstream. Non-literal segments stay unresolved. * feat(group): infer OkHttp request verb from builder chain (Java) Walk up from the matched .url(...) call to the sibling verb helper (.post()/.method("X")), defaulting to GET. Variable-bound verbs stay GET. Re-document the Kotlin OkHttp GET-default pin as an accepted Java/Kotlin asymmetry (Kotlin verb-walk is a tracked follow-up). * feat(group): Java HttpClient HEAD, .method("X"), and default-GET Add HEAD to the verb-helper regex and two dedicated pattern families: .method("VERB", body) (covers PATCH) and a bare .build() defaulting to GET. The three families terminate at distinct calls, so each chain matches exactly one (no double-emit); variable-bound verbs stay unresolved. * feat(group): extract fully-qualified Java route annotations Widen JAVA_ROUTE_ANNOTATION_PATTERNS @ann to match scoped_identifier (predicate-free node-type change), normalizing to the trailing segment via simpleName in the scan loop. Snapshot-verified: only previously unmatched FQN annotations gain routes; existing contracts unchanged. Brings Java to FQN parity with the Kotlin plugin (closes #2254 limitation). * refactor(group): reuse simpleName helper in hasAnnotation Drop the inline split('.').pop() (which shadowed the new module-level simpleName) and call the helper. Note appendPath's deliberate divergence from the shared joinPath so it is not accidentally unified. * docs(test): fix reversed Java/Kotlin OkHttp asymmetry comment The comment stated .kt->POST / .java->GET; it is the opposite — Java infers the verb (inferOkHttpMethod) so .java emits POST, while Kotlin still defaults so .kt emits GET. Aligns the prose with the assertion below it. * docs(group): correct stale kotlin.ts OkHttp parity comment The comment claimed Kotlin's GET-default 'mirrors java.ts:OK_HTTP_PATTERNS' and is 'the same trade-off Java has accepted'. The Java plugin now infers the verb (inferOkHttpMethod), so this is now a documented Java/Kotlin asymmetry; the Kotlin verb-walk is the tracked follow-up. * test(group): pin OkHttp default-GET branch on a distinct path The bare `.build()` (no verb call) now uses /api/bare-build and is asserted individually, so the default-GET branch of inferOkHttpMethod is no longer masked by the explicit .get() case collapsing into the same {param} slot. * fix(group): skip OkHttp emission for variable-bound .method(verb) inferOkHttpMethod now returns string|null: an explicit .method(verb, …) with a non-literal verb returns null and the loop skips it, instead of asserting a wrong GET contract. A bare .url().build() with no verb call still defaults to GET (OkHttp's real default). Matches WebClient long-form, which also skips variable-bound verbs. * fix(group): strip query from UriComponentsBuilder seed literal A query baked into the seed (fromUriString("/base?x=1")) was returned verbatim, so a later .path("/sub") glued onto it (/base?x=1/sub) and normalizeHttpPath truncated the tail at ? to /base. Strip ?query at the seed so .path() appends to a clean base → /base/sub. Host prefixes are preserved and stripped downstream by normalizeConsumerPath. * fix(group): add recursion depth guard to extractUriComponentsBuilderPath The recursive builder-chain walk was unbounded; a pathological or machine-generated chain could overflow the stack. Cap recursion at MAX_BUILDER_DEPTH (100) and return null past it — consistent with the project's other AST-depth guards. * docs(group): document accepted FQN simple-name collision trade-off The route discriminator matches on the trailing annotation segment, so a non-Spring annotation sharing a route name (@com.evil.GetMapping) is treated as a route — the same trade-off hasAnnotation makes and the intended Kotlin parity. Note why package-origin gating is deliberately not added. * refactor(group): extract static-path helpers to java-static-path.ts Move the URI.create / UriComponentsBuilder resolution helpers (methodInvocation*, firstLiteralArgument, appendPath, extractUri*, extractStaticPathExpression) out of java.ts (back under ~1000 lines). java.ts imports the four it consumes; inferOkHttpMethod stays. Pure move, behavior-preserving — full group suite unchanged. * fix(group): walk builder chain for Java HttpClient verb (#2268) Replace the three rigid JAVA_HTTP_CLIENT_* pattern families with one .uri()-anchored query plus inferHttpClientMethod, which walks up the fluent chain for the verb (mirroring inferOkHttpMethod). The walk is transparent to intervening .header()/.timeout()/.version() calls, so a header/timeout hop before the terminal no longer silently drops the consumer contract. Relocate both verb-walks onto a shared inferBuilderVerb in java-static-path.ts and de-export the now-internal methodInvocation* primitives; java.ts drops 1015 -> 910 lines. * fix(group): append UriComponentsBuilder .path() verbatim (#2268) Spring's UriComponentsBuilder.path(p) appends p as-is without inserting a slash (then collapses duplicate slashes), unlike .pathSegment() which slash-joins. The resolver used the always-one-slash appendPath for both, so fromPath("/api").path("users") resolved to /api/users instead of Spring's /apiusers. Switch the .path() branch to verbatim append plus a colon-aware duplicate-slash collapse (preserving a host seed's ://); .pathSegment() keeps appendPath. * fix(group): skip empty-string verb literal in builder verb-walk (#2268) `.method("", body)` produced a malformed `http::::/path` consumer: unquoteLiteral('""') returns "" (not null), so the `=== null` guard let an empty method through. Treat a falsy literal verb as unresolvable (return null from the shared inferBuilderVerb) and switch the OkHttp and HttpClient emission guards to falsiness, so an empty verb skips like a variable-bound one. * test(group): harden Java HTTP consumer coverage (#2268) Add coverage beyond the tri-review findings: a count guard on the UriComponentsBuilder query-seed test (so a double-emit can't slip past the two find assertions), an exchange()+UriComponentsBuilder end-to-end case (the widened (_) @path exchange capture was only covered with URI.create), and an HttpClient .method("REPORT") custom-verb pass-through pin. * docs(group): document pre-path builder rigidity + fix stale refs (#2268) Document the OkHttp pre-.url() limitation (a builder call before .url() is missed) at OK_HTTP_PATTERNS, cross-referencing the Java-HttpClient pre-.uri() dual the verb-walk rewrite leaves in place — so neither comment overclaims that the chain is walked before the path call. Update the now-stale 'inferOkHttpMethod in java.ts' references in kotlin.ts and the test to point at java-static-path.ts after the relocation. * feat(group): match Java HTTP consumer chains with a pre-path builder call (#2268) The OkHttp .url() and HttpClient .uri() queries required the path call to sit directly on the construction, so a builder call BEFORE it — new Request.Builder().addHeader(...).url(...) or HttpRequest.newBuilder().version(v).uri(...) — silently dropped the consumer contract. Match the path call on any receiver and re-impose the framework anchor in JS (okHttpUrlRootsAtBuilder / httpClientUriRootsAtNewBuilder: the chain must root at new Request.Builder() / HttpRequest.newBuilder()), so a preceding call is captured while an unrelated .url()/.uri() is rejected. The verb-walk now scans the whole chain, so a verb set before the path call also resolves. Also extract the HttpRequest.newBuilder(URI.create(...)) constructor-arg form (skipped when a later .uri() overrides it). Resolves the deferred follow-ups from the round-2 tri-review. * feat(group): Kotlin OkHttp verb-walk parity with Java (#2268) The Kotlin OkHttp consumer always emitted GET while the Java side walks the builder chain to recover the verb — a documented Java/Kotlin asymmetry. Mirror the verb-walk into kotlin.ts, adapted to the tree-sitter-kotlin call_expression/navigation_expression grammar: match .url("literal") on any receiver, gate to chains rooting at Request.Builder() (kotlinUrlRootsAtRequestBuilder), and scan the whole chain for the verb (inferKotlinOkHttpMethod — last-wins, null-skip for a variable/empty .method(verb), resolves a named-argument .method(method="X")). This brings .kt to full parity with .java — verb inference, a builder call before .url(), and verb-before-url — pinned by two new Java<->Kotlin parity-harness rows. Flips the former GET-default asymmetry test. --- .../http-patterns/java-static-path.ts | 269 ++++++ .../group/extractors/http-patterns/java.ts | 197 +++-- .../group/extractors/http-patterns/kotlin.ts | 166 +++- .../unit/group/http-route-extractor.test.ts | 834 +++++++++++++++++- 4 files changed, 1346 insertions(+), 120 deletions(-) create mode 100644 gitnexus/src/core/group/extractors/http-patterns/java-static-path.ts diff --git a/gitnexus/src/core/group/extractors/http-patterns/java-static-path.ts b/gitnexus/src/core/group/extractors/http-patterns/java-static-path.ts new file mode 100644 index 0000000000..cf51e5d3f0 --- /dev/null +++ b/gitnexus/src/core/group/extractors/http-patterns/java-static-path.ts @@ -0,0 +1,269 @@ +import Parser from 'tree-sitter'; +import { unquoteLiteral } from '../tree-sitter-scanner.js'; + +// ─── Statically-resolvable consumer path + builder verb-walk helpers ────── +// RestTemplate calls increasingly pass a non-literal path argument that is +// still statically derivable — `URI.create("/x")` or a `UriComponentsBuilder` +// fluent chain. These helpers resolve those shapes to a literal path; a +// genuinely dynamic argument (a variable, a non-`URI`/`UriComponentsBuilder` +// call) resolves to null and the call site is skipped. This module also owns +// the OkHttp / Java-HttpClient builder verb-walks (`inferOkHttpMethod` / +// `inferHttpClientMethod`), which recover the request verb by walking UP the +// fluent chain from the matched `.url(...)` / `.uri(...)` call. Extracted from +// java.ts (#2268) so that plugin stays under ~1000 lines; the `methodInvocation*` +// primitives + `firstLiteralArgument` are module-internal (shared by the path +// resolvers and the verb-walks), while the path resolver and the two verb-walks +// are java.ts's only entry points here. + +function methodInvocationName(node: Parser.SyntaxNode): string | null { + return node.type === 'method_invocation' ? (node.childForFieldName('name')?.text ?? null) : null; +} + +function methodInvocationObject(node: Parser.SyntaxNode): Parser.SyntaxNode | null { + return node.type === 'method_invocation' ? node.childForFieldName('object') : null; +} + +function methodInvocationArguments(node: Parser.SyntaxNode): Parser.SyntaxNode[] { + const argsNode = node.type === 'method_invocation' ? node.childForFieldName('arguments') : null; + return argsNode?.namedChildren ?? []; +} + +function firstLiteralArgument(node: Parser.SyntaxNode): string | null { + const first = methodInvocationArguments(node)[0]; + return first?.type === 'string_literal' ? unquoteLiteral(first.text) : null; +} + +/** Resolve a `URI.create("/path")` call to its literal path; null otherwise. */ +function extractUriCreatePath(node: Parser.SyntaxNode): string | null { + if (node.type !== 'method_invocation') return null; + if (methodInvocationObject(node)?.text !== 'URI' || methodInvocationName(node) !== 'create') + return null; + return firstLiteralArgument(node); +} + +// Join a builder base with a sub-path using exactly one separating slash. This +// is intentionally NOT the shared `joinPath`: `joinPath` force-prepends `/`, +// whereas `appendPath` must preserve an absolute/host base (`fromHttpUrl` +// "https://host/api") so the host survives until `normalizeConsumerPath` strips +// it downstream. Do not unify the two. +function appendPath(base: string, subPath: string): string { + if (!base) return subPath.startsWith('/') ? subPath : `/${subPath}`; + if (!subPath) return base; + return `${base.replace(/\/+$/, '')}/${subPath.replace(/^\/+/, '')}`; +} + +/** + * Resolve a `UriComponentsBuilder` fluent chain to its literal path. Seed + * methods (`fromPath`/`fromUriString`/`fromHttpUrl`) return the literal arg + * VERBATIM — a `fromHttpUrl("https://host/api")` seed keeps its host, which the + * shared `normalizeConsumerPath` later reduces to the path (the same single + * normalization point every other consumer path goes through). `path` and + * `pathSegment` append literal segments; `build`/`toUriString`/`toUri`/`encode` + * and the `query*` family pass through (query attributes do not change the + * path). Any non-literal segment or unknown call → null. + */ +// A UriComponentsBuilder chain deeper than this is not realistic source; cap the +// recursion so a pathological / machine-generated chain returns null instead of +// overflowing the stack (mirrors the project's other AST-depth guards). +const MAX_BUILDER_DEPTH = 100; + +function extractUriComponentsBuilderPath(node: Parser.SyntaxNode, depth = 0): string | null { + if (depth > MAX_BUILDER_DEPTH) return null; + if (node.type !== 'method_invocation') return null; + const name = methodInvocationName(node); + const objectNode = methodInvocationObject(node); + if ( + (name === 'fromPath' || name === 'fromUriString' || name === 'fromHttpUrl') && + objectNode?.text === 'UriComponentsBuilder' + ) { + // Strip any `?query` baked into the seed literal so a later `.path()` appends + // to a clean base; otherwise the sub-path is glued after the query + // (`/base?x=1/sub`) and normalizeHttpPath truncates the whole tail at `?`. + // A host prefix (`https://h/api`) is preserved and stripped downstream by + // normalizeConsumerPath. + const seed = firstLiteralArgument(node); + return seed === null ? null : seed.split('?')[0]; + } + if (!objectNode) return null; + if (name === 'path') { + const base = extractUriComponentsBuilderPath(objectNode, depth + 1); + const subPath = firstLiteralArgument(node); + if (base === null || subPath === null) return null; + // Spring `UriComponentsBuilder.path(p)` appends `p` VERBATIM (no slash + // inserted — unlike `pathSegment`, which slash-joins), then normalizes the + // full path to collapse duplicate slashes. So `fromPath("/api").path("users")` + // → `/apiusers`, while `.path("/users")` → `/api/users`, and a trailing-slash + // base collapses (`/api/` + `/users` → `/api/users`). The `(? (arg.type === 'string_literal' ? unquoteLiteral(arg.text) : null)) + .filter((segment): segment is string => segment !== null); + if (segments.length !== args.length) return null; // a non-literal segment defeats static resolution + return segments.reduce((acc, segment) => appendPath(acc, segment), base); + } + if ( + name === 'build' || + name === 'toUriString' || + name === 'toUri' || + name === 'encode' || + name === 'query' || + name === 'queryParam' || + name === 'queryParams' || + name === 'replaceQuery' || + name === 'replaceQueryParam' || + name === 'replaceQueryParams' + ) + return extractUriComponentsBuilderPath(objectNode, depth + 1); + return null; +} + +/** + * Resolve a statically-derivable path argument to a literal path: a bare + * string literal, a `URI.create("/x")` call, or a `UriComponentsBuilder` + * fluent chain. Genuinely dynamic arguments → null. + */ +export function extractStaticPathExpression(node: Parser.SyntaxNode): string | null { + if (node.type === 'string_literal') return unquoteLiteral(node.text); + return extractUriCreatePath(node) ?? extractUriComponentsBuilderPath(node); +} + +// ─── Builder verb-walks ─────────────────────────────────────────────── +// OkHttp / Java-HttpClient encode the request verb on a SIBLING call elsewhere in +// the fluent chain, not on the matched path call. Both queries capture the path +// call (`.url(...)` / `.uri(...)`); these helpers recover the verb by scanning +// the WHOLE chain — transparent to neutral calls (`.addHeader()`/`.header()`/ +// `.timeout()`/`.version()`) wherever they sit relative to the path call, so the +// verb resolves whether it precedes or follows the path call. + +/** Every `method_invocation` in the fluent chain `pathCall` belongs to, ordered + * innermost (next to the construction) → outermost (terminal). Lets the verb-walk + * find the verb wherever it sits, and lets the root gates inspect the chain base. */ +function builderChainCalls(pathCall: Parser.SyntaxNode): Parser.SyntaxNode[] { + let innermost = pathCall; + let obj = methodInvocationObject(innermost); + while (obj?.type === 'method_invocation') { + innermost = obj; + obj = methodInvocationObject(innermost); + } + const calls: Parser.SyntaxNode[] = [innermost]; + let cur = innermost; + let parent = cur.parent; + while (parent?.type === 'method_invocation' && methodInvocationObject(parent)?.id === cur.id) { + calls.push(parent); + cur = parent; + parent = parent.parent; + } + return calls; +} + +/** Scan the chain for the verb a `name`-matching helper or a `.method("LITERAL")` + * call sets, resolving the LAST such call — each verb-setter overwrites the + * previous at runtime, so on the (non-idiomatic) chain that sets two verbs the + * one nearest the terminal wins. `null` means "verb is set but not statically + * resolvable" (a non-literal/empty `.method(verb)`) → the caller skips rather + * than guessing. `defaultVerb` is returned only when the chain has no verb call + * at all (a bare build). `verbHelpers` are matched on the method NAME (OkHttp + * helpers are lowercase, HttpClient helpers uppercase). */ +function inferBuilderVerb( + pathCall: Parser.SyntaxNode, + verbHelpers: readonly string[], + defaultVerb: string, +): string | null { + let lastVerbCall: Parser.SyntaxNode | null = null; + for (const call of builderChainCalls(pathCall)) { + if (call.id === pathCall.id) continue; // the path call itself is never the verb + const name = methodInvocationName(call); + if (name === 'method' || (name !== null && verbHelpers.includes(name))) lastVerbCall = call; + } + if (lastVerbCall === null) return defaultVerb; // no verb call → builder default + const name = methodInvocationName(lastVerbCall); + // Explicit `.method(...)`: a non-empty string-literal verb resolves; a + // non-literal (variable-bound) OR empty-string verb is unresolvable → null + // (skip), NOT a guessed default or a malformed empty-method contract. + if (name === 'method') { + const verb = firstLiteralArgument(lastVerbCall); + return verb ? verb.toUpperCase() : null; + } + return name === null ? defaultVerb : name.toUpperCase(); // a verb-helper name +} + +const OK_HTTP_VERB_HELPERS = ['get', 'head', 'post', 'put', 'delete', 'patch'] as const; +const HTTP_CLIENT_VERB_HELPERS = ['GET', 'POST', 'PUT', 'DELETE', 'HEAD'] as const; + +/** + * Infer an OkHttp request verb by scanning the builder chain around the matched + * `.url(...)` call: a `.get()/.head()/.post()/.put()/.delete()/.patch()` helper + * (before or after `.url()`), or a `.method("VERB", …)` literal, wins. Returns + * `'GET'` only when the chain has NO verb call at all (a bare `.url(...).build()` + * — OkHttp's real default). Returns `null` for an explicit `.method(verb, …)` + * whose verb is a non-literal: the verb is set but not statically resolvable, so + * the caller skips the call rather than asserting a wrong GET (parity with the + * WebClient long-form variable-bound-verb behavior, which also skips). + */ +export function inferOkHttpMethod(urlCall: Parser.SyntaxNode): string | null { + return inferBuilderVerb(urlCall, OK_HTTP_VERB_HELPERS, 'GET'); +} + +/** + * Infer a Java-`HttpClient` request verb by walking UP the builder chain from the + * matched `.uri(URI.create("..."))` call: the first `.GET()/.POST()/.PUT()/` + * `.DELETE()/.HEAD()` verb-helper, or a `.method("VERB", body)` literal, wins. + * Returns `'GET'` only when the chain has no verb call (a bare `.build()` — the + * builder's real default). Returns `null` for an explicit `.method(verb, …)` with + * a non-literal verb (skip, not a guessed GET). The scan is transparent to + * neutral calls anywhere in the chain (`.header()`/`.timeout()`/`.version()`), + * before or after `.uri()`, so neither a header/timeout hop nor a verb call's + * position drops the contract. + */ +export function inferHttpClientMethod(uriCall: Parser.SyntaxNode): string | null { + return inferBuilderVerb(uriCall, HTTP_CLIENT_VERB_HELPERS, 'GET'); +} + +// ─── Builder-chain root gates (anti-overreach) ──────────────────────── +// The path queries match a bare `.url(...)` / `.uri(URI.create(...))` call on ANY +// receiver so that a builder call BEFORE the path call (`new Request.Builder()` +// `.addHeader(...).url(...)`, `HttpRequest.newBuilder().version(v).uri(...)`) is +// still captured. These gates re-impose the framework anchor in JS — only a chain +// that bottoms out on the right construction emits, so a `.url(...)`/`.uri(...)` +// on an unrelated object does not. + +/** True when `urlCall`'s receiver chain bottoms out on a `new Request.Builder()` + * object-creation (descending the `.object` chain past any intervening calls). */ +export function okHttpUrlRootsAtBuilder(urlCall: Parser.SyntaxNode): boolean { + let obj = methodInvocationObject(urlCall); + while (obj?.type === 'method_invocation') obj = methodInvocationObject(obj); + return ( + obj?.type === 'object_creation_expression' && + obj.childForFieldName('type')?.text === 'Request.Builder' + ); +} + +/** True when `uriCall`'s receiver chain includes a `HttpRequest.newBuilder()`. */ +export function httpClientUriRootsAtNewBuilder(uriCall: Parser.SyntaxNode): boolean { + let obj = methodInvocationObject(uriCall); + while (obj?.type === 'method_invocation') { + if ( + methodInvocationName(obj) === 'newBuilder' && + methodInvocationObject(obj)?.text === 'HttpRequest' + ) + return true; + obj = methodInvocationObject(obj); + } + return false; +} + +/** True when a `HttpRequest.newBuilder(URI.create(...))` chain ALSO calls `.uri(...)` + * later — a later `.uri()` overrides the constructor URI at runtime, so the + * constructor-arg path must NOT be emitted (the `.uri()` query emits the override). */ +export function httpClientChainHasUriCall(newBuilderCall: Parser.SyntaxNode): boolean { + return builderChainCalls(newBuilderCall).some( + (call) => call.id !== newBuilderCall.id && methodInvocationName(call) === 'uri', + ); +} diff --git a/gitnexus/src/core/group/extractors/http-patterns/java.ts b/gitnexus/src/core/group/extractors/http-patterns/java.ts index 5afebbaced..7d70864cda 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/java.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/java.ts @@ -27,6 +27,14 @@ import { REQUEST_LINE_CONFIDENCE, EXCHANGE_CONFIDENCE, } from './spring-consumer-shared.js'; +import { + extractStaticPathExpression, + inferOkHttpMethod, + inferHttpClientMethod, + okHttpUrlRootsAtBuilder, + httpClientUriRootsAtNewBuilder, + httpClientChainHasUriCall, +} from './java-static-path.js'; import type { HttpDetection, HttpFileDetections, @@ -98,17 +106,15 @@ import type { // sidesteps that hazard entirely; all name/key discrimination lives in the // for-loop, where it reads as straight-line code. // -// KNOWN LIMITATION — fully-qualified route annotations are not matched. `@ann` -// binds `name: (identifier)`, but a FQN annotation (`@org.springframework… -// GetMapping("/x")`) parses its name as a `scoped_identifier`, which this query -// does not match, so its route is not extracted. (The class is still recognized -// as a controller — `hasAnnotation` trailing-segment-matches the FQN — only the -// route-string extraction is missed.) In practice annotations are imported and -// written by simple name, so this is rare. It is a minor asymmetry with the -// Kotlin plugin, whose grammar models a FQN as separate `type_identifier` -// segments that its route queries DO match. Aligning Java would mean matching -// `scoped_identifier` too; that is deferred to avoid re-keying existing Java -// contracts via the predicate hazard above. Pinned by an anti-overreach test. +// FULLY-QUALIFIED route annotations ARE matched. `@ann` binds either an +// `identifier` (simple name) or a `scoped_identifier` (a FQN annotation such as +// `@org.springframework…GetMapping("/x")`); the for-loop normalizes the name to +// its trailing segment via `simpleName` before discriminating. This is a node- +// type widening only — the query stays predicate-free, so it does NOT reintroduce +// the bucket hazard above, and a simple name maps to itself so existing Java +// contracts are unchanged (only previously-unmatched FQN annotations gain +// routes). This brings Java to parity with the Kotlin plugin, whose grammar +// already matches FQN route annotations. const JAVA_ROUTE_ANNOTATION_PATTERNS = compilePatterns({ name: 'java-route-annotation', language: Java, @@ -120,17 +126,17 @@ const JAVA_ROUTE_ANNOTATION_PATTERNS = compilePatterns({ (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 (interface_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 @@ -138,7 +144,7 @@ const JAVA_ROUTE_ANNOTATION_PATTERNS = compilePatterns({ (interface_declaration (modifiers (annotation - name: (identifier) @ann + name: [(identifier) (scoped_identifier)] @ann arguments: (annotation_argument_list (element_value_pair key: (identifier) @key @@ -146,13 +152,13 @@ const JAVA_ROUTE_ANNOTATION_PATTERNS = compilePatterns({ (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)]))) name: (identifier) @member) @node (method_declaration (modifiers (annotation - name: (identifier) @ann + name: [(identifier) (scoped_identifier)] @ann arguments: (annotation_argument_list (element_value_pair key: (identifier) @key @@ -199,7 +205,7 @@ const REST_TEMPLATE_PATTERNS = compilePatterns({ (method_invocation object: (identifier) @obj (#eq? @obj "restTemplate") name: (identifier) @method - arguments: (argument_list . (string_literal) @path)) + arguments: (argument_list . (_) @path)) `, }, ], @@ -216,7 +222,7 @@ const REST_TEMPLATE_EXCHANGE_PATTERNS = compilePatterns({ object: (identifier) @obj (#eq? @obj "restTemplate") name: (identifier) @method (#eq? @method "exchange") arguments: (argument_list - . (string_literal) @path + . (_) @path (field_access object: (identifier) @httpMethodCls (#eq? @httpMethodCls "HttpMethod") field: (identifier) @http_method))) @@ -275,10 +281,14 @@ const WEB_CLIENT_LONG_FORM_PATTERNS = compilePatterns({ ], } satisfies LanguagePatterns>); -// ─── Consumer: OkHttp `new Request.Builder().url("path")` ───────────── -// Note: `Request.Builder` is a `scoped_type_identifier` whose text includes -// the dot, so `#eq?` against the literal string matches cleanly (no need -// to escape a regex dot). +// ─── Consumer: OkHttp `Request.Builder()…url("path")` ───────────────── +// Match a bare `.url("literal")` call on ANY receiver, capturing it as `@call`. +// `okHttpUrlRootsAtBuilder` (JS) then verifies the receiver chain bottoms out on +// `new Request.Builder()` — re-imposing the framework anchor while allowing +// builder calls BEFORE `.url()` (`new Request.Builder().addHeader(...).url("/x")`) +// that the old object-direct query dropped, and rejecting a `.url(...)` on an +// unrelated object. The verb is recovered by `inferOkHttpMethod` scanning the +// whole chain (java-static-path.ts). const OK_HTTP_PATTERNS = compilePatterns({ name: 'java-okhttp', language: Java, @@ -287,15 +297,22 @@ const OK_HTTP_PATTERNS = compilePatterns({ meta: {}, query: ` (method_invocation - object: (object_creation_expression - type: (scoped_type_identifier) @type (#eq? @type "Request.Builder")) name: (identifier) @method (#eq? @method "url") - arguments: (argument_list . (string_literal) @path)) + arguments: (argument_list . (string_literal) @path)) @call `, }, ], } satisfies LanguagePatterns>); +// Match a bare `.uri(URI.create("literal"))` call on ANY receiver, capturing it +// as `@call`. `httpClientUriRootsAtNewBuilder` (JS) verifies the chain includes +// `HttpRequest.newBuilder()` — allowing calls BEFORE `.uri()` (`.version(v)` +// `.uri(...)`) that the old object-direct query dropped, and rejecting a `.uri(...)` +// on an unrelated object (e.g. WebClient). The verb (a `.GET()/.POST()/.PUT()/` +// `.DELETE()/.HEAD()` helper, a `.method("VERB", body)` literal, or the bare-build +// default) is recovered by `inferHttpClientMethod` scanning the whole chain. +// Matching `.uri(...)` regardless of a trailing `.build()` mirrors the accepted +// OkHttp over-match posture. const JAVA_HTTP_CLIENT_PATTERNS = compilePatterns({ name: 'java-http-client', language: Java, @@ -304,18 +321,36 @@ const JAVA_HTTP_CLIENT_PATTERNS = compilePatterns({ meta: {}, query: ` (method_invocation - object: (method_invocation - object: (method_invocation - object: (identifier) @builderCls (#eq? @builderCls "HttpRequest") - name: (identifier) @newBuilder (#eq? @newBuilder "newBuilder") - arguments: (argument_list)) - name: (identifier) @uri_method (#eq? @uri_method "uri") - arguments: (argument_list - (method_invocation - object: (identifier) @uriCls (#eq? @uriCls "URI") - name: (identifier) @create (#eq? @create "create") - arguments: (argument_list . (string_literal) @path)))) - name: (identifier) @http_method (#match? @http_method "^(GET|POST|PUT|DELETE)$")) + name: (identifier) @uri_method (#eq? @uri_method "uri") + arguments: (argument_list + (method_invocation + object: (identifier) @uriCls (#eq? @uriCls "URI") + name: (identifier) @create (#eq? @create "create") + arguments: (argument_list . (string_literal) @path)))) @call + `, + }, + ], +} satisfies LanguagePatterns>); + +// Constructor-arg form: `HttpRequest.newBuilder(URI.create("..."))` — the path is +// the `newBuilder(...)` argument, with no `.uri()` call. Captures the `newBuilder` +// call as `@call`; the emission skips it when a later `.uri(...)` overrides the +// constructor URI (`httpClientChainHasUriCall`), so the `.uri()` query owns that case. +const JAVA_HTTP_CLIENT_CTOR_PATTERNS = compilePatterns({ + name: 'java-http-client-ctor', + language: Java, + patterns: [ + { + meta: {}, + query: ` + (method_invocation + object: (identifier) @builderCls (#eq? @builderCls "HttpRequest") + name: (identifier) @newBuilder (#eq? @newBuilder "newBuilder") + arguments: (argument_list + (method_invocation + object: (identifier) @uriCls (#eq? @uriCls "URI") + name: (identifier) @create (#eq? @create "create") + arguments: (argument_list . (string_literal) @path)))) @call `, }, ], @@ -361,6 +396,18 @@ function getNodeName(node: Parser.SyntaxNode): string | null { return node.childForFieldName('name')?.text ?? null; } +/** + * Trailing segment of a possibly fully-qualified annotation name + * (`org.springframework.web.bind.annotation.GetMapping` → `GetMapping`). The + * route query binds `@ann` to either an `identifier` (simple) or a + * `scoped_identifier` (FQN); normalizing here lets the one for-loop discriminate + * on the simple name in both cases. A simple name maps to itself, so this never + * changes how a non-FQN annotation is classified. + */ +function simpleName(text: string): string { + return text.split('.').pop() ?? text; +} + function hasAnnotation(node: Parser.SyntaxNode, names: string | readonly string[]): boolean { const modifiers = node.namedChildren.find((child) => child.type === 'modifiers'); if (!modifiers) return false; @@ -369,10 +416,9 @@ function hasAnnotation(node: Parser.SyntaxNode, names: string | readonly string[ while (stack.length > 0) { const cur = stack.pop()!; const annotationName = cur.childForFieldName('name')?.text ?? ''; - const simpleName = annotationName.split('.').pop() ?? annotationName; if ( (cur.type === 'annotation' || cur.type === 'marker_annotation') && - (allowed.has(annotationName) || allowed.has(simpleName)) + (allowed.has(annotationName) || allowed.has(simpleName(annotationName))) ) { return true; } @@ -381,6 +427,11 @@ function hasAnnotation(node: Parser.SyntaxNode, names: string | readonly string[ return false; } +// The statically-resolvable consumer path helpers (URI.create / +// UriComponentsBuilder resolution) and the OkHttp / HttpClient builder verb-walks +// (`inferOkHttpMethod` / `inferHttpClientMethod`) live in ./java-static-path.ts +// (#2268), shared with the RestTemplate, OkHttp, and HttpClient consumer loops. + interface MethodRouteAnnotation { methodNode: Parser.SyntaxNode; methodName: string | null; @@ -444,7 +495,13 @@ function scanRouteAnnotations(tree: Parser.Tree): RouteAnnotationScan { const node = captures.node; const valueNode = captures.value; if (!annNode || !node || !valueNode) continue; - const ann = annNode.text; + // Discrimination is on the trailing segment only (`simpleName`), so a + // non-Spring annotation whose last segment collides with a route annotation + // (e.g. `@com.evil.GetMapping("/x")`) is treated as a route. This is the + // same accepted trailing-segment trade-off `hasAnnotation` already makes and + // the intended parity with the Kotlin plugin — package-origin gating would + // break that parity and is deliberately not done. + const ann = simpleName(annNode.text); const keyNode = captures.key; // undefined for the positional shape if (node.type === 'method_declaration') { @@ -727,7 +784,7 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = { if (!methodNode || !pathNode) continue; const httpMethod = REST_TEMPLATE_TO_HTTP[methodNode.text]; if (!httpMethod) continue; - const path = unquoteLiteral(pathNode.text); + const path = extractStaticPathExpression(pathNode); if (path === null) continue; out.push({ role: 'consumer', @@ -743,7 +800,7 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = { const httpMethodNode = match.captures.http_method; const pathNode = match.captures.path; if (!httpMethodNode || !pathNode) continue; - const path = unquoteLiteral(pathNode.text); + const path = extractStaticPathExpression(pathNode); if (path === null) continue; out.push({ role: 'consumer', @@ -801,15 +858,25 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = { } // ─── Consumers: OkHttp Request.Builder().url("path") ──────────── + // Match any `.url("literal")`, gate to chains rooting at `new Request.Builder()` + // (so a call before `.url()` is captured but an unrelated `.url()` is not), then + // recover the verb (`.post()`/`.method("X")`) by scanning the builder chain. for (const match of runCompiledPatterns(OK_HTTP_PATTERNS, tree)) { + const callNode = match.captures.call; const pathNode = match.captures.path; - if (!pathNode) continue; + if (!callNode || !pathNode) continue; + if (!okHttpUrlRootsAtBuilder(callNode)) continue; const path = unquoteLiteral(pathNode.text); if (path === null) continue; + const method = inferOkHttpMethod(callNode); + // An explicit `.method(verb, …)` with a non-literal or empty verb is + // unresolvable (null) — emit nothing rather than a wrong GET or an empty + // `http::::/path` contract. + if (!method) continue; out.push({ role: 'consumer', framework: 'okhttp', - method: 'GET', + method, path, name: null, confidence: 0.7, @@ -817,18 +884,48 @@ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = { } // ─── Consumers: Java HttpClient request builder ───────────────── - // Java's builder exposes GET/POST/PUT/DELETE helpers. PATCH uses - // `.method("PATCH", body)`, which is intentionally deferred. + // Match any `.uri(URI.create("..."))`, gate to chains including `HttpRequest` + // `.newBuilder()` (so a call before `.uri()` is captured but an unrelated + // `.uri()` is not); `inferHttpClientMethod` scans the chain for the verb — a + // `.GET()/.POST()/.PUT()/.DELETE()/.HEAD()` helper, a `.method("VERB", body)` + // literal, or the bare-build default GET. A variable-bound `.method(verb, …)` + // is unresolvable → emit nothing. Mirrors the OkHttp loop above. The + // constructor-arg form (`newBuilder(URI.create(...))`, no `.uri()`) follows. for (const match of runCompiledPatterns(JAVA_HTTP_CLIENT_PATTERNS, tree)) { - const httpMethodNode = match.captures.http_method; + const callNode = match.captures.call; const pathNode = match.captures.path; - if (!httpMethodNode || !pathNode) continue; + if (!callNode || !pathNode) continue; + if (!httpClientUriRootsAtNewBuilder(callNode)) continue; const path = unquoteLiteral(pathNode.text); if (path === null) continue; + const method = inferHttpClientMethod(callNode); + if (!method) continue; out.push({ role: 'consumer', framework: 'java-http-client', - method: httpMethodNode.text.toUpperCase(), + method, + path, + name: null, + confidence: 0.65, + }); + } + + // Constructor-arg form: `HttpRequest.newBuilder(URI.create("..."))` with the + // path in the constructor and no overriding `.uri(...)` later in the chain + // (a later `.uri()` wins at runtime, so the loop above owns that case). + for (const match of runCompiledPatterns(JAVA_HTTP_CLIENT_CTOR_PATTERNS, tree)) { + const callNode = match.captures.call; + const pathNode = match.captures.path; + if (!callNode || !pathNode) continue; + if (httpClientChainHasUriCall(callNode)) continue; + const path = unquoteLiteral(pathNode.text); + if (path === null) continue; + const method = inferHttpClientMethod(callNode); + if (!method) continue; + out.push({ + role: 'consumer', + framework: 'java-http-client', + method, path, name: null, confidence: 0.65, diff --git a/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts b/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts index 4286ba7587..3b51dc7227 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/kotlin.ts @@ -131,6 +131,118 @@ const arrayOfArg = (cap: string): string => `(call_expression (simple_identifier) @arrayOf (#eq? @arrayOf "arrayOf") (call_suffix (value_arguments (value_argument (string_literal) ${cap}))))`; +// ─── Kotlin OkHttp builder verb-walk (parity with java-static-path.ts) ── +// Mirrors `inferOkHttpMethod`, adapted to the Kotlin grammar: a call `X.name(args)` +// is a `call_expression` whose callee is a `navigation_expression` (receiver + +// `navigation_suffix` → the method name) and whose `call_suffix` holds the +// `value_arguments`. The chain is left-nested via `navigation_expression`, so we +// walk UP from the matched `.url(...)` call to the sibling verb call (`.post()` / +// `.method("X")`), exactly as the Java side does — so `.java` and `.kt` infer the +// same verb for the same OkHttp shape. +const OK_HTTP_VERB_HELPERS = ['get', 'head', 'post', 'put', 'delete', 'patch']; + +/** The method name a Kotlin `call_expression` invokes (its `navigation_suffix`). */ +function kotlinCallName(call: Parser.SyntaxNode): string | null { + const callee = call.namedChild(0); + if (callee?.type !== 'navigation_expression') return null; + for (let i = 0; i < callee.namedChildCount; i++) { + const child = callee.namedChild(i); + if (child?.type === 'navigation_suffix') return child.namedChild(0)?.text ?? null; + } + return null; +} + +/** The first string-literal argument of a Kotlin `call_expression`, else null. */ +function kotlinFirstStringArg(call: Parser.SyntaxNode): string | null { + for (let i = 0; i < call.namedChildCount; i++) { + const callSuffix = call.namedChild(i); + if (callSuffix?.type !== 'call_suffix') continue; + for (let j = 0; j < callSuffix.namedChildCount; j++) { + const args = callSuffix.namedChild(j); + if (args?.type !== 'value_arguments') continue; + const firstArg = args.namedChild(0); + if (firstArg?.type !== 'value_argument') return null; + // Positional literal `"X"`, or named-argument `name = "X"` (the label is a + // leading `simple_identifier` and the literal follows). A non-literal value + // (a variable) → null → unresolvable. + const positional = firstArg.namedChild(0); + if (positional?.type === 'string_literal') return unquoteLiteral(positional.text); + const labeled = positional?.type === 'simple_identifier' ? firstArg.namedChild(1) : null; + return labeled?.type === 'string_literal' ? unquoteLiteral(labeled.text) : null; + } + } + return null; +} + +/** The receiver expression a Kotlin `call_expression` is invoked on. */ +function kotlinReceiver(call: Parser.SyntaxNode): Parser.SyntaxNode | null { + const nav = call.namedChild(0); + return nav?.type === 'navigation_expression' ? nav.namedChild(0) : null; +} + +/** The call that invokes a method ON `call` as its receiver — one hop up the chain. */ +function kotlinChainParentCall(call: Parser.SyntaxNode): Parser.SyntaxNode | null { + const nav = call.parent; + if (nav?.type !== 'navigation_expression' || nav.namedChild(0)?.id !== call.id) return null; + return nav.parent?.type === 'call_expression' ? nav.parent : null; +} + +/** Every `call_expression` in the fluent chain `pathCall` belongs to, innermost + * (next to the construction) → outermost. Lets the verb-walk find a verb call + * wherever it sits relative to `.url(...)` — parity with java-static-path.ts + * `builderChainCalls`. */ +function kotlinBuilderChainCalls(pathCall: Parser.SyntaxNode): Parser.SyntaxNode[] { + let innermost = pathCall; + let recv = kotlinReceiver(innermost); + while (recv?.type === 'call_expression') { + innermost = recv; + recv = kotlinReceiver(innermost); + } + const calls: Parser.SyntaxNode[] = [innermost]; + let cur = innermost; + for (let next = kotlinChainParentCall(cur); next; next = kotlinChainParentCall(cur)) { + calls.push(next); + cur = next; + } + return calls; +} + +/** True when `urlCall`'s receiver chain bottoms out on `Request.Builder()` — the + * Kotlin anti-overreach gate (mirror of okHttpUrlRootsAtBuilder), so a `.url(...)` + * on an unrelated object is rejected while a builder call before `.url()` is kept. */ +function kotlinUrlRootsAtRequestBuilder(urlCall: Parser.SyntaxNode): boolean { + let cur: Parser.SyntaxNode | null = kotlinReceiver(urlCall); + while (cur?.type === 'call_expression') { + const recv = kotlinReceiver(cur); + if (recv?.type === 'simple_identifier') + return recv.text === 'Request' && kotlinCallName(cur) === 'Builder'; + cur = recv; + } + return false; +} + +/** Infer the OkHttp verb by scanning the builder chain around the matched + * `.url(...)` call — parity with java-static-path.ts `inferOkHttpMethod`. The + * LAST verb call wins (runtime overwrite); `null` for an unresolvable + * `.method(verb)` (non-literal/empty) so the caller skips; `'GET'` when the + * chain has no verb call. */ +function inferKotlinOkHttpMethod(urlCall: Parser.SyntaxNode): string | null { + let lastVerbCall: Parser.SyntaxNode | null = null; + for (const call of kotlinBuilderChainCalls(urlCall)) { + if (call.id === urlCall.id) continue; + const name = kotlinCallName(call); + if (name === 'method' || (name !== null && OK_HTTP_VERB_HELPERS.includes(name))) + lastVerbCall = call; + } + if (lastVerbCall === null) return 'GET'; // no verb call → OkHttp default + const name = kotlinCallName(lastVerbCall); + if (name === 'method') { + const verb = kotlinFirstStringArg(lastVerbCall); + return verb ? verb.toUpperCase() : null; + } + return name === null ? 'GET' : name.toUpperCase(); +} + /** * Build the plugin only if the Kotlin grammar is available. Compiling * the queries against a null grammar would throw at module load time @@ -423,26 +535,25 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin { } satisfies LanguagePatterns>); // ─── Consumer: OkHttp Request.Builder().url("/x") ───────────────────── - // Kotlin parses `Request.Builder()` as a `call_expression` whose - // callee is a `navigation_expression` (Request → .Builder), NOT as - // Java's `object_creation_expression`. The chain `.url("/x")` then - // wraps that in another `call_expression`. The query mirrors Java's - // `OK_HTTP_PATTERNS` (java.ts) but adapts the node types. + // Full parity with the Java OkHttp extraction (java.ts + java-static-path.ts), + // adapted to the Kotlin grammar (a call is a `call_expression` whose callee is a + // `navigation_expression`, not Java's `object_creation_expression`): // - // Receiver `Request` is constrained by name (#eq? @cls); a project - // that imports OkHttp's `Request` under an alias (`import okhttp3.Request as OkRequest`) - // would not be picked up — this matches the Java plugin's heuristic. + // • Match a bare `.url("literal")` call on ANY receiver (capture it as `@call`); + // `kotlinUrlRootsAtRequestBuilder` (JS) then verifies the receiver chain + // bottoms out on `Request.Builder()` — re-imposing the framework anchor while + // allowing a builder call BEFORE `.url()` (`Request.Builder().addHeader(...)` + // `.url(...)`) and rejecting a `.url(...)` on an unrelated object. + // • `inferKotlinOkHttpMethod` scans the whole chain for the verb (`.post(body)` + // / `.get()` / `.method("X")`, before or after `.url()`) — the mirror of + // `inferOkHttpMethod`. So `Request.Builder().url("/x").post(body).build()` + // becomes `http::POST::/x` on both `.java` and `.kt` (pinned by the Java↔Kotlin + // parity harness). A variable-bound/empty `.method(verb)` is unresolvable → + // the call is skipped (not a guessed GET), matching the Java side. // - // **Known limitation — verb defaults to GET.** OkHttp encodes the - // verb on a *sibling* call further down the builder chain (e.g. - // `.post(body)` / `.get()` / `.delete()`), not on `.url(...)` itself. - // This query intentionally does not walk the chain to recover the - // verb — it emits `method: 'GET'` for every match, mirroring - // `java.ts:OK_HTTP_PATTERNS`. So a `Request.Builder().url("/x").post(body).build()` - // call becomes `http::GET::/x`, not `http::POST::/x`. This is the - // same trade-off Java has accepted; pinned by an anti-overreach - // test in `http-route-extractor.test.ts` so a future verb-walk - // implementation has to update this comment in lockstep. + // Receiver `Request` is constrained by name (in the JS gate); a project that + // imports OkHttp's `Request` under an alias would not be picked up — matching the + // Java plugin's heuristic. const OK_HTTP_PATTERNS = compilePatterns({ name: 'kotlin-okhttp', language, @@ -452,14 +563,9 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin { query: ` (call_expression (navigation_expression - (call_expression - (navigation_expression - (simple_identifier) @cls (#eq? @cls "Request") - (navigation_suffix (simple_identifier) @builder (#eq? @builder "Builder"))) - (call_suffix (value_arguments))) (navigation_suffix (simple_identifier) @method (#eq? @method "url"))) (call_suffix - (value_arguments . (value_argument . (string_literal) @path)))) + (value_arguments . (value_argument . (string_literal) @path)))) @call `, }, ], @@ -983,15 +1089,23 @@ function buildKotlinPlugin(language: unknown): HttpLanguagePlugin { } // ─── Consumers: OkHttp Request.Builder().url("path") ──────────── + // Gate to chains rooting at `Request.Builder()` (so a call before `.url()` is + // captured but an unrelated `.url()` is not), then recover the verb by scanning + // the chain (parity with Java); a variable-bound or empty `.method(verb)` is + // unresolvable → emit nothing rather than a GET. for (const match of runCompiledPatterns(OK_HTTP_PATTERNS, tree)) { + const callNode = match.captures.call; const pathNode = match.captures.path; - if (!pathNode) continue; + if (!callNode || !pathNode) continue; + if (!kotlinUrlRootsAtRequestBuilder(callNode)) continue; const path = unquoteLiteral(pathNode.text); if (path === null) continue; + const method = inferKotlinOkHttpMethod(callNode); + if (!method) continue; out.push({ role: 'consumer', framework: 'okhttp', - method: 'GET', + method, path, name: null, confidence: 0.7, diff --git a/gitnexus/test/unit/group/http-route-extractor.test.ts b/gitnexus/test/unit/group/http-route-extractor.test.ts index 2d688c7e8c..f1af4ff626 100644 --- a/gitnexus/test/unit/group/http-route-extractor.test.ts +++ b/gitnexus/test/unit/group/http-route-extractor.test.ts @@ -1158,12 +1158,13 @@ public class StatusController implements StatusApi { ).toHaveLength(0); }); - it('does not extract fully-qualified Java route annotations (documented limitation #2254)', async () => { - // JAVA_ROUTE_ANNOTATION_PATTERNS binds `name: (identifier)`; a FQN route - // annotation parses its name as `scoped_identifier` and is not matched, so - // its route is not extracted (only the route string — the controller itself - // is still recognised). This pins that documented limitation / asymmetry - // with Kotlin. If FQN matching is ever added, flip this assertion. + it('extracts fully-qualified Java route annotations (#2254 FQN follow-through)', async () => { + // JAVA_ROUTE_ANNOTATION_PATTERNS now binds `name: [(identifier) + // (scoped_identifier)]`; a deep FQN route annotation + // (`@org.springframework…GetMapping`) is matched and the for-loop + // normalizes the name to its trailing segment (`simpleName`). The + // controller was already recognised (hasAnnotation trailing-segment match); + // now the route string is extracted too — parity with the Kotlin plugin. const dir = path.join(tmpDir, 'java-fqn-route-annotation'); fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); fs.writeFileSync( @@ -1181,12 +1182,57 @@ class FqnController { const contracts = await extractor.extract(null, dir, makeRepo(dir)); const providers = contracts.filter((c) => c.role === 'provider'); - // The FQN route annotation is not extracted (documented limitation). - expect(providers.find((c) => c.contractId === 'http::GET::/api/users')).toBeUndefined(); - // And it must not over-match into a bogus contract either. expect( - providers.filter((c) => c.symbolRef.filePath.endsWith('FqnController.java')), - ).toHaveLength(0); + providers.find( + (c) => + c.contractId === 'http::GET::/api/users' && + c.meta.framework === 'spring' && + c.confidence === 0.8, + ), + ).toBeDefined(); + }); + + it('extracts FQN OpenFeign consumers + two-segment FQN, with anti-overreach', async () => { + const dir = path.join(tmpDir, 'java-fqn-feign'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'QualifiedClient.java'), + ` +@org.springframework.cloud.openfeign.FeignClient(name = "order-service", path = "/api") +interface QualifiedClient { + @org.springframework.web.bind.annotation.GetMapping("/orders/{id}") + OrderDto getOrder(String id); +} + +@foo.RestController +@foo.RequestMapping("/v2") +class ShortFqnController { + @foo.GetMapping("/items") + Object items() { return null; } +} + +@com.example.NotARoute("/should-not-extract") +class Unrelated { + @com.example.AlsoNotARoute("/nope") + Object noop() { return null; } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const fromFile = contracts.filter((c) => + c.symbolRef.filePath.endsWith('QualifiedClient.java'), + ); + + // Exactly two contracts: the FQN @FeignClient(path)+@GetMapping consumer and + // the two-segment-FQN provider. The `Unrelated` class's non-route FQN + // annotations contribute nothing (anti-overreach — simpleName misses them). + expect(new Set(fromFile.map((c) => `${c.role} ${c.contractId} ${c.meta.framework}`))).toEqual( + new Set([ + 'consumer http::GET::/api/orders/{param} openfeign', + 'provider http::GET::/v2/items spring', + ]), + ); }); it('extracts Express router.get patterns', async () => { @@ -1771,6 +1817,366 @@ class ApiClient { ).toBeDefined(); }); + it('extracts Java RestTemplate URI.create(...) static paths', async () => { + const dir = path.join(tmpDir, 'java-rest-template-uri-create'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'UriClient.java'), + ` +import java.net.URI; +import org.springframework.http.HttpMethod; +import org.springframework.web.client.RestTemplate; + +class UriClient { + void run(RestTemplate restTemplate) { + String dynamicPath = "/api/dynamic-users/99"; + restTemplate.getForEntity(URI.create("/api/uri-users/42"), String.class); + restTemplate.exchange(URI.create("/api/uri-users/42/details"), HttpMethod.POST, null, String.class); + restTemplate.getForObject(dynamicPath, String.class); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect( + consumers.find( + (c) => + c.contractId === 'http::GET::/api/uri-users/{param}' && + c.meta.framework === 'spring-rest-template' && + c.confidence === 0.7, + ), + ).toBeDefined(); + expect( + consumers.find((c) => c.contractId === 'http::POST::/api/uri-users/{param}/details'), + ).toBeDefined(); + // Anti-overreach: a variable-bound path is not statically resolvable, so + // the widened `(_) @path` capture must NOT emit a consumer for it. + expect( + consumers.find((c) => c.contractId === 'http::GET::/api/dynamic-users/{param}'), + ).toBeUndefined(); + }); + + it('extracts Java RestTemplate UriComponentsBuilder fluent-chain paths', async () => { + const dir = path.join(tmpDir, 'java-rest-template-builder'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'BuilderClient.java'), + ` +import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; + +class BuilderClient { + void run(RestTemplate restTemplate, String idVar) { + restTemplate.getForObject( + UriComponentsBuilder.fromPath("/api").path("/builder-users").pathSegment("42").build().toUriString(), + String.class); + restTemplate.getForObject( + UriComponentsBuilder.fromUriString("/base").path("/sub").queryParam("page", "1").build().toUriString(), + String.class); + restTemplate.getForObject( + UriComponentsBuilder.fromHttpUrl("https://example.com/api").path("/external-users").query("page=1").build().toUriString(), + String.class); + restTemplate.getForObject( + UriComponentsBuilder.fromPath("/api").pathSegment(idVar).build().toUriString(), + String.class); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + // fromPath + path + numeric pathSegment → joined, numeric → {param}. + expect( + consumers.find((c) => c.contractId === 'http::GET::/api/builder-users/{param}'), + ).toBeDefined(); + // fromUriString seed + path; the queryParam attribute does not alter the path. + expect(consumers.find((c) => c.contractId === 'http::GET::/base/sub')).toBeDefined(); + // fromHttpUrl host seed: helper keeps the host; normalizeConsumerPath strips it. + expect( + consumers.find((c) => c.contractId === 'http::GET::/api/external-users'), + ).toBeDefined(); + // Anti-overreach: the non-literal `pathSegment(idVar)` call defeats static + // resolution, so exactly the three resolvable chains emit — not a fourth. + expect( + consumers.filter((c) => c.symbolRef.filePath.endsWith('BuilderClient.java')), + ).toHaveLength(3); + }); + + it('strips a query string baked into a UriComponentsBuilder seed (#2268)', async () => { + const dir = path.join(tmpDir, 'java-rest-template-builder-query-seed'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'QuerySeedClient.java'), + ` +import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; + +class QuerySeedClient { + void run(RestTemplate restTemplate) { + restTemplate.getForObject( + UriComponentsBuilder.fromUriString("/base?x=1").path("/sub").build().toUriString(), + String.class); + restTemplate.getForObject( + UriComponentsBuilder.fromHttpUrl("https://example.com/api?x=1").path("/y").build().toUriString(), + String.class); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + // Query in the seed must NOT swallow the later .path() segment: + // fromUriString("/base?x=1").path("/sub") → /base/sub (was /base before the fix). + expect(consumers.find((c) => c.contractId === 'http::GET::/base/sub')).toBeDefined(); + // Host + query seed: query stripped at the seed, host stripped downstream. + expect(consumers.find((c) => c.contractId === 'http::GET::/api/y')).toBeDefined(); + // Count guard: exactly the two resolvable chains. A regression that + // double-emitted (e.g. both /base and /base/sub) would slip past the two + // `find` assertions above without this. + expect( + consumers.filter((c) => c.symbolRef.filePath.endsWith('QuerySeedClient.java')), + ).toHaveLength(2); + }); + + it('resolves a UriComponentsBuilder argument passed to restTemplate.exchange (#2268)', async () => { + // The exchange() path capture was widened to `(_) @path` alongside the + // plain RestTemplate loop, but was only covered with URI.create. Pin that a + // UriComponentsBuilder chain through exchange() also resolves end-to-end. + const dir = path.join(tmpDir, 'java-rest-template-exchange-builder'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'ExchangeBuilderClient.java'), + ` +import org.springframework.http.HttpMethod; +import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; + +class ExchangeBuilderClient { + void run(RestTemplate restTemplate) { + restTemplate.exchange( + UriComponentsBuilder.fromPath("/api").path("/exchange-users").build().toUriString(), + HttpMethod.PUT, null, String.class); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + expect( + consumers.find((c) => c.contractId === 'http::PUT::/api/exchange-users'), + ).toBeDefined(); + }); + + it('appends UriComponentsBuilder .path() verbatim per Spring semantics (#2268)', async () => { + // Spring `.path(p)` appends `p` as-is (no slash inserted) then collapses + // duplicate slashes — unlike `.pathSegment`, which slash-joins. So a + // no-leading-slash arg is NOT given a phantom slash, a leading-slash arg + // joins cleanly, a trailing-slash base collapses, and a host seed keeps its + // `://` until the downstream normalizer strips the host. + const dir = path.join(tmpDir, 'java-rest-template-builder-path'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'PathClient.java'), + ` +import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; + +class PathClient { + void run(RestTemplate restTemplate) { + restTemplate.getForObject(UriComponentsBuilder.fromPath("/api").path("noslash").build().toUriString(), String.class); + restTemplate.getForObject(UriComponentsBuilder.fromPath("/svc").path("/withslash").build().toUriString(), String.class); + restTemplate.getForObject(UriComponentsBuilder.fromPath("/trail/").path("/seg").build().toUriString(), String.class); + restTemplate.getForObject(UriComponentsBuilder.fromPath("/first-").path("value/").path("/end").build().toUriString(), String.class); + restTemplate.getForObject(UriComponentsBuilder.fromHttpUrl("https://example.com/api").path("/ext").build().toUriString(), String.class); + restTemplate.getForObject(UriComponentsBuilder.fromPath("/empty").path("").build().toUriString(), String.class); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const fromFile = contracts.filter( + (c) => c.role === 'consumer' && c.symbolRef.filePath.endsWith('PathClient.java'), + ); + + // Exactly these six — no phantom slash on the no-leading-slash arg, no + // double slash from the trailing-slash base, no `://` corruption. + expect(new Set(fromFile.map((c) => c.contractId))).toEqual( + new Set([ + 'http::GET::/apinoslash', // fromPath("/api").path("noslash") — verbatim, no slash + 'http::GET::/svc/withslash', // leading-slash arg joins cleanly + 'http::GET::/trail/seg', // trailing-slash base collapses + 'http::GET::/first-value/end', // value/ + /end → one slash + 'http::GET::/api/ext', // host seed: `://` kept, host stripped downstream + 'http::GET::/empty', // empty .path("") is a no-op + ]), + ); + }); + + it('does not overflow on a pathological UriComponentsBuilder chain (#2268)', async () => { + const dir = path.join(tmpDir, 'java-rest-template-builder-deep'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + // A chain far deeper than the recursion cap — exercises the depth guard. + const deepChain = `UriComponentsBuilder.fromPath("/r")${'.path("/x")'.repeat(200)}.build().toUriString()`; + fs.writeFileSync( + path.join(dir, 'src', 'DeepClient.java'), + ` +import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; + +class DeepClient { + void run(RestTemplate restTemplate) { + restTemplate.getForObject(${deepChain}, String.class); + } +} +`, + ); + + // Must not throw (the depth guard caps recursion). A chain past the cap + // resolves to null, so no consumer is emitted for it (without the guard it + // would either overflow or resolve to a bogus deep path). + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + expect( + contracts.filter( + (c) => c.role === 'consumer' && c.symbolRef.filePath.endsWith('DeepClient.java'), + ), + ).toHaveLength(0); + }); + + it('infers Java OkHttp verbs from sibling Request.Builder calls', async () => { + const dir = path.join(tmpDir, 'java-okhttp-verbs'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'OkHttpVerbs.java'), + ` +import okhttp3.Request; +import okhttp3.RequestBody; + +class OkHttpVerbs { + void run(RequestBody body, String verb) { + new Request.Builder().url("/api/orders/0").get().build(); + new Request.Builder().url("/api/orders/head").head().build(); + new Request.Builder().url("/api/orders").post(body).build(); + new Request.Builder().url("/api/orders/1").put(body).build(); + new Request.Builder().url("/api/orders/2").delete().build(); + new Request.Builder().url("/api/orders/3").method("PATCH", body).build(); + new Request.Builder().url("/api/bare-build").build(); + new Request.Builder().url("/api/dyn-verb").method(verb, body).build(); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const okhttp = contracts.filter( + (c) => + c.role === 'consumer' && + c.meta.framework === 'okhttp' && + c.symbolRef.filePath.endsWith('OkHttpVerbs.java'), + ); + + // The bare `.build()` with no verb call gets its OWN path (`/api/bare-build`) + // so the default-GET branch of inferOkHttpMethod is pinned independently. + // An explicit `.method(verb, …)` with a *variable* verb (`/api/dyn-verb`) is + // unresolvable and emits NOTHING — not a guessed GET (parity with WebClient + // long-form). Explicit verbs resolve; numeric segments normalize to {param}. + expect(okhttp.find((c) => c.contractId === 'http::GET::/api/bare-build')).toBeDefined(); + // Variable-bound `.method(verb)` produces no contract at all (any verb). + expect(okhttp.find((c) => c.contractId.includes('/api/dyn-verb'))).toBeUndefined(); + expect(new Set(okhttp.map((c) => c.contractId))).toEqual( + new Set([ + 'http::GET::/api/bare-build', + 'http::GET::/api/orders/{param}', + 'http::POST::/api/orders', + 'http::PUT::/api/orders/{param}', + 'http::DELETE::/api/orders/{param}', + 'http::PATCH::/api/orders/{param}', + 'http::HEAD::/api/orders/head', + ]), + ); + expect(okhttp.every((c) => c.confidence === 0.7)).toBe(true); + }); + + it('does not emit a contract for an empty-string verb literal (#2268)', async () => { + // `.method("", body)` is an explicit-but-unresolvable verb — `unquoteLiteral` + // returns "" (not null), so the falsiness guard must skip it rather than + // emit a malformed `http::::/path` contract or a guessed GET. Covers both + // the OkHttp and Java-HttpClient verb-walks (shared `inferBuilderVerb`). + const dir = path.join(tmpDir, 'java-empty-verb'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'EmptyVerb.java'), + ` +import java.net.URI; +import java.net.http.HttpRequest; +import okhttp3.Request; +import okhttp3.RequestBody; + +class EmptyVerb { + void run(RequestBody body) throws Exception { + new Request.Builder().url("/api/okhttp-empty").method("", body).build(); + HttpRequest hc = HttpRequest.newBuilder().uri(URI.create("/api/hc-empty")).method("", body).build(); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const fromFile = contracts.filter( + (c) => c.role === 'consumer' && c.symbolRef.filePath.endsWith('EmptyVerb.java'), + ); + + // No contract at all — neither a malformed empty-method id nor a guessed GET. + expect(fromFile.map((c) => c.contractId)).toEqual([]); + }); + + it('extracts OkHttp chains with a builder call before .url(), with anti-overreach (#2268)', async () => { + // A builder call BEFORE .url() (`new Request.Builder().addHeader(...).url(...)`) + // no longer drops the contract — the chain is matched as long as it roots at + // `new Request.Builder()`. The verb-walk scans the whole chain, so a verb set + // before .url() also resolves. A `.url(...)` on an unrelated object does NOT + // emit (the root gate rejects it). + const dir = path.join(tmpDir, 'java-okhttp-pre-url'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'OkHttpPreUrl.java'), + ` +import okhttp3.Request; +import okhttp3.RequestBody; + +class OkHttpPreUrl { + void run(RequestBody body, SomeClient other) { + new Request.Builder().addHeader("A", "b").url("/api/pre-url").build(); + new Request.Builder().post(body).url("/api/verb-first").build(); + other.url("/api/not-okhttp").build(); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const okhttp = contracts.filter( + (c) => + c.role === 'consumer' && + c.meta.framework === 'okhttp' && + c.symbolRef.filePath.endsWith('OkHttpPreUrl.java'), + ); + + // The header-before-url chain (default GET) and the verb-before-url chain + // (POST) both emit; `other.url(...)` does not (not a Request.Builder chain). + expect(new Set(okhttp.map((c) => c.contractId))).toEqual( + new Set(['http::GET::/api/pre-url', 'http::POST::/api/verb-first']), + ); + }); + it('extracts Java WebClient long-form method(HttpMethod.X).uri(...) — #2254 parity', async () => { // Parity with the Kotlin plugin: a single structural query matches the // verb (HttpMethod.X field access) and path. Previously deferred on the @@ -2553,6 +2959,208 @@ class HttpClients { ).toBeDefined(); }); + it('extracts Java HttpClient HEAD, .method(), and default-GET forms', async () => { + const dir = path.join(tmpDir, 'java-http-client-verbs'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'HttpClientVerbs.java'), + ` +import java.net.URI; +import java.net.http.HttpRequest; + +class HttpClientVerbs { + void run(String verb) throws Exception { + HttpRequest head = HttpRequest.newBuilder().uri(URI.create("/api/users/head")).HEAD().build(); + HttpRequest patch = HttpRequest.newBuilder().uri(URI.create("/api/users/2")).method("PATCH", HttpRequest.BodyPublishers.ofString("{}")).build(); + HttpRequest def = HttpRequest.newBuilder().uri(URI.create("/api/default-users/3")).build(); + HttpRequest dyn = HttpRequest.newBuilder().uri(URI.create("/api/dyn/4")).method(verb, HttpRequest.BodyPublishers.noBody()).build(); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const hc = contracts.filter( + (c) => + c.role === 'consumer' && + c.meta.framework === 'java-http-client' && + c.symbolRef.filePath.endsWith('HttpClientVerbs.java'), + ); + + // HEAD via verb helper, PATCH via `.method("X")`, default GET via bare + // `.build()`. The variable-bound `.method(verb, …)` is NOT resolved + // (string literal only) → no contract. Exact set-equality also pins that + // no chain double-emits (e.g. HEAD would never also yield a GET). + expect(new Set(hc.map((c) => c.contractId))).toEqual( + new Set([ + 'http::HEAD::/api/users/head', + 'http::PATCH::/api/users/{param}', + 'http::GET::/api/default-users/{param}', + ]), + ); + expect(hc.every((c) => c.confidence === 0.65)).toBe(true); + }); + + it('extracts Java HttpClient verbs across intervening builder calls (#2268)', async () => { + // The verb-walk is transparent to neutral calls AFTER `.uri(...)`, so a + // `.header()`/`.timeout()` hop before the terminal no longer drops the + // contract. Each verb-producing branch gets a DISTINCT non-numeric path so + // the set-equality assertion cannot mask a branch. A call BEFORE `.uri()`, + // a constructor-arg `newBuilder(uri)`, and a non-literal `.uri()` arg are + // documented misses (must NOT extract). + const dir = path.join(tmpDir, 'java-http-client-intervening'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'HttpClientIntervening.java'), + ` +import java.net.URI; +import java.net.http.HttpRequest; +import java.net.http.HttpClient; +import java.time.Duration; + +class HttpClientIntervening { + void run(URI uriVar, Duration dur, HttpClient.Version ver, HttpRequest.BodyPublisher body) throws Exception { + // Intervening calls AFTER .uri() — header/timeout transparent to the verb-walk. + HttpRequest a = HttpRequest.newBuilder().uri(URI.create("/api/hdr")).header("Accept", "application/json").build(); + HttpRequest b = HttpRequest.newBuilder().uri(URI.create("/api/tmo")).timeout(dur).method("PUT", body).build(); + HttpRequest c = HttpRequest.newBuilder().uri(URI.create("/api/hdr-verb")).header("X", "y").DELETE().build(); + // Verb helper BEFORE an intervening call (walk does not stop at the first non-verb). + HttpRequest d = HttpRequest.newBuilder().uri(URI.create("/api/verb-then-hdr")).POST(body).header("X", "y").build(); + // Unbuilt .uri() — no .build(); over-match emits the default GET (mirrors OkHttp). + HttpRequest.Builder e = HttpRequest.newBuilder().uri(URI.create("/api/unbuilt")); + // Call BEFORE .uri() and the constructor-arg form are now captured too (the + // chain roots at HttpRequest.newBuilder); only a non-literal .uri() is a miss. + HttpRequest f = HttpRequest.newBuilder().version(ver).uri(URI.create("/api/pre-uri")).build(); // call before .uri() + HttpRequest g = HttpRequest.newBuilder(URI.create("/api/ctor")).build(); // constructor-arg, no .uri() + HttpRequest h = HttpRequest.newBuilder().uri(uriVar).build(); // non-literal .uri() arg → miss + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const hc = contracts.filter( + (c) => + c.role === 'consumer' && + c.meta.framework === 'java-http-client' && + c.symbolRef.filePath.endsWith('HttpClientIntervening.java'), + ); + + // The seven resolvable chains (incl. the pre-`.uri()` and constructor-arg + // forms); only the non-literal `.uri(uriVar)` contributes nothing. + expect(new Set(hc.map((c) => c.contractId))).toEqual( + new Set([ + 'http::GET::/api/hdr', + 'http::PUT::/api/tmo', + 'http::DELETE::/api/hdr-verb', + 'http::POST::/api/verb-then-hdr', + 'http::GET::/api/unbuilt', + 'http::GET::/api/pre-uri', + 'http::GET::/api/ctor', + ]), + ); + expect(hc.every((c) => c.confidence === 0.65)).toBe(true); + }); + + it('passes through a non-standard HttpClient .method("VERB") verb (#2268)', async () => { + // A custom verb literal passes through (uppercased), matching the OkHttp + // .method() precedent — the verb-walk does not restrict to known verbs. + const dir = path.join(tmpDir, 'java-http-client-custom-verb'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'CustomVerb.java'), + ` +import java.net.URI; +import java.net.http.HttpRequest; + +class CustomVerb { + void run(HttpRequest.BodyPublisher body) throws Exception { + HttpRequest r = HttpRequest.newBuilder().uri(URI.create("/api/report")).method("REPORT", body).build(); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const hc = contracts.filter( + (c) => + c.role === 'consumer' && + c.meta.framework === 'java-http-client' && + c.symbolRef.filePath.endsWith('CustomVerb.java'), + ); + expect(new Set(hc.map((c) => c.contractId))).toEqual(new Set(['http::REPORT::/api/report'])); + }); + + it('handles HttpClient constructor-URI override and rejects non-newBuilder .uri (#2268)', async () => { + // A constructor URI overridden by a later `.uri(...)` emits ONLY the override + // (not both), and a `.uri(URI.create(...))` on a chain that does not root at + // HttpRequest.newBuilder (e.g. WebClient) is NOT a java-http-client consumer. + const dir = path.join(tmpDir, 'java-http-client-ctor-override'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'CtorOverride.java'), + ` +import java.net.URI; +import java.net.http.HttpRequest; +import org.springframework.web.reactive.function.client.WebClient; + +class CtorOverride { + void run(WebClient webClient) { + HttpRequest a = HttpRequest.newBuilder(URI.create("/api/ctor-overridden")).uri(URI.create("/api/override-wins")).build(); + webClient.get().uri(URI.create("/api/webclient-not-hc")).retrieve(); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const hc = contracts.filter( + (c) => + c.role === 'consumer' && + c.meta.framework === 'java-http-client' && + c.symbolRef.filePath.endsWith('CtorOverride.java'), + ); + + // Only the override URI, exactly once; the overridden constructor URI and the + // WebClient `.uri()` are not java-http-client contracts. + expect(new Set(hc.map((c) => c.contractId))).toEqual( + new Set(['http::GET::/api/override-wins']), + ); + }); + + it('resolves the last verb when a chain sets two (runtime last-wins) (#2268)', async () => { + // Each verb-setter overwrites the previous at runtime, so a chain that sets + // two verbs resolves to the one nearest the terminal — not the first found. + const dir = path.join(tmpDir, 'java-http-two-verb'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'TwoVerb.java'), + ` +import java.net.URI; +import java.net.http.HttpRequest; +import okhttp3.Request; +import okhttp3.RequestBody; + +class TwoVerb { + void run(RequestBody body, HttpRequest.BodyPublisher pub) throws Exception { + HttpRequest hc = HttpRequest.newBuilder().GET().uri(URI.create("/api/hc-two")).POST(pub).build(); + new Request.Builder().get().url("/api/ok-two").post(body).build(); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const fromFile = contracts.filter( + (c) => c.role === 'consumer' && c.symbolRef.filePath.endsWith('TwoVerb.java'), + ); + + // Both resolve to the LAST verb (POST), not the first (GET). + expect(new Set(fromFile.map((c) => `${c.meta.framework} ${c.contractId}`))).toEqual( + new Set(['java-http-client http::POST::/api/hc-two', 'okhttp http::POST::/api/ok-two']), + ); + }); + // ─── Kotlin consumers (RestTemplate / WebClient short+long / OkHttp) ── // Same shape as the Java consumer test above, but parsed by the // tree-sitter-kotlin grammar via `KOTLIN_HTTP_PLUGIN`. Four @@ -2678,26 +3286,14 @@ class OkClient(private val client: OkHttpClient) { }); itKotlinConsumer( - 'OkHttp Request.Builder().url("/x").post(body) — verb defaults to GET (Java parity)', + 'Kotlin OkHttp .url("/x").post(body) infers POST — verb-walk parity with Java (#2268)', async () => { - // Anti-overreach / known-limitation pin: OkHttp encodes the - // HTTP verb on a sibling call (`.post(body)` / `.delete()` / - // ...), not on `.url(...)`. The query at `kotlin.ts:OK_HTTP_PATTERNS` - // intentionally does not walk the chain to recover the verb — - // it emits `method: 'GET'` for every match, mirroring the Java - // plugin's `OK_HTTP_PATTERNS` (java.ts). - // - // This test pins the accepted behavior so a future verb-walk - // implementation must update kotlin.ts's known-limitation - // comment in lockstep. Concretely: - // - `Request.Builder().url("/api/users").post(body).build()` - // → ONE consumer: `http::GET::/api/users` (heuristic-default) - // → NO `http::POST::/api/users` consumer - // - // Test signal: - // - if this becomes correct (POST detected) without updating - // the kotlin.ts comment + java.ts behavior together, this - // test goes red and the reviewer must reconcile both sides. + // OkHttp encodes the HTTP verb on a sibling call (`.post(body)` / `.delete()` + // / `.method("X")`), not on `.url(...)`. The Kotlin plugin now WALKS the + // builder chain (`inferKotlinOkHttpMethod`) to recover it — the mirror of + // the Java side's `inferOkHttpMethod`. So `Request.Builder().url("/api/users")` + // `.post(body).build()` emits `http::POST::/api/users` (not GET) on `.kt`, + // identical to `.java` (pinned by the Java↔Kotlin parity harness below). const dir = path.join(tmpDir, 'kotlin-okhttp-post-chain'); fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); fs.writeFileSync( @@ -2717,24 +3313,102 @@ class OkPostClient(private val client: OkHttpClient, private val body: RequestBo ); const contracts = await extractor.extract(null, dir, makeRepo(dir)); - const consumers = contracts.filter((c) => c.role === 'consumer'); - - const fromThisFile = consumers.filter((c) => - c.symbolRef.filePath.endsWith('OkPostClient.kt'), + const fromThisFile = contracts.filter( + (c) => c.role === 'consumer' && c.symbolRef.filePath.endsWith('OkPostClient.kt'), ); - // Heuristic-default GET: exactly one consumer is emitted for - // the .url("/x") capture, with method=GET regardless of the - // sibling .post(body) call. + // The sibling `.post(body)` is recovered: exactly one POST consumer, no GET. expect(fromThisFile).toHaveLength(1); - expect(fromThisFile[0].contractId).toBe('http::GET::/api/users'); - expect(fromThisFile[0].meta.method).toBe('GET'); + expect(fromThisFile[0].contractId).toBe('http::POST::/api/users'); + expect(fromThisFile[0].meta.method).toBe('POST'); + expect(fromThisFile.find((c) => c.contractId === 'http::GET::/api/users')).toBeUndefined(); + }, + ); - // Anti-overreach: no second contract with POST should appear. - // If a future verb-walk lands and this assertion needs to flip - // (i.e. POST is now detected), bump kotlin.ts's known-limitation - // comment and java.ts in the same PR. - expect(fromThisFile.find((c) => c.contractId === 'http::POST::/api/users')).toBeUndefined(); + itKotlinConsumer( + 'Kotlin OkHttp verb-walk: helpers, .method("X"), default GET, variable skip (#2268)', + async () => { + // Parity with the Java OkHttp verb cases: a verb helper resolves, a literal + // `.method("X")` resolves, a bare `.build()` defaults to GET, and a + // variable-bound `.method(verb)` is unresolvable → emits nothing. + const dir = path.join(tmpDir, 'kotlin-okhttp-verbs'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'OkVerbs.kt'), + `package com.example +import okhttp3.Request +import okhttp3.RequestBody + +class OkVerbs(private val body: RequestBody, private val verb: String) { + fun run() { + Request.Builder().url("/api/k-get").build() + Request.Builder().url("/api/k-delete").delete().build() + Request.Builder().url("/api/k-patch").method("PATCH", body).build() + Request.Builder().url("/api/k-named").method(method = "REPORT", body = body).build() + Request.Builder().url("/api/k-dyn").method(verb, body).build() + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const okhttp = contracts.filter( + (c) => + c.role === 'consumer' && + c.meta.framework === 'okhttp' && + c.symbolRef.filePath.endsWith('OkVerbs.kt'), + ); + + // The variable-bound `.method(verb)` (`/api/k-dyn`) emits nothing; the + // named-argument `.method(method = "REPORT")` resolves its literal verb. + expect(new Set(okhttp.map((c) => c.contractId))).toEqual( + new Set([ + 'http::GET::/api/k-get', + 'http::DELETE::/api/k-delete', + 'http::PATCH::/api/k-patch', + 'http::REPORT::/api/k-named', + ]), + ); + }, + ); + + itKotlinConsumer( + 'Kotlin OkHttp: builder call before .url(), with anti-overreach (#2268)', + async () => { + // Parity with the Java OkHttp pre-`.url()` support: a builder call BEFORE + // `.url()` is captured (the chain roots at Request.Builder), the verb-walk + // scans the whole chain so a verb before `.url()` resolves, and a `.url(...)` + // on an unrelated object does NOT emit. + const dir = path.join(tmpDir, 'kotlin-okhttp-pre-url'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'OkPreUrl.kt'), + `package com.example +import okhttp3.Request +import okhttp3.RequestBody + +class OkPreUrl(private val body: RequestBody, private val other: SomeClient) { + fun run() { + Request.Builder().addHeader("A", "b").url("/api/k-pre-url").build() + Request.Builder().post(body).url("/api/k-verb-first").build() + other.url("/api/k-not-okhttp").build() + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const okhttp = contracts.filter( + (c) => + c.role === 'consumer' && + c.meta.framework === 'okhttp' && + c.symbolRef.filePath.endsWith('OkPreUrl.kt'), + ); + + // header-before-url → GET, verb-before-url → POST; `other.url(...)` emits nothing. + expect(new Set(okhttp.map((c) => c.contractId))).toEqual( + new Set(['http::GET::/api/k-pre-url', 'http::POST::/api/k-verb-first']), + ); }, ); @@ -4547,6 +5221,78 @@ class GatewayController : OrdersApi, UsersApi { ); const rows: ParityRow[] = [ + { + name: 'OkHttp builder verb inference', + files: [ + { + name: 'OkClient', + java: ` +import okhttp3.Request; +import okhttp3.RequestBody; + +class OkClient { + void run(RequestBody body) { + new Request.Builder().url("/api/things").post(body).build(); + } +} +`, + kotlin: `package com.example +import okhttp3.Request +import okhttp3.RequestBody + +class OkClient(private val body: RequestBody) { + fun run() { + Request.Builder().url("/api/things").post(body).build() + } +} +`, + }, + ], + expected: [ + { + role: 'consumer', + contractId: 'http::POST::/api/things', + framework: 'okhttp', + confidence: 0.7, + }, + ], + }, + { + name: 'OkHttp verb + builder call before .url()', + files: [ + { + name: 'OkPre', + java: ` +import okhttp3.Request; +import okhttp3.RequestBody; + +class OkPre { + void run(RequestBody body) { + new Request.Builder().post(body).url("/api/pre").build(); + } +} +`, + kotlin: `package com.example +import okhttp3.Request +import okhttp3.RequestBody + +class OkPre(private val body: RequestBody) { + fun run() { + Request.Builder().post(body).url("/api/pre").build() + } +} +`, + }, + ], + expected: [ + { + role: 'consumer', + contractId: 'http::POST::/api/pre', + framework: 'okhttp', + confidence: 0.7, + }, + ], + }, { name: '@RequestLine with @RequestMapping prefix fallback', files: [