Fix phpstan/phpstan#13876: Covariance: Error reported on L6, but not L10#5165
Closed
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Closed
Fix phpstan/phpstan#13876: Covariance: Error reported on L6, but not L10#5165phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
… off - At rule levels 3-7, RuleLevelHelper strips null from accepted type arguments - This caused Trap<int|null, A|null> to become Trap<int, A> on the accepted side - Invariant template check uses equals(), so int|null ≠ int → false positive - Added fallback in GenericObjectType::isSuperTypeOfInternal() to detect when the only difference is null stripping and accept the type in that case - New regression test in tests/PHPStan/Rules/Properties/data/bug-13876.php Closes phpstan/phpstan#13876
Contributor
|
It might require more work but I think the right fix would be this one #4210 |
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
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
Fix false positive error "Property B::$b (Trap<int|null, A|null>) does not accept Trap<int|null, A|null>" that occurred at rule levels 3-7 when assigning a generic object with nullable template arguments to a property with the same generic type.
Changes
src/Type/Generic/GenericObjectType.php: Added a fallback check inisSuperTypeOfInternal()for accepts context when invariant template variance check fails. If the only difference between the accepting and accepted type arguments is null (stripped byRuleLevelHelper::transformAcceptedType()), the type is accepted.$checkNullablesproperty totests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.phpto allow testing withcheckNullables = false.tests/PHPStan/Rules/Properties/data/bug-13876.php.Root cause
At rule levels 3-7,
RuleLevelHelper::transformAcceptedType()traverses the accepted type and removesnullviaTypeCombinator::removeNull(). This traversal descends into generic type arguments throughGenericObjectType::traverse(), transformingTrap<int|null, A|null>intoTrap<int, A>on the accepted side. The accepting type (property type) retains its nulls asTrap<int|null, A|null>.For invariant template parameters,
TemplateTypeVariance::isValidVariance()uses strictType::equals()checking. Sinceint|null ≠ intandA|null ≠ A, the check fails and a false positive is reported - even though both types are semantically identical before null stripping.The fix detects this situation in accepts context: when the accepting type argument is a supertype of the accepted argument AND the accepted argument is a supertype of the accepting argument with null removed, the types differ only by null stripping and should be accepted.
Test
Added
testBug13876()inTypesAssignedToPropertiesRuleTestthat setscheckNullables = falseand verifies no errors are reported when assigningTrap<int|null, A|null>to a property of typeTrap<int|null, A|null>.Fixes phpstan/phpstan#13876