2 commits

Author	SHA1	Message	Date
Przemek Poppe	b059ab3541	PHP: detect generated-client Request(method, host . resourcePath) consumer shape (#3079) ... * feat(group/php): detect generated-client Request(method, host . resourcePath) openapi-generator-php / swagger-codegen PHP clients build every operation as `$resourcePath = '/foo/bar'; ...; new Request($method, $host . $resourcePath);` — a shape the PHP consumer patterns didn't cover (only `$client->verb('/path')` literal calls were matched), documented in the module's own docblock as a follow-up ("constant-folding the surrounding scope"). Adds a pattern for `new [Qualified\]Request(...)` constructor calls, with a conservative, single-scope backward constant fold: the last variable in the path argument's concatenation chain (generated clients build `<host> . <resourcePath>`) is resolved to a `$var = '<literal>';` assignment in the same enclosing function/method body (or file scope) if one exists earlier in the same scope. No interprocedural resolution — a miss just leaves the endpoint undetected, never a wrong one. The HTTP verb is often itself a parameter in these generated clients (not a literal at the call site), so when it can't be resolved to a literal the detection reports a wildcard method (`'*'`), consistent with this project's existing manifest-link convention for a contract whose verb isn't pinned. `hasConsumerSignals` is widened to stay a proven superset of what `scan()` now detects (required by its own contract, checked by `http-consumer-signals.test.ts`). The docblock notes this is a deliberately narrow, single-scope fallback, not this language's entry into the shared cross-file constant-fold the other languages use (`constant-resolver.ts`, wired in via `java-const-resolver.ts` / `python-const-resolver.ts` / `js-const-resolver.ts`) — PHP has no such binding yet; adding one is a separate, larger project (this repo's PHP import resolution for `use`-statements is its own multi-file subsystem built for symbol/scope resolution, not constant extraction) and is out of scope here. Tests: 7 scan()-level cases (resolution across a member-access host, fully-qualified class name, purely literal call, negative — different function scope, negative — non-Request constructor, negative — non-HTTP literal, picking the LAST var in a 3-part concatenation), plus 2 new hasConsumerSignals cases and a negative. `tsc --noEmit` clean, `eslint` clean, full test/unit/group green (1035+/1037; 2 pre-existing native EBUSY failures on a `.lbug` file in bridge-meta-swap-window.test.ts, unrelated subsystem, reproduces in isolation on unchanged upstream too). * fix(group/php): fix two code-review findings in guzzle-request-ctor 1. lastConcatVariable silently returned the WRONG variable for a parenthesized right operand: `$host . ($resourcePath . $suffix)` fell through to the left operand (unhandled parenthesized_expression) and returned $host instead of looking inside the parens. Restored parenthesis unwrapping (present in an earlier draft, dropped during a simplification pass that didn't account for this fallthrough). 2. resolveLocalStringLiteral stopped at the nearest compound_statement, so a `new Request(...)` call nested in `if`/`try`/`foreach` inside the same function couldn't see an assignment made just above that block — despite the docblock's claim of covering the "enclosing function/method body". Now widens level by level (search the immediate block's preceding statements, then its own enclosing block, and so on), stopping at `program` so it still never crosses into a different function or the containing class body — verified by a regression test asserting exactly that boundary. Also documents the line-number choice (path argument, not the `new Request(` call site — the two differ for this pattern's characteristically multi-line calls) inline, matching the other three consumer patterns' convention in this file. 4 new regression tests (35 total in this file's suite): parenthesized right operand no longer mismatches, enclosing-block resolution across an `if`, and a negative case proving the widened search still respects the function boundary. tsc --noEmit clean, eslint clean. * fix(group/php): address gitnexus-check bot review on PR #3079 1. resolveLocalStringLiteral fell through an intervening non-literal reassignment: `$v = '/old'; $v = buildPath(); new Request(..., $v)` resolved to '/old' even though $v never holds that literal at the call site. The NEAREST assignment to the target variable now decides the outcome unconditionally — a non-string RHS stops the search (returns null) instead of letting the scan continue past it to an older, shadowed literal. This was a real "wrong answer", not a miss, directly contradicting the function's own documented invariant. 2. lastConcatVariable recursed into every binary_expression regardless of operator, so `$host && $resourcePath`, `$host + $resourcePath`, and `$host ?? $resourcePath` were walked exactly like `.` concatenation. Now checks operator === '.' before recursing. 3. hasConsumerSignals matches case-insensitively (`/i`), correctly, since PHP class names are case-insensitive at the language level — but scan() compared the resolved class name to 'Request' case-sensitively, so a valid `new request(...)` / `new \NS\REQUEST(...)` call would pass the parse-skip gate as a signal and then be silently dropped by scan() itself. Both sides now agree (case-insensitive compare in scan() too). 4. The first test's own PHP source assigned `$method = 'POST';` as a local variable but asserted `method: '*'` with a comment calling it "a parameter" — it wasn't; it was exactly the same locally-resolvable shape as $resourcePath. Fixed by (a) rewriting that test's source to show $method as a genuine function parameter (the shape generated clients actually use — the verb is fixed by the caller of the builder method), which is what the test intended to demonstrate, and (b) actually implementing symmetric resolution: method now resolves through the same resolveLocalStringLiteral fold as path when it IS a local variable, with a new test proving that case resolves to a literal method instead of a wildcard. 5 new regression tests (39 total in this file's suite, up from 35): non-literal-reassignment shadowing, non-concatenation operator rejected, case-insensitive class name match, and local-variable method resolution. tsc --noEmit clean, eslint clean. * fix(group/php): address second round of gitnexus-check bot review 1. Backward fold missed reassignments nested inside a preceding if/foreach/ try/switch: the scan only recognized direct expression_statement siblings as candidate assignments, so `if ($cond) { $v = '/new'; }` right before the call was invisible, and an OLDER, now-shadowed literal outside that block was returned instead — a real wrong answer whenever that branch runs. Any non-assignment sibling that contains an assignment to the target ANYWHERE inside it now stops the search (miss) rather than being skipped over, since whether that branch ran is unknown. 2. Level-by-level scope widening crossed anonymous-function boundaries without checking PHP's actual capture rule: closures capture NOTHING automatically, only variables listed in `use (...)` are visible inside — unlike arrow functions, which auto-capture everything and have no `compound_statement` body (never seen as a scope by this walk at all). Widening past a closure's body now checks its `use (...)` clause first; real PHP would throw "Undefined variable" for anything not captured, not resolve to a value from the enclosing scope. 3. lastConcatVariable still fell through to the LEFT operand whenever the right one wasn't a variable-or-nestable-expression — `new Request($m, $host . '/users')` (a trailing string literal, not a variable) resolved to $host instead of recognizing there's simply nothing to resolve at that position. Removed the left-operand fallback entirely: the rightmost position decides, full stop, matching the function's own "single lookup, not a fallback list" docblock (which the previous round already stated but the code didn't yet fully honor for this case). Also strengthened a test that the bot correctly flagged as non-diagnostic: "ignores an unrelated constructor" used an unresolvable $resourcePath, so it would have passed even with the class-name filter deleted. Now uses a fully resolvable path so the class-name filter is what the assertion actually exercises. 4 new regression tests (43 total, up from 39): shadowed-by-conditional- reassignment, closure boundary without use()-capture (negative), closure boundary WITH use()-capture (positive control), and trailing-literal concatenation no longer mistaken for the host variable. tsc --noEmit clean, eslint clean. * chore: trigger re-review (previous gitnexus-check report cited stale line numbers) * fix(group/php): stop scope widening at a function/method boundary The digest posted on PR #3079 (verified against the current file, not the stale HEAD it was generated from — three of its four findings were already fixed in prior commits) reproduced a real, still-present fourth issue: after exhausting a method's own body, widening continued straight to `program` (file/script scope) and could resolve a top-level variable into a class method — but PHP methods (and plain functions) have NO access to file-level variables without an explicit `global $v;`, which this resolver intentionally never adds support for. A file-level `$resourcePath = '/x';` could therefore leak into an unrelated method's `new Request(...)` as a real, wrong answer. Widening now stops unconditionally at a `function_definition` or `method_declaration` boundary — these get no automatic capture and no implicit global in PHP, unlike closures (already handled: an `anonymous_function` boundary stops unless `$target` is `use()`-captured). The call-site-at-file-scope case still resolves correctly, since `program` is reached directly there with no boundary to cross. 3 new regression tests (46 total): file-scope variable does not leak into a class method, does not leak into a plain top-level function either, and a positive control confirming file-scope-to-file-scope resolution still works when there's no function boundary at all. tsc --noEmit clean, eslint clean. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-08-28 17:47:41 +00:00
henry201605	b16ec344f7	perf(group/http): skip source parse for graph-covered route files (#2138 Part 2) (#2265) ... * feat(routes): resolve + persist handler symbol on Route nodes (#2138 part 2, WIP) Part 2 groundwork for #2138: give the graph-assisted HTTP provider path the handler symbol directly, so it no longer re-parses source to recover the handler name. (The remaining parse-skip in extract() + a call-count benchmark land in a follow-up commit.) - ExtractedDecoratorRoute gains `handlerName`; the Spring extractor captures the decorated method's name (the method_declaration node is in hand). - New `resolveRouteHandlerSymbols` (call-processor) resolves each route's handler to a real symbol UID, keyed by normalized route URL — Laravel framework routes (controller + method) and decorator routes (Spring/FastAPI) both reduce to `(filePath, name) -> nodeId`. Threaded through the parse phase onto `ParseOutput.routeHandlerSymbols`. - routes phase stamps `Route.handlerSymbolId`; persisted end-to-end (schema + Route CSV row + getCopyQuery COPY columns), mirroring Part 1's `method`. - HttpRouteExtractor: `HANDLES_ROUTE_QUERY` returns `handlerSymbolId`; `extractProvidersGraph` uses it as the authoritative symbol and SKIPS `getDetections()` for resolved rows (CONTAINS is a cheap graph lookup for the display name only — no tree-sitter parse). Fully backward compatible: an unresolved/old-index route with no `handlerSymbolId` keeps the source-scan fallback. - Extracted `normalizeExtractedRoutePath` to `route-extractors/route-path.ts` (shared by routes phase + resolver without an import cycle). - SCHEMA_BUMP 6->7 (ParseWorkerResult gained `handlerName`); regenerated the emit-persistence byte-identity baseline (route.csv header gained two columns). - Tests: Spring pipeline asserts the Route node carries a handlerSymbolId resolving to the handler method; extractor fast-path test proves the handler resolves with zero source detections. Refs #2138 * perf(group/http): skip source parse for graph-covered route files (#2138 Part 2) Builds on the persisted Route.handlerSymbolId (U0–U3a). When a file's HANDLES_ROUTE rows all resolve a handler symbol AND its language plugin declares routeCoverage: 'complete' (Java/Python/PHP), the graph is authoritative for that file's providers, so the source scan + tree-sitter parse can be skipped — the scan would only re-discover routes the graph already has. This is the measurable parse reduction #2167 could not show. Consumer safety: routeCoverage: 'complete' asserts *provider* Route-node completeness only. The scan() of those same languages also emits consumer detections (RestTemplate/WebClient/OkHttp/Feign, Guzzle/Http::, requests/httpx), and ingestion's FETCHES edges are JS/TS-only — so the graph cannot back up server-side consumers. A provider-covered controller that also calls out would otherwise lose its consumer contract. Guarded by a cheap, parse-free text gate. - types: HttpLanguagePlugin gains - routeCoverage?: 'complete' | 'partial' (default 'partial') - hasConsumerSignals?(content): false only when the raw source provably has no outbound-HTTP call this plugin detects (conservative). - java/python/php: mark routeCoverage 'complete' + implement hasConsumerSignals with a token regex over their consumer idioms. - http-route-extractor: run the graph provider pass first to build a coveredFiles set; then keep a file covered only when hasConsumerSignals(content) === false (read via readSafe, no parse). scanFiles = files not covered → drives collectProjectDetections + both source scans. Fail-open per file: any unresolved row, a 'partial' language, a positive consumer signal, a missing hook, or an unreadable file leaves the file in the scan set. The orchestrator names no languages — token knowledge stays in the plugins. Net: pure-provider controllers skip the parse (the win); controllers that also call out are still parsed (no consumer loss); partial-coverage languages and graph-less runs are unchanged. - test: route-parse-skip integration test spies the real parseSourceSafe to COUNT parses over a temp repo of Spring controllers with a mock DB — baseline (every file parsed), fully-covered (0 parses), mixed (unresolved file falls back, resolved stays skipped), and provider+consumer (a covered controller that also calls restTemplate is parsed; its consumer contract survives). * fix(group/http): cover Spring HTTP Interface @*Exchange in Java consumer-signal gate #2254 (merged) added Spring 6 HTTP Interface `@(Get|...)Exchange` / `@HttpExchange` as a new Java *consumer* idiom. The #2138 parse-skip consumer-safety gate must recognize it, or a provider-covered file carrying an `@GetExchange` could be parse-skipped and lose that consumer contract. Add `Exchange` to JAVA_HTTP_PLUGIN.hasConsumerSignals (conservative; also matches `restTemplate.exchange(`). * style(group/http): prettier formatting for #2138 Part 2 files * style(ingestion): prettier formatting for call-processor.ts (#2138 Part 2) * fix(group/http): P1 (Java over-claim) + P2 (handler mis-attribution) on top of #2268 (#2138 Part 2) Re-applied on the maintainer's #2268 (expanded Java/Kotlin consumer extraction) base. P1 — `routeCoverage: 'complete'` over-claimed for Java: the graph provider set is a strict subset of the group scan (array-form `@GetMapping({...})`, interface-inherited routes, same-URL multi-verb have no graph Route node), so parse-skip could drop those group-only providers. - java/python → default 'partial' (always source-scanned). Java flips to 'complete' only once ingestion provider extraction matches the group scan (a separate follow-up). Python was a no-op anyway (no handlerName resolved); 'complete' was a latent trap. PHP stays 'complete' (Laravel ingestion ⊇ the group scan, the one language the skip engages for). - python hasConsumerSignals widened to a true superset of scan() (uri=/url= wrapper, aiohttp, urllib). Java's gate already covers #2268's consumer set (same receivers; the @*Exchange token is present). P2 — resolveRouteHandlerSymbols: reserve the URL slot on first encounter even when unresolved (mirrors addRoute first-writer-wins, so a later same-URL route can't stamp the node-winner's slot); refuse to guess on an ambiguous same-name lookup (exactly one match → use it; zero/many → fail-open, never a wrong handler). The cross-source case (filesystem route winning a URL a framework route also normalizes to) is unchanged — the resolver never receives filesystem routes — and stays fail-open. Tests: - route-parse-skip rewritten: the parse-skip win is proven on PHP (fully covered → 0 parses; mixed fallback; consumer-covered file still parsed), plus three Java P1 regression guards (array-form / interface-inherited / multi-verb) asserting the group-only routes survive — verified they go red if Java is flipped back to 'complete'. - resolve-route-handler-symbols: direct unit tests (the fn had none) — unique resolve, ambiguous/unknown fail-open, same-URL reservation, first-writer-wins. - http-consumer-signals: each plugin's hasConsumerSignals is a superset of its scan() consumer idioms; pure providers return false. - route-handler-symbol-roundtrip: real-LadybugDB CSV→COPY→query for Route.handlerSymbolId. --------- Co-authored-by: henry <zhangwei2017@unipus.cn> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-06-23 07:22:43 +01:00