Skip to content

Fix #6120: Null coalesce non-null result implies the caller not to be null - not reflected in consequent conditions#4989

Closed
phpstan-bot wants to merge 1 commit into2.1.xfrom
create-pull-request/patch-qeu1t20
Closed

Fix #6120: Null coalesce non-null result implies the caller not to be null - not reflected in consequent conditions#4989
phpstan-bot wants to merge 1 commit into2.1.xfrom
create-pull-request/patch-qeu1t20

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

When a nullsafe property access or method call result is assigned to a variable (e.g., $result = $clazz?->foo) and then checked against null (if ($result !== null)), PHPStan did not narrow the caller ($clazz) to non-null. This fix adds that narrowing.

Changes

  • Added non-null/null conditional expression creation in src/Analyser/NodeScopeResolver.php (in processAssignVar), alongside the existing truthy/falsey conditional expressions
  • When the assigned type contains null and the non-null type differs from the truthy type, additional conditional expressions are created by simulating $assignedExpr !== null and $assignedExpr === null through the TypeSpecifier
  • Updated tests/PHPStan/Analyser/nsrt/bug-13546.php: the firstInCondition function now correctly narrows to non-empty-list<string> (was list<string>) and array{} (was list<string>) when checking array_key_first result against null
  • Updated tests/PHPStan/Analyser/nsrt/bug-6120.php: added test cases for nullsafe property access, nullable property access, and nullsafe method call narrowing through null checks
  • Removed a baseline entry in phpstan-baseline.neon for ClassReflection::getCacheKey() — the improved narrowing now correctly resolves the return type

Root cause

PHPStan's processAssignVar in NodeScopeResolver creates ConditionalExpressionHolder entries that link an assigned variable's type to narrowing of sub-expressions. Previously, only truthy/falsey conditional expressions were created. For $result = $clazz?->foo where foo is int, the truthy type was int<min, -1>|int<1, max> (non-zero int). When checking $result !== null, the narrowed type of $result is int, which didn't match the truthy condition (int<min, -1>|int<1, max>), so the conditional narrowing of $clazz never fired.

The fix adds separate conditional expressions specifically for the non-null type (TypeCombinator::removeNull($type)) and null type, so that !== null checks now trigger the appropriate narrowing.

Test

Added regression test in tests/PHPStan/Analyser/nsrt/bug-6120.php covering:

  • Nullsafe property access with non-nullable property ($clazz?->foo where foo is int)
  • Nullsafe property access with nullable property ($clazz?->nullableProperty where property is ?string)
  • Nullsafe method call ($clazz?->getFoo() where method returns int)
  • Both !== null and === null branches

Fixes phpstan/phpstan#6120

- When assigning from a nullsafe expression (e.g. $result = $clazz?->foo),
  checking $result !== null now correctly narrows $clazz to non-null
- Added non-null/null conditional expressions in processAssignVar alongside
  the existing truthy/falsey ones, so !== null checks trigger narrowing
- Updated bug-13546 test: array_key_first assignment narrowing now correctly
  narrows to non-empty-list/array{} when checking result !== null
- Removed baseline entry for ClassReflection::getCacheKey() which is now
  correctly resolved by the improved narrowing
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.

2 participants