Skip to content

[DeadCode] Skip early return in constructor in RemoveDefaultValueFromAssignedPropertyRector#8209

Merged
TomasVotruba merged 1 commit into
mainfrom
fix-early-return-default-value
Jul 27, 2026
Merged

[DeadCode] Skip early return in constructor in RemoveDefaultValueFromAssignedPropertyRector#8209
TomasVotruba merged 1 commit into
mainfrom
fix-early-return-default-value

Conversation

@TomasVotruba

@TomasVotruba TomasVotruba commented Jul 27, 2026

Copy link
Copy Markdown
Member

The rule removed the default value whenever a first-level $this->prop = ... was found in the constructor. But an early return can skip that assign, leaving a typed property uninitialized:

class Foo {
    public int $x = 0;
    public function __construct(bool $return = false) {
        if ($return) {
            return;
        }
        $this->x = 0;
    }
}

print (new Foo())->x . ';' . (new Foo(true))->x;

Before this fix, the rule produced:

 class Foo {
-    public int $x = 0;
+    public int $x;
     public function __construct(bool $return = false) {

which turns (new Foo(true))->x into Error: Typed property Foo::$x must not be accessed before initialization.

Now any return in the constructor body makes the rule bail out. Returns inside nested closures/arrow functions/anonymous classes are ignored, as they do not skip the constructor assign:

final class ReturnInClosure
{
    private ?string $name = null; // still changed to `private ?string $name;`

    public function __construct(string $name)
    {
        $this->name = (function () use ($name): string {
            return $name;
        })();
    }
}

@TomasVotruba
TomasVotruba force-pushed the fix-early-return-default-value branch from 42b98e6 to 45e7ee3 Compare July 27, 2026 08:39
@TomasVotruba
TomasVotruba merged commit 4853fdb into main Jul 27, 2026
65 checks passed
@TomasVotruba
TomasVotruba deleted the fix-early-return-default-value branch July 27, 2026 08:41
@staabm

staabm commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

properties might also be initialized from methods which are considered 'additionalConstructors'.

I think most robust would be to base this rule on the PHPStan builtin mechanics which already can identify uninitialized properties, see e.g. https://github.com/phpstan/phpstan-src/blob/d7e9e6b1a05d4c6bdefc771f4cf3b12122daa1ad/src/Rules/Properties/MissingReadOnlyPropertyAssignRule.php

@TomasVotruba

Copy link
Copy Markdown
Member Author

I know about the rule, but it's very complex. I wish there was single re-usable service.

Asking agents if they can do something perf-wise about it.

@staabm

staabm commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

there is $node->getUninitializedProperties() right in the code I quoted above which is part of ClassPropertiesNode which is public @api

see https://github.com/phpstan/phpstan-src/blob/d7e9e6b1a05d4c6bdefc771f4cf3b12122daa1ad/src/Rules/Properties/MissingReadOnlyPropertyAssignRule.php#L35

@TomasVotruba

Copy link
Copy Markdown
Member Author

That's not a printable node we can use, only a wrapper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants