Skip to content

Commit b94a0b1

Browse files
mpesarimpesari
authored
Bleeding edge - Add operand types to DisallowedLooseComparisonRule error message
1 parent f6f5b3e commit b94a0b1

5 files changed

Lines changed: 130 additions & 13 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
| `switchConditionsMatchingType` | Types in `switch` condition and `case` value must match. PHP compares them loosely by default and that can lead to unexpected results. |
1717
| `dynamicCallOnStaticMethod` | Check that statically declared methods are called statically. |
1818
| `disallowedEmpty` | Disallow `empty()` - it's a very loose comparison (see [manual](https://php.net/empty)), it's recommended to use more strict one. |
19+
| `disallowedLooseComparison` | Disallow loose comparison via `==` and `!=`. |
1920
| `disallowedShortTernary` | Disallow short ternary operator (`?:`) - implies weak comparison, it's recommended to use null coalesce operator (`??`) or ternary operator with strict condition. |
2021
| `noVariableVariables` | Disallow variable variables (`$$foo`, `$this->$method()` etc.). |
2122
| `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. |

‎rules.neon‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,8 @@ services:
164164

165165
-
166166
class: PHPStan\Rules\DisallowedConstructs\DisallowedLooseComparisonRule
167+
arguments:
168+
includeOperandTypesInErrorMessage: %featureToggles.bleedingEdge%
167169

168170
-
169171
class: PHPStan\Rules\BooleansInConditions\BooleanInBooleanAndRule

‎src/Rules/DisallowedConstructs/DisallowedLooseComparisonRule.php‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,40 +9,57 @@
99
use PHPStan\Analyser\Scope;
1010
use PHPStan\Rules\Rule;
1111
use PHPStan\Rules\RuleErrorBuilder;
12+
use PHPStan\Type\VerbosityLevel;
13+
use function sprintf;
1214

1315
/**
1416
* @implements Rule<BinaryOp>
1517
*/
1618
class DisallowedLooseComparisonRule implements Rule
1719
{
1820

21+
private bool $includeOperandTypesInErrorMessage;
22+
23+
public function __construct(bool $includeOperandTypesInErrorMessage)
24+
{
25+
$this->includeOperandTypesInErrorMessage = $includeOperandTypesInErrorMessage;
26+
}
27+
1928
public function getNodeType(): string
2029
{
2130
return BinaryOp::class;
2231
}
2332

2433
public function processNode(Node $node, Scope $scope): array
2534
{
35+
if (!$node instanceof Equal && !$node instanceof NotEqual) {
36+
return [];
37+
}
38+
39+
$left = $scope->getType($node->left)->describe(VerbosityLevel::typeOnly());
40+
$right = $scope->getType($node->right)->describe(VerbosityLevel::typeOnly());
41+
2642
if ($node instanceof Equal) {
2743
return [
2844
RuleErrorBuilder::message(
29-
'Loose comparison via "==" is not allowed.',
45+
$this->includeOperandTypesInErrorMessage
46+
? sprintf('Loose comparison via "==" between %s and %s is not allowed.', $left, $right)
47+
: 'Loose comparison via "==" is not allowed.',
3048
)->tip('Use strict comparison via "===" instead.')
3149
->identifier('equal.notAllowed')
3250
->build(),
3351
];
3452
}
35-
if ($node instanceof NotEqual) {
36-
return [
37-
RuleErrorBuilder::message(
38-
'Loose comparison via "!=" is not allowed.',
39-
)->tip('Use strict comparison via "!==" instead.')
40-
->identifier('notEqual.notAllowed')
41-
->build(),
42-
];
43-
}
4453

45-
return [];
54+
return [
55+
RuleErrorBuilder::message(
56+
$this->includeOperandTypesInErrorMessage
57+
? sprintf('Loose comparison via "!=" between %s and %s is not allowed.', $left, $right)
58+
: 'Loose comparison via "!=" is not allowed.',
59+
)->tip('Use strict comparison via "!==" instead.')
60+
->identifier('notEqual.notAllowed')
61+
->build(),
62+
];
4663
}
4764

4865
}

‎tests/Rules/DisallowedConstructs/DisallowedLooseComparisonRuleTest.php‎

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,18 @@
1111
class DisallowedLooseComparisonRuleTest extends RuleTestCase
1212
{
1313

14+
private bool $includeOperandTypesInErrorMessage;
15+
1416
protected function getRule(): Rule
1517
{
16-
return new DisallowedLooseComparisonRule();
18+
return new DisallowedLooseComparisonRule(
19+
$this->includeOperandTypesInErrorMessage,
20+
);
1721
}
1822

19-
public function testRule(): void
23+
public function testRuleWithoutOperandTypesInErrorMessage(): void
2024
{
25+
$this->includeOperandTypesInErrorMessage = false;
2126
$this->analyse([__DIR__ . '/data/weak-comparison.php'], [
2227
[
2328
'Loose comparison via "==" is not allowed.',
@@ -29,6 +34,83 @@ public function testRule(): void
2934
5,
3035
'Use strict comparison via "!==" instead.',
3136
],
37+
[
38+
'Loose comparison via "==" is not allowed.',
39+
8,
40+
'Use strict comparison via "===" instead.',
41+
],
42+
[
43+
'Loose comparison via "!=" is not allowed.',
44+
10,
45+
'Use strict comparison via "!==" instead.',
46+
],
47+
[
48+
'Loose comparison via "==" is not allowed.',
49+
13,
50+
'Use strict comparison via "===" instead.',
51+
],
52+
[
53+
'Loose comparison via "!=" is not allowed.',
54+
15,
55+
'Use strict comparison via "!==" instead.',
56+
],
57+
[
58+
'Loose comparison via "==" is not allowed.',
59+
18,
60+
'Use strict comparison via "===" instead.',
61+
],
62+
[
63+
'Loose comparison via "!=" is not allowed.',
64+
20,
65+
'Use strict comparison via "!==" instead.',
66+
],
67+
]);
68+
}
69+
70+
public function testRuleWithOperandTypesInErrorMessage(): void
71+
{
72+
$this->includeOperandTypesInErrorMessage = true;
73+
$this->analyse([__DIR__ . '/data/weak-comparison.php'], [
74+
[
75+
'Loose comparison via "==" between int and int is not allowed.',
76+
3,
77+
'Use strict comparison via "===" instead.',
78+
],
79+
[
80+
'Loose comparison via "!=" between int and int is not allowed.',
81+
5,
82+
'Use strict comparison via "!==" instead.',
83+
],
84+
[
85+
'Loose comparison via "==" between DateTime and float is not allowed.',
86+
8,
87+
'Use strict comparison via "===" instead.',
88+
],
89+
[
90+
'Loose comparison via "!=" between float and DateTime is not allowed.',
91+
10,
92+
'Use strict comparison via "!==" instead.',
93+
],
94+
[
95+
'Loose comparison via "==" between DateTime and null is not allowed.',
96+
13,
97+
'Use strict comparison via "===" instead.',
98+
],
99+
[
100+
'Loose comparison via "!=" between null and DateTime is not allowed.',
101+
15,
102+
'Use strict comparison via "!==" instead.',
103+
],
104+
[
105+
'Loose comparison via "==" between DateTime and DateTime is not allowed.',
106+
18,
107+
'Use strict comparison via "===" instead.',
108+
],
109+
[
110+
'Loose comparison via "!=" between DateTime and DateTime is not allowed.',
111+
20,
112+
'Use strict comparison via "!==" instead.',
113+
],
32114
]);
33115
}
34116

‎tests/Rules/DisallowedConstructs/data/weak-comparison.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,18 @@
44
$bool2 = 123 === 456;
55
$bool3 = 123 != 456;
66
$bool4 = 123 !== 456;
7+
8+
$bool5 = new DateTime('now') == 1.23;
9+
$bool6 = new DateTime('now') === 1.23;
10+
$bool7 = 1.23 != new DateTime('now');
11+
$bool8 = 1.23 !== new DateTime('now');
12+
13+
$bool9 = new DateTime('now') == null;
14+
$bool10 = new DateTime('now') === null;
15+
$bool11 = null != new DateTime('now');
16+
$bool12 = null !== new DateTime('now');
17+
18+
$bool13 = new DateTime('now') == new DateTime('now');
19+
$bool14 = new DateTime('now') === new DateTime('now');
20+
$bool15 = new DateTime('now') != new DateTime('now');
21+
$bool16 = new DateTime('now') !== new DateTime('now');

0 commit comments

Comments
 (0)