Skip to content

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
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-xngd077
Open

Do not remember readonly properties' constructor-narrowed types in other methods when clone with is available (PHP 8.5)#5893
phpstan-bot wants to merge 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-xngd077

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

On PHP 8.5, the new clone with syntax (clone($obj, ['prop' => $value])) can
reinitialize readonly properties. PHPStan reported a false positive
Strict comparison using !== between null and null will always evaluate to false
for a readonly property that was assigned null in the constructor but later
reinitialized through clone with:

readonly class CoffeeBreak
{
    public ?int $durationInMinutes;

    public function __construct(public string $name) {
        $this->durationInMinutes = null;
    }

    public function setDuration(): self {
        return clone($this, ['durationInMinutes' => 15]);
    }

    public function hasDuration(): bool {
        return $this->durationInMinutes !== null; // wrongly reported as always false
    }
}

The fix stops remembering a readonly property's constructor-narrowed value in
other methods when clone with is available, so the property widens back to its
declared type (?int here).

Changes

  • src/Analyser/MutatingScope.phprememberConstructorExpressions() no longer
    carries readonly PropertyFetch types into other methods when
    getPhpVersion()->supportsCloneWith() is not no(). The check uses the
    scope-narrowable Scope::getPhpVersion() so it honors an enclosing
    if (PHP_VERSION_ID >= 80500).
  • src/Php/PhpVersions.php — added supportsCloneWith(): TrinaryLogic (>= 8.5).
  • tests/PHPStan/Analyser/nsrt/bug-14838.php — new regression test (the issue's
    reproducer).
  • Updated existing tests that relied on readonly constructor remembering to
    assert the widened types under PHP 8.5 via PHP_VERSION_ID branches:
    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's
narrowed types of readonly property fetches (e.g. null, 4|10,
class-string) and makes them visible in every other method, because before PHP
8.5 a readonly property could not change after construction.

clone with breaks 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 $this may
be 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 with on them. Remembering the constructor
value 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 PHP
8.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 __clone bodies at
remember-time (a much larger change). Since clone with makes the unsoundness
universal 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, which clone with does not
mutate).

Test

  • nsrt/bug-14838.php (// lint >= 8.5) reproduces the report: on PHP 8.5
    $this->durationInMinutes is int|null (not null), so the comparison is no
    longer reported as always-false.
  • The updated remember-* / bug-12902* tests assert, via
    if (PHP_VERSION_ID >= 80500) { … } else { … }, that readonly properties
    widen 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

…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

false-positive error when read-only object is modified through php8.5 clone with

1 participant