Skip to content

Commit e8572b7

Browse files
mhujermhujer
committed
[ci skip] AssertEqualsIsDiscouragedRule
1 parent 58e52ad commit e8572b7

File tree

5 files changed

+186
-0
lines changed

5 files changed

+186
-0
lines changed

‎README.md

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ It also contains this strict framework-specific rules (can be enabled separately
2222
* Check that you are not using `assertSame()` with `false` as expected value. `assertFalse()` should be used instead.
2323
* Check that you are not using `assertSame()` with `null` as expected value. `assertNull()` should be used instead.
2424
* Check that you are not using `assertSame()` with `count($variable)` as second parameter. `assertCount($variable)` should be used instead.
25+
* Check that you are not using `assertEquals()` with same types (`assertSame()` should be used) or without explanatory comment above.
2526

2627
## How to document mock objects in phpDocs?
2728

‎rules.neon

+1
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@ rules:
1111
- PHPStan\Rules\PHPUnit\AssertSameNullExpectedRule
1212
- PHPStan\Rules\PHPUnit\AssertSameWithCountRule
1313
- PHPStan\Rules\PHPUnit\MockMethodCallRule
14+
- PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule

‎src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php

+94
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\PHPUnit;
4+
5+
use PhpParser\Node;
6+
use PHPStan\Analyser\Scope;
7+
use PHPStan\Type\BooleanType;
8+
use PHPStan\Type\FloatType;
9+
use PHPStan\Type\IntegerType;
10+
use PHPStan\Type\StringType;
11+
use PHPStan\Type\VerbosityLevel;
12+
13+
/**
14+
* @implements \PHPStan\Rules\Rule<\PhpParser\NodeAbstract>
15+
*/
16+
class AssertEqualsIsDiscouragedRule implements \PHPStan\Rules\Rule
17+
{
18+
19+
public function getNodeType(): string
20+
{
21+
return \PhpParser\NodeAbstract::class;
22+
}
23+
24+
public function processNode(Node $node, Scope $scope): array
25+
{
26+
if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) {
27+
return [];
28+
}
29+
30+
/** @var \PhpParser\Node\Expr\MethodCall|\PhpParser\Node\Expr\StaticCall $node */
31+
$node = $node;
32+
33+
if (count($node->args) < 2) {
34+
return [];
35+
}
36+
if (!$node->name instanceof Node\Identifier || strtolower($node->name->name) !== 'assertequals') {
37+
return [];
38+
}
39+
40+
$leftType = $scope->getType($node->args[0]->value);
41+
$rightType = $scope->getType($node->args[1]->value);
42+
43+
if (
44+
($leftType instanceof BooleanType && $rightType instanceof BooleanType)
45+
|| ($leftType instanceof IntegerType && $rightType instanceof IntegerType)
46+
|| ($leftType instanceof StringType && $rightType instanceof StringType)
47+
) {
48+
$typeDescription = $leftType->describe(VerbosityLevel::typeOnly());
49+
if ($leftType instanceof BooleanType) {
50+
$typeDescription = 'bool';
51+
}
52+
return [
53+
sprintf(
54+
'You should use assertSame instead of assertEquals, because both values are of the same type "%s"',
55+
$typeDescription
56+
),
57+
];
58+
}
59+
if (
60+
($leftType instanceof FloatType && $rightType instanceof FloatType)
61+
&& count($node->args) < 4 // is not using delta for comparing floats
62+
) {
63+
return [
64+
'You should use assertSame instead of assertEquals, because both values are of the same type "float" and you are not using $delta argument',
65+
];
66+
}
67+
68+
if (!$node->hasAttribute('comments')) {
69+
return [
70+
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ...',
71+
];
72+
}
73+
74+
/** @var \PhpParser\Comment[] $comments */
75+
$comments = $node->getAttribute('comments');
76+
$comment = $comments[count($comments) - 1];
77+
78+
// the comment should be on the line above the assertEquals()
79+
if ($comment->getLine() !== ($node->getLine() - 1)) {
80+
return [
81+
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (The comment is not directly above the assertEquals)',
82+
];
83+
}
84+
85+
if (preg_match('~^//\s+assertEquals because(.*)~', $comment->getReformattedText()) === 0) {
86+
return [
87+
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (There is a different comment)',
88+
];
89+
}
90+
91+
return [];
92+
}
93+
94+
}

‎tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\PHPUnit;
4+
5+
use PHPStan\Rules\Rule;
6+
7+
/**
8+
* @extends \PHPStan\Testing\RuleTestCase<AssertEqualsIsDiscouragedRule>
9+
*/
10+
class AssertEqualsIsDiscouragedRuleTest extends \PHPStan\Testing\RuleTestCase
11+
{
12+
13+
protected function getRule(): Rule
14+
{
15+
return new AssertEqualsIsDiscouragedRule();
16+
}
17+
18+
public function testRule(): void
19+
{
20+
$this->analyse([__DIR__ . '/data/assert-equals-is-discouraged.php'], [
21+
[
22+
'You should use assertSame instead of assertEquals, because both values are of the same type "string"',
23+
11,
24+
],
25+
[
26+
'You should use assertSame instead of assertEquals, because both values are of the same type "int"',
27+
12,
28+
],
29+
[
30+
'You should use assertSame instead of assertEquals, because both values are of the same type "bool"',
31+
13,
32+
],
33+
[
34+
'You should use assertSame instead of assertEquals, because both values are of the same type "float" and you are not using $delta argument',
35+
16,
36+
],
37+
[
38+
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (There is a different comment)',
39+
19,
40+
],
41+
[
42+
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ...',
43+
21,
44+
],
45+
[
46+
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (There is a different comment)',
47+
24,
48+
],
49+
[
50+
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (The comment is not directly above the assertEquals)',
51+
28,
52+
],
53+
]);
54+
}
55+
56+
}

‎tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ExampleTestCase;
4+
5+
class AssertEqualsIsDiscouragedTestCase extends \PHPUnit\Framework\TestCase
6+
{
7+
8+
public function testAssertEqualsIsDiscouraged()
9+
{
10+
// assertSame can be used as both are of same type
11+
$this->assertEquals('a', 'b');
12+
$this->assertEquals(1, 2);
13+
$this->assertEquals(true, false);
14+
15+
// comparing floats without delta
16+
$this->assertEquals(1.0, 2.0);
17+
18+
// comparing floats with delta
19+
$this->assertEquals(1.0, 2.0, '', 0.01);
20+
21+
$this->assertEquals(1, '1'); // assertEquals without comment on previous line
22+
23+
// with incorrect comment
24+
$this->assertEquals(1, '1');
25+
26+
// assertEquals because I want it! But sadly, the comment is not just above the assert.
27+
28+
$this->assertEquals(1, '1');
29+
30+
// assertEquals because I want it!
31+
$this->assertEquals(1, '1');
32+
}
33+
34+
}

0 commit comments

Comments
 (0)