From 87417a7f76e1d38b84be464a17faba861584f00b Mon Sep 17 00:00:00 2001 From: Matthias Nagel Date: Sun, 12 Jun 2022 18:47:41 +0200 Subject: [PATCH 1/8] Added rule to disallow weak comparisons --- rules.neon | 1 + .../DisallowedWeakComparisonRule.php | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php diff --git a/rules.neon b/rules.neon index 48011c37..219bbc32 100644 --- a/rules.neon +++ b/rules.neon @@ -27,6 +27,7 @@ rules: - PHPStan\Rules\DisallowedConstructs\DisallowedEmptyRule - PHPStan\Rules\DisallowedConstructs\DisallowedImplicitArrayCreationRule - PHPStan\Rules\DisallowedConstructs\DisallowedShortTernaryRule + - PHPStan\Rules\DisallowedConstructs\DisallowedWeakComparisonRule - PHPStan\Rules\ForeachLoop\OverwriteVariablesWithForeachRule - PHPStan\Rules\ForLoop\OverwriteVariablesWithForLoopInitRule - PHPStan\Rules\Functions\ClosureUsesThisRule diff --git a/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php b/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php new file mode 100644 index 00000000..9059030b --- /dev/null +++ b/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php @@ -0,0 +1,34 @@ + + */ +class DisallowedWeakComparisonRule implements Rule +{ + + public function getNodeType(): string + { + return BinaryOp::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if ($node instanceof Equal || $node instanceof NotEqual) { + return [ + 'Weak comparison via "' . $node->getOperatorSigil() . '" is not allowed. Use strong comparison.', + ]; + } else { + return []; + } + } + +} From 98202c4291d14ee1e9311c38b303ec2ad099a341 Mon Sep 17 00:00:00 2001 From: Matthias Nagel Date: Sun, 12 Jun 2022 18:54:14 +0200 Subject: [PATCH 2/8] Code style --- .../DisallowedConstructs/DisallowedWeakComparisonRule.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php b/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php index 9059030b..36021039 100644 --- a/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php +++ b/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php @@ -26,9 +26,9 @@ public function processNode(Node $node, Scope $scope): array return [ 'Weak comparison via "' . $node->getOperatorSigil() . '" is not allowed. Use strong comparison.', ]; - } else { - return []; } + + return []; } } From c19efb5e166d55f56443435545cbd134525b9b2a Mon Sep 17 00:00:00 2001 From: Matthias Nagel Date: Mon, 13 Jun 2022 10:43:08 +0200 Subject: [PATCH 3/8] Added test --- .../DisallowedWeakComparisonRule.php | 2 +- .../DisallowedWeakComparisonRuleTest.php | 29 +++++++++++++++++++ .../data/weak-comparison.php | 6 ++++ 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php create mode 100644 tests/Rules/DisallowedConstructs/data/weak-comparison.php diff --git a/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php b/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php index 36021039..504341a9 100644 --- a/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php +++ b/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php @@ -24,7 +24,7 @@ public function processNode(Node $node, Scope $scope): array { if ($node instanceof Equal || $node instanceof NotEqual) { return [ - 'Weak comparison via "' . $node->getOperatorSigil() . '" is not allowed. Use strong comparison.', + 'Weak comparison via "' . $node->getOperatorSigil() . '" is not allowed. Use strong comparison instead.', ]; } diff --git a/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php b/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php new file mode 100644 index 00000000..52936425 --- /dev/null +++ b/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php @@ -0,0 +1,29 @@ +analyse([__DIR__ . '/data/weak-comparison.php'], [ + [ + 'Weak comparison via "==" is not allowed. Use strong comparison instead.', + 3, + ],[ + 'Weak comparison via "!=" is not allowed. Use strong comparison instead.', + 5, + ], + ]); + } + +} diff --git a/tests/Rules/DisallowedConstructs/data/weak-comparison.php b/tests/Rules/DisallowedConstructs/data/weak-comparison.php new file mode 100644 index 00000000..2e7e3a97 --- /dev/null +++ b/tests/Rules/DisallowedConstructs/data/weak-comparison.php @@ -0,0 +1,6 @@ + Date: Mon, 13 Jun 2022 10:49:25 +0200 Subject: [PATCH 4/8] Added PhpStan errors in test class to baseline. --- phpstan-baseline.neon | 10 ++++++++++ .../DisallowedWeakComparisonRuleTest.php | 3 ++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 327481a5..ba74deef 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -342,6 +342,16 @@ parameters: count: 1 path: tests/Rules/DisallowedConstructs/DisallowedImplicitArrayCreationRuleTest.php + - + message: "#^Class PHPStan\\\\Rules\\\\DisallowedConstructs\\\\DisallowedWeakComparisonRuleTest extends generic class PHPStan\\\\Testing\\\\RuleTestCase but does not specify its types\\: TRule$#" + count: 1 + path: tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php + + - + message: "#^Method PHPStan\\\\Rules\\\\DisallowedConstructs\\\\DisallowedWeakComparisonRuleTest\\:\\:getRule\\(\\) return type with generic interface PHPStan\\\\Rules\\\\Rule does not specify its types\\: TNodeType$#" + count: 1 + path: tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php + - message: "#^Class PHPStan\\\\Rules\\\\ForeachLoop\\\\OverwriteVariablesWithForeachRuleTest extends generic class PHPStan\\\\Testing\\\\RuleTestCase but does not specify its types\\: TRule$#" count: 1 diff --git a/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php b/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php index 52936425..9c464d6e 100644 --- a/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php +++ b/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php @@ -19,7 +19,8 @@ public function testRule(): void [ 'Weak comparison via "==" is not allowed. Use strong comparison instead.', 3, - ],[ + ], + [ 'Weak comparison via "!=" is not allowed. Use strong comparison instead.', 5, ], From 02152d28916218c620907e2dc074a93900555fbd Mon Sep 17 00:00:00 2001 From: Matthias Nagel Date: Mon, 13 Jun 2022 11:04:55 +0200 Subject: [PATCH 5/8] Reverted PhpStan baseline and added template parameter to test class --- phpstan-baseline.neon | 10 ---------- .../DisallowedWeakComparisonRuleTest.php | 3 +++ 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index ba74deef..327481a5 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -342,16 +342,6 @@ parameters: count: 1 path: tests/Rules/DisallowedConstructs/DisallowedImplicitArrayCreationRuleTest.php - - - message: "#^Class PHPStan\\\\Rules\\\\DisallowedConstructs\\\\DisallowedWeakComparisonRuleTest extends generic class PHPStan\\\\Testing\\\\RuleTestCase but does not specify its types\\: TRule$#" - count: 1 - path: tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php - - - - message: "#^Method PHPStan\\\\Rules\\\\DisallowedConstructs\\\\DisallowedWeakComparisonRuleTest\\:\\:getRule\\(\\) return type with generic interface PHPStan\\\\Rules\\\\Rule does not specify its types\\: TNodeType$#" - count: 1 - path: tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php - - message: "#^Class PHPStan\\\\Rules\\\\ForeachLoop\\\\OverwriteVariablesWithForeachRuleTest extends generic class PHPStan\\\\Testing\\\\RuleTestCase but does not specify its types\\: TRule$#" count: 1 diff --git a/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php b/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php index 9c464d6e..e49f0ec1 100644 --- a/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php +++ b/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php @@ -5,6 +5,9 @@ use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; +/** + * @extends RuleTestCase + */ class DisallowedWeakComparisonRuleTest extends RuleTestCase { From 56a7475d7185c829770fc9d1e721c5a3aaadceb1 Mon Sep 17 00:00:00 2001 From: Matthias Nagel Date: Mon, 13 Jun 2022 21:03:47 +0200 Subject: [PATCH 6/8] Applied suggested changes from code review. --- rules.neon | 8 +++++++- .../DisallowedWeakComparisonRule.php | 14 ++++++++++++-- .../DisallowedWeakComparisonRuleTest.php | 6 ++++-- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/rules.neon b/rules.neon index 219bbc32..445ffe79 100644 --- a/rules.neon +++ b/rules.neon @@ -27,7 +27,6 @@ rules: - PHPStan\Rules\DisallowedConstructs\DisallowedEmptyRule - PHPStan\Rules\DisallowedConstructs\DisallowedImplicitArrayCreationRule - PHPStan\Rules\DisallowedConstructs\DisallowedShortTernaryRule - - PHPStan\Rules\DisallowedConstructs\DisallowedWeakComparisonRule - PHPStan\Rules\ForeachLoop\OverwriteVariablesWithForeachRule - PHPStan\Rules\ForLoop\OverwriteVariablesWithForLoopInitRule - PHPStan\Rules\Functions\ClosureUsesThisRule @@ -53,6 +52,10 @@ rules: - PHPStan\Rules\VariableVariables\VariableStaticPropertyFetchRule - PHPStan\Rules\VariableVariables\VariableVariablesRule +conditionalTags: + PHPStan\Rules\DisallowedConstructs\DisallowedWeakComparisonRule: + phpstan.rules.rule: %featureToggles.bleedingEdge% + services: - class: PHPStan\Rules\BooleansInConditions\BooleanRuleHelper @@ -72,3 +75,6 @@ services: universalObjectCratesClasses: %universalObjectCratesClasses% tags: - phpstan.rules.rule + + - + class: PHPStan\Rules\DisallowedConstructs\DisallowedWeakComparisonRule diff --git a/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php b/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php index 504341a9..fc8a2e7f 100644 --- a/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php +++ b/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php @@ -8,6 +8,7 @@ use PhpParser\Node\Expr\BinaryOp\NotEqual; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleErrorBuilder; /** * @implements Rule @@ -22,9 +23,18 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - if ($node instanceof Equal || $node instanceof NotEqual) { + if ($node instanceof Equal) { return [ - 'Weak comparison via "' . $node->getOperatorSigil() . '" is not allowed. Use strong comparison instead.', + RuleErrorBuilder::message( + 'Loose comparison via "==" is not allowed.' + )->tip('Use strict comparison via "===" instead.')->build(), + ]; + } + if ($node instanceof NotEqual) { + return [ + RuleErrorBuilder::message( + 'Loose comparison via "!=" is not allowed.' + )->tip('Use strict comparison via "!==" instead.')->build(), ]; } diff --git a/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php b/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php index e49f0ec1..533b9e92 100644 --- a/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php +++ b/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php @@ -20,12 +20,14 @@ public function testRule(): void { $this->analyse([__DIR__ . '/data/weak-comparison.php'], [ [ - 'Weak comparison via "==" is not allowed. Use strong comparison instead.', + 'Loose comparison via "==" is not allowed.', 3, + 'Use strict comparison via "===" instead.', ], [ - 'Weak comparison via "!=" is not allowed. Use strong comparison instead.', + 'Loose comparison via "!=" is not allowed.', 5, + 'Use strict comparison via "!==" instead.', ], ]); } From e2401eb71290556d4cf5b6ca466575909aaea448 Mon Sep 17 00:00:00 2001 From: Matthias Nagel Date: Wed, 15 Jun 2022 17:05:50 +0200 Subject: [PATCH 7/8] Renamed classes and files --- ...ComparisonRule.php => DisallowedLooseComparisonRule.php} | 2 +- ...onRuleTest.php => DisallowedLooseComparisonRuleTest.php} | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) rename src/Rules/DisallowedConstructs/{DisallowedWeakComparisonRule.php => DisallowedLooseComparisonRule.php} (94%) rename tests/Rules/DisallowedConstructs/{DisallowedWeakComparisonRuleTest.php => DisallowedLooseComparisonRuleTest.php} (76%) diff --git a/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php b/src/Rules/DisallowedConstructs/DisallowedLooseComparisonRule.php similarity index 94% rename from src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php rename to src/Rules/DisallowedConstructs/DisallowedLooseComparisonRule.php index fc8a2e7f..a1425c76 100644 --- a/src/Rules/DisallowedConstructs/DisallowedWeakComparisonRule.php +++ b/src/Rules/DisallowedConstructs/DisallowedLooseComparisonRule.php @@ -13,7 +13,7 @@ /** * @implements Rule */ -class DisallowedWeakComparisonRule implements Rule +class DisallowedLooseComparisonRule implements Rule { public function getNodeType(): string diff --git a/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php b/tests/Rules/DisallowedConstructs/DisallowedLooseComparisonRuleTest.php similarity index 76% rename from tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php rename to tests/Rules/DisallowedConstructs/DisallowedLooseComparisonRuleTest.php index 533b9e92..9e56b583 100644 --- a/tests/Rules/DisallowedConstructs/DisallowedWeakComparisonRuleTest.php +++ b/tests/Rules/DisallowedConstructs/DisallowedLooseComparisonRuleTest.php @@ -6,14 +6,14 @@ use PHPStan\Testing\RuleTestCase; /** - * @extends RuleTestCase + * @extends RuleTestCase */ -class DisallowedWeakComparisonRuleTest extends RuleTestCase +class DisallowedLooseComparisonRuleTest extends RuleTestCase { protected function getRule(): Rule { - return new DisallowedWeakComparisonRule(); + return new DisallowedLooseComparisonRule(); } public function testRule(): void From ecc05ce03c13b372ae0e944755eb7047f122af86 Mon Sep 17 00:00:00 2001 From: Matthias Nagel Date: Wed, 15 Jun 2022 17:10:47 +0200 Subject: [PATCH 8/8] Renamed forgotten references --- rules.neon | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rules.neon b/rules.neon index 445ffe79..d70a9111 100644 --- a/rules.neon +++ b/rules.neon @@ -53,7 +53,7 @@ rules: - PHPStan\Rules\VariableVariables\VariableVariablesRule conditionalTags: - PHPStan\Rules\DisallowedConstructs\DisallowedWeakComparisonRule: + PHPStan\Rules\DisallowedConstructs\DisallowedLooseComparisonRule: phpstan.rules.rule: %featureToggles.bleedingEdge% services: @@ -77,4 +77,4 @@ services: - phpstan.rules.rule - - class: PHPStan\Rules\DisallowedConstructs\DisallowedWeakComparisonRule + class: PHPStan\Rules\DisallowedConstructs\DisallowedLooseComparisonRule