Skip to content

Commit

Permalink
BooleanAndConstantConditionRule - check LogicalAnd (bleeding edge), u…
Browse files Browse the repository at this point in the history
…se virtual node for more correct result
  • Loading branch information
ondrejmirtes committed Jan 11, 2021
1 parent 9c39e0f commit 40a76e8
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 18 deletions.
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ parameters:
readComposerPhpVersion: true
dateTimeInstantiation: true
detectDuplicateStubFiles: true
checkLogicalAndConstantCondition: true
1 change: 1 addition & 0 deletions conf/config.level4.neon
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ services:
class: PHPStan\Rules\Comparison\BooleanAndConstantConditionRule
arguments:
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
checkLogicalAndConstantCondition: %featureToggles.checkLogicalAndConstantCondition%
tags:
- phpstan.rules.rule

Expand Down
4 changes: 3 additions & 1 deletion conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ parameters:
readComposerPhpVersion: false
dateTimeInstantiation: false
detectDuplicateStubFiles: false
checkLogicalAndConstantCondition: false
fileExtensions:
- php
checkAlwaysTrueCheckTypeFunctionCall: false
Expand Down Expand Up @@ -168,7 +169,8 @@ parametersSchema:
unusedClassElements: bool(),
readComposerPhpVersion: bool(),
dateTimeInstantiation: bool(),
detectDuplicateStubFiles: bool()
detectDuplicateStubFiles: bool(),
checkLogicalAndConstantCondition: bool()
])
fileExtensions: listOf(string())
checkAlwaysTrueCheckTypeFunctionCall: bool()
Expand Down
3 changes: 3 additions & 0 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
use PHPStan\DependencyInjection\Reflection\ClassReflectionExtensionRegistryProvider;
use PHPStan\File\FileHelper;
use PHPStan\File\FileReader;
use PHPStan\Node\BooleanAndNode;
use PHPStan\Node\ClassConstantsNode;
use PHPStan\Node\ClassMethodsNode;
use PHPStan\Node\ClassPropertiesNode;
Expand Down Expand Up @@ -1905,6 +1906,8 @@ static function () use ($scope, $expr): MutatingScope {
$rightResult = $this->processExprNode($expr->right, $leftResult->getTruthyScope(), $nodeCallback, $context);
$leftMergedWithRightScope = $leftResult->getScope()->mergeWith($rightResult->getScope());

$nodeCallback(new BooleanAndNode($expr, $leftResult->getTruthyScope()), $scope);

return new ExpressionResult(
$leftMergedWithRightScope,
$leftResult->hasYield() || $rightResult->hasYield(),
Expand Down
55 changes: 55 additions & 0 deletions src/Node/BooleanAndNode.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php declare(strict_types = 1);

namespace PHPStan\Node;

use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\BinaryOp\LogicalAnd;
use PhpParser\NodeAbstract;
use PHPStan\Analyser\Scope;

class BooleanAndNode extends NodeAbstract implements VirtualNode
{

/** @var BooleanAnd|LogicalAnd */
private $originalNode;

private Scope $rightScope;

/**
* @param BooleanAnd|LogicalAnd $originalNode
* @param Scope $rightScope
*/
public function __construct($originalNode, Scope $rightScope)
{
parent::__construct($originalNode->getAttributes());
$this->originalNode = $originalNode;
$this->rightScope = $rightScope;
}

/**
* @return BooleanAnd|LogicalAnd
*/
public function getOriginalNode()
{
return $this->originalNode;
}

public function getRightScope(): Scope
{
return $this->rightScope;
}

public function getType(): string
{
return 'PHPStan_Node_BooleanAndNode';
}

/**
* @return string[]
*/
public function getSubNodeNames(): array
{
return [];
}

}
47 changes: 31 additions & 16 deletions src/Rules/Comparison/BooleanAndConstantConditionRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

namespace PHPStan\Rules\Comparison;

use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\BinaryOp\LogicalAnd;
use PHPStan\Node\BooleanAndNode;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\Constant\ConstantBooleanType;

/**
* @implements \PHPStan\Rules\Rule<\PhpParser\Node\Expr\BinaryOp\BooleanAnd>
* @implements \PHPStan\Rules\Rule<BooleanAndNode>
*/
class BooleanAndConstantConditionRule implements \PHPStan\Rules\Rule
{
Expand All @@ -15,18 +18,22 @@ class BooleanAndConstantConditionRule implements \PHPStan\Rules\Rule

private bool $treatPhpDocTypesAsCertain;

private bool $checkLogicalAndConstantCondition;

public function __construct(
ConstantConditionRuleHelper $helper,
bool $treatPhpDocTypesAsCertain
bool $treatPhpDocTypesAsCertain,
bool $checkLogicalAndConstantCondition
)
{
$this->helper = $helper;
$this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain;
$this->checkLogicalAndConstantCondition = $checkLogicalAndConstantCondition;
}

public function getNodeType(): string
{
return \PhpParser\Node\Expr\BinaryOp\BooleanAnd::class;
return BooleanAndNode::class;
}

public function processNode(
Expand All @@ -35,15 +42,22 @@ public function processNode(
): array
{
$errors = [];
$leftType = $this->helper->getBooleanType($scope, $node->left);

/** @var BooleanAnd|LogicalAnd $originalNode */
$originalNode = $node->getOriginalNode();
if (!$originalNode instanceof BooleanAnd && !$this->checkLogicalAndConstantCondition) {
return [];
}

$leftType = $this->helper->getBooleanType($scope, $originalNode->left);
$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%</>.';
if ($leftType instanceof ConstantBooleanType) {
$addTipLeft = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node, $tipText): RuleErrorBuilder {
$addTipLeft = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $tipText, $originalNode): RuleErrorBuilder {
if (!$this->treatPhpDocTypesAsCertain) {
return $ruleErrorBuilder;
}

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

$rightScope = $node->getRightScope();
$rightType = $this->helper->getBooleanType(
$scope->filterByTruthyValue($node->left),
$node->right
$rightScope,
$originalNode->right
);
if ($rightType instanceof ConstantBooleanType) {
$addTipRight = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node, $tipText): RuleErrorBuilder {
$addTipRight = function (RuleErrorBuilder $ruleErrorBuilder) use ($rightScope, $originalNode, $tipText): RuleErrorBuilder {
if (!$this->treatPhpDocTypesAsCertain) {
return $ruleErrorBuilder;
}

$booleanNativeType = $this->helper->getNativeBooleanType(
$scope->doNotTreatPhpDocTypesAsCertain()->filterByTruthyValue($node->left),
$node->right
$rightScope->doNotTreatPhpDocTypesAsCertain(),
$originalNode->right
);
if ($booleanNativeType instanceof ConstantBooleanType) {
return $ruleErrorBuilder;
Expand All @@ -79,18 +94,18 @@ public function processNode(
$errors[] = $addTipRight(RuleErrorBuilder::message(sprintf(
'Right side of && is always %s.',
$rightType->getValue() ? 'true' : 'false'
)))->line($node->right->getLine())->build();
)))->line($originalNode->right->getLine())->build();
}

if (count($errors) === 0) {
$nodeType = $scope->getType($node);
$nodeType = $scope->getType($originalNode);
if ($nodeType instanceof ConstantBooleanType) {
$addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node, $tipText): RuleErrorBuilder {
$addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $originalNode, $tipText): RuleErrorBuilder {
if (!$this->treatPhpDocTypesAsCertain) {
return $ruleErrorBuilder;
}

$booleanNativeType = $scope->doNotTreatPhpDocTypesAsCertain()->getType($node);
$booleanNativeType = $scope->doNotTreatPhpDocTypesAsCertain()->getType($originalNode);
if ($booleanNativeType instanceof ConstantBooleanType) {
return $ruleErrorBuilder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ protected function getRule(): \PHPStan\Rules\Rule
),
$this->treatPhpDocTypesAsCertain
),
$this->treatPhpDocTypesAsCertain
$this->treatPhpDocTypesAsCertain,
true
);
}

Expand Down

0 comments on commit 40a76e8

Please sign in to comment.