diff --git a/gitnexus/src/core/group/extractors/http-patterns/php.ts b/gitnexus/src/core/group/extractors/http-patterns/php.ts index bf2eb7aba..65a1896c9 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/php.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/php.ts @@ -15,20 +15,36 @@ import type { HttpDetection, HttpLanguagePlugin } from './types.js'; * Providers: * - Laravel `Route::get/post/...` * - * Consumers (string-literal URLs only): + * Consumers (string-literal URLs only, unless noted): * - Laravel HTTP client: `Http::get/post/put/delete/patch($url)` * - Guzzle / generic object method: `$client->get/post/...($url)` * - `file_get_contents($url)` + * - `new Request($method, $host . $resourcePath)` — the openapi-generator-php + * / swagger-codegen client shape. `$resourcePath` is resolved via a + * single-scope backward constant fold (see `resolveLocalStringLiteral`), + * not a string literal at the call site itself. * * The pipeline already uses `PHP.php_only` for ingesting plain `.php` * files (see `core/tree-sitter/parser-loader.ts`), and we do the same * here so Laravel route files are parsed with the right grammar dialect. * - * Scope notes: consumer patterns match string literals only. URLs built - * via binary concatenation (`$base . '/path'`), `sprintf`, or config - * lookup (`config('services.foo.base').'/path'`) are intentionally left - * for a follow-up — they require constant-folding the surrounding - * scope to be meaningful. + * Scope notes: consumer patterns match string literals only, with one + * narrow exception (above). URLs built via `sprintf`, config lookup + * (`config('services.foo.base').'/path'`), or a variable resolved from + * outside its own function/method body are intentionally left for a + * follow-up — they require constant-folding beyond one local scope to + * be meaningful. + * + * That narrow exception (`resolveLocalStringLiteral`) is a temporary, + * single-scope fallback, not this language's entry into the shared + * cross-file constant-fold used by the other languages in this plugin + * (`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 real, separate project (this repo's PHP + * import resolution for `use`-statements is its own multi-file subsystem + * under `ingestion/import-resolvers/php.ts`, built for symbol/scope + * resolution, not constant extraction) and is intentionally out of scope + * here. Tracked as a follow-up, not silently punted. */ const LARAVEL_ROUTE_SPEC: PatternSpec> = { @@ -71,11 +87,31 @@ const FILE_GET_CONTENTS_SPEC: PatternSpec> = { `, }; +/** + * `new Request($method, $host . $resourcePath)` — the shape swagger-codegen / + * openapi-generator-php emit for every operation of a generated API client + * (Guzzle's `\GuzzleHttp\Psr7\Request`, or a bare `Request` behind a `use` + * import). Matches both `(name)` and `(qualified_name)` class references; + * `scan()` below filters to the last path segment being exactly `Request` + * and resolves the concatenated path argument (see `resolveLocalStringLiteral`). + */ +const GUZZLE_REQUEST_CTOR_SPEC: PatternSpec> = { + meta: {}, + query: ` + (object_creation_expression + [(name) (qualified_name)] @class + (arguments + . (argument (_) @methodArg) + . (argument (_) @pathArg))) + `, +}; + interface PhpPatternBundle { laravelRoute: CompiledPatterns>; httpFacade: CompiledPatterns>; guzzleMember: CompiledPatterns>; fileGetContents: CompiledPatterns>; + guzzleRequestCtor: CompiledPatterns>; } const mk = (spec: PatternSpec>, suffix: string) => @@ -90,6 +126,7 @@ const PHP_PATTERNS: PhpPatternBundle = { httpFacade: mk(HTTP_FACADE_SPEC, 'http-facade'), guzzleMember: mk(GUZZLE_MEMBER_SPEC, 'guzzle-member'), fileGetContents: mk(FILE_GET_CONTENTS_SPEC, 'file-get-contents'), + guzzleRequestCtor: mk(GUZZLE_REQUEST_CTOR_SPEC, 'guzzle-request-ctor'), }; /** @@ -129,6 +166,183 @@ function isHttpUrlLiteral(path: string): boolean { return path.startsWith('http://') || path.startsWith('https://'); } +/** + * Last identifier segment of a class-name reference: `(name)` returns its + * own text, `(qualified_name)` returns the text of its last child (the + * unqualified class name — `\GuzzleHttp\Psr7\Request` → `Request`). + */ +function lastNameSegment(node: import('tree-sitter').SyntaxNode): string { + if (node.type === 'qualified_name') { + const last = node.child(node.childCount - 1); + return last ? last.text : node.text; + } + return node.text; +} + +/** + * Return the variable at the LAST position of a `.`-concatenation + * expression, if (and only if) that position is a plain variable — + * generated clients build ` . `, so the path segment + * is the one closest to the end. + * + * No fallback to an earlier operand: if the rightmost position is anything + * other than a variable, a parenthesized sub-expression, or a nested `.` + * concatenation (a literal, a function call, ...), that position is a real + * value we simply can't resolve — falling back to an EARLIER operand would + * silently substitute a different value (e.g. the host) for the one that's + * actually there. `null` here is a miss, not a signal to keep looking. + */ +function lastConcatVariable( + node: import('tree-sitter').SyntaxNode, +): import('tree-sitter').SyntaxNode | null { + if (node.type === 'variable_name') return node; + if (node.type === 'parenthesized_expression') { + const inner = node.namedChild(0); + return inner ? lastConcatVariable(inner) : null; + } + if (node.type === 'binary_expression') { + const operator = node.childForFieldName('operator'); + if (!operator || operator.text !== '.') return null; // not concatenation + const right = node.childForFieldName('right'); + return right ? lastConcatVariable(right) : null; + } + return null; +} + +/** + * True if `node`'s subtree assigns to `$target` ANYWHERE inside it, at any + * depth (including inside nested functions — deliberately over-broad: a + * false positive here only costs a miss in the caller, never a wrong + * answer, so there's no need to be precise about scoping inside the probe + * itself). + */ +function containsAssignmentTo(node: import('tree-sitter').SyntaxNode, target: string): boolean { + if (node.type === 'assignment_expression') { + const lhs = node.childForFieldName('left'); + if (lhs && lhs.type === 'variable_name' && lhs.text === target) return true; + } + for (let i = 0; i < node.namedChildCount; i++) { + const child = node.namedChild(i); + if (child && containsAssignmentTo(child, target)) return true; + } + return false; +} + +/** + * True if an `anonymous_function` node's `use (...)` clause lists + * `$target`. PHP closures capture NOTHING automatically — only variables + * named in `use (...)` are visible inside — unlike arrow functions + * (`fn() => ...`), which auto-capture everything by value and have no + * `compound_statement` body of their own, so they're never seen as a + * `scope` by the walk below in the first place. + */ +function anonymousFunctionCaptures( + anonFn: import('tree-sitter').SyntaxNode, + target: string, +): boolean { + for (let i = 0; i < anonFn.namedChildCount; i++) { + const child = anonFn.namedChild(i); + if (!child || child.type !== 'anonymous_function_use_clause') continue; + for (let j = 0; j < child.namedChildCount; j++) { + const v = child.namedChild(j); + if (v && v.type === 'variable_name' && v.text === target) return true; + } + return false; // has a use(...) clause, but $target isn't in it + } + return false; // no use(...) clause at all — nothing is captured +} + +/** + * Best-effort, single-scope constant fold: given a `variable_name` node + * referenced inside a `new Request(...)` argument, walk BACKWARD through + * the preceding statements of its immediately enclosing function/method + * body (or file scope, for top-level script code) looking for the nearest + * `$var = '';` assignment. + * + * "Enclosing body" is resolved level by level, not just the nearest + * `compound_statement` — a call site nested in `if`/`foreach`/`try` inside + * that function is still within the same function/method body, and a + * preceding assignment above that conditional must still be found. Each + * level searches only its own preceding siblings, then the search + * continues from the enclosing block itself one level up, UNLESS that + * block IS the body of a function/method/closure: + * - a regular `function_definition` or `method_declaration` boundary + * always stops the search — PHP gives a function or method no access + * to anything outside its own body (no automatic capture, no implicit + * global), so widening past one into the containing class or + * file-level scope would resolve a variable the call site could never + * actually see at runtime; + * - an `anonymous_function` boundary stops UNLESS `$target` is + * explicitly captured via `use (...)` — closures capture nothing + * automatically either. + * It stops at `program` regardless, for the case where the call site was + * at file/script scope all along. + * + * A preceding sibling that ISN'T a plain assignment but might reassign the + * target somewhere inside itself (an `if`/`foreach`/`try`/`switch`, ...) + * stops the search rather than being skipped over: whether that branch ran + * is unknown, so an older literal further back can't be trusted either. + * + * Deliberately conservative and bounded — no interprocedural resolution, + * no constant/property lookups. A miss just means the endpoint stays + * undetected, never a wrong one: this is exactly the class of case the + * module docblock flags as in-scope only for one local scope. + */ +function resolveLocalStringLiteral(varNode: import('tree-sitter').SyntaxNode): string | null { + const target = varNode.text; // includes the `$` sigil, e.g. "$resourcePath" + let cursor: import('tree-sitter').SyntaxNode = varNode; + + for (;;) { + let scope: import('tree-sitter').SyntaxNode | null = cursor.parent; + while (scope && scope.type !== 'compound_statement' && scope.type !== 'program') { + scope = scope.parent; + } + if (!scope) return null; + + let stmt: import('tree-sitter').SyntaxNode | null = cursor; + while (stmt && stmt.parent !== scope) stmt = stmt.parent; + if (!stmt) return null; + + let sibling = stmt.previousNamedSibling; + while (sibling) { + if (sibling.type === 'expression_statement') { + const inner = sibling.namedChild(0); + if (inner && inner.type === 'assignment_expression') { + const lhs = inner.childForFieldName('left'); + if (lhs && lhs.type === 'variable_name' && lhs.text === target) { + // The NEAREST assignment to this variable wins, full stop — an + // older literal further back is shadowed by this one even when + // this one isn't itself a resolvable string (`$v = f();`). + const rhs = inner.childForFieldName('right'); + return rhs && rhs.type === 'string' ? phpStringText(rhs) : null; + } + } + } else if (containsAssignmentTo(sibling, target)) { + return null; // reassigned somewhere inside a conditional/loop/try + } + sibling = sibling.previousNamedSibling; + } + + if (scope.type === 'program') return null; + const enclosing = scope.parent; + if (enclosing && enclosing.type === 'anonymous_function') { + // Closures capture nothing automatically — only what's use()'d. + if (!anonymousFunctionCaptures(enclosing, target)) return null; + } else if ( + enclosing && + (enclosing.type === 'function_definition' || enclosing.type === 'method_declaration') + ) { + // A regular function or method boundary — NOT a closure. PHP gives + // these no access to anything outside their own body (no automatic + // capture, no implicit global): widening past one into the + // containing class body or file-level scope would resolve a + // variable the call site could never actually see at runtime. + return null; + } + cursor = scope; // one block up: search resumes from this block's own position + } +} + export const PHP_HTTP_PLUGIN: HttpLanguagePlugin = { name: 'php-http', language: PHP.php_only, @@ -136,12 +350,16 @@ export const PHP_HTTP_PLUGIN: HttpLanguagePlugin = { // ingestion, so the graph is authoritative for PHP providers (#2138 Part 2). routeCoverage: 'complete', // Consumer signals scan() can detect: Laravel `Http::`, Guzzle client - // `->get/post/.../request(...)`, and `file_get_contents` of an HTTP URL. A - // provider-covered file with any of these must still be parsed (ingestion - // emits no FETCHES for PHP). Conservative — the `->verb(` shape over-matches - // ordinary method calls, which only costs a parse, never data. + // `->get/post/.../request(...)`, `file_get_contents` of an HTTP URL, and a + // generated-client `new ...Request(...)` constructor call. A provider-covered + // file with any of these must still be parsed (ingestion emits no FETCHES for + // PHP). Conservative — the `->verb(`/`new ...Request(` shapes over-match + // ordinary method calls and unrelated constructors, which only costs a + // parse, never data. hasConsumerSignals(content) { - return /Http::|file_get_contents|->\s*(get|post|put|delete|patch|request)\s*\(/i.test(content); + return /Http::|file_get_contents|->\s*(get|post|put|delete|patch|request)\s*\(|new\s+[\\\w]*Request\s*\(/i.test( + content, + ); }, scan(tree) { const out: HttpDetection[] = []; @@ -222,6 +440,62 @@ export const PHP_HTTP_PLUGIN: HttpLanguagePlugin = { }); } + for (const match of runCompiledPatterns(PHP_PATTERNS.guzzleRequestCtor, tree)) { + const classNode = match.captures.class; + const methodArg = match.captures.methodArg; + const pathArg = match.captures.pathArg; + if (!classNode || !methodArg || !pathArg) continue; + // PHP class names are case-insensitive at the language level, and + // `hasConsumerSignals` above matches case-insensitively (`/i`) for + // the same reason — this comparison must agree with it, or a valid + // `new request(...)` / `new \NS\REQUEST(...)` call would be waved + // through the parse-skip gate as a signal and then silently dropped + // here. + if (lastNameSegment(classNode).toLowerCase() !== 'request') continue; + + // Path: a direct string literal, or the last variable in a + // concatenation chain (see `lastConcatVariable`) resolved to a + // locally-assigned literal. + let path: string | null = null; + if (pathArg.type === 'string') { + path = phpStringText(pathArg); + } else { + const lastVar = lastConcatVariable(pathArg); + path = lastVar ? resolveLocalStringLiteral(lastVar) : null; + } + if (path === null || !isHttpClientPath(path)) continue; + + // The HTTP verb is a literal, a local variable resolved the same way + // as the path (see `resolveLocalStringLiteral` above), or — commonly + // in generated clients — a parameter of the enclosing builder method + // fixed by ITS caller, not by this call site. That last case needs + // the same interprocedural reach the module docblock rules out, so it + // falls through to a wildcard verb, matching this project's own + // convention for a contract whose verb isn't pinned (see manifest + // links, `http::*::`). + let method: string | null = null; + if (methodArg.type === 'string') { + method = phpStringText(methodArg); + } else if (methodArg.type === 'variable_name') { + method = resolveLocalStringLiteral(methodArg); + } + + out.push({ + role: 'consumer', + framework: 'guzzle-request-ctor', + method: method ? method.toUpperCase() : '*', + path, + name: null, + // Line of the path ARGUMENT, not the `new Request(` call — same + // choice the other three consumer patterns in this file make, but + // this is the one pattern where the two routinely differ (generated + // clients wrap the call across multiple lines). Line-span + // containment still resolves to the right symbol either way. + line: pathArg.startPosition.row + 1, + confidence: 0.6, + }); + } + return out; }, }; diff --git a/gitnexus/test/unit/group/http-consumer-signals.test.ts b/gitnexus/test/unit/group/http-consumer-signals.test.ts index 087524cfe..6d98f44c7 100644 --- a/gitnexus/test/unit/group/http-consumer-signals.test.ts +++ b/gitnexus/test/unit/group/http-consumer-signals.test.ts @@ -46,6 +46,8 @@ describe('PHP hasConsumerSignals — superset of scan() consumer idioms', () => ['Laravel Http facade', "Http::get('/api/x');"], ['Guzzle member call', "$client->post('/api/x', []);"], ['file_get_contents', "file_get_contents('https://x/api');"], + ['bare Request constructor', 'new Request($method, $host . $resourcePath);'], + ['namespaced Request constructor', "new Foo\\Bar\\Request('GET', $url);"], ])('detects %s', (_label, src) => { expect(has(PHP_HTTP_PLUGIN, src)).toBe(true); }); @@ -53,6 +55,10 @@ describe('PHP hasConsumerSignals — superset of scan() consumer idioms', () => it('returns false for a pure Laravel route file (provider only)', () => { expect(has(PHP_HTTP_PLUGIN, "Route::get('/api/a/list', 'AController@list');")).toBe(false); }); + + it('returns false for an unrelated constructor call (not *Request)', () => { + expect(has(PHP_HTTP_PLUGIN, 'new Response($body, 200);')).toBe(false); + }); }); describe('Python hasConsumerSignals — superset of scan() consumer idioms', () => { diff --git a/gitnexus/test/unit/group/php-guzzle-request-ctor.test.ts b/gitnexus/test/unit/group/php-guzzle-request-ctor.test.ts new file mode 100644 index 000000000..9e6f44b4a --- /dev/null +++ b/gitnexus/test/unit/group/php-guzzle-request-ctor.test.ts @@ -0,0 +1,352 @@ +import { describe, expect, it } from 'vitest'; +import Parser from 'tree-sitter'; +import PHP from 'tree-sitter-php'; +import { PHP_HTTP_PLUGIN } from '../../../src/core/group/extractors/http-patterns/php.js'; + +const parser = new Parser(); +parser.setLanguage(PHP.php_only); + +const scan = (src: string) => PHP_HTTP_PLUGIN.scan(parser.parse(src)); +const consumers = (src: string) => scan(src).filter((d) => d.role === 'consumer'); + +describe('PHP guzzle-request-ctor pattern', () => { + it('resolves a locally-assigned $resourcePath concatenated with a member-access host, method is a real parameter', () => { + // $method is a FUNCTION PARAMETER here (the shape openapi-generator-php + // actually emits — the verb is fixed by the caller of this builder + // method, not assigned inside its body), not a local variable that + // happens to share the resolver's single-scope shape. A local + // `$method = 'POST';` immediately before the call would in fact resolve + // via the same fold as `$resourcePath` — that's a different case, + // covered separately below. + const src = `operationHost . $resourcePath + ); + return $this->client->send($request); + } +} +`; + const found = consumers(src); + expect(found).toHaveLength(1); + expect(found[0]).toMatchObject({ + framework: 'guzzle-request-ctor', + method: '*', // $method is a parameter, not a literal at this call site + path: '/payments/pay', + }); + }); + + it('resolves a fully-qualified GuzzleHttp/Psr7/Request with a literal verb', () => { + // Built via join(), not a literal backslash in this source file: a + // template-literal backslash-escape is easy to mis-transcribe (dropped + // silently by the JS/TS escape rules for an unrecognized `\`), so + // this sidesteps that entirely and is robust regardless of how the file + // itself gets written to disk. + const bs = String.fromCharCode(92); + const qualified = ['', 'GuzzleHttp', 'Psr7', 'Request'].join(bs); + const src = [ + ' { + const src = ` { + const src = `host . $resourcePath); + return $request; +} +`; + expect(consumers(src)).toHaveLength(0); + }); + + it('ignores an unrelated constructor whose class name does not end in "Request"', () => { + // $resourcePath IS resolvable here (unlike an earlier version of this + // test) — the class-name filter must be the reason this produces no + // detection, not an incidental miss elsewhere in the pipeline. Without + // a resolvable path, this test would pass even with the class-name + // filter deleted. + const src = `host . $resourcePath); + return $response; +} +`; + expect(consumers(src)).toHaveLength(0); + }); + + it('rejects a resolved literal that is not an HTTP-looking path', () => { + const src = `host . $resourcePath); + return $request; +} +`; + expect(consumers(src)).toHaveLength(0); + }); + + it('prefers the LAST variable in a 3-part concatenation (path, not an earlier segment)', () => { + const src = ` { + // Regression: an earlier version of lastConcatVariable treated an + // unhandled `parenthesized_expression` as "no variable here" and fell + // through to the LEFT operand — silently returning $host instead of + // failing to find anything inside the parens. + const src = ` { + // Regression: resolveLocalStringLiteral stopped at the nearest + // compound_statement (the `if` block), not the enclosing function body, + // so an assignment made just above the `if` — in the same function — + // was invisible to a `new Request(...)` call nested inside it. + const src = `isValid()) { + $request = new Request($method, $this->host . $resourcePath); + return $request; + } + return null; +} +`; + const found = consumers(src); + expect(found).toHaveLength(1); + expect(found[0].path).toBe('/payments/pay'); + }); + + it('still does not cross into a sibling function even when searching level by level', () => { + // The level-by-level widening must stop at `program` / the enclosing + // function boundary — it must not walk into a DIFFERENT function's body + // just because that function is a preceding sibling statement. + const src = `host . $resourcePath); + return $request; + } +} +`; + expect(consumers(src)).toHaveLength(0); + }); + + it('does not resolve through an intervening non-literal reassignment (last write wins)', () => { + // Regression: the backward scan used to skip PAST an assignment whose + // RHS wasn't a string literal, landing on an older literal that the + // variable no longer holds at the call site — a wrong answer, not a + // miss. The nearest assignment must decide the outcome, full stop. + const src = `host . $resourcePath); + return $request; +} +`; + expect(consumers(src)).toHaveLength(0); + }); + + it('does not treat a non-concatenation binary expression as a path candidate', () => { + // Regression: lastConcatVariable recursed into ANY binary_expression + // without checking the operator, so `$host ?? $resourcePath` (or `&&`, + // `+`, ...) was walked exactly like `.` concatenation. + const src = `host ?? $resourcePath); + return $request; +} +`; + expect(consumers(src)).toHaveLength(0); + }); + + it('matches a lowercase "request" constructor — PHP class names are case-insensitive', () => { + const src = ` { + const src = `host . $resourcePath); + return $request; +} +`; + const found = consumers(src); + expect(found).toHaveLength(1); + expect(found[0]).toMatchObject({ method: 'POST', path: '/payments/pay' }); + }); + + it('does not use a stale literal shadowed by a reassignment inside a preceding if block', () => { + // Regression: the backward scan only inspected direct expression_statement + // siblings, so `$resourcePath = '/new';` nested inside an `if` right + // before the call was invisible, and the OLDER `/old` (outside the `if`) + // was returned instead — an unconditional wrong answer whenever that + // branch runs, not a miss. + const src = `host . $resourcePath); + return $request; +} +`; + expect(consumers(src)).toHaveLength(0); + }); + + it('does not resolve a variable across an anonymous-function boundary it was not use()-captured into', () => { + // Regression: level-by-level widening climbed straight from the + // closure's body to the enclosing method's body without checking PHP's + // actual capture rule (closures capture NOTHING unless listed in + // `use (...)`), resolving a variable the closure can't actually see — + // real PHP would throw "Undefined variable" here, not build this path. + const src = `host . $resourcePath); + }; + return $build(); +} +`; + expect(consumers(src)).toHaveLength(0); + }); + + it('DOES resolve a variable across an anonymous-function boundary it WAS use()-captured into', () => { + const src = `host . $resourcePath); + }; + return $build(); +} +`; + const found = consumers(src); + expect(found).toHaveLength(1); + expect(found[0].path).toBe('/payments/pay'); + }); + + it('does not fall back to the host when the concatenation ends in a string literal, not a variable', () => { + // Regression: lastConcatVariable fell through to the LEFT operand when + // the right one wasn't a variable, so `$host . '/users'` resolved to + // $host instead of recognizing the trailing literal isn't a variable at + // all — if $host happened to be an HTTP URL locally, that URL would be + // emitted as the path instead of a miss. + const src = ` { + // Regression: after exhausting a method's own body, widening went + // straight to `program` (file scope) and found the top-level literal — + // but PHP methods have NO access to file-level variables without an + // explicit `global $v;`, which this resolver deliberately never adds + // support for. This produced a wrong contract, not a miss. + const src = `host . $resourcePath); + return $request; + } +} +`; + expect(consumers(src)).toHaveLength(0); + }); + + it('does not resolve a variable from file scope into a plain top-level function either', () => { + const src = ` { + const src = `