Skip to content

Commit

Permalink
Bleeding edge - LogicalXorConstantConditionRule
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Dec 7, 2023
1 parent 0a3a968 commit 3a12724
Show file tree
Hide file tree
Showing 8 changed files with 211 additions and 0 deletions.
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ parameters:
unescapeStrings: true
alwaysCheckTooWideReturnTypeFinalMethods: true
duplicateStubs: true
logicalXor: true
invarianceComposition: true
alwaysTrueAlwaysReported: true
disableUnreachableBranchesRules: true
Expand Down
8 changes: 8 additions & 0 deletions conf/config.level4.neon
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ conditionalTags:
phpstan.collector: %featureToggles.notAnalysedTrait%
PHPStan\Rules\Traits\NotAnalysedTraitRule:
phpstan.rules.rule: %featureToggles.notAnalysedTrait%
PHPStan\Rules\Comparison\LogicalXorConstantConditionRule:
phpstan.rules.rule: %featureToggles.logicalXor%

parameters:
checkAdvancedIsset: true
Expand Down Expand Up @@ -124,6 +126,12 @@ services:
tags:
- phpstan.rules.rule

-
class: PHPStan\Rules\Comparison\LogicalXorConstantConditionRule
arguments:
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
reportAlwaysTrueInLastCondition: %reportAlwaysTrueInLastCondition%

-
class: PHPStan\Rules\Comparison\MatchExpressionRule
arguments:
Expand Down
1 change: 1 addition & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ parameters:
unescapeStrings: false
alwaysCheckTooWideReturnTypeFinalMethods: false
duplicateStubs: false
logicalXor: false
invarianceComposition: false
alwaysTrueAlwaysReported: false
disableUnreachableBranchesRules: false
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ parametersSchema:
unescapeStrings: bool()
alwaysCheckTooWideReturnTypeFinalMethods: bool()
duplicateStubs: bool()
logicalXor: bool()
invarianceComposition: bool()
alwaysTrueAlwaysReported: bool()
disableUnreachableBranchesRules: bool()
Expand Down
5 changes: 5 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,11 @@ parameters:
count: 1
path: src/Rules/Comparison/ImpossibleCheckTypeHelper.php

-
message: "#^Doing instanceof PHPStan\\\\Type\\\\Constant\\\\ConstantBooleanType is error\\-prone and deprecated\\. Use Type\\:\\:isTrue\\(\\) or Type\\:\\:isFalse\\(\\) instead\\.$#"
count: 4
path: src/Rules/Comparison/LogicalXorConstantConditionRule.php

-
message: "#^Doing instanceof PHPStan\\\\Type\\\\Constant\\\\ConstantBooleanType is error\\-prone and deprecated\\. Use Type\\:\\:isTrue\\(\\) or Type\\:\\:isFalse\\(\\) instead\\.$#"
count: 4
Expand Down
99 changes: 99 additions & 0 deletions src/Rules/Comparison/LogicalXorConstantConditionRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Comparison;

use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp\LogicalXor;
use PHPStan\Analyser\Scope;
use PHPStan\Parser\LastConditionVisitor;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\Constant\ConstantBooleanType;
use function sprintf;

/**
* @implements Rule<LogicalXor>
*/
class LogicalXorConstantConditionRule implements Rule
{

public function __construct(
private ConstantConditionRuleHelper $helper,
private bool $treatPhpDocTypesAsCertain,
private bool $reportAlwaysTrueInLastCondition,
)
{
}

public function getNodeType(): string
{
return LogicalXor::class;
}

public function processNode(Node $node, Scope $scope): array
{
$errors = [];
$leftType = $this->helper->getBooleanType($scope, $node->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, $tipText, $node): RuleErrorBuilder {
if (!$this->treatPhpDocTypesAsCertain) {
return $ruleErrorBuilder;
}

$booleanNativeType = $this->helper->getNativeBooleanType($scope, $node->left);
if ($booleanNativeType instanceof ConstantBooleanType) {
return $ruleErrorBuilder;
}

return $ruleErrorBuilder->tip($tipText);
};

$isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME);
if (!$leftType->getValue() || $isLast !== true || $this->reportAlwaysTrueInLastCondition) {
$errorBuilder = $addTipLeft(RuleErrorBuilder::message(sprintf(
'Left side of xor is always %s.',
$leftType->getValue() ? 'true' : 'false',
)))->line($node->left->getLine());
if ($leftType->getValue() && $isLast === false && !$this->reportAlwaysTrueInLastCondition) {
$errorBuilder->tip('Remove remaining cases below this one and this error will disappear too.');
}
$errors[] = $errorBuilder->build();
}
}

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

$booleanNativeType = $this->helper->getNativeBooleanType(
$scope,
$node->right,
);
if ($booleanNativeType instanceof ConstantBooleanType) {
return $ruleErrorBuilder;
}

return $ruleErrorBuilder->tip($tipText);
};

$isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME);
if (!$rightType->getValue() || $isLast !== true || $this->reportAlwaysTrueInLastCondition) {
$errorBuilder = $addTipRight(RuleErrorBuilder::message(sprintf(
'Right side of xor is always %s.',
$rightType->getValue() ? 'true' : 'false',
)))->line($node->right->getLine());
if ($rightType->getValue() && $isLast === false && !$this->reportAlwaysTrueInLastCondition) {
$errorBuilder->tip('Remove remaining cases below this one and this error will disappear too.');
}
$errors[] = $errorBuilder->build();
}
}

return $errors;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Comparison;

use PHPStan\Rules\Rule as TRule;
use PHPStan\Testing\RuleTestCase;

/**
* @extends RuleTestCase<LogicalXorConstantConditionRule>
*/
class LogicalXorConstantConditionRuleTest extends RuleTestCase
{

private bool $treatPhpDocTypesAsCertain;

private bool $reportAlwaysTrueInLastCondition = false;

protected function getRule(): TRule
{
return new LogicalXorConstantConditionRule(
new ConstantConditionRuleHelper(
new ImpossibleCheckTypeHelper(
$this->createReflectionProvider(),
$this->getTypeSpecifier(),
[],
$this->treatPhpDocTypesAsCertain,
true,
),
$this->treatPhpDocTypesAsCertain,
true,
),
$this->treatPhpDocTypesAsCertain,
$this->reportAlwaysTrueInLastCondition,
);
}

public function testRule(): void
{
$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%</>.';
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/logical-xor.php'], [
[
'Left side of xor is always true.',
14,
],
[
'Right side of xor is always false.',
14,
],
[
'Left side of xor is always false.',
17,
],
[
'Right side of xor is always true.',
17,
],
[
'Left side of xor is always true.',
20,
$tipText,
],
[
'Right side of xor is always true.',
20,
$tipText,
],
]);
}

}
25 changes: 25 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/logical-xor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

namespace ConstantLogicalXor;

class Foo
{

/**
* @param object $a
* @param object $b
*/
public function doFoo($a, $b)
{
if (1 xor 0) {

}
if (0 xor 1) {

}
if ($a xor $b) {

}
}

}

0 comments on commit 3a12724

Please sign in to comment.