Allow to remember constant and impure expressions in match condition

Closes phpstan/phpstan#4451 Closes phpstan/phpstan#9357 Closes phpstan/phpstan#9007
phpstan · Jun 19, 2023 · 0cdda0b · 0cdda0b
1 parent b5cf52b
commit 0cdda0b
Show file tree

Hide file tree

Showing 12 changed files with 236 additions and 13 deletions.
diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php
@@ -29,6 +29,7 @@
 use PhpParser\Node\Scalar\String_;
 use PhpParser\NodeFinder;
 use PHPStan\Node\ExecutionEndNode;
+use PHPStan\Node\Expr\AlwaysRememberedExpr;
 use PHPStan\Node\Expr\GetIterableKeyTypeExpr;
 use PHPStan\Node\Expr\GetIterableValueTypeExpr;
 use PHPStan\Node\Expr\GetOffsetValueTypeExpr;
@@ -677,6 +678,10 @@ private function resolveType(string $exprString, Expr $node): Type
 			return $this->expressionTypes[$exprString]->getType();
 		}
 
+		if ($node instanceof AlwaysRememberedExpr) {
+			return $node->getExprType();
+		}
+
 		if ($node instanceof Expr\BinaryOp\Smaller) {
 			return $this->getType($node->left)->isSmallerThan($this->getType($node->right))->toBooleanType();
 		}
@@ -3089,6 +3094,20 @@ public function getFunctionType($type, bool $isNullable, bool $isVariadic): Type
 		return ParserNodeTypeToPHPStanType::resolve($type, $this->isInClass() ? $this->getClassReflection() : null);
 	}
 
+	public function enterMatch(Expr\Match_ $expr): self
+	{
+		if ($expr->cond instanceof Variable) {
+			return $this;
+		}
+
+		$type = $this->getType($expr->cond);
+		$nativeType = $this->getNativeType($expr->cond);
+		$condExpr = new AlwaysRememberedExpr($expr->cond, $type, $nativeType);
+		$expr->cond = $condExpr;
+
+		return $this->assignExpression($condExpr, $type, $nativeType);
+	}
+
 	public function enterForeach(Expr $iteratee, string $valueName, ?string $keyName): self
 	{
 		$iterateeType = $this->getType($iteratee);

diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php
@@ -2705,7 +2705,7 @@ static function (Node $node, Scope $scope) use ($nodeCallback): void {
 			$scope = $condResult->getScope();
 			$hasYield = $condResult->hasYield();
 			$throwPoints = $condResult->getThrowPoints();
-			$matchScope = $scope;
+			$matchScope = $scope->enterMatch($expr);
 			$armNodes = [];
 			$hasDefaultCond = false;
 			$hasAlwaysTrueCond = false;

diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php
@@ -18,6 +18,7 @@
 use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Expr\StaticPropertyFetch;
 use PhpParser\Node\Name;
+use PHPStan\Node\Expr\AlwaysRememberedExpr;
 use PHPStan\Node\Printer\ExprPrinter;
 use PHPStan\Reflection\Assertions;
 use PHPStan\Reflection\ParametersAcceptor;
@@ -191,24 +192,33 @@ public function specifyTypesInCondition(
 				}
 			}
 
-			$rightType = $scope->getType($expr->right);
+			$rightExpr = $expr->right;
+			if ($rightExpr instanceof AlwaysRememberedExpr) {
+				$rightExpr = $rightExpr->getExpr();
+			}
+
+			$leftExpr = $expr->left;
+			if ($leftExpr instanceof AlwaysRememberedExpr) {
+				$leftExpr = $leftExpr->getExpr();
+			}
+			$rightType = $scope->getType($rightExpr);
 			if (
-				$expr->left instanceof ClassConstFetch &&
-				$expr->left->class instanceof Expr &&
-				$expr->left->name instanceof Node\Identifier &&
-				$expr->right instanceof ClassConstFetch &&
+				$leftExpr instanceof ClassConstFetch &&
+				$leftExpr->class instanceof Expr &&
+				$leftExpr->name instanceof Node\Identifier &&
+				$rightExpr instanceof ClassConstFetch &&
 				$rightType instanceof ConstantStringType &&
-				strtolower($expr->left->name->toString()) === 'class'
+				strtolower($leftExpr->name->toString()) === 'class'
 			) {
 				return $this->specifyTypesInCondition(
 					$scope,
 					new Instanceof_(
-						$expr->left->class,
+						$leftExpr->class,
 						new Name($rightType->getValue()),
 					),
 					$context,
 					$rootExpr,
-				);
+				)->unionWith($this->create($expr->left, $rightType, $context, false, $scope, $rootExpr));
 			}
 			if ($context->false()) {
 				$identicalType = $scope->getType($expr);
@@ -1420,16 +1430,27 @@ private function findTypeExpressionsFromBinaryOperation(Scope $scope, Node\Expr\
 	{
 		$leftType = $scope->getType($binaryOperation->left);
 		$rightType = $scope->getType($binaryOperation->right);
+
+		$rightExpr = $binaryOperation->right;
+		if ($rightExpr instanceof AlwaysRememberedExpr) {
+			$rightExpr = $rightExpr->getExpr();
+		}
+
+		$leftExpr = $binaryOperation->left;
+		if ($leftExpr instanceof AlwaysRememberedExpr) {
+			$leftExpr = $leftExpr->getExpr();
+		}
+
 		if (
 			$leftType instanceof ConstantScalarType
-			&& !$binaryOperation->right instanceof ConstFetch
-			&& !$binaryOperation->right instanceof ClassConstFetch
+			&& !$rightExpr instanceof ConstFetch
+			&& !$rightExpr instanceof ClassConstFetch
 		) {
 			return [$binaryOperation->right, $leftType];
 		} elseif (
 			$rightType instanceof ConstantScalarType
-			&& !$binaryOperation->left instanceof ConstFetch
-			&& !$binaryOperation->left instanceof ClassConstFetch
+			&& !$leftExpr instanceof ConstFetch
+			&& !$leftExpr instanceof ClassConstFetch
 		) {
 			return [$binaryOperation->left, $rightType];
 		}

diff --git a/src/Node/Expr/AlwaysRememberedExpr.php b/src/Node/Expr/AlwaysRememberedExpr.php
@@ -0,0 +1,45 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Node\Expr;
+
+use PhpParser\Node\Expr;
+use PHPStan\Node\VirtualNode;
+use PHPStan\Type\Type;
+
+class AlwaysRememberedExpr extends Expr implements VirtualNode
+{
+
+	public function __construct(private Expr $expr, private Type $type, private Type $nativeType)
+	{
+		parent::__construct([]);
+	}
+
+	public function getExpr(): Expr
+	{
+		return $this->expr;
+	}
+
+	public function getExprType(): Type
+	{
+		return $this->type;
+	}
+
+	public function getNativeExprType(): Type
+	{
+		return $this->nativeType;
+	}
+
+	public function getType(): string
+	{
+		return 'PHPStan_Node_AlwaysRememberedExpr';
+	}
+
+	/**
+	 * @return string[]
+	 */
+	public function getSubNodeNames(): array
+	{
+		return [];
+	}
+
+}
diff --git a/src/Node/Printer/Printer.php b/src/Node/Printer/Printer.php
@@ -3,6 +3,7 @@
 namespace PHPStan\Node\Printer;
 
 use PhpParser\PrettyPrinter\Standard;
+use PHPStan\Node\Expr\AlwaysRememberedExpr;
 use PHPStan\Node\Expr\GetIterableKeyTypeExpr;
 use PHPStan\Node\Expr\GetIterableValueTypeExpr;
 use PHPStan\Node\Expr\GetOffsetValueTypeExpr;
@@ -45,4 +46,9 @@ protected function pPHPStan_Node_SetOffsetValueTypeExpr(SetOffsetValueTypeExpr $
 		return sprintf('__phpstanSetOffsetValueType(%s, %s, %s)', $this->p($expr->getVar()), $expr->getDim() !== null ? $this->p($expr->getDim()) : 'null', $this->p($expr->getValue()));
 	}
 
+	protected function pPHPStan_Node_AlwaysRememberedExpr(AlwaysRememberedExpr $expr): string // phpcs:ignore
+	{
+		return sprintf('__phpstanRembered(%s)', $this->p($expr->getExpr()));
+	}
+
 }
diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php
@@ -1195,6 +1195,7 @@ public function dataFileAsserts(): iterable
 		}
 
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8520.php');
+		yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Comparison/data/bug-9007.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/filter-var-dynamic-return-type-extension-regression.php');
 
 		if (PHP_VERSION_ID >= 80000) {

diff --git a/tests/PHPStan/Rules/Comparison/MatchExpressionDoNotRememberPossiblyImpureValuesRuleTest.php b/tests/PHPStan/Rules/Comparison/MatchExpressionDoNotRememberPossiblyImpureValuesRuleTest.php
@@ -0,0 +1,49 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Comparison;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use function array_merge;
+use const PHP_VERSION_ID;
+
+/**
+ * @extends RuleTestCase<MatchExpressionRule>
+ */
+class MatchExpressionDoNotRememberPossiblyImpureValuesRuleTest extends RuleTestCase
+{
+
+	protected function getRule(): Rule
+	{
+		return self::getContainer()->getByType(MatchExpressionRule::class);
+	}
+
+	public function testBug9357(): void
+	{
+		if (PHP_VERSION_ID < 80100) {
+			$this->markTestSkipped('Test requires PHP 8.1.');
+		}
+
+		$this->analyse([__DIR__ . '/data/bug-9357.php'], []);
+	}
+
+	public function testBug9007(): void
+	{
+		if (PHP_VERSION_ID < 80100) {
+			$this->markTestSkipped('Test requires PHP 8.1.');
+		}
+
+		$this->analyse([__DIR__ . '/data/bug-9007.php'], []);
+	}
+
+	public static function getAdditionalConfigFiles(): array
+	{
+		return array_merge(
+			parent::getAdditionalConfigFiles(),
+			[
+				__DIR__ . '/doNotRememberPossiblyImpureValues.neon',
+			],
+		);
+	}
+
+}
diff --git a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php
@@ -471,4 +471,22 @@ public function testBug8900(): void
 		$this->analyse([__DIR__ . '/data/bug-8900.php'], []);
 	}
 
+	public function testBug4451(): void
+	{
+		if (PHP_VERSION_ID < 80100) {
+			$this->markTestSkipped('Test requires PHP 8.1.');
+		}
+
+		$this->analyse([__DIR__ . '/data/bug-4451.php'], []);
+	}
+
+	public function testBug9007(): void
+	{
+		if (PHP_VERSION_ID < 80100) {
+			$this->markTestSkipped('Test requires PHP 8.1.');
+		}
+
+		$this->analyse([__DIR__ . '/data/bug-9007.php'], []);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Comparison/data/bug-4451.php b/tests/PHPStan/Rules/Comparison/data/bug-4451.php
@@ -0,0 +1,19 @@
+<?php
+
+namespace Bug4451;
+
+class HelloWorld
+{
+	public function sayHello(): int
+	{
+		$verified = fn(): bool => rand() === 1;
+
+		return match([$verified(), $verified()]) {
+			[true, true] => 1,
+			[true, false] => 2,
+			[false, true] => 3,
+			[false, false] => 4,
+		};
+
+	}
+}
diff --git a/tests/PHPStan/Rules/Comparison/data/bug-9007.php b/tests/PHPStan/Rules/Comparison/data/bug-9007.php
@@ -0,0 +1,20 @@
+<?php // lint >= 8.1
+
+namespace Bug9007;
+
+use function PHPStan\Testing\assertType;
+
+enum Country: string {
+	case Usa = 'USA';
+	case Canada = 'CAN';
+	case Mexico = 'MEX';
+}
+
+function doStuff(string $countryString): int {
+	assertType(Country::class, Country::from($countryString));
+	return match (Country::from($countryString)) {
+		Country::Usa => 1,
+		Country::Canada => 2,
+		Country::Mexico => 3,
+	};
+}
diff --git a/tests/PHPStan/Rules/Comparison/data/bug-9357.php b/tests/PHPStan/Rules/Comparison/data/bug-9357.php
@@ -0,0 +1,23 @@
+<?php // lint >= 8.1
+
+namespace Bug9357;
+
+enum MyEnum: string {
+	case A = 'a';
+	case B = 'b';
+}
+
+class My {
+	/** @phpstan-impure */
+	public function getType(): MyEnum {
+		echo "called!";
+		return rand() > 0.5 ? MyEnum::A : MyEnum::B;
+	}
+}
+
+function test(My $m): void {
+	echo match ($m->getType()) {
+		MyEnum::A => 1,
+		MyEnum::B => 2,
+	};
+}
diff --git a/tests/PHPStan/Rules/Comparison/doNotRememberPossiblyImpureValues.neon b/tests/PHPStan/Rules/Comparison/doNotRememberPossiblyImpureValues.neon
@@ -0,0 +1,2 @@
+parameters:
+	rememberPossiblyImpureFunctionValues: false