diff --git a/README.md b/README.md index ee04c34b..caef824f 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,7 @@ | `switchConditionsMatchingType` | Types in `switch` condition and `case` value must match. PHP compares them loosely by default and that can lead to unexpected results. | | `dynamicCallOnStaticMethod` | Check that statically declared methods are called statically. | | `disallowedEmpty` | Disallow `empty()` - it's a very loose comparison (see [manual](https://php.net/empty)), it's recommended to use more strict one. | +| `disallowedLooseComparison` | Disallow loose comparison via `==` and `!=`. | | `disallowedShortTernary` | Disallow short ternary operator (`?:`) - implies weak comparison, it's recommended to use null coalesce operator (`??`) or ternary operator with strict condition. | | `noVariableVariables` | Disallow variable variables (`$$foo`, `$this->$method()` etc.). | | `checkAlwaysTrueInstanceof`, `checkAlwaysTrueCheckTypeFunctionCall`, `checkAlwaysTrueStrictComparison` | Always true `instanceof`, type-checking `is_*` functions and strict comparisons `===`/`!==`. These checks can be turned off by setting `checkAlwaysTrueInstanceof`, `checkAlwaysTrueCheckTypeFunctionCall` and `checkAlwaysTrueStrictComparison` to false. | diff --git a/rules.neon b/rules.neon index 0def6d87..e33fbaa6 100644 --- a/rules.neon +++ b/rules.neon @@ -164,6 +164,8 @@ services: - class: PHPStan\Rules\DisallowedConstructs\DisallowedLooseComparisonRule + arguments: + includeOperandTypesInErrorMessage: %featureToggles.bleedingEdge% - class: PHPStan\Rules\BooleansInConditions\BooleanInBooleanAndRule diff --git a/src/Rules/DisallowedConstructs/DisallowedLooseComparisonRule.php b/src/Rules/DisallowedConstructs/DisallowedLooseComparisonRule.php index 70b8514f..4eed0c5e 100644 --- a/src/Rules/DisallowedConstructs/DisallowedLooseComparisonRule.php +++ b/src/Rules/DisallowedConstructs/DisallowedLooseComparisonRule.php @@ -9,6 +9,8 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\Type\VerbosityLevel; +use function sprintf; /** * @implements Rule @@ -16,6 +18,13 @@ class DisallowedLooseComparisonRule implements Rule { + private bool $includeOperandTypesInErrorMessage; + + public function __construct(bool $includeOperandTypesInErrorMessage) + { + $this->includeOperandTypesInErrorMessage = $includeOperandTypesInErrorMessage; + } + public function getNodeType(): string { return BinaryOp::class; @@ -23,26 +32,34 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { + if (!$node instanceof Equal && !$node instanceof NotEqual) { + return []; + } + + $left = $scope->getType($node->left)->describe(VerbosityLevel::typeOnly()); + $right = $scope->getType($node->right)->describe(VerbosityLevel::typeOnly()); + if ($node instanceof Equal) { return [ RuleErrorBuilder::message( - 'Loose comparison via "==" is not allowed.', + $this->includeOperandTypesInErrorMessage + ? sprintf('Loose comparison via "==" between %s and %s is not allowed.', $left, $right) + : 'Loose comparison via "==" is not allowed.', )->tip('Use strict comparison via "===" instead.') ->identifier('equal.notAllowed') ->build(), ]; } - if ($node instanceof NotEqual) { - return [ - RuleErrorBuilder::message( - 'Loose comparison via "!=" is not allowed.', - )->tip('Use strict comparison via "!==" instead.') - ->identifier('notEqual.notAllowed') - ->build(), - ]; - } - return []; + return [ + RuleErrorBuilder::message( + $this->includeOperandTypesInErrorMessage + ? sprintf('Loose comparison via "!=" between %s and %s is not allowed.', $left, $right) + : 'Loose comparison via "!=" is not allowed.', + )->tip('Use strict comparison via "!==" instead.') + ->identifier('notEqual.notAllowed') + ->build(), + ]; } } diff --git a/tests/Rules/DisallowedConstructs/DisallowedLooseComparisonRuleTest.php b/tests/Rules/DisallowedConstructs/DisallowedLooseComparisonRuleTest.php index 9e56b583..87fd3868 100644 --- a/tests/Rules/DisallowedConstructs/DisallowedLooseComparisonRuleTest.php +++ b/tests/Rules/DisallowedConstructs/DisallowedLooseComparisonRuleTest.php @@ -11,13 +11,18 @@ class DisallowedLooseComparisonRuleTest extends RuleTestCase { + private bool $includeOperandTypesInErrorMessage; + protected function getRule(): Rule { - return new DisallowedLooseComparisonRule(); + return new DisallowedLooseComparisonRule( + $this->includeOperandTypesInErrorMessage, + ); } - public function testRule(): void + public function testRuleWithoutOperandTypesInErrorMessage(): void { + $this->includeOperandTypesInErrorMessage = false; $this->analyse([__DIR__ . '/data/weak-comparison.php'], [ [ 'Loose comparison via "==" is not allowed.', @@ -29,6 +34,83 @@ public function testRule(): void 5, 'Use strict comparison via "!==" instead.', ], + [ + 'Loose comparison via "==" is not allowed.', + 8, + 'Use strict comparison via "===" instead.', + ], + [ + 'Loose comparison via "!=" is not allowed.', + 10, + 'Use strict comparison via "!==" instead.', + ], + [ + 'Loose comparison via "==" is not allowed.', + 13, + 'Use strict comparison via "===" instead.', + ], + [ + 'Loose comparison via "!=" is not allowed.', + 15, + 'Use strict comparison via "!==" instead.', + ], + [ + 'Loose comparison via "==" is not allowed.', + 18, + 'Use strict comparison via "===" instead.', + ], + [ + 'Loose comparison via "!=" is not allowed.', + 20, + 'Use strict comparison via "!==" instead.', + ], + ]); + } + + public function testRuleWithOperandTypesInErrorMessage(): void + { + $this->includeOperandTypesInErrorMessage = true; + $this->analyse([__DIR__ . '/data/weak-comparison.php'], [ + [ + 'Loose comparison via "==" between int and int is not allowed.', + 3, + 'Use strict comparison via "===" instead.', + ], + [ + 'Loose comparison via "!=" between int and int is not allowed.', + 5, + 'Use strict comparison via "!==" instead.', + ], + [ + 'Loose comparison via "==" between DateTime and float is not allowed.', + 8, + 'Use strict comparison via "===" instead.', + ], + [ + 'Loose comparison via "!=" between float and DateTime is not allowed.', + 10, + 'Use strict comparison via "!==" instead.', + ], + [ + 'Loose comparison via "==" between DateTime and null is not allowed.', + 13, + 'Use strict comparison via "===" instead.', + ], + [ + 'Loose comparison via "!=" between null and DateTime is not allowed.', + 15, + 'Use strict comparison via "!==" instead.', + ], + [ + 'Loose comparison via "==" between DateTime and DateTime is not allowed.', + 18, + 'Use strict comparison via "===" instead.', + ], + [ + 'Loose comparison via "!=" between DateTime and DateTime is not allowed.', + 20, + 'Use strict comparison via "!==" instead.', + ], ]); } diff --git a/tests/Rules/DisallowedConstructs/data/weak-comparison.php b/tests/Rules/DisallowedConstructs/data/weak-comparison.php index 2e7e3a97..0a6a1671 100644 --- a/tests/Rules/DisallowedConstructs/data/weak-comparison.php +++ b/tests/Rules/DisallowedConstructs/data/weak-comparison.php @@ -4,3 +4,18 @@ $bool2 = 123 === 456; $bool3 = 123 != 456; $bool4 = 123 !== 456; + +$bool5 = new DateTime('now') == 1.23; +$bool6 = new DateTime('now') === 1.23; +$bool7 = 1.23 != new DateTime('now'); +$bool8 = 1.23 !== new DateTime('now'); + +$bool9 = new DateTime('now') == null; +$bool10 = new DateTime('now') === null; +$bool11 = null != new DateTime('now'); +$bool12 = null !== new DateTime('now'); + +$bool13 = new DateTime('now') == new DateTime('now'); +$bool14 = new DateTime('now') === new DateTime('now'); +$bool15 = new DateTime('now') != new DateTime('now'); +$bool16 = new DateTime('now') !== new DateTime('now');