diff --git a/conf/config.level1.neon b/conf/config.level1.neon index 2499484180..99006e0133 100644 --- a/conf/config.level1.neon +++ b/conf/config.level1.neon @@ -7,17 +7,15 @@ parameters: reportMagicMethods: true reportMagicProperties: true - -conditionalTags: - PHPStan\Rules\Variables\VariableCertaintyNullCoalesceRule: - phpstan.rules.rule: %featureToggles.nullCoalesce% - rules: - PHPStan\Rules\Classes\UnusedConstructorParametersRule - PHPStan\Rules\Constants\ConstantRule - PHPStan\Rules\Functions\UnusedClosureUsesRule - - PHPStan\Rules\Variables\VariableCertaintyInIssetRule services: - - class: PHPStan\Rules\Variables\VariableCertaintyNullCoalesceRule + class: PHPStan\Rules\Variables\VariableCertaintyInIssetRule + arguments: + bleedingEdge: %featureToggles.bleedingEdge% + tags: + - phpstan.rules.rule diff --git a/conf/config.neon b/conf/config.neon index 3c183f59bf..6236f4d147 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -859,6 +859,8 @@ services: - class: PHPStan\Rules\IssetCheck + arguments: + bleedingEdge: %featureToggles.bleedingEdge% - # checked as part of OverridingMethodRule diff --git a/src/Rules/IssetCheck.php b/src/Rules/IssetCheck.php index e737ee9cc7..47e44e49d3 100644 --- a/src/Rules/IssetCheck.php +++ b/src/Rules/IssetCheck.php @@ -18,13 +18,17 @@ class IssetCheck private \PHPStan\Rules\Properties\PropertyReflectionFinder $propertyReflectionFinder; + private bool $bleedingEdge; + public function __construct( PropertyDescriptor $propertyDescriptor, - PropertyReflectionFinder $propertyReflectionFinder + PropertyReflectionFinder $propertyReflectionFinder, + bool $bleedingEdge = false ) { $this->propertyDescriptor = $propertyDescriptor; $this->propertyReflectionFinder = $propertyReflectionFinder; + $this->bleedingEdge = $bleedingEdge; } /** @@ -38,6 +42,25 @@ public function check(Expr $expr, Scope $scope, string $operatorDescription, cal return null; } + if ( + $error === null + && $this->bleedingEdge + ) { + if ($hasVariable->yes()) { + if ($expr->name === '_SESSION') { + return null; + } + + return $this->generateError( + $scope->getVariableType($expr->name), + sprintf('Variable $%s %s always exists and', $expr->name, $operatorDescription), + $typeMessageCallback + ); + } + + return RuleErrorBuilder::message(sprintf('Variable $%s %s is never defined.', $expr->name, $operatorDescription))->build(); + } + return $error; } elseif ($expr instanceof Node\Expr\ArrayDimFetch && $expr->dim !== null) { diff --git a/src/Rules/Variables/EmptyRule.php b/src/Rules/Variables/EmptyRule.php index ba0911ac44..ab9ce3c38d 100644 --- a/src/Rules/Variables/EmptyRule.php +++ b/src/Rules/Variables/EmptyRule.php @@ -6,7 +6,6 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\IssetCheck; use PHPStan\Type\Constant\ConstantBooleanType; -use PHPStan\Type\ErrorType; use PHPStan\Type\NullType; use PHPStan\Type\Type; @@ -61,14 +60,8 @@ public function processNode(Node $node, Scope $scope): array return 'is not nullable'; }); - if ($error === null) { - return []; - } - $exprType = $scope->getType($node->expr); - $exprBooleanType = $exprType->toBoolean(); - $isFalse = (new ConstantBooleanType(false))->isSuperTypeOf($exprBooleanType); - if (!$exprType instanceof ErrorType && $isFalse->maybe()) { + if ($error === null) { return []; } diff --git a/src/Rules/Variables/VariableCertaintyInIssetRule.php b/src/Rules/Variables/VariableCertaintyInIssetRule.php index 242951798d..d8226c4a74 100644 --- a/src/Rules/Variables/VariableCertaintyInIssetRule.php +++ b/src/Rules/Variables/VariableCertaintyInIssetRule.php @@ -13,6 +13,13 @@ class VariableCertaintyInIssetRule implements \PHPStan\Rules\Rule { + private bool $bleedingEdge; + + public function __construct(bool $bleedingEdge = false) + { + $this->bleedingEdge = $bleedingEdge; + } + public function getNodeType(): string { return Node\Expr\Isset_::class; @@ -20,6 +27,10 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { + if ($this->bleedingEdge) { + return []; + } + $messages = []; foreach ($node->vars as $var) { $isSubNode = false; diff --git a/src/Rules/Variables/VariableCertaintyNullCoalesceRule.php b/src/Rules/Variables/VariableCertaintyNullCoalesceRule.php deleted file mode 100644 index 5a8b9e1323..0000000000 --- a/src/Rules/Variables/VariableCertaintyNullCoalesceRule.php +++ /dev/null @@ -1,81 +0,0 @@ - - */ -class VariableCertaintyNullCoalesceRule implements \PHPStan\Rules\Rule -{ - - public function getNodeType(): string - { - return Node\Expr::class; - } - - public function processNode(Node $node, Scope $scope): array - { - if ($node instanceof Node\Expr\AssignOp\Coalesce) { - $var = $node->var; - $description = '??='; - } elseif ($node instanceof Node\Expr\BinaryOp\Coalesce) { - $var = $node->left; - $description = '??'; - } else { - return []; - } - - $isSubNode = false; - while ( - $var instanceof Node\Expr\ArrayDimFetch - || $var instanceof Node\Expr\PropertyFetch - || ( - $var instanceof Node\Expr\StaticPropertyFetch - && $var->class instanceof Node\Expr - ) - ) { - if ($var instanceof Node\Expr\StaticPropertyFetch) { - $var = $var->class; - } else { - $var = $var->var; - } - $isSubNode = true; - } - - if (!$var instanceof Node\Expr\Variable || !is_string($var->name)) { - return []; - } - - $certainty = $scope->hasVariableType($var->name); - if ($certainty->no()) { - return [RuleErrorBuilder::message(sprintf( - 'Variable $%s on left side of %s is never defined.', - $var->name, - $description - ))->build()]; - } elseif ($certainty->yes() && !$isSubNode) { - $variableType = $scope->getVariableType($var->name); - if ($variableType->isSuperTypeOf(new NullType())->no()) { - return [RuleErrorBuilder::message(sprintf( - 'Variable $%s on left side of %s always exists and is not nullable.', - $var->name, - $description - ))->build()]; - } elseif ((new NullType())->isSuperTypeOf($variableType)->yes()) { - return [RuleErrorBuilder::message(sprintf( - 'Variable $%s on left side of %s is always null.', - $var->name, - $description - ))->build()]; - } - } - - return []; - } - -} diff --git a/tests/PHPStan/Levels/data/coalesce.php b/tests/PHPStan/Levels/data/coalesce.php index 6010637b0a..fffbf433ca 100644 --- a/tests/PHPStan/Levels/data/coalesce.php +++ b/tests/PHPStan/Levels/data/coalesce.php @@ -10,3 +10,15 @@ function (\ReflectionClass $ref): void { echo $ref->name ?? 'foo'; echo $ref->nonexistent ?? 'bar'; }; + +function (?string $s): void { + echo $a ?? 'foo'; + + echo $s ?? 'bar'; + + if ($s !== null) { + return; + } + + echo $s ?? 'bar'; +}; diff --git a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php index af2206ea2d..bfd6cdefdb 100644 --- a/tests/PHPStan/Rules/Variables/EmptyRuleTest.php +++ b/tests/PHPStan/Rules/Variables/EmptyRuleTest.php @@ -15,7 +15,7 @@ class EmptyRuleTest extends RuleTestCase protected function getRule(): \PHPStan\Rules\Rule { - return new EmptyRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder())); + return new EmptyRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder(), true)); } public function testRule(): void @@ -45,6 +45,24 @@ public function testRule(): void 'Offset 2 on array(\'\', \'0\', \'foo\', \'\'|\'foo\') in empty() always exists and is not falsy.', 38, ], + [ + 'Variable $a in empty() is never defined.', + 44, + ], + [ + 'Variable $b in empty() always exists and is not falsy.', + 47, + ], + ]); + } + + public function testBug970(): void + { + $this->analyse([__DIR__ . '/data/bug-970.php'], [ + [ + 'aaa', + 10, + ], ]); } diff --git a/tests/PHPStan/Rules/Variables/IssetRuleTest.php b/tests/PHPStan/Rules/Variables/IssetRuleTest.php index 7e4ee5150a..5c20822123 100644 --- a/tests/PHPStan/Rules/Variables/IssetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/IssetRuleTest.php @@ -16,7 +16,7 @@ class IssetRuleTest extends RuleTestCase protected function getRule(): Rule { - return new IssetRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder())); + return new IssetRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder(), true)); } public function testRule(): void @@ -26,6 +26,10 @@ public function testRule(): void 'Property IssetRule\FooCoalesce::$string (string) in isset() is not nullable.', 32, ], + [ + 'Variable $scalar in isset() always exists and is not nullable.', + 41, + ], [ 'Offset \'string\' on array(1, 2, 3) in isset() does not exist.', 45, @@ -34,6 +38,10 @@ public function testRule(): void 'Offset \'string\' on array(array(1), array(2), array(3)) in isset() does not exist.', 49, ], + [ + 'Variable $doesNotExist in isset() is never defined.', + 51, + ], [ 'Offset \'dim\' on array(\'dim\' => 1, \'dim-null\' => 1|null, \'dim-null-offset\' => array(\'a\' => true|null), \'dim-empty\' => array()) in isset() always exists and is not nullable.', 67, @@ -62,6 +70,10 @@ public function testRule(): void 'Static property IssetRule\FooCoalesce::$staticAlwaysNull (null) in isset() is always null.', 97, ], + [ + 'Variable $a in isset() always exists and is always null.', + 111, + ], [ 'Property IssetRule\FooCoalesce::$string (string) in isset() is not nullable.', 116, @@ -114,4 +126,97 @@ public function testBug4671(): void ]]); } + public function testVariableCertaintyInIsset(): void + { + $this->analyse([__DIR__ . '/data/variable-certainty-isset.php'], [ + [ + 'Variable $alwaysDefinedNotNullable in isset() always exists and is not nullable.', + 14, + ], + [ + 'Variable $neverDefinedVariable in isset() is never defined.', + 22, + ], + [ + 'Variable $anotherNeverDefinedVariable in isset() is never defined.', + 42, + ], + [ + 'Variable $yetAnotherNeverDefinedVariable in isset() is never defined.', + 46, + ], + [ + 'Variable $yetYetAnotherNeverDefinedVariableInIsset in isset() is never defined.', + 56, + ], + [ + 'Variable $anotherVariableInDoWhile in isset() always exists and is not nullable.', + 104, + ], + [ + 'Variable $variableInSecondCase in isset() is never defined.', + 110, + ], + [ + 'Variable $variableInFirstCase in isset() always exists and is not nullable.', + 112, + ], + [ + 'Variable $variableInFirstCase in isset() always exists and is not nullable.', + 116, + ], + [ + 'Variable $variableInSecondCase in isset() always exists and is not nullable.', + 117, + ], + [ + 'Variable $variableAssignedInSecondCase in isset() is never defined.', + 119, + ], + [ + 'Variable $alwaysDefinedForSwitchCondition in isset() always exists and is not nullable.', + 139, + ], + [ + 'Variable $alwaysDefinedForCaseNodeCondition in isset() always exists and is not nullable.', + 140, + ], + [ + 'Variable $alwaysDefinedNotNullable in isset() always exists and is not nullable.', + 152, + ], + [ + 'Variable $neverDefinedVariable in isset() is never defined.', + 152, + ], + [ + 'Variable $a in isset() always exists and is not nullable.', + 214, + ], + [ + 'Variable $null in isset() always exists and is always null.', + 225, + ], + ]); + } + + public function testIssetInGlobalScope(): void + { + $this->analyse([__DIR__ . '/data/isset-global-scope.php'], [ + [ + 'Variable $alwaysDefinedNotNullable in isset() always exists and is not nullable.', + 8, + ], + ]); + } + + public function testNullsafe(): void + { + if (PHP_VERSION_ID < 80000 && !self::$useStaticReflectionProvider) { + $this->markTestSkipped('Test requires PHP 8.0.'); + } + + $this->analyse([__DIR__ . '/data/isset-nullsafe.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php index 681ee02a24..33636a032c 100644 --- a/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php +++ b/tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php @@ -14,7 +14,7 @@ class NullCoalesceRuleTest extends \PHPStan\Testing\RuleTestCase protected function getRule(): \PHPStan\Rules\Rule { - return new NullCoalesceRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder())); + return new NullCoalesceRule(new IssetCheck(new PropertyDescriptor(), new PropertyReflectionFinder(), true)); } public function testCoalesceRule(): void @@ -24,6 +24,10 @@ public function testCoalesceRule(): void 'Property CoalesceRule\FooCoalesce::$string (string) on left side of ?? is not nullable.', 32, ], + [ + 'Variable $scalar on left side of ?? always exists and is not nullable.', + 41, + ], [ 'Offset \'string\' on array(1, 2, 3) on left side of ?? does not exist.', 45, @@ -32,6 +36,10 @@ public function testCoalesceRule(): void 'Offset \'string\' on array(array(1), array(2), array(3)) on left side of ?? does not exist.', 49, ], + [ + 'Variable $doesNotExist on left side of ?? is never defined.', + 51, + ], [ 'Offset \'dim\' on array(\'dim\' => 1, \'dim-null\' => 1|null, \'dim-null-offset\' => array(\'a\' => true|null), \'dim-empty\' => array()) on left side of ?? always exists and is not nullable.', 67, @@ -64,6 +72,10 @@ public function testCoalesceRule(): void 'Static property CoalesceRule\FooCoalesce::$staticAlwaysNull (null) on left side of ?? is always null.', 101, ], + [ + 'Variable $a on left side of ?? always exists and is always null.', + 115, + ], [ 'Property CoalesceRule\FooCoalesce::$string (string) on left side of ?? is not nullable.', 120, @@ -106,6 +118,10 @@ public function testCoalesceAssignRule(): void 'Property CoalesceAssignRule\FooCoalesce::$string (string) on left side of ??= is not nullable.', 32, ], + [ + 'Variable $scalar on left side of ??= always exists and is not nullable.', + 41, + ], [ 'Offset \'string\' on array(1, 2, 3) on left side of ??= does not exist.', 45, @@ -114,6 +130,10 @@ public function testCoalesceAssignRule(): void 'Offset \'string\' on array(array(1), array(2), array(3)) on left side of ??= does not exist.', 49, ], + [ + 'Variable $doesNotExist on left side of ??= is never defined.', + 51, + ], [ 'Offset \'dim\' on array(\'dim\' => 1, \'dim-null\' => 1|null, \'dim-null-offset\' => array(\'a\' => true|null), \'dim-empty\' => array()) on left side of ??= always exists and is not nullable.', 67, @@ -142,6 +162,10 @@ public function testCoalesceAssignRule(): void 'Static property CoalesceAssignRule\FooCoalesce::$staticAlwaysNull (null) on left side of ??= is always null.', 101, ], + [ + 'Variable $a on left side of ??= always exists and is always null.', + 115, + ], ]); } @@ -154,4 +178,54 @@ public function testNullsafe(): void $this->analyse([__DIR__ . '/data/null-coalesce-nullsafe.php'], []); } + public function testVariableCertaintyInNullCoalesce(): void + { + $this->analyse([__DIR__ . '/data/variable-certainty-null.php'], [ + [ + 'Variable $scalar on left side of ?? always exists and is not nullable.', + 6, + ], + [ + 'Variable $doesNotExist on left side of ?? is never defined.', + 8, + ], + [ + 'Variable $a on left side of ?? always exists and is always null.', + 13, + ], + ]); + } + + public function testVariableCertaintyInNullCoalesceAssign(): void + { + if (!self::$useStaticReflectionProvider && PHP_VERSION_ID < 70400) { + $this->markTestSkipped('Test requires PHP 7.4.'); + } + + $this->analyse([__DIR__ . '/data/variable-certainty-null-assign.php'], [ + [ + 'Variable $scalar on left side of ??= always exists and is not nullable.', + 6, + ], + [ + 'Variable $doesNotExist on left side of ??= is never defined.', + 8, + ], + [ + 'Variable $a on left side of ??= always exists and is always null.', + 13, + ], + ]); + } + + public function testNullCoalesceInGlobalScope(): void + { + $this->analyse([__DIR__ . '/data/null-coalesce-global-scope.php'], [ + [ + 'Variable $bar on left side of ?? always exists and is not nullable.', + 6, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/VariableCertaintyNullCoalesceRuleTest.php b/tests/PHPStan/Rules/Variables/VariableCertaintyNullCoalesceRuleTest.php deleted file mode 100644 index c9c7fe7b22..0000000000 --- a/tests/PHPStan/Rules/Variables/VariableCertaintyNullCoalesceRuleTest.php +++ /dev/null @@ -1,66 +0,0 @@ - - */ -class VariableCertaintyNullCoalesceRuleTest extends \PHPStan\Testing\RuleTestCase -{ - - protected function getRule(): \PHPStan\Rules\Rule - { - return new VariableCertaintyNullCoalesceRule(); - } - - public function testVariableCertaintyInNullCoalesce(): void - { - $this->analyse([__DIR__ . '/data/variable-certainty-null.php'], [ - [ - 'Variable $scalar on left side of ?? always exists and is not nullable.', - 6, - ], - [ - 'Variable $doesNotExist on left side of ?? is never defined.', - 8, - ], - [ - 'Variable $a on left side of ?? is always null.', - 13, - ], - ]); - } - - public function testVariableCertaintyInNullCoalesceAssign(): void - { - if (!self::$useStaticReflectionProvider && PHP_VERSION_ID < 70400) { - $this->markTestSkipped('Test requires PHP 7.4.'); - } - - $this->analyse([__DIR__ . '/data/variable-certainty-null-assign.php'], [ - [ - 'Variable $scalar on left side of ??= always exists and is not nullable.', - 6, - ], - [ - 'Variable $doesNotExist on left side of ??= is never defined.', - 8, - ], - [ - 'Variable $a on left side of ??= is always null.', - 13, - ], - ]); - } - - public function testNullCoalesceInGlobalScope(): void - { - $this->analyse([__DIR__ . '/data/null-coalesce-global-scope.php'], [ - [ - 'Variable $bar on left side of ?? always exists and is not nullable.', - 6, - ], - ]); - } - -} diff --git a/tests/PHPStan/Rules/Variables/data/bug-970.php b/tests/PHPStan/Rules/Variables/data/bug-970.php new file mode 100644 index 0000000000..c1b988f68e --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-970.php @@ -0,0 +1,14 @@ +