From 7d8c62993ff53857b97ce500412c6bb407cacb3f Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 10 Dec 2024 11:16:58 +0100 Subject: [PATCH] Implement AssertSameOverAssertEqualsRule --- README.md | 1 + rules.neon | 1 + .../PHPUnit/AssertEqualsIsDiscouragedRule.php | 64 +++++++++++++++++++ .../AssertEqualsIsDiscouragedRuleTest.php | 38 +++++++++++ .../data/assert-equals-is-discouraged.php | 37 +++++++++++ 5 files changed, 141 insertions(+) create mode 100644 src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php create mode 100644 tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php create mode 100644 tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php diff --git a/README.md b/README.md index 205cbe4..526085b 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ It also contains this strict framework-specific rules (can be enabled separately * Check that you are not using `assertSame()` with `false` as expected value. `assertFalse()` should be used instead. * Check that you are not using `assertSame()` with `null` as expected value. `assertNull()` should be used instead. * Check that you are not using `assertSame()` with `count($variable)` as second parameter. `assertCount($variable)` should be used instead. +* Check that you are not using `assertEquals()` with same types (`assertSame()` should be used) ## How to document mock objects in phpDocs? diff --git a/rules.neon b/rules.neon index 023e11c..85afa76 100644 --- a/rules.neon +++ b/rules.neon @@ -2,6 +2,7 @@ rules: - PHPStan\Rules\PHPUnit\AssertSameBooleanExpectedRule - PHPStan\Rules\PHPUnit\AssertSameNullExpectedRule - PHPStan\Rules\PHPUnit\AssertSameWithCountRule + - PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule - PHPStan\Rules\PHPUnit\ClassCoversExistsRule - PHPStan\Rules\PHPUnit\ClassMethodCoversExistsRule - PHPStan\Rules\PHPUnit\MockMethodCallRule diff --git a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php new file mode 100644 index 0000000..3c83671 --- /dev/null +++ b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php @@ -0,0 +1,64 @@ + + */ +class AssertEqualsIsDiscouragedRule implements Rule +{ + + public function getNodeType(): string + { + return NodeAbstract::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { + return []; + } + + if (count($node->getArgs()) < 2) { + return []; + } + if (!$node->name instanceof Node\Identifier || strtolower($node->name->name) !== 'assertequals') { + return []; + } + + $leftType = TypeCombinator::removeNull($scope->getType($node->getArgs()[0]->value)); + $rightType = TypeCombinator::removeNull($scope->getType($node->getArgs()[1]->value)); + + if ($leftType->isConstantScalarValue()->yes()) { + $leftType = $leftType->generalize(GeneralizePrecision::lessSpecific()); + } + if ($rightType->isConstantScalarValue()->yes()) { + $rightType = $rightType->generalize(GeneralizePrecision::lessSpecific()); + } + + if ( + ($leftType->isScalar()->yes() && $rightType->isScalar()->yes()) + && ($leftType->isSuperTypeOf($rightType)->yes()) + && ($rightType->isSuperTypeOf($leftType)->yes()) + ) { + return [ + RuleErrorBuilder::message( + 'You should use assertSame() instead of assertEquals(), because both values are scalars of the same type', + )->identifier('phpunit.assertEquals')->build(), + ]; + } + + return []; + } + +} diff --git a/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php new file mode 100644 index 0000000..e739ee4 --- /dev/null +++ b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php @@ -0,0 +1,38 @@ + + */ +final class AssertEqualsIsDiscouragedRuleTest extends RuleTestCase +{ + + private const ERROR_MESSAGE = 'You should use assertSame() instead of assertEquals(), because both values are scalars of the same type'; + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/assert-equals-is-discouraged.php'], [ + [self::ERROR_MESSAGE, 19], + [self::ERROR_MESSAGE, 22], + [self::ERROR_MESSAGE, 23], + [self::ERROR_MESSAGE, 24], + [self::ERROR_MESSAGE, 25], + [self::ERROR_MESSAGE, 26], + [self::ERROR_MESSAGE, 27], + [self::ERROR_MESSAGE, 28], + [self::ERROR_MESSAGE, 29], + [self::ERROR_MESSAGE, 30], + [self::ERROR_MESSAGE, 32], + ]); + } + + protected function getRule(): Rule + { + return new AssertEqualsIsDiscouragedRule(); + } + +} diff --git a/tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php b/tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php new file mode 100644 index 0000000..727408d --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php @@ -0,0 +1,37 @@ +assertSame(5, $integer); + static::assertSame(5, $integer); + + $this->assertEquals('', $string); + $this->assertEquals(null, $string); + static::assertEquals(null, $string); + static::assertEquals($nullableString, $string); + $this->assertEquals(2, $integer); + $this->assertEquals(2.2, $float); + static::assertEquals((int) '2', (int) $string); + $this->assertEquals(true, $boolean); + $this->assertEquals($string, $string); + $this->assertEquals($integer, $integer); + $this->assertEquals($boolean, $boolean); + $this->assertEquals($float, $float); + $this->assertEquals($null, $null); + $this->assertEquals((string) new Exception(), (string) new Exception()); + $this->assertEquals([], []); + $this->assertEquals(new Exception(), new Exception()); + static::assertEquals(new Exception(), new Exception()); + } +}