From 6ebf2361a3c831dd105a815521889428c295dc9f Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Thu, 28 Apr 2022 09:16:44 +0200 Subject: [PATCH] Bleeding edge level 4 - ConstantLooseComparisonRule --- conf/bleedingEdge.neon | 1 + conf/config.level4.neon | 9 +++ conf/config.neon | 4 ++ .../ConstantLooseComparisonRule.php | 65 +++++++++++++++++++ .../ConstantLooseComparisonRuleTest.php | 47 ++++++++++++++ .../Comparison/data/loose-comparison.php | 25 +++++++ 6 files changed, 151 insertions(+) create mode 100644 src/Rules/Comparison/ConstantLooseComparisonRule.php create mode 100644 tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php create mode 100644 tests/PHPStan/Rules/Comparison/data/loose-comparison.php diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 2ec54b5985..cec6f561c3 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -12,3 +12,4 @@ parameters: illegalConstructorMethodCall: true disableCheckMissingIterableValueType: true strictUnnecessaryNullsafePropertyFetch: true + looseComparison: true diff --git a/conf/config.level4.neon b/conf/config.level4.neon index a0912191b3..5464be9e01 100644 --- a/conf/config.level4.neon +++ b/conf/config.level4.neon @@ -19,6 +19,10 @@ rules: - PHPStan\Rules\TooWideTypehints\TooWideClosureReturnTypehintRule - PHPStan\Rules\TooWideTypehints\TooWideFunctionReturnTypehintRule +conditionalTags: + PHPStan\Rules\Comparison\ConstantLooseComparisonRule: + phpstan.rules.rule: %featureToggles.looseComparison% + parameters: checkAdvancedIsset: true @@ -120,6 +124,11 @@ services: tags: - phpstan.rules.rule + - + class: PHPStan\Rules\Comparison\ConstantLooseComparisonRule + arguments: + checkAlwaysTrueLooseComparison: %checkAlwaysTrueLooseComparison% + - class: PHPStan\Rules\Comparison\TernaryOperatorConstantConditionRule arguments: diff --git a/conf/config.neon b/conf/config.neon index 88a6568232..84a957bf87 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -36,12 +36,14 @@ parameters: illegalConstructorMethodCall: false disableCheckMissingIterableValueType: false strictUnnecessaryNullsafePropertyFetch: false + looseComparison: false fileExtensions: - php checkAdvancedIsset: false checkAlwaysTrueCheckTypeFunctionCall: false checkAlwaysTrueInstanceof: false checkAlwaysTrueStrictComparison: false + checkAlwaysTrueLooseComparison: false checkClassCaseSensitivity: false checkExplicitMixed: false checkFunctionArgumentTypes: false @@ -228,12 +230,14 @@ parametersSchema: illegalConstructorMethodCall: bool(), disableCheckMissingIterableValueType: bool(), strictUnnecessaryNullsafePropertyFetch: bool(), + looseComparison: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() checkAlwaysTrueCheckTypeFunctionCall: bool() checkAlwaysTrueInstanceof: bool() checkAlwaysTrueStrictComparison: bool() + checkAlwaysTrueLooseComparison: bool() checkClassCaseSensitivity: bool() checkExplicitMixed: bool() checkFunctionArgumentTypes: bool() diff --git a/src/Rules/Comparison/ConstantLooseComparisonRule.php b/src/Rules/Comparison/ConstantLooseComparisonRule.php new file mode 100644 index 0000000000..78766f823f --- /dev/null +++ b/src/Rules/Comparison/ConstantLooseComparisonRule.php @@ -0,0 +1,65 @@ + + */ +class ConstantLooseComparisonRule implements Rule +{ + + public function __construct(private bool $checkAlwaysTrueLooseComparison) + { + } + + public function getNodeType(): string + { + return Node\Expr\BinaryOp::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node instanceof Node\Expr\BinaryOp\Equal && !$node instanceof Node\Expr\BinaryOp\NotEqual) { + return []; + } + + $nodeType = $scope->getType($node); + if (!$nodeType instanceof ConstantBooleanType) { + return []; + } + + $leftType = $scope->getType($node->left); + $rightType = $scope->getType($node->right); + + if (!$nodeType->getValue()) { + return [ + RuleErrorBuilder::message(sprintf( + 'Loose comparison using %s between %s and %s will always evaluate to false.', + $node instanceof Node\Expr\BinaryOp\Equal ? '==' : '!=', + $leftType->describe(VerbosityLevel::value()), + $rightType->describe(VerbosityLevel::value()), + ))->build(), + ]; + } elseif ($this->checkAlwaysTrueLooseComparison) { + return [ + RuleErrorBuilder::message(sprintf( + 'Loose comparison using %s between %s and %s will always evaluate to true.', + $node instanceof Node\Expr\BinaryOp\Equal ? '==' : '!=', + $leftType->describe(VerbosityLevel::value()), + $rightType->describe(VerbosityLevel::value()), + ))->build(), + ]; + } + + return []; + } + +} diff --git a/tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php b/tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php new file mode 100644 index 0000000000..2234e05ba9 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php @@ -0,0 +1,47 @@ + + */ +class ConstantLooseComparisonRuleTest extends RuleTestCase +{ + + private bool $checkAlwaysTrueStrictComparison; + + protected function getRule(): Rule + { + return new ConstantLooseComparisonRule($this->checkAlwaysTrueStrictComparison); + } + + public function testRule(): void + { + $this->checkAlwaysTrueStrictComparison = false; + $this->analyse([__DIR__ . '/data/loose-comparison.php'], [ + [ + "Loose comparison using == between 0 and '1' will always evaluate to false.", + 20, + ], + ]); + } + + public function testRuleAlwaysTrue(): void + { + $this->checkAlwaysTrueStrictComparison = true; + $this->analyse([__DIR__ . '/data/loose-comparison.php'], [ + [ + "Loose comparison using == between 0 and '0' will always evaluate to true.", + 16, + ], + [ + "Loose comparison using == between 0 and '1' will always evaluate to false.", + 20, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Comparison/data/loose-comparison.php b/tests/PHPStan/Rules/Comparison/data/loose-comparison.php new file mode 100644 index 0000000000..7e3b1b194d --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/loose-comparison.php @@ -0,0 +1,25 @@ +