mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-16 23:43:12 +00:00
b059ab3541
* 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>
|
||
|---|---|---|
| .. | ||
| fixtures | ||
| helpers | ||
| integration | ||
| unit | ||
| utils | ||