Skip to content

Commit 122e716

Browse files
mhujermhujer
committed
AssertEqualsIsDiscouragedRule
1 parent b26d14d commit 122e716

File tree

5 files changed

+184
-0
lines changed

5 files changed

+184
-0
lines changed

Diff for: ‎README.md

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

2829
## How to document mock objects in phpDocs?
2930

Diff for: ‎src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php

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

Diff for: ‎strictRules.neon

+4
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,7 @@ services:
1111
class: PHPStan\Rules\PHPUnit\AssertSameWithCountRule
1212
tags:
1313
- phpstan.rules.rule
14+
-
15+
class: PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule
16+
tags:
17+
- phpstan.rules.rule

Diff for: ‎tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php

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

Diff for: ‎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)