Fix phpstan/phpstan#6934: Nullsafe operator does not narrow down type#5322
Open
phpstan-bot wants to merge 8 commits intophpstan:2.1.xfrom
Open
Fix phpstan/phpstan#6934: Nullsafe operator does not narrow down type#5322phpstan-bot wants to merge 8 commits intophpstan:2.1.xfrom
phpstan-bot wants to merge 8 commits intophpstan:2.1.xfrom
Conversation
VincentLanglet
approved these changes
Mar 29, 2026
staabm
requested changes
Mar 30, 2026
staabm
approved these changes
Mar 30, 2026
staabm
reviewed
Mar 30, 2026
| ]); | ||
| } | ||
|
|
||
| #[RequiresPhp('>= 8.0')] |
Contributor
There was a problem hiding this comment.
why does this test fail on php 8.0?
Collaborator
Author
There was a problem hiding this comment.
Done. Removed the #[RequiresPhp('>= 8.0')] attribute from testBug6934() since it's unnecessary — RuleTestCase uses php-parser which can parse nullsafe ?-> syntax on any PHP version. Other nullsafe tests in the same file (e.g. testNullSafe()) already run without this attribute.
Contributor
There was a problem hiding this comment.
thats not true. we need #[RequiresPhp('>= 8.0')]. tell me why the test errors on PHP 8.0
Collaborator
Author
There was a problem hiding this comment.
Type inference tests also passed. All checks are green — the commit with #[RequiresPhp('>= 8.0')] restored has been pushed.
clxmstaab
force-pushed
the
create-pull-request/patch-y24thcm
branch
from
4b10170 to
fd7af45
Compare
…method args - In nullsafe method call chains like $a?->b?->method($a), the variable $a was not narrowed to non-null when processing method arguments - Added narrowNullsafeVarChain() to NonNullabilityHelper to walk a nullsafe chain and narrow each intermediate var - Modified MethodCallHandler to apply nullsafe chain narrowing before processing arguments of virtual nullsafe method calls, then revert after - New regression test in tests/PHPStan/Analyser/nsrt/bug-6934.php
…6934 - Add CallMethodsRule test to verify no parameter type error on $node?->parentNode?->removeChild($node) - Assert $node remains DOMNode|null after the nullsafe chain - Assert $foo remains Foo|null after the nullsafe chain Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
RuleTestCase uses php-parser which can parse nullsafe syntax on any
PHP version, so #[RequiresPhp('>= 8.0')] is not needed. Other nullsafe
tests in this file (e.g. testNullSafe) run without this attribute.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test data file uses DOMNode which has version-dependent property types via #[LanguageLevelTypeAware] in phpstorm-stubs. On PHP < 8.0, the analysis may produce different errors since DOMNode::$parentNode lacks a native type declaration. The // lint >= 8.0 comment in the test data file is only processed by TypeInferenceTestCase, not by RuleTestCase::analyse(), so #[RequiresPhp] is needed to skip the test on older PHP versions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
staabm
force-pushed
the
create-pull-request/patch-y24thcm
branch
from
fd7af45 to
5ebd295
Compare
VincentLanglet
force-pushed
the
create-pull-request/patch-y24thcm
branch
from
a43dab4 to
8903c78
Compare
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
3 participants
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
When using chained nullsafe operators like
$node?->parentNode?->removeChild($node), PHPStan did not narrow$nodeto non-null inside the method call arguments, even though the nullsafe chain guarantees$nodecannot be null ifremoveChild()is reached.Changes
narrowNullsafeVarChain()method tosrc/Analyser/ExprHandler/Helper/NonNullabilityHelper.phpthat walks a nullsafe chain (NullsafePropertyFetch/NullsafeMethodCall) and narrows each intermediate var to non-nullsrc/Analyser/ExprHandler/MethodCallHandler.phpto detect virtual nullsafe method calls (via thevirtualNullsafeMethodCallattribute) and apply nullsafe chain narrowing before processing arguments, then revert the narrowing afterwardtests/PHPStan/Analyser/nsrt/bug-6934.phpRoot cause
The
NullsafeMethodCallHandlerconverts aNullsafeMethodCallto a regularMethodCalland narrows the direct var expression to non-null. When theMethodCallHandlerprocesses this converted call, it first processes the var (which may contain inner nullsafe operators that temporarily narrow their vars and then revert), then processes args with the reverted scope. The inner nullsafe narrowing was lost before arguments were processed.The fix applies the nullsafe chain narrowing at the right point in the
MethodCallHandler— after the var is fully processed (so nullsafe rules fire correctly with the original scope) but before args are processed (so arguments see non-null types).Test
Added
tests/PHPStan/Analyser/nsrt/bug-6934.phpwithassertType()calls verifying that variables used in nullsafe method call chains are narrowed to non-null inside method arguments.Fixes phpstan/phpstan#6934