Skip to content

Commit 40a76e8

Browse files
ondrejmirtesondrejmirtes
committed
BooleanAndConstantConditionRule - check LogicalAnd (bleeding edge), use virtual node for more correct result
1 parent 9c39e0f commit 40a76e8

7 files changed

Lines changed: 96 additions & 18 deletions

File tree

‎conf/bleedingEdge.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@ parameters:
99
readComposerPhpVersion: true
1010
dateTimeInstantiation: true
1111
detectDuplicateStubFiles: true
12+
checkLogicalAndConstantCondition: true

‎conf/config.level4.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ services:
4242
class: PHPStan\Rules\Comparison\BooleanAndConstantConditionRule
4343
arguments:
4444
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
45+
checkLogicalAndConstantCondition: %featureToggles.checkLogicalAndConstantCondition%
4546
tags:
4647
- phpstan.rules.rule
4748

‎conf/config.neon‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ parameters:
2222
readComposerPhpVersion: false
2323
dateTimeInstantiation: false
2424
detectDuplicateStubFiles: false
25+
checkLogicalAndConstantCondition: false
2526
fileExtensions:
2627
- php
2728
checkAlwaysTrueCheckTypeFunctionCall: false
@@ -168,7 +169,8 @@ parametersSchema:
168169
unusedClassElements: bool(),
169170
readComposerPhpVersion: bool(),
170171
dateTimeInstantiation: bool(),
171-
detectDuplicateStubFiles: bool()
172+
detectDuplicateStubFiles: bool(),
173+
checkLogicalAndConstantCondition: bool()
172174
])
173175
fileExtensions: listOf(string())
174176
checkAlwaysTrueCheckTypeFunctionCall: bool()

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
use PHPStan\DependencyInjection\Reflection\ClassReflectionExtensionRegistryProvider;
5050
use PHPStan\File\FileHelper;
5151
use PHPStan\File\FileReader;
52+
use PHPStan\Node\BooleanAndNode;
5253
use PHPStan\Node\ClassConstantsNode;
5354
use PHPStan\Node\ClassMethodsNode;
5455
use PHPStan\Node\ClassPropertiesNode;
@@ -1905,6 +1906,8 @@ static function () use ($scope, $expr): MutatingScope {
19051906
$rightResult = $this->processExprNode($expr->right, $leftResult->getTruthyScope(), $nodeCallback, $context);
19061907
$leftMergedWithRightScope = $leftResult->getScope()->mergeWith($rightResult->getScope());
19071908

1909+
$nodeCallback(new BooleanAndNode($expr, $leftResult->getTruthyScope()), $scope);
1910+
19081911
return new ExpressionResult(
19091912
$leftMergedWithRightScope,
19101913
$leftResult->hasYield() || $rightResult->hasYield(),

‎src/Node/BooleanAndNode.php‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Node;
4+
5+
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
6+
use PhpParser\Node\Expr\BinaryOp\LogicalAnd;
7+
use PhpParser\NodeAbstract;
8+
use PHPStan\Analyser\Scope;
9+
10+
class BooleanAndNode extends NodeAbstract implements VirtualNode
11+
{
12+
13+
/** @var BooleanAnd|LogicalAnd */
14+
private $originalNode;
15+
16+
private Scope $rightScope;
17+
18+
/**
19+
* @param BooleanAnd|LogicalAnd $originalNode
20+
* @param Scope $rightScope
21+
*/
22+
public function __construct($originalNode, Scope $rightScope)
23+
{
24+
parent::__construct($originalNode->getAttributes());
25+
$this->originalNode = $originalNode;
26+
$this->rightScope = $rightScope;
27+
}
28+
29+
/**
30+
* @return BooleanAnd|LogicalAnd
31+
*/
32+
public function getOriginalNode()
33+
{
34+
return $this->originalNode;
35+
}
36+
37+
public function getRightScope(): Scope
38+
{
39+
return $this->rightScope;
40+
}
41+
42+
public function getType(): string
43+
{
44+
return 'PHPStan_Node_BooleanAndNode';
45+
}
46+
47+
/**
48+
* @return string[]
49+
*/
50+
public function getSubNodeNames(): array
51+
{
52+
return [];
53+
}
54+
55+
}

‎src/Rules/Comparison/BooleanAndConstantConditionRule.php‎

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,14 @@
22

33
namespace PHPStan\Rules\Comparison;
44

5+
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
6+
use PhpParser\Node\Expr\BinaryOp\LogicalAnd;
7+
use PHPStan\Node\BooleanAndNode;
58
use PHPStan\Rules\RuleErrorBuilder;
69
use PHPStan\Type\Constant\ConstantBooleanType;
710

811
/**
9-
* @implements \PHPStan\Rules\Rule<\PhpParser\Node\Expr\BinaryOp\BooleanAnd>
12+
* @implements \PHPStan\Rules\Rule<BooleanAndNode>
1013
*/
1114
class BooleanAndConstantConditionRule implements \PHPStan\Rules\Rule
1215
{
@@ -15,18 +18,22 @@ class BooleanAndConstantConditionRule implements \PHPStan\Rules\Rule
1518

1619
private bool $treatPhpDocTypesAsCertain;
1720

21+
private bool $checkLogicalAndConstantCondition;
22+
1823
public function __construct(
1924
ConstantConditionRuleHelper $helper,
20-
bool $treatPhpDocTypesAsCertain
25+
bool $treatPhpDocTypesAsCertain,
26+
bool $checkLogicalAndConstantCondition
2127
)
2228
{
2329
$this->helper = $helper;
2430
$this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain;
31+
$this->checkLogicalAndConstantCondition = $checkLogicalAndConstantCondition;
2532
}
2633

2734
public function getNodeType(): string
2835
{
29-
return \PhpParser\Node\Expr\BinaryOp\BooleanAnd::class;
36+
return BooleanAndNode::class;
3037
}
3138

3239
public function processNode(
@@ -35,15 +42,22 @@ public function processNode(
3542
): array
3643
{
3744
$errors = [];
38-
$leftType = $this->helper->getBooleanType($scope, $node->left);
45+
46+
/** @var BooleanAnd|LogicalAnd $originalNode */
47+
$originalNode = $node->getOriginalNode();
48+
if (!$originalNode instanceof BooleanAnd && !$this->checkLogicalAndConstantCondition) {
49+
return [];
50+
}
51+
52+
$leftType = $this->helper->getBooleanType($scope, $originalNode->left);
3953
$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';
4054
if ($leftType instanceof ConstantBooleanType) {
41-
$addTipLeft = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node, $tipText): RuleErrorBuilder {
55+
$addTipLeft = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $tipText, $originalNode): RuleErrorBuilder {
4256
if (!$this->treatPhpDocTypesAsCertain) {
4357
return $ruleErrorBuilder;
4458
}
4559

46-
$booleanNativeType = $this->helper->getNativeBooleanType($scope, $node->left);
60+
$booleanNativeType = $this->helper->getNativeBooleanType($scope, $originalNode->left);
4761
if ($booleanNativeType instanceof ConstantBooleanType) {
4862
return $ruleErrorBuilder;
4963
}
@@ -53,22 +67,23 @@ public function processNode(
5367
$errors[] = $addTipLeft(RuleErrorBuilder::message(sprintf(
5468
'Left side of && is always %s.',
5569
$leftType->getValue() ? 'true' : 'false'
56-
)))->line($node->left->getLine())->build();
70+
)))->line($originalNode->left->getLine())->build();
5771
}
5872

73+
$rightScope = $node->getRightScope();
5974
$rightType = $this->helper->getBooleanType(
60-
$scope->filterByTruthyValue($node->left),
61-
$node->right
75+
$rightScope,
76+
$originalNode->right
6277
);
6378
if ($rightType instanceof ConstantBooleanType) {
64-
$addTipRight = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node, $tipText): RuleErrorBuilder {
79+
$addTipRight = function (RuleErrorBuilder $ruleErrorBuilder) use ($rightScope, $originalNode, $tipText): RuleErrorBuilder {
6580
if (!$this->treatPhpDocTypesAsCertain) {
6681
return $ruleErrorBuilder;
6782
}
6883

6984
$booleanNativeType = $this->helper->getNativeBooleanType(
70-
$scope->doNotTreatPhpDocTypesAsCertain()->filterByTruthyValue($node->left),
71-
$node->right
85+
$rightScope->doNotTreatPhpDocTypesAsCertain(),
86+
$originalNode->right
7287
);
7388
if ($booleanNativeType instanceof ConstantBooleanType) {
7489
return $ruleErrorBuilder;
@@ -79,18 +94,18 @@ public function processNode(
7994
$errors[] = $addTipRight(RuleErrorBuilder::message(sprintf(
8095
'Right side of && is always %s.',
8196
$rightType->getValue() ? 'true' : 'false'
82-
)))->line($node->right->getLine())->build();
97+
)))->line($originalNode->right->getLine())->build();
8398
}
8499

85100
if (count($errors) === 0) {
86-
$nodeType = $scope->getType($node);
101+
$nodeType = $scope->getType($originalNode);
87102
if ($nodeType instanceof ConstantBooleanType) {
88-
$addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node, $tipText): RuleErrorBuilder {
103+
$addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $originalNode, $tipText): RuleErrorBuilder {
89104
if (!$this->treatPhpDocTypesAsCertain) {
90105
return $ruleErrorBuilder;
91106
}
92107

93-
$booleanNativeType = $scope->doNotTreatPhpDocTypesAsCertain()->getType($node);
108+
$booleanNativeType = $scope->doNotTreatPhpDocTypesAsCertain()->getType($originalNode);
94109
if ($booleanNativeType instanceof ConstantBooleanType) {
95110
return $ruleErrorBuilder;
96111
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ protected function getRule(): \PHPStan\Rules\Rule
2323
),
2424
$this->treatPhpDocTypesAsCertain
2525
),
26-
$this->treatPhpDocTypesAsCertain
26+
$this->treatPhpDocTypesAsCertain,
27+
true
2728
);
2829
}
2930

0 commit comments

Comments
 (0)