From 0cdda0b210a623ee299323da80771c2c84c602f9 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 19 Jun 2023 11:40:49 +0200 Subject: [PATCH] Allow to remember constant and impure expressions in match condition Closes https://github.com/phpstan/phpstan/issues/4451 Closes https://github.com/phpstan/phpstan/issues/9357 Closes https://github.com/phpstan/phpstan/issues/9007 --- src/Analyser/MutatingScope.php | 19 +++++++ src/Analyser/NodeScopeResolver.php | 2 +- src/Analyser/TypeSpecifier.php | 45 ++++++++++++----- src/Node/Expr/AlwaysRememberedExpr.php | 45 +++++++++++++++++ src/Node/Printer/Printer.php | 6 +++ .../Analyser/NodeScopeResolverTest.php | 1 + ...otRememberPossiblyImpureValuesRuleTest.php | 49 +++++++++++++++++++ .../Comparison/MatchExpressionRuleTest.php | 18 +++++++ .../Rules/Comparison/data/bug-4451.php | 19 +++++++ .../Rules/Comparison/data/bug-9007.php | 20 ++++++++ .../Rules/Comparison/data/bug-9357.php | 23 +++++++++ .../doNotRememberPossiblyImpureValues.neon | 2 + 12 files changed, 236 insertions(+), 13 deletions(-) create mode 100644 src/Node/Expr/AlwaysRememberedExpr.php create mode 100644 tests/PHPStan/Rules/Comparison/MatchExpressionDoNotRememberPossiblyImpureValuesRuleTest.php create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-4451.php create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-9007.php create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-9357.php create mode 100644 tests/PHPStan/Rules/Comparison/doNotRememberPossiblyImpureValues.neon diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 5823dda61d..71e145d2c6 100644 --- 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 index 011c49f341..88546889ff 100644 --- 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 index 1a725bcd70..81f7f4a64d 100644 --- 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 new file mode 100644 index 0000000000..6df48fc9aa --- /dev/null +++ b/src/Node/Expr/AlwaysRememberedExpr.php @@ -0,0 +1,45 @@ +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 index d8127309c4..e35f5d8190 100644 --- 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 index eb136244ae..8d8bfb1818 100644 --- 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 new file mode 100644 index 0000000000..b52785c6f5 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/MatchExpressionDoNotRememberPossiblyImpureValuesRuleTest.php @@ -0,0 +1,49 @@ + + */ +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 index a90e9da670..98eed429eb 100644 --- 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 new file mode 100644 index 0000000000..95cdc059f8 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-4451.php @@ -0,0 +1,19 @@ + 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 new file mode 100644 index 0000000000..ae2fa03d6c --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-9007.php @@ -0,0 +1,20 @@ += 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 new file mode 100644 index 0000000000..c0bae16688 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-9357.php @@ -0,0 +1,23 @@ += 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 new file mode 100644 index 0000000000..971a1184f4 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/doNotRememberPossiblyImpureValues.neon @@ -0,0 +1,2 @@ +parameters: + rememberPossiblyImpureFunctionValues: false