MutatingScope - process left side of BooleanAnd and BooleanOr before …

…getting type of the whole expression Closes phpstan/phpstan#5365 Closes phpstan/phpstan#6551
phpstan · Jun 30, 2023 · 989dd6f · 989dd6f
1 parent ddf64ef
commit 989dd6f
Show file tree

Hide file tree

Showing 7 changed files with 106 additions and 15 deletions.
diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php
@@ -760,18 +760,17 @@ private function resolveType(string $exprString, Expr $node): Type
 			$node instanceof Node\Expr\BinaryOp\BooleanAnd
 			|| $node instanceof Node\Expr\BinaryOp\LogicalAnd
 		) {
+			$noopCallback = static function (): void {
+			};
+			$leftResult = $this->nodeScopeResolver->processExprNode($node->left, $this, $noopCallback, ExpressionContext::createDeep());
 			$leftBooleanType = $this->getType($node->left)->toBoolean();
-			if (
-				$leftBooleanType->isFalse()->yes()
-			) {
+			if ($leftBooleanType->isFalse()->yes()) {
 				return new ConstantBooleanType(false);
 			}
 
-			$rightBooleanType = $this->filterByTruthyValue($node->left)->getType($node->right)->toBoolean();
+			$rightBooleanType = $leftResult->getTruthyScope()->getType($node->right)->toBoolean();
 
-			if (
-				$rightBooleanType->isFalse()->yes()
-			) {
+			if ($rightBooleanType->isFalse()->yes()) {
 				return new ConstantBooleanType(false);
 			}
 
@@ -789,18 +788,17 @@ private function resolveType(string $exprString, Expr $node): Type
 			$node instanceof Node\Expr\BinaryOp\BooleanOr
 			|| $node instanceof Node\Expr\BinaryOp\LogicalOr
 		) {
+			$noopCallback = static function (): void {
+			};
+			$leftResult = $this->nodeScopeResolver->processExprNode($node->left, $this, $noopCallback, ExpressionContext::createDeep());
 			$leftBooleanType = $this->getType($node->left)->toBoolean();
-			if (
-				$leftBooleanType->isTrue()->yes()
-			) {
+			if ($leftBooleanType->isTrue()->yes()) {
 				return new ConstantBooleanType(true);
 			}
 
-			$rightBooleanType = $this->filterByFalseyValue($node->left)->getType($node->right)->toBoolean();
+			$rightBooleanType = $leftResult->getFalseyScope()->getType($node->right)->toBoolean();
 
-			if (
-				$rightBooleanType->isTrue()->yes()
-			) {
+			if ($rightBooleanType->isTrue()->yes()) {
 				return new ConstantBooleanType(true);
 			}
 

diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php
@@ -1812,7 +1812,7 @@ private function findEarlyTerminatingExpr(Expr $expr, Scope $scope): ?Expr
 	/**
 	 * @param callable(Node $node, Scope $scope): void $nodeCallback
 	 */
-	private function processExprNode(Expr $expr, MutatingScope $scope, callable $nodeCallback, ExpressionContext $context): ExpressionResult
+	public function processExprNode(Expr $expr, MutatingScope $scope, callable $nodeCallback, ExpressionContext $context): ExpressionResult
 	{
 		if ($expr instanceof Expr\CallLike && $expr->isFirstClassCallable()) {
 			if ($expr instanceof FuncCall) {

diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php
@@ -1270,6 +1270,8 @@ public function dataFileAsserts(): iterable
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/asymmetric-properties.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-9062.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8092.php');
+		yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Comparison/data/bug-5365.php');
+		yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Comparison/data/bug-6551.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Variables/data/bug-9403.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/object-shape.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/memcache-get.php');

diff --git a/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/BooleanAndConstantConditionRuleTest.php
@@ -514,4 +514,11 @@ public function testReportAlwaysTrueInLastCondition(bool $reportAlwaysTrueInLast
 		$this->analyse([__DIR__ . '/data/boolean-and-report-always-true-last-condition.php'], $expectedErrors);
 	}
 
+	public function testBug5365(): void
+	{
+		$this->treatPhpDocTypesAsCertain = true;
+		$this->reportAlwaysTrueInLastCondition = true;
+		$this->analyse([__DIR__ . '/data/bug-5365.php'], []);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Comparison/BooleanOrConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/BooleanOrConstantConditionRuleTest.php
@@ -431,4 +431,11 @@ public function testReportAlwaysTrueInLastCondition(bool $reportAlwaysTrueInLast
 		$this->analyse([__DIR__ . '/data/boolean-or-report-always-true-last-condition.php'], $expectedErrors);
 	}
 
+	public function testBug6551(): void
+	{
+		$this->treatPhpDocTypesAsCertain = true;
+		$this->reportAlwaysTrueInLastCondition = true;
+		$this->analyse([__DIR__ . '/data/bug-6551.php'], []);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Comparison/data/bug-5365.php b/tests/PHPStan/Rules/Comparison/data/bug-5365.php
@@ -0,0 +1,14 @@
+<?php
+
+namespace Bug5365;
+
+use function PHPStan\Testing\assertType;
+
+function (): void {
+	$matches = [];
+	$pattern = '#^C\s+(?<productId>\d+)$#i';
+	$subject = 'C 1234567890';
+
+	$found = (bool)preg_match( $pattern, $subject, $matches ) && isset( $matches['productId'] );
+	assertType('bool', $found);
+};
diff --git a/tests/PHPStan/Rules/Comparison/data/bug-6551.php b/tests/PHPStan/Rules/Comparison/data/bug-6551.php
@@ -0,0 +1,63 @@
+<?php
+
+namespace Bug6551;
+
+use function PHPStan\Testing\assertType;
+
+function (): void {
+	$data = [
+		'c1' => 12,
+		'rasd' => 13,
+		'c34' => 15,
+	];
+
+	foreach ($data as $key => $value) {
+		$match = [];
+		if (false === preg_match('/^c(\d+)$/', $key, $match) || empty($match)) {
+			continue;
+		}
+		var_dump($key);
+		var_dump($value);
+	}
+};
+
+function (): void {
+	$data = [
+		'c1' => 12,
+		'rasd' => 13,
+		'c34' => 15,
+	];
+
+	foreach ($data as $key => $value) {
+		if (false === preg_match('/^c(\d+)$/', $key, $match) || empty($match)) {
+			continue;
+		}
+		var_dump($key);
+		var_dump($value);
+	}
+};
+
+function (): void {
+	$data = [
+		'c1' => 12,
+		'rasd' => 13,
+		'c34' => 15,
+	];
+
+	foreach ($data as $key => $value) {
+		$match = [];
+		assertType('bool', preg_match('/^c(\d+)$/', $key, $match) || empty($match));
+	}
+};
+
+function (): void {
+	$data = [
+		'c1' => 12,
+		'rasd' => 13,
+		'c34' => 15,
+	];
+
+	foreach ($data as $key => $value) {
+		assertType('bool', preg_match('/^c(\d+)$/', $key, $match) || empty($match));
+	}
+};