Make isset() and null-coalesce (??) operators consistent with each other

* add regression tests for property access with dynamic properties related issues * allow undefined expression for `ObjectWithoutClassType` property fetch * fix test for solved issue * make isset and coalesce consistent * allow overwriting allowedUndefinedExpression because not needed anymore * add regression tests for fixed issues * cs fix * fix levels test for coalesce
phpstan · Apr 20, 2022 · 070b0b2 · 070b0b2
1 parent c12cf94
commit 070b0b2
Show file tree

Hide file tree

Showing 10 changed files with 122 additions and 45 deletions.
diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php
@@ -3909,9 +3909,6 @@ public function isInExpressionAssign(Expr $expr): bool
 	public function setAllowedUndefinedExpression(Expr $expr, bool $isAllowed): self
 	{
 		$exprString = $this->getNodeKey($expr);
-		if (array_key_exists($exprString, $this->currentlyAllowedUndefinedExpressions)) {
-			return $this;
-		}
 		$currentlyAllowedUndefinedExpressions = $this->currentlyAllowedUndefinedExpressions;
 		$currentlyAllowedUndefinedExpressions[$exprString] = $isAllowed;
 

diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php
@@ -2320,14 +2320,8 @@ static function (?Type $offsetType, Type $valueType) use (&$arrayType): void {
 				static fn (): MutatingScope => $rightResult->getScope()->filterByFalseyValue($expr),
 			);
 		} elseif ($expr instanceof Coalesce) {
-			// backwards compatibility: coalesce doesn't allow dynamic properties
 			$nonNullabilityResult = $this->ensureNonNullability($scope, $expr->left, false);
-			if ($expr->left instanceof PropertyFetch || $expr->left instanceof Expr\NullsafePropertyFetch || $expr->left instanceof StaticPropertyFetch) {
-				$condScope = $nonNullabilityResult->getScope()->setAllowedUndefinedExpression($expr->left, false);
-			} else {
-				$condScope = $nonNullabilityResult->getScope();
-			}
-			$condScope = $this->lookForEnterAllowedUndefinedVariable($condScope, $expr->left, true);
+			$condScope = $this->lookForEnterAllowedUndefinedVariable($nonNullabilityResult->getScope(), $expr->left, true);
 			$condResult = $this->processExprNode($expr->left, $condScope, $nodeCallback, $context->enterDeep());
 			$scope = $this->revertNonNullability($condResult->getScope(), $nonNullabilityResult->getSpecifiedExpressions());
 			$scope = $this->lookForExitAllowedUndefinedVariable($scope, $expr->left);

diff --git a/tests/PHPStan/Levels/data/coalesce-2.json b/tests/PHPStan/Levels/data/coalesce-2.json
diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php
@@ -129,18 +129,6 @@ public function testAccessProperties(): void
 					'Access to an undefined property TestAccessProperties\FooAccessProperties::$dolor.',
 					250,
 				],
-				[
-					'Access to an undefined property TestAccessProperties\NullCoalesce::$bar.',
-					264,
-				],
-				[
-					'Access to an undefined property TestAccessProperties\NullCoalesce::$bar.',
-					266,
-				],
-				[
-					'Access to an undefined property TestAccessProperties\NullCoalesce::$bar.',
-					270,
-				],
 				[
 					'Cannot access property $bar on TestAccessProperties\NullCoalesce|null.',
 					272,
@@ -271,18 +259,6 @@ public function testAccessPropertiesWithoutUnionTypes(): void
 					'Access to an undefined property TestAccessProperties\FooAccessProperties::$dolor.',
 					250,
 				],
-				[
-					'Access to an undefined property TestAccessProperties\NullCoalesce::$bar.',
-					264,
-				],
-				[
-					'Access to an undefined property TestAccessProperties\NullCoalesce::$bar.',
-					266,
-				],
-				[
-					'Access to an undefined property TestAccessProperties\NullCoalesce::$bar.',
-					270,
-				],
 				[
 					'Cannot access property $bar on TestAccessProperties\NullCoalesce|null.',
 					272,
@@ -570,11 +546,36 @@ public function testBug6899(): void
 				'Cannot access property $prop on string.',
 				15,
 			],
-			[
-				'Cannot access property $prop on object|string.', // open issue https://github.com/phpstan/phpstan/issues/3659, https://github.com/phpstan/phpstan/issues/6026
-				25,
-			],
 		]);
 	}
 
+	public function testBug6026(): void
+	{
+		$this->checkThisOnly = false;
+		$this->checkUnionTypes = true;
+		$this->analyse([__DIR__ . '/data/bug-6026.php'], []);
+	}
+
+	public function testBug3659(): void
+	{
+		$this->checkThisOnly = false;
+		$this->checkUnionTypes = true;
+		$this->analyse([__DIR__ . '/data/bug-3659.php'], []);
+	}
+
+	public function testDynamicProperties(): void
+	{
+		$this->checkThisOnly = false;
+		$this->checkUnionTypes = true;
+		$this->analyse([__DIR__ . '/data/dynamic-properties.php'], []);
+	}
+
+
+	public function testBug4559(): void
+	{
+		$this->checkThisOnly = false;
+		$this->checkUnionTypes = true;
+		$this->analyse([__DIR__ . '/data/bug-4559.php'], []);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Properties/AccessStaticPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessStaticPropertiesRuleTest.php
@@ -207,4 +207,9 @@ public function testBug5143(): void
 		$this->analyse([__DIR__ . '/data/bug-5143.php'], []);
 	}
 
+	public function testBug6809(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-6809.php'], []);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Properties/data/bug-3659.php b/tests/PHPStan/Rules/Properties/data/bug-3659.php
@@ -0,0 +1,17 @@
+<?php
+
+namespace Bug3659;
+
+class Foo
+{
+	public function func1(object $obj): void
+	{
+		$this->func2($obj->someProperty ?? null);
+	}
+
+	public function func2(?string $param): void
+	{
+		echo $param ?? 'test';
+	}
+}
+
diff --git a/tests/PHPStan/Rules/Properties/data/bug-4559.php b/tests/PHPStan/Rules/Properties/data/bug-4559.php
@@ -0,0 +1,14 @@
+<?php declare(strict_types = 1);
+
+namespace Bug4559;
+
+class HelloWorld
+{
+	public function doBar()
+	{
+		$response = json_decode('');
+		if (isset($response->error->code)) {
+			echo $response->error->message ?? '';
+		}
+	}
+}
diff --git a/tests/PHPStan/Rules/Properties/data/bug-6026.php b/tests/PHPStan/Rules/Properties/data/bug-6026.php
@@ -0,0 +1,23 @@
+<?php
+
+namespace Bug6026;
+
+class Foo {
+	/**
+	 * @param resource $theResource
+	 * @return bool
+	 */
+	public function Process($theResource) : bool
+	{
+		$bucket = \stream_bucket_make_writeable($theResource);
+		if ( $bucket === null )
+		{
+			return false;
+		}
+		$bucketLen = $bucket->datalen ?? 0;
+		$bucketLen = isset($bucket->datalen) ? $bucket->datalen : 0;
+
+		return true;
+	}
+}
+
diff --git a/tests/PHPStan/Rules/Properties/data/bug-6809.php b/tests/PHPStan/Rules/Properties/data/bug-6809.php
@@ -0,0 +1,14 @@
+<?php declare(strict_types = 1);
+
+namespace Bug6809;
+
+trait SomeTrait {
+	public function __construct() {
+		$isClassCool = static::$coolClass ?? true;
+	}
+}
+
+class HelloWorld
+{
+	use SomeTrait;
+}
diff --git a/tests/PHPStan/Rules/Properties/data/dynamic-properties.php b/tests/PHPStan/Rules/Properties/data/dynamic-properties.php
@@ -0,0 +1,19 @@
+<?php
+
+namespace DynamicProperties;
+
+class Bar {}
+
+class Foo {
+	public function doBar() {
+		isset($this->dynamicProperty);
+		empty($this->dynamicProperty);
+		$this->dynamicProperty ?? 'test';
+
+		$bar = new Bar();
+		isset($bar->dynamicProperty);
+		empty($bar->dynamicProperty);
+		$bar->dynamicProperty ?? 'test';
+	}
+}
+