Skip to content

Commit 989dd6f

Browse files
ondrejmirtesondrejmirtes
committed
MutatingScope - process left side of BooleanAnd and BooleanOr before getting type of the whole expression
Closes phpstan/phpstan#5365 Closes phpstan/phpstan#6551
1 parent ddf64ef commit 989dd6f

7 files changed

Lines changed: 106 additions & 15 deletions

File tree

‎src/Analyser/MutatingScope.php‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -760,18 +760,17 @@ private function resolveType(string $exprString, Expr $node): Type
760760
$node instanceof Node\Expr\BinaryOp\BooleanAnd
761761
|| $node instanceof Node\Expr\BinaryOp\LogicalAnd
762762
) {
763+
$noopCallback = static function (): void {
764+
};
765+
$leftResult = $this->nodeScopeResolver->processExprNode($node->left, $this, $noopCallback, ExpressionContext::createDeep());
763766
$leftBooleanType = $this->getType($node->left)->toBoolean();
764-
if (
765-
$leftBooleanType->isFalse()->yes()
766-
) {
767+
if ($leftBooleanType->isFalse()->yes()) {
767768
return new ConstantBooleanType(false);
768769
}
769770

770-
$rightBooleanType = $this->filterByTruthyValue($node->left)->getType($node->right)->toBoolean();
771+
$rightBooleanType = $leftResult->getTruthyScope()->getType($node->right)->toBoolean();
771772

772-
if (
773-
$rightBooleanType->isFalse()->yes()
774-
) {
773+
if ($rightBooleanType->isFalse()->yes()) {
775774
return new ConstantBooleanType(false);
776775
}
777776

@@ -789,18 +788,17 @@ private function resolveType(string $exprString, Expr $node): Type
789788
$node instanceof Node\Expr\BinaryOp\BooleanOr
790789
|| $node instanceof Node\Expr\BinaryOp\LogicalOr
791790
) {
791+
$noopCallback = static function (): void {
792+
};
793+
$leftResult = $this->nodeScopeResolver->processExprNode($node->left, $this, $noopCallback, ExpressionContext::createDeep());
792794
$leftBooleanType = $this->getType($node->left)->toBoolean();
793-
if (
794-
$leftBooleanType->isTrue()->yes()
795-
) {
795+
if ($leftBooleanType->isTrue()->yes()) {
796796
return new ConstantBooleanType(true);
797797
}
798798

799-
$rightBooleanType = $this->filterByFalseyValue($node->left)->getType($node->right)->toBoolean();
799+
$rightBooleanType = $leftResult->getFalseyScope()->getType($node->right)->toBoolean();
800800

801-
if (
802-
$rightBooleanType->isTrue()->yes()
803-
) {
801+
if ($rightBooleanType->isTrue()->yes()) {
804802
return new ConstantBooleanType(true);
805803
}
806804

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1812,7 +1812,7 @@ private function findEarlyTerminatingExpr(Expr $expr, Scope $scope): ?Expr
18121812
/**
18131813
* @param callable(Node $node, Scope $scope): void $nodeCallback
18141814
*/
1815-
private function processExprNode(Expr $expr, MutatingScope $scope, callable $nodeCallback, ExpressionContext $context): ExpressionResult
1815+
public function processExprNode(Expr $expr, MutatingScope $scope, callable $nodeCallback, ExpressionContext $context): ExpressionResult
18161816
{
18171817
if ($expr instanceof Expr\CallLike && $expr->isFirstClassCallable()) {
18181818
if ($expr instanceof FuncCall) {

‎tests/PHPStan/Analyser/NodeScopeResolverTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,6 +1270,8 @@ public function dataFileAsserts(): iterable
12701270
yield from $this->gatherAssertTypes(__DIR__ . '/data/asymmetric-properties.php');
12711271
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-9062.php');
12721272
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8092.php');
1273+
yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Comparison/data/bug-5365.php');
1274+
yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Comparison/data/bug-6551.php');
12731275
yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Variables/data/bug-9403.php');
12741276
yield from $this->gatherAssertTypes(__DIR__ . '/data/object-shape.php');
12751277
yield from $this->gatherAssertTypes(__DIR__ . '/data/memcache-get.php');

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,4 +514,11 @@ public function testReportAlwaysTrueInLastCondition(bool $reportAlwaysTrueInLast
514514
$this->analyse([__DIR__ . '/data/boolean-and-report-always-true-last-condition.php'], $expectedErrors);
515515
}
516516

517+
public function testBug5365(): void
518+
{
519+
$this->treatPhpDocTypesAsCertain = true;
520+
$this->reportAlwaysTrueInLastCondition = true;
521+
$this->analyse([__DIR__ . '/data/bug-5365.php'], []);
522+
}
523+
517524
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,4 +431,11 @@ public function testReportAlwaysTrueInLastCondition(bool $reportAlwaysTrueInLast
431431
$this->analyse([__DIR__ . '/data/boolean-or-report-always-true-last-condition.php'], $expectedErrors);
432432
}
433433

434+
public function testBug6551(): void
435+
{
436+
$this->treatPhpDocTypesAsCertain = true;
437+
$this->reportAlwaysTrueInLastCondition = true;
438+
$this->analyse([__DIR__ . '/data/bug-6551.php'], []);
439+
}
440+
434441
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
namespace Bug5365;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
function (): void {
8+
$matches = [];
9+
$pattern = '#^C\s+(?<productId>\d+)$#i';
10+
$subject = 'C 1234567890';
11+
12+
$found = (bool)preg_match( $pattern, $subject, $matches ) && isset( $matches['productId'] );
13+
assertType('bool', $found);
14+
};

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
3+
namespace Bug6551;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
function (): void {
8+
$data = [
9+
'c1' => 12,
10+
'rasd' => 13,
11+
'c34' => 15,
12+
];
13+
14+
foreach ($data as $key => $value) {
15+
$match = [];
16+
if (false === preg_match('/^c(\d+)$/', $key, $match) || empty($match)) {
17+
continue;
18+
}
19+
var_dump($key);
20+
var_dump($value);
21+
}
22+
};
23+
24+
function (): void {
25+
$data = [
26+
'c1' => 12,
27+
'rasd' => 13,
28+
'c34' => 15,
29+
];
30+
31+
foreach ($data as $key => $value) {
32+
if (false === preg_match('/^c(\d+)$/', $key, $match) || empty($match)) {
33+
continue;
34+
}
35+
var_dump($key);
36+
var_dump($value);
37+
}
38+
};
39+
40+
function (): void {
41+
$data = [
42+
'c1' => 12,
43+
'rasd' => 13,
44+
'c34' => 15,
45+
];
46+
47+
foreach ($data as $key => $value) {
48+
$match = [];
49+
assertType('bool', preg_match('/^c(\d+)$/', $key, $match) || empty($match));
50+
}
51+
};
52+
53+
function (): void {
54+
$data = [
55+
'c1' => 12,
56+
'rasd' => 13,
57+
'c34' => 15,
58+
];
59+
60+
foreach ($data as $key => $value) {
61+
assertType('bool', preg_match('/^c(\d+)$/', $key, $match) || empty($match));
62+
}
63+
};

0 commit comments

Comments
 (0)