Skip to content

Commit a0de679

Browse files
committed
Improve str_replace and json_encode return types
1 parent cbd271b commit a0de679

14 files changed

Lines changed: 903 additions & 476 deletions

File tree

docs/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
375375
- **A return type that depends on an argument's value is read from the value passed, or from the argument's default when it is left out.** A conditional `@return` keyed on a value (`($format is 0 ? int : list<string>)`) only ever recognised a quoted string, so an `int` literal decided nothing and an omitted argument decided nothing either, leaving every call to read back the union of every branch and any use of the result in a typed position reported. The literal at the call site now decides the branch, compared by value rather than by spelling, and an omitted argument is decided by the default it declares, since that is the value it takes at runtime. `str_word_count()` is the standard library's example of the shape and now resolves to the count, the word list, or the offset-keyed map according to its `$format`, so returning it from an `int` method is no longer reported. A negated condition is honoured in both directions, and a value the call site cannot pin down still reads back everything the call could return.
376376
- **A tag written on a docblock's opening line is read.** A docblock that starts its first tag on the `/**` line itself, as in `/** @param 'a'|'b' $key` followed by more tags below, had that first tag ignored for `@param` and `@var`. The same tag moved down a line worked, and so did the fully single-line spelling, so only the shape that shares the opening line was affected. The parameter or variable fell back to its native hint, which is wider than what was declared, so narrowing, argument checks, and hover all read the wide type, and a `@return` further down the same docblock (which was read) could then be reported as incompatible with the body's widened value.
377377
- **`?->` on a `null` subject is no longer reported as a crash.** `echo $customer?->id;` where `$customer` is `null` was reported as "Cannot access property 'id' on type 'null'", the same diagnostic a plain `->` earns for the same subject. The nullsafe operator exists precisely so that case doesn't crash: it short-circuits to `null` without touching the property. The diagnostic now tells the two operators apart and only reports a `null` subject under a plain `->`, where accessing it is still a real crash.
378+
- **A replace on a string comes back a string, and one on an array comes back an array.** `preg_replace()`, `preg_replace_callback()`, `preg_filter()`, `str_replace()`, `str_ireplace()` and `substr_replace()` return whatever shape their subject was, but their signatures can only name the flat union of both overloads, so every call was read as `array|string` no matter what it was handed. Passing the result of a replace on a plain string straight into a `string` parameter or returning it from a `string` function was reported for an array branch the call could never take, which was the single largest source of argument and return mismatches in real code. Each of them now resolves against the subject at the call site: a string subject rules the array branch out, an array subject rules the string branch out and keeps the keys it was given, and a subject whose shape is genuinely unknown still reports both, since that is all the call can promise. `preg_replace()`'s `null` error result survives for a string subject, where PHP really can return it, and is dropped for an array subject, where it cannot.
379+
- **`json_encode()` with `JSON_THROW_ON_ERROR` can no longer be `false`.** The flag is how modern code asks for a `JsonException` instead of a silent `false`, and the declared `string|false` return type has no way to say so, so every such call was still read as possibly `false`: handing the result to a `string` parameter, returning it, or concatenating it were all reported for a branch the flag had already ruled out. The flag is now read at the call site, whether it is passed on its own, OR-ed together with other JSON flags such as `JSON_PRETTY_PRINT`, written as a plain number, or held in a constant. A call that leaves the flag out, or whose flags cannot be read, keeps the failure branch, because there it is real.
380+
- **A cast argument is read as the type it casts to.** `(string) $customer->mobile` is a `string` whatever the property holds, but an argument written that way resolved to nothing at all, so anything that reads a call's arguments to work out its return type was left guessing: a `@template` bound from that argument stayed unbound, and a return type that depends on the argument fell back to naming every branch at once. Every cast now answers with what it produces, so `str_replace('a', 'b', (string) $value)` is a `string` and not the string-or-array union its signature also allows.
378381

379382
## [0.9.0] - 2026-07-20
380383

docs/todo.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,14 @@ unlikely to move the needle for most users.
7272
| C6 | `#[ExpectedValues]` parameter value suggestions | Low | Medium |
7373
| C10 | [Deprecation markers on class-name completions from all sources](todo/completion.md#c10-deprecation-markers-on-class-name-completions-from-all-sources) | Low | Low |
7474
| | **[Type Inference](todo/type-inference.md)** | | |
75-
| T38 | [Several core builtins have a return type that depends on an argument's value or shape](todo/type-inference.md#t38-several-core-builtins-have-a-return-type-that-depends-on-an-arguments-value-or-shape) (`preg_replace`/`str_replace` family, `json_encode`/`json_decode`) | High | Medium |
7675
| T20 | [Type narrowing reconciliation engine](todo/type-inference.md#t20-type-narrowing-reconciliation-engine) (CNF clause algebra, sure/sureNot tracking) | Medium-High | High |
7776
| T28 | [Template inference depth priority (shallowest bound wins)](todo/type-inference.md#t28-template-inference-depth-priority-shallowest-bound-wins) | Medium | Low-Medium |
7877
| T29 | [Definite vs possible variable existence tracking](todo/type-inference.md#t29-definite-vs-possible-variable-existence-tracking) | Medium | Medium |
7978
| T3 | [Property hooks (PHP 8.4)](todo/type-inference.md#t3-property-hooks-php-84) | Medium | Medium |
8079
| T32 | [Audit `is_type_compatible`'s MAYBE escape hatches for core-engine gaps](todo/type-inference.md#t32-audit-is_type_compatibles-maybe-escape-hatches-for-core-engine-gaps) | Medium | Medium |
8180
| T34 | [`static::CONST` over-narrows to the declaring class's value](todo/type-inference.md#t34-staticconst-over-narrows-to-the-declaring-classs-value) | Medium | Medium |
8281
| T30 | [Literal type collapse limit](todo/type-inference.md#t30-literal-type-collapse-limit) | Low-Medium | Low |
82+
| T40 | [`pathinfo()` returns a shape or a string depending on the flags argument](todo/type-inference.md#t40-pathinfo-returns-a-shape-or-a-string-depending-on-the-flags-argument) | Low-Medium | Low |
8383
| T6 | `Closure::bind()` / `Closure::fromCallable()` return type preservation | Low-Medium | Low-Medium |
8484
| T13 | [Closure variables lose callable signature detail](todo/type-inference.md#t13-closure-variables-lose-callable-signature-detail) | Low-Medium | Medium |
8585
| T26 | [Globbed constant unions (`Foo::BAR_*`)](todo/type-inference.md#t26-globbed-constant-unions-foobar_) | Low-Medium | Medium |
@@ -93,11 +93,12 @@ unlikely to move the needle for most users.
9393
| D14 | [Tighten argument type mismatch diagnostic (Phase 2)](todo/diagnostics.md#d14-tighten-argument-type-mismatch-diagnostic-phase-2) | Medium | Low |
9494
| D6 | [Unreachable code diagnostic](todo/diagnostics.md#d6-unreachable-code-diagnostic) | Low-Medium | Low |
9595
| D16 | [`unreachable_match_arm` ignores literal subject types](todo/diagnostics.md#d16-unreachable_match_arm-ignores-literal-subject-types) | Low-Medium | Low |
96-
| D5 | [External tool diagnostic suppression actions](todo/diagnostics.md#d5-external-tool-diagnostic-suppression-actions) | Low | Low |
96+
| D5 | [External tool diagnostic suppression actions](todo/diagnostics.md#d5-external-tool-diagnostic-suppression-actions) | Low | Low-Medium |
9797
| D15 | [Unused parameter diagnostic](todo/diagnostics.md#d15-unused-parameter-diagnostic) | Low | Low |
9898
| D17 | [`docblock_native_mismatch` only judges nullability](todo/diagnostics.md#d17-docblock_native_mismatch-only-judges-nullability) | Low | Medium |
9999
| | **[Bug Fixes](todo/bugs.md)** | | |
100100
| B99 | [Formatting a `.blade.php` file has no extension guard](todo/bugs.md#b99-formatting-a-bladephp-file-has-no-extension-guard) | Medium | Low |
101+
| B124 | [An argument's type is read from its source text, and several ordinary spellings read as nothing](todo/bugs.md#b124-an-arguments-type-is-read-from-its-source-text-and-several-ordinary-spellings-read-as-nothing) | Medium | Medium |
101102
| | **[Code Actions](todo/actions.md)** | | |
102103
| A40 | [Generate method from call](todo/actions.md#a40-generate-method-from-call) | Medium-High | Medium |
103104
| A41 | [Create class from non-existing name](todo/actions.md#a41-create-class-from-non-existing-name) | Medium | Medium |

docs/todo/bugs.md

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,48 @@ edits, rather than being a no-op.
2828

2929
**Fix:** add an extension check in the formatting handler that returns no
3030
edits for `.blade.php` documents, ahead of BL16's real implementation.
31+
32+
### B124. An argument's type is read from its source text, and several ordinary spellings read as nothing
33+
34+
**Impact: Medium · Effort: Medium**
35+
36+
```php
37+
/** @param array{message: string} $data */
38+
function report(array $data, string $body): void {
39+
$out = str_replace('a', 'b', $data['message']); // array<array-key, string>|string
40+
$subject = $data['message'];
41+
$sameThing = str_replace('a', 'b', $subject); // string — correct
42+
43+
$version = preg_replace('/-.*/', '', PHP_VERSION); // array<array-key, string>|string
44+
$trimmed = preg_replace('/\s+/', ' ', $body ?: ''); // array<array-key, string>|string
45+
}
46+
```
47+
48+
`Backend::resolve_arg_text_to_type` is the shared "what type is this
49+
argument" helper that conditional return types and `@template` binding both
50+
consult, and it works from the argument's *source text*. It answers for
51+
literals, casts, variables, property chains, calls, `::class` and static
52+
access, but several ordinary spellings resolve to nothing:
53+
54+
- an array element (`$data['message']`, `$rows[0]`): the general expression
55+
path reports only class-backed results, so an element holding a scalar
56+
comes back empty, and the raw-type fallback skips any text containing `[`
57+
outright;
58+
- a global constant (`PHP_VERSION`, `PHP_EOL`): there is no constant branch,
59+
and `ResolutionCtx` does not carry the constant loader that
60+
`VarResolutionCtx` has, so one would have nothing to consult;
61+
- an operator expression (`$body ?: ''`, `$a . $b`, `$n + 1`): nothing reads
62+
the operator, even where it alone decides the type.
63+
64+
Assigning the same expression to a variable first resolves fine, so the
65+
answer depends on how the call was spelled. The visible effects are a
66+
conditional return type that stays undecided (and therefore returns the
67+
union of both branches, as `str_replace` does above) and a template
68+
parameter that stays unbound.
69+
70+
**Fix:** each spelling needs its own small branch in the text resolver,
71+
answering with what the same expression resolves to when it is assigned to a
72+
variable first: array access via `SubjectExpr::ArrayAccess` plus the
73+
array-shape key lookup the forward walker already has, a global constant via
74+
the constant loader (which `ResolutionCtx` has to start carrying), and the
75+
operators whose result type is fixed regardless of their operands.

docs/todo/type-inference.md

Lines changed: 26 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -679,50 +679,35 @@ upstream's `nsrt/class-constant-types.php` were dropped when
679679
`tests/phpstan_nsrt/class-constant-types.php` was ported (only the
680680
`self::` cases survive); port them back.
681681

682+
682683
---
683684

684-
## T38. Several core builtins have a return type that depends on an argument's value or shape
685-
**Impact: High · Effort: Medium**
685+
## T40. `pathinfo()` returns a shape or a string depending on the flags argument
686+
**Impact: Low-Medium · Effort: Low**
686687

687688
```php
688-
$json = json_encode($value, JSON_THROW_ON_ERROR);
689-
needsString($json); // reported: got string|false — THROW_ON_ERROR makes false impossible
690-
691-
$out = preg_replace($pattern, $replacement, $subject); // $subject: string
692-
needsString($out); // reported: got string|array<string> — $subject is a string, so the array branch can't happen
689+
$parts = pathinfo($path); // array{dirname: …, basename: …, extension?: …, filename: …}
690+
$name = pathinfo($path, PATHINFO_FILENAME); // string
691+
needsString($name); // reported: got string|array{…} — a single component is always a string
693692
```
694693

695-
phpstorm-stubs (and PHPantom's own signatures) declare these functions
696-
with the flat union of every possible overload's return type, because
697-
the stub format has no way to express "the return type depends on the
698-
value of this specific argument." PHPStan instead ships a
699-
`DynamicFunctionReturnTypeExtension` per case:
700-
`JsonThrowOnErrorDynamicReturnTypeExtension` drops the `false` branch of
701-
`json_encode`/`json_decode` when the flags argument provably includes
702-
the `JSON_THROW_ON_ERROR` bit; a `preg_replace`/`preg_replace_callback`/
703-
`str_replace`/`str_ireplace` family extension picks the `array` or
704-
`string` branch based on whether the `$subject` argument is an array or
705-
a string; `pathinfo()` similarly depends on whether a `$flags` argument
706-
is present. None of this is modelled today, so PHPantom always reports
707-
the full declared union regardless of the actual call site — one of the
708-
largest sources of `type_mismatch_argument`/`_return` false positives
709-
found in the 2026-08-12 sample-project sweep (over 150 instances of the
710-
`preg_replace`/`str_replace`-family shape alone, plus dozens more from
711-
`json_encode`/`json_decode`).
712-
713-
**Fix:** add a small dynamic-return-type mechanism for builtin functions,
714-
mirroring the existing PHPStan conditional-return-type support
715-
(`php_type/transform.rs`'s `ConditionalType` handling, added for
716-
user-defined `@return ($x is Y ? A : B)` docblocks): for a short,
717-
explicit list of builtin functions, inspect the resolved argument type
718-
or literal value at the call site and pick the matching branch instead
719-
of returning the raw stub union. Start with `json_encode`/`json_decode`
720-
(`JSON_THROW_ON_ERROR` bit test) and the `preg_replace`/`str_replace`
721-
family (array-vs-string `$subject`), since those account for most of
722-
the volume found.
723-
724-
A case the condition can name outright (an argument's value, or whether it
725-
is an array or a string) needs no new mechanism: a conditional return type
726-
in `stub_patches.rs` covers it, as `range()` and `str_word_count()` do. The
727-
`json_encode`/`json_decode` flag test is the case that does, since a bit
728-
test on the flags argument is not something a condition can express.
694+
`pathinfo()` hands back an array of every component when its `$flags`
695+
argument is left at the default `PATHINFO_ALL`, and a plain string when
696+
the caller names one component. phpstorm-stubs declare the flat union, so
697+
both call shapes carry the branch the other one takes.
698+
699+
The mechanism this needs already exists, in the shape used for
700+
`json_encode`'s `JSON_THROW_ON_ERROR`
701+
(`type_engine/types/flag_returns.rs`): read the flags argument's text at
702+
the call site and pick a branch from it. A conditional return type in
703+
`stub_patches.rs` cannot express this one, because the deciding value
704+
arrives as a global constant (`PATHINFO_FILENAME`) and a condition can
705+
only name a literal value or a class constant.
706+
707+
**Fix:** add `pathinfo` to `flag_returns.rs`: the string branch when the
708+
flags argument names one of `PATHINFO_DIRNAME`, `PATHINFO_BASENAME`,
709+
`PATHINFO_EXTENSION` or `PATHINFO_FILENAME` (or an integer with a single
710+
bit set), and the array branch when the argument is left out. Anything
711+
else keeps the declared union. Left over from the work that took the
712+
`preg_replace`/`str_replace` family and `json_encode`, the two shapes
713+
that accounted for the bulk of the volume.

0 commit comments

Comments
 (0)