Skip to content

Commit 10880da

Browse files
authored
Implement AssertEqualsIsDiscouragedRule (#216)
1 parent 4b6ad7f commit 10880da

6 files changed

+148
-1
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)
2526

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

composer.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
],
88
"require": {
99
"php": "^7.4 || ^8.0",
10-
"phpstan/phpstan": "^2.0"
10+
"phpstan/phpstan": "^2.0.4"
1111
},
1212
"conflict": {
1313
"phpunit/phpunit": "<7.0"

rules.neon

+7
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ rules:
99
- PHPStan\Rules\PHPUnit\NoMissingSpaceInMethodAnnotationRule
1010
- PHPStan\Rules\PHPUnit\ShouldCallParentMethodsRule
1111

12+
conditionalTags:
13+
PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule:
14+
phpstan.rules.rule: [%strictRulesInstalled%, %featureToggles.bleedingEdge%]
15+
1216
services:
1317
-
1418
class: PHPStan\Rules\PHPUnit\DataProviderDeclarationRule
@@ -17,3 +21,6 @@ services:
1721
deprecationRulesInstalled: %deprecationRulesInstalled%
1822
tags:
1923
- phpstan.rules.rule
24+
25+
-
26+
class: PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\PHPUnit;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr\CallLike;
7+
use PHPStan\Analyser\Scope;
8+
use PHPStan\Rules\Rule;
9+
use PHPStan\Rules\RuleErrorBuilder;
10+
use PHPStan\Type\GeneralizePrecision;
11+
use PHPStan\Type\TypeCombinator;
12+
use function count;
13+
use function strtolower;
14+
15+
/**
16+
* @implements Rule<CallLike>
17+
*/
18+
class AssertEqualsIsDiscouragedRule implements Rule
19+
{
20+
21+
public function getNodeType(): string
22+
{
23+
return CallLike::class;
24+
}
25+
26+
public function processNode(Node $node, Scope $scope): array
27+
{
28+
if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) {
29+
return [];
30+
}
31+
32+
if (count($node->getArgs()) < 2) {
33+
return [];
34+
}
35+
if (!$node->name instanceof Node\Identifier || strtolower($node->name->name) !== 'assertequals') {
36+
return [];
37+
}
38+
39+
$leftType = TypeCombinator::removeNull($scope->getType($node->getArgs()[0]->value));
40+
$rightType = TypeCombinator::removeNull($scope->getType($node->getArgs()[1]->value));
41+
42+
if ($leftType->isConstantScalarValue()->yes()) {
43+
$leftType = $leftType->generalize(GeneralizePrecision::lessSpecific());
44+
}
45+
if ($rightType->isConstantScalarValue()->yes()) {
46+
$rightType = $rightType->generalize(GeneralizePrecision::lessSpecific());
47+
}
48+
49+
if (
50+
($leftType->isScalar()->yes() && $rightType->isScalar()->yes())
51+
&& ($leftType->isSuperTypeOf($rightType)->yes())
52+
&& ($rightType->isSuperTypeOf($leftType)->yes())
53+
) {
54+
return [
55+
RuleErrorBuilder::message(
56+
'You should use assertSame() instead of assertEquals(), because both values are scalars of the same type',
57+
)->identifier('phpunit.assertEquals')->build(),
58+
];
59+
}
60+
61+
return [];
62+
}
63+
64+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\PHPUnit;
4+
5+
use PHPStan\Rules\Rule;
6+
use PHPStan\Testing\RuleTestCase;
7+
8+
/**
9+
* @extends RuleTestCase<AssertEqualsIsDiscouragedRule>
10+
*/
11+
final class AssertEqualsIsDiscouragedRuleTest extends RuleTestCase
12+
{
13+
14+
private const ERROR_MESSAGE = 'You should use assertSame() instead of assertEquals(), because both values are scalars of the same type';
15+
16+
public function testRule(): void
17+
{
18+
$this->analyse([__DIR__ . '/data/assert-equals-is-discouraged.php'], [
19+
[self::ERROR_MESSAGE, 19],
20+
[self::ERROR_MESSAGE, 22],
21+
[self::ERROR_MESSAGE, 23],
22+
[self::ERROR_MESSAGE, 24],
23+
[self::ERROR_MESSAGE, 25],
24+
[self::ERROR_MESSAGE, 26],
25+
[self::ERROR_MESSAGE, 27],
26+
[self::ERROR_MESSAGE, 28],
27+
[self::ERROR_MESSAGE, 29],
28+
[self::ERROR_MESSAGE, 30],
29+
[self::ERROR_MESSAGE, 32],
30+
]);
31+
}
32+
33+
protected function getRule(): Rule
34+
{
35+
return new AssertEqualsIsDiscouragedRule();
36+
}
37+
38+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace SameOverEqualsTest;
6+
7+
use Exception;
8+
use PHPUnit\Framework\TestCase;
9+
10+
final class AssertSameOverAssertEqualsRule extends TestCase
11+
{
12+
public function dummyTest(string $string, int $integer, bool $boolean, float $float, ?string $nullableString): void
13+
{
14+
$null = null;
15+
16+
$this->assertSame(5, $integer);
17+
static::assertSame(5, $integer);
18+
19+
$this->assertEquals('', $string);
20+
$this->assertEquals(null, $string);
21+
static::assertEquals(null, $string);
22+
static::assertEquals($nullableString, $string);
23+
$this->assertEquals(2, $integer);
24+
$this->assertEquals(2.2, $float);
25+
static::assertEquals((int) '2', (int) $string);
26+
$this->assertEquals(true, $boolean);
27+
$this->assertEquals($string, $string);
28+
$this->assertEquals($integer, $integer);
29+
$this->assertEquals($boolean, $boolean);
30+
$this->assertEquals($float, $float);
31+
$this->assertEquals($null, $null);
32+
$this->assertEquals((string) new Exception(), (string) new Exception());
33+
$this->assertEquals([], []);
34+
$this->assertEquals(new Exception(), new Exception());
35+
static::assertEquals(new Exception(), new Exception());
36+
}
37+
}

0 commit comments

Comments
 (0)