Use dedicated Type methods over isSuperTypeOf() #2772
Conversation
| @@ -281,7 +281,7 @@ public function testVariableCertaintyInIsset(): void | |||
| 116, | |||
| ], | |||
| [ | |||
| 'Variable $variableInSecondCase in isset() always exists and is always null.', | |||
| 'Variable $variableInSecondCase in isset() always exists and is not nullable.', | |||
There was a problem hiding this comment.
this one is actually interessting, as I think it actually fixes a bug - which is suprising.
There was a problem hiding this comment.
having another look at the test-case I think our assertions could be more precise:
phpstan-src/tests/PHPStan/Rules/Variables/data/variable-certainty-isset.php
Lines 108 to 124 in 3a5328b
in case 1 and 2 the case expression is always true and we are only assigning values. as the switch-case is constructed we will always run into break on line 114, which effectively means the cases below line 114 are dead code.
There was a problem hiding this comment.
found the reason for the change in behaviour:
$neverType->isNull() -> no
$nullType->isSuperTypeOf($neverType) -> yes
|
the doctrine error is valid: upstream fix in doctrine/orm#11085 |
| $type = $this->treatPhpDocTypesAsCertain ? $scope->getType($expr) : $scope->getNativeType($expr); | ||
| if (!$type instanceof NeverType) { | ||
| return $this->generateError( | ||
| $type, | ||
| sprintf('Variable $%s %s always exists and', $expr->name, $operatorDescription), | ||
| $typeMessageCallback, | ||
| ); | ||
| } |
There was a problem hiding this comment.
this changes address the comments in #2772 (comment)
everything else in this PR was a mechnical change from
((new ConstantBooleanType(false))->isSuperTypeOf($type)->yes()) to $type->isFalse()->yes() etc.
|
the drupal error is caused by invalid parameter types: phpstan/phpstan#10189 therefore I think this is good to go. |
|
This pull request has been marked as ready for review. |
|
Thank you! |
while reviewing #2710 I noticed that we can simplify stuff by using dedicated Type methods