Skip to content

[Php74] Fix isset guard never converting on IfToNullCoalescingAssignRector - #8389

Merged
TomasVotruba merged 1 commit into
rectorphp:mainfrom
Soean:fix-null-coalescing-assign-isset-narrowed-type
Aug 28, 2026
Merged

[Php74] Fix isset guard never converting on IfToNullCoalescingAssignRector#8389
TomasVotruba merged 1 commit into
rectorphp:mainfrom
Soean:fix-null-coalescing-assign-isset-narrowed-type

Conversation

@Soean

@Soean Soean commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

The non-nullable property skip from #8387 reads the type off the fetch node returned by matchNullGuardedExpr(). For the ! isset($prop) form that node sits inside Isset_, where PHPStan has already narrowed null out of the type. TypeCombinator::containsNull() is therefore always false, isNonNullableProperty() always returns true, and the skip fires for every property under isset, nullable or not.

Before (same nullable property, three guard forms)

class Demo
{
    private ?array $values = null;

    public function get(): array
    {
        // skipped, but should convert
        if (! isset($this->values)) {
            $this->values = ['default'];
        }

        // converts
        if (null === $this->values) {
            $this->values = ['default'];
        }

        // converts
        if (is_null($this->values)) {
            $this->values = ['default'];
        }

        return $this->values;
    }
}

Only the isset form is affected, so it was silently disabled for properties, including the case the rule advertises in its CodeSample.

Fix

Resolve the declared type through ReflectionResolver::resolvePropertyReflectionFromPropertyFetch() instead of nodeTypeResolver->getType() on the fetch node. getReadableType() stays phpdoc-aware like the previous call, but carries no scope narrowing.

After

// nullable property, isset form
$this->values ??= ['default'];

skip_typed_non_nullable_property.php.inc keeps skipping, so the boundary #8387 intended is unchanged, the two fixtures now pin it from both sides.

@Soean
Soean marked this pull request as ready for review August 27, 2026 12:35

@samsonasik samsonasik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@TomasVotruba
TomasVotruba merged commit 10120e0 into rectorphp:main Aug 28, 2026
44 checks passed
@TomasVotruba

Copy link
Copy Markdown
Member

Thank you

@Soean
Soean deleted the fix-null-coalescing-assign-isset-narrowed-type branch August 28, 2026 06:47
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.

3 participants