Do not remember readonly properties' constructor-narrowed types in other methods when clone with is available (PHP 8.5)#5893
Open
phpstan-bot wants to merge 1 commit into
Conversation
…her methods when `clone with` is available (PHP 8.5)
- `MutatingScope::rememberConstructorExpressions()` carries readonly property
fetches narrowed in the constructor into the scope of other methods. Since
PHP 8.5, `clone($obj, [...])` ("clone with") can reinitialize any readonly
property — even private ones, reinitialized by the declaring class — and the
modified instance escapes, so `$this` in another method is no longer
guaranteed to hold the constructor value. Skip remembering readonly property
fetches then, so they widen back to their declared type.
- The version check uses the scope-narrowable `Scope::getPhpVersion()` (new
`PhpVersions::supportsCloneWith()`), so the behavior respects an enclosing
`if (PHP_VERSION_ID >= 80500)` and is testable on older runtimes.
- Added `PhpVersions::supportsCloneWith()` (>= 8.0500).
- Regression test `nsrt/bug-14838.php` (the issue's clone-with reproducer).
- Updated existing remember-readonly tests (`bug-12902(.|-non-strict)`,
`remember-non-nullable-property-non-strict`,
`remember-readonly-constructor-narrowed(.|-hooks)`) to assert the widened
types under PHP 8.5 via `PHP_VERSION_ID` branches.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On PHP 8.5, the new
clone withsyntax (clone($obj, ['prop' => $value])) canreinitialize readonly properties. PHPStan reported a false positive
Strict comparison using !== between null and null will always evaluate to falsefor a readonly property that was assigned
nullin the constructor but laterreinitialized through
clone with:The fix stops remembering a readonly property's constructor-narrowed value in
other methods when
clone withis available, so the property widens back to itsdeclared type (
?inthere).Changes
src/Analyser/MutatingScope.php—rememberConstructorExpressions()no longercarries readonly
PropertyFetchtypes into other methods whengetPhpVersion()->supportsCloneWith()is notno(). The check uses thescope-narrowable
Scope::getPhpVersion()so it honors an enclosingif (PHP_VERSION_ID >= 80500).src/Php/PhpVersions.php— addedsupportsCloneWith(): TrinaryLogic(>= 8.5).tests/PHPStan/Analyser/nsrt/bug-14838.php— new regression test (the issue'sreproducer).
assert the widened types under PHP 8.5 via
PHP_VERSION_IDbranches:bug-12902.php,bug-12902-non-strict.php,remember-non-nullable-property-non-strict.php,remember-readonly-constructor-narrowed.php,remember-readonly-constructor-narrowed-hooks.php.Root cause
MutatingScope::rememberConstructorExpressions()keeps the constructor'snarrowed types of readonly property fetches (e.g.
null,4|10,class-string) and makes them visible in every other method, because before PHP8.5 a readonly property could not change after construction.
clone withbreaks that invariant:clone($obj, ['prop' => $value])reinitializes the property and produces a new instance. That instance can be
returned and have its methods called, so inside any instance method
$thismaybe a clone-modified object whose readonly property differs from the constructor
value. This is true even for private readonly properties, since the
declaring class itself can run
clone withon them. Remembering the constructorvalue is therefore unsound on PHP 8.5, and the fix widens readonly properties
back to their declared type there. Pre-8.5 behavior is unchanged.
Probed but intentionally out of scope
A
__clone()method that reinitializes a readonly property (allowed since PHP8.3) can produce the same kind of stale narrowing on 8.3/8.4. Fixing that
soundly would require either widening all readonly remembering on 8.3+
(a large precision regression — there are deliberate tests asserting narrowed
readonly values from 8.2 onward) or per-property analysis of
__clonebodies atremember-time (a much larger change). Since
clone withmakes the unsoundnessuniversal and unconditional on 8.5, this PR scopes the fix to 8.5, where the
trade-off is unambiguous. The sibling closure-capture and
method-call-invalidation paths for readonly properties were also checked and are
sound on 8.5 (they reason about the current
$this, whichclone withdoes notmutate).
Test
nsrt/bug-14838.php(// lint >= 8.5) reproduces the report: on PHP 8.5$this->durationInMinutesisint|null(notnull), so the comparison is nolonger reported as always-false.
remember-*/bug-12902*tests assert, viaif (PHP_VERSION_ID >= 80500) { … } else { … }, that readonly propertieswiden to their declared type on PHP 8.5 while keeping the narrowed type on
older versions. These branches are exercised and were confirmed to fail
without the source fix.
Fixes phpstan/phpstan#14838