Bleeding edge - teach IssetRule everything what VariableCertaintyInIs…

…setRule does
phpstan · Sep 10, 2021 · 9689fbd · 9689fbd
1 parent 601460c
commit 9689fbd
Show file tree

Hide file tree

Showing 13 changed files with 286 additions and 166 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -13,13 +13,24 @@
 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;
 	}
 
 	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
diff --git 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
@@ -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
@@ -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'], []);
+	}
+
 }