Skip to content

Commit 0cdda0b

Browse files
ondrejmirtesondrejmirtes
committed
Allow to remember constant and impure expressions in match condition
Closes phpstan/phpstan#4451 Closes phpstan/phpstan#9357 Closes phpstan/phpstan#9007
1 parent b5cf52b commit 0cdda0b

12 files changed

+236
-13
lines changed

‎src/Analyser/MutatingScope.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use PhpParser\Node\Scalar\String_;
3030
use PhpParser\NodeFinder;
3131
use PHPStan\Node\ExecutionEndNode;
32+
use PHPStan\Node\Expr\AlwaysRememberedExpr;
3233
use PHPStan\Node\Expr\GetIterableKeyTypeExpr;
3334
use PHPStan\Node\Expr\GetIterableValueTypeExpr;
3435
use PHPStan\Node\Expr\GetOffsetValueTypeExpr;
@@ -677,6 +678,10 @@ private function resolveType(string $exprString, Expr $node): Type
677678
return $this->expressionTypes[$exprString]->getType();
678679
}
679680

681+
if ($node instanceof AlwaysRememberedExpr) {
682+
return $node->getExprType();
683+
}
684+
680685
if ($node instanceof Expr\BinaryOp\Smaller) {
681686
return $this->getType($node->left)->isSmallerThan($this->getType($node->right))->toBooleanType();
682687
}
@@ -3089,6 +3094,20 @@ public function getFunctionType($type, bool $isNullable, bool $isVariadic): Type
30893094
return ParserNodeTypeToPHPStanType::resolve($type, $this->isInClass() ? $this->getClassReflection() : null);
30903095
}
30913096

3097+
public function enterMatch(Expr\Match_ $expr): self
3098+
{
3099+
if ($expr->cond instanceof Variable) {
3100+
return $this;
3101+
}
3102+
3103+
$type = $this->getType($expr->cond);
3104+
$nativeType = $this->getNativeType($expr->cond);
3105+
$condExpr = new AlwaysRememberedExpr($expr->cond, $type, $nativeType);
3106+
$expr->cond = $condExpr;
3107+
3108+
return $this->assignExpression($condExpr, $type, $nativeType);
3109+
}
3110+
30923111
public function enterForeach(Expr $iteratee, string $valueName, ?string $keyName): self
30933112
{
30943113
$iterateeType = $this->getType($iteratee);

‎src/Analyser/NodeScopeResolver.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2705,7 +2705,7 @@ static function (Node $node, Scope $scope) use ($nodeCallback): void {
27052705
$scope = $condResult->getScope();
27062706
$hasYield = $condResult->hasYield();
27072707
$throwPoints = $condResult->getThrowPoints();
2708-
$matchScope = $scope;
2708+
$matchScope = $scope->enterMatch($expr);
27092709
$armNodes = [];
27102710
$hasDefaultCond = false;
27112711
$hasAlwaysTrueCond = false;

‎src/Analyser/TypeSpecifier.php

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use PhpParser\Node\Expr\StaticCall;
1919
use PhpParser\Node\Expr\StaticPropertyFetch;
2020
use PhpParser\Node\Name;
21+
use PHPStan\Node\Expr\AlwaysRememberedExpr;
2122
use PHPStan\Node\Printer\ExprPrinter;
2223
use PHPStan\Reflection\Assertions;
2324
use PHPStan\Reflection\ParametersAcceptor;
@@ -191,24 +192,33 @@ public function specifyTypesInCondition(
191192
}
192193
}
193194

194-
$rightType = $scope->getType($expr->right);
195+
$rightExpr = $expr->right;
196+
if ($rightExpr instanceof AlwaysRememberedExpr) {
197+
$rightExpr = $rightExpr->getExpr();
198+
}
199+
200+
$leftExpr = $expr->left;
201+
if ($leftExpr instanceof AlwaysRememberedExpr) {
202+
$leftExpr = $leftExpr->getExpr();
203+
}
204+
$rightType = $scope->getType($rightExpr);
195205
if (
196-
$expr->left instanceof ClassConstFetch &&
197-
$expr->left->class instanceof Expr &&
198-
$expr->left->name instanceof Node\Identifier &&
199-
$expr->right instanceof ClassConstFetch &&
206+
$leftExpr instanceof ClassConstFetch &&
207+
$leftExpr->class instanceof Expr &&
208+
$leftExpr->name instanceof Node\Identifier &&
209+
$rightExpr instanceof ClassConstFetch &&
200210
$rightType instanceof ConstantStringType &&
201-
strtolower($expr->left->name->toString()) === 'class'
211+
strtolower($leftExpr->name->toString()) === 'class'
202212
) {
203213
return $this->specifyTypesInCondition(
204214
$scope,
205215
new Instanceof_(
206-
$expr->left->class,
216+
$leftExpr->class,
207217
new Name($rightType->getValue()),
208218
),
209219
$context,
210220
$rootExpr,
211-
);
221+
)->unionWith($this->create($expr->left, $rightType, $context, false, $scope, $rootExpr));
212222
}
213223
if ($context->false()) {
214224
$identicalType = $scope->getType($expr);
@@ -1420,16 +1430,27 @@ private function findTypeExpressionsFromBinaryOperation(Scope $scope, Node\Expr\
14201430
{
14211431
$leftType = $scope->getType($binaryOperation->left);
14221432
$rightType = $scope->getType($binaryOperation->right);
1433+
1434+
$rightExpr = $binaryOperation->right;
1435+
if ($rightExpr instanceof AlwaysRememberedExpr) {
1436+
$rightExpr = $rightExpr->getExpr();
1437+
}
1438+
1439+
$leftExpr = $binaryOperation->left;
1440+
if ($leftExpr instanceof AlwaysRememberedExpr) {
1441+
$leftExpr = $leftExpr->getExpr();
1442+
}
1443+
14231444
if (
14241445
$leftType instanceof ConstantScalarType
1425-
&& !$binaryOperation->right instanceof ConstFetch
1426-
&& !$binaryOperation->right instanceof ClassConstFetch
1446+
&& !$rightExpr instanceof ConstFetch
1447+
&& !$rightExpr instanceof ClassConstFetch
14271448
) {
14281449
return [$binaryOperation->right, $leftType];
14291450
} elseif (
14301451
$rightType instanceof ConstantScalarType
1431-
&& !$binaryOperation->left instanceof ConstFetch
1432-
&& !$binaryOperation->left instanceof ClassConstFetch
1452+
&& !$leftExpr instanceof ConstFetch
1453+
&& !$leftExpr instanceof ClassConstFetch
14331454
) {
14341455
return [$binaryOperation->left, $rightType];
14351456
}

‎src/Node/Expr/AlwaysRememberedExpr.php

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Node\Expr;
4+
5+
use PhpParser\Node\Expr;
6+
use PHPStan\Node\VirtualNode;
7+
use PHPStan\Type\Type;
8+
9+
class AlwaysRememberedExpr extends Expr implements VirtualNode
10+
{
11+
12+
public function __construct(private Expr $expr, private Type $type, private Type $nativeType)
13+
{
14+
parent::__construct([]);
15+
}
16+
17+
public function getExpr(): Expr
18+
{
19+
return $this->expr;
20+
}
21+
22+
public function getExprType(): Type
23+
{
24+
return $this->type;
25+
}
26+
27+
public function getNativeExprType(): Type
28+
{
29+
return $this->nativeType;
30+
}
31+
32+
public function getType(): string
33+
{
34+
return 'PHPStan_Node_AlwaysRememberedExpr';
35+
}
36+
37+
/**
38+
* @return string[]
39+
*/
40+
public function getSubNodeNames(): array
41+
{
42+
return [];
43+
}
44+
45+
}

‎src/Node/Printer/Printer.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace PHPStan\Node\Printer;
44

55
use PhpParser\PrettyPrinter\Standard;
6+
use PHPStan\Node\Expr\AlwaysRememberedExpr;
67
use PHPStan\Node\Expr\GetIterableKeyTypeExpr;
78
use PHPStan\Node\Expr\GetIterableValueTypeExpr;
89
use PHPStan\Node\Expr\GetOffsetValueTypeExpr;
@@ -45,4 +46,9 @@ protected function pPHPStan_Node_SetOffsetValueTypeExpr(SetOffsetValueTypeExpr $
4546
return sprintf('__phpstanSetOffsetValueType(%s, %s, %s)', $this->p($expr->getVar()), $expr->getDim() !== null ? $this->p($expr->getDim()) : 'null', $this->p($expr->getValue()));
4647
}
4748

49+
protected function pPHPStan_Node_AlwaysRememberedExpr(AlwaysRememberedExpr $expr): string // phpcs:ignore
50+
{
51+
return sprintf('__phpstanRembered(%s)', $this->p($expr->getExpr()));
52+
}
53+
4854
}

‎tests/PHPStan/Analyser/NodeScopeResolverTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,6 +1195,7 @@ public function dataFileAsserts(): iterable
11951195
}
11961196

11971197
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8520.php');
1198+
yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Comparison/data/bug-9007.php');
11981199
yield from $this->gatherAssertTypes(__DIR__ . '/data/filter-var-dynamic-return-type-extension-regression.php');
11991200

12001201
if (PHP_VERSION_ID >= 80000) {

‎tests/PHPStan/Rules/Comparison/MatchExpressionDoNotRememberPossiblyImpureValuesRuleTest.php

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Comparison;
4+
5+
use PHPStan\Rules\Rule;
6+
use PHPStan\Testing\RuleTestCase;
7+
use function array_merge;
8+
use const PHP_VERSION_ID;
9+
10+
/**
11+
* @extends RuleTestCase<MatchExpressionRule>
12+
*/
13+
class MatchExpressionDoNotRememberPossiblyImpureValuesRuleTest extends RuleTestCase
14+
{
15+
16+
protected function getRule(): Rule
17+
{
18+
return self::getContainer()->getByType(MatchExpressionRule::class);
19+
}
20+
21+
public function testBug9357(): void
22+
{
23+
if (PHP_VERSION_ID < 80100) {
24+
$this->markTestSkipped('Test requires PHP 8.1.');
25+
}
26+
27+
$this->analyse([__DIR__ . '/data/bug-9357.php'], []);
28+
}
29+
30+
public function testBug9007(): void
31+
{
32+
if (PHP_VERSION_ID < 80100) {
33+
$this->markTestSkipped('Test requires PHP 8.1.');
34+
}
35+
36+
$this->analyse([__DIR__ . '/data/bug-9007.php'], []);
37+
}
38+
39+
public static function getAdditionalConfigFiles(): array
40+
{
41+
return array_merge(
42+
parent::getAdditionalConfigFiles(),
43+
[
44+
__DIR__ . '/doNotRememberPossiblyImpureValues.neon',
45+
],
46+
);
47+
}
48+
49+
}

‎tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,4 +471,22 @@ public function testBug8900(): void
471471
$this->analyse([__DIR__ . '/data/bug-8900.php'], []);
472472
}
473473

474+
public function testBug4451(): void
475+
{
476+
if (PHP_VERSION_ID < 80100) {
477+
$this->markTestSkipped('Test requires PHP 8.1.');
478+
}
479+
480+
$this->analyse([__DIR__ . '/data/bug-4451.php'], []);
481+
}
482+
483+
public function testBug9007(): void
484+
{
485+
if (PHP_VERSION_ID < 80100) {
486+
$this->markTestSkipped('Test requires PHP 8.1.');
487+
}
488+
489+
$this->analyse([__DIR__ . '/data/bug-9007.php'], []);
490+
}
491+
474492
}

‎tests/PHPStan/Rules/Comparison/data/bug-4451.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
namespace Bug4451;
4+
5+
class HelloWorld
6+
{
7+
public function sayHello(): int
8+
{
9+
$verified = fn(): bool => rand() === 1;
10+
11+
return match([$verified(), $verified()]) {
12+
[true, true] => 1,
13+
[true, false] => 2,
14+
[false, true] => 3,
15+
[false, false] => 4,
16+
};
17+
18+
}
19+
}

‎tests/PHPStan/Rules/Comparison/data/bug-9007.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php // lint >= 8.1
2+
3+
namespace Bug9007;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
enum Country: string {
8+
case Usa = 'USA';
9+
case Canada = 'CAN';
10+
case Mexico = 'MEX';
11+
}
12+
13+
function doStuff(string $countryString): int {
14+
assertType(Country::class, Country::from($countryString));
15+
return match (Country::from($countryString)) {
16+
Country::Usa => 1,
17+
Country::Canada => 2,
18+
Country::Mexico => 3,
19+
};
20+
}

‎tests/PHPStan/Rules/Comparison/data/bug-9357.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php // lint >= 8.1
2+
3+
namespace Bug9357;
4+
5+
enum MyEnum: string {
6+
case A = 'a';
7+
case B = 'b';
8+
}
9+
10+
class My {
11+
/** @phpstan-impure */
12+
public function getType(): MyEnum {
13+
echo "called!";
14+
return rand() > 0.5 ? MyEnum::A : MyEnum::B;
15+
}
16+
}
17+
18+
function test(My $m): void {
19+
echo match ($m->getType()) {
20+
MyEnum::A => 1,
21+
MyEnum::B => 2,
22+
};
23+
}

‎tests/PHPStan/Rules/Comparison/doNotRememberPossiblyImpureValues.neon

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
parameters:
2+
rememberPossiblyImpureFunctionValues: false

0 commit comments

Comments
 (0)