Skip to content

Commit ae9a558

Browse files
ondrejmirtesondrejmirtes
committed
BooleanOrConstantConditionRule - check LogicalOr (bleeding edge), use virtual node for more correct result
1 parent 40a76e8 commit ae9a558

File tree

7 files changed

+93
-18
lines changed

7 files changed

+93
-18
lines changed

‎conf/bleedingEdge.neon

+1
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@ parameters:
1010
dateTimeInstantiation: true
1111
detectDuplicateStubFiles: true
1212
checkLogicalAndConstantCondition: true
13+
checkLogicalOrConstantCondition: true

‎conf/config.level4.neon

+1
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ services:
5050
class: PHPStan\Rules\Comparison\BooleanOrConstantConditionRule
5151
arguments:
5252
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
53+
checkLogicalOrConstantCondition: %featureToggles.checkLogicalOrConstantCondition%
5354
tags:
5455
- phpstan.rules.rule
5556

‎conf/config.neon

+3-1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ parameters:
2323
dateTimeInstantiation: false
2424
detectDuplicateStubFiles: false
2525
checkLogicalAndConstantCondition: false
26+
checkLogicalOrConstantCondition: false
2627
fileExtensions:
2728
- php
2829
checkAlwaysTrueCheckTypeFunctionCall: false
@@ -170,7 +171,8 @@ parametersSchema:
170171
readComposerPhpVersion: bool(),
171172
dateTimeInstantiation: bool(),
172173
detectDuplicateStubFiles: bool(),
173-
checkLogicalAndConstantCondition: bool()
174+
checkLogicalAndConstantCondition: bool(),
175+
checkLogicalOrConstantCondition: bool()
174176
])
175177
fileExtensions: listOf(string())
176178
checkAlwaysTrueCheckTypeFunctionCall: bool()

‎src/Analyser/NodeScopeResolver.php

+3
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
use PHPStan\File\FileHelper;
5151
use PHPStan\File\FileReader;
5252
use PHPStan\Node\BooleanAndNode;
53+
use PHPStan\Node\BooleanOrNode;
5354
use PHPStan\Node\ClassConstantsNode;
5455
use PHPStan\Node\ClassMethodsNode;
5556
use PHPStan\Node\ClassPropertiesNode;
@@ -1923,6 +1924,8 @@ static function () use ($leftMergedWithRightScope, $expr): MutatingScope {
19231924
$rightResult = $this->processExprNode($expr->right, $leftResult->getFalseyScope(), $nodeCallback, $context);
19241925
$leftMergedWithRightScope = $leftResult->getScope()->mergeWith($rightResult->getScope());
19251926

1927+
$nodeCallback(new BooleanOrNode($expr, $leftResult->getFalseyScope()), $scope);
1928+
19261929
return new ExpressionResult(
19271930
$leftMergedWithRightScope,
19281931
$leftResult->hasYield() || $rightResult->hasYield(),

‎src/Node/BooleanOrNode.php

+55
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\BooleanOr;
6+
use PhpParser\Node\Expr\BinaryOp\LogicalOr;
7+
use PhpParser\NodeAbstract;
8+
use PHPStan\Analyser\Scope;
9+
10+
class BooleanOrNode extends NodeAbstract implements VirtualNode
11+
{
12+
13+
/** @var BooleanOr|LogicalOr */
14+
private $originalNode;
15+
16+
private Scope $rightScope;
17+
18+
/**
19+
* @param BooleanOr|LogicalOr $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 BooleanOr|LogicalOr
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_BooleanOrNode';
45+
}
46+
47+
/**
48+
* @return string[]
49+
*/
50+
public function getSubNodeNames(): array
51+
{
52+
return [];
53+
}
54+
55+
}

‎src/Rules/Comparison/BooleanOrConstantConditionRule.php

+28-16
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22

33
namespace PHPStan\Rules\Comparison;
44

5+
use PhpParser\Node\Expr\BinaryOp\BooleanOr;
6+
use PHPStan\Node\BooleanOrNode;
57
use PHPStan\Rules\RuleErrorBuilder;
68
use PHPStan\Type\Constant\ConstantBooleanType;
79

810
/**
9-
* @implements \PHPStan\Rules\Rule<\PhpParser\Node\Expr\BinaryOp\BooleanOr>
11+
* @implements \PHPStan\Rules\Rule<BooleanOrNode>
1012
*/
1113
class BooleanOrConstantConditionRule implements \PHPStan\Rules\Rule
1214
{
@@ -15,35 +17,44 @@ class BooleanOrConstantConditionRule implements \PHPStan\Rules\Rule
1517

1618
private bool $treatPhpDocTypesAsCertain;
1719

20+
private bool $checkLogicalOrConstantCondition;
21+
1822
public function __construct(
1923
ConstantConditionRuleHelper $helper,
20-
bool $treatPhpDocTypesAsCertain
24+
bool $treatPhpDocTypesAsCertain,
25+
bool $checkLogicalOrConstantCondition
2126
)
2227
{
2328
$this->helper = $helper;
2429
$this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain;
30+
$this->checkLogicalOrConstantCondition = $checkLogicalOrConstantCondition;
2531
}
2632

2733
public function getNodeType(): string
2834
{
29-
return \PhpParser\Node\Expr\BinaryOp\BooleanOr::class;
35+
return BooleanOrNode::class;
3036
}
3137

3238
public function processNode(
3339
\PhpParser\Node $node,
3440
\PHPStan\Analyser\Scope $scope
3541
): array
3642
{
43+
$originalNode = $node->getOriginalNode();
44+
if (!$originalNode instanceof BooleanOr && !$this->checkLogicalOrConstantCondition) {
45+
return [];
46+
}
47+
3748
$messages = [];
38-
$leftType = $this->helper->getBooleanType($scope, $node->left);
49+
$leftType = $this->helper->getBooleanType($scope, $originalNode->left);
3950
$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%</>.';
4051
if ($leftType instanceof ConstantBooleanType) {
41-
$addTipLeft = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node, $tipText): RuleErrorBuilder {
52+
$addTipLeft = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $originalNode, $tipText): RuleErrorBuilder {
4253
if (!$this->treatPhpDocTypesAsCertain) {
4354
return $ruleErrorBuilder;
4455
}
4556

46-
$booleanNativeType = $this->helper->getNativeBooleanType($scope, $node->left);
57+
$booleanNativeType = $this->helper->getNativeBooleanType($scope, $originalNode->left);
4758
if ($booleanNativeType instanceof ConstantBooleanType) {
4859
return $ruleErrorBuilder;
4960
}
@@ -53,22 +64,23 @@ public function processNode(
5364
$messages[] = $addTipLeft(RuleErrorBuilder::message(sprintf(
5465
'Left side of || is always %s.',
5566
$leftType->getValue() ? 'true' : 'false'
56-
)))->line($node->left->getLine())->build();
67+
)))->line($originalNode->left->getLine())->build();
5768
}
5869

70+
$rightScope = $node->getRightScope();
5971
$rightType = $this->helper->getBooleanType(
60-
$scope->filterByFalseyValue($node->left),
61-
$node->right
72+
$rightScope,
73+
$originalNode->right
6274
);
6375
if ($rightType instanceof ConstantBooleanType) {
64-
$addTipRight = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node, $tipText): RuleErrorBuilder {
76+
$addTipRight = function (RuleErrorBuilder $ruleErrorBuilder) use ($rightScope, $originalNode, $tipText): RuleErrorBuilder {
6577
if (!$this->treatPhpDocTypesAsCertain) {
6678
return $ruleErrorBuilder;
6779
}
6880

6981
$booleanNativeType = $this->helper->getNativeBooleanType(
70-
$scope->doNotTreatPhpDocTypesAsCertain()->filterByFalseyValue($node->left),
71-
$node->right
82+
$rightScope->doNotTreatPhpDocTypesAsCertain(),
83+
$originalNode->right
7284
);
7385
if ($booleanNativeType instanceof ConstantBooleanType) {
7486
return $ruleErrorBuilder;
@@ -79,18 +91,18 @@ public function processNode(
7991
$messages[] = $addTipRight(RuleErrorBuilder::message(sprintf(
8092
'Right side of || is always %s.',
8193
$rightType->getValue() ? 'true' : 'false'
82-
)))->line($node->right->getLine())->build();
94+
)))->line($originalNode->right->getLine())->build();
8395
}
8496

8597
if (count($messages) === 0) {
86-
$nodeType = $scope->getType($node);
98+
$nodeType = $scope->getType($originalNode);
8799
if ($nodeType instanceof ConstantBooleanType) {
88-
$addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node, $tipText): RuleErrorBuilder {
100+
$addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $originalNode, $tipText): RuleErrorBuilder {
89101
if (!$this->treatPhpDocTypesAsCertain) {
90102
return $ruleErrorBuilder;
91103
}
92104

93-
$booleanNativeType = $scope->doNotTreatPhpDocTypesAsCertain()->getType($node);
105+
$booleanNativeType = $scope->doNotTreatPhpDocTypesAsCertain()->getType($originalNode);
94106
if ($booleanNativeType instanceof ConstantBooleanType) {
95107
return $ruleErrorBuilder;
96108
}

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

+2-1
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)