From 10880dad5190e788634a62ebb211802669382f3d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 17 Dec 2024 18:22:01 +0100 Subject: [PATCH] Implement AssertEqualsIsDiscouragedRule (#216) --- README.md | 1 + composer.json | 2 +- rules.neon | 7 ++ .../PHPUnit/AssertEqualsIsDiscouragedRule.php | 64 +++++++++++++++++++ .../AssertEqualsIsDiscouragedRuleTest.php | 38 +++++++++++ .../data/assert-equals-is-discouraged.php | 37 +++++++++++ 6 files changed, 148 insertions(+), 1 deletion(-) 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/composer.json b/composer.json index 5237f06..3453041 100644 --- a/composer.json +++ b/composer.json @@ -7,7 +7,7 @@ ], "require": { "php": "^7.4 || ^8.0", - "phpstan/phpstan": "^2.0" + "phpstan/phpstan": "^2.0.4" }, "conflict": { "phpunit/phpunit": "<7.0" diff --git a/rules.neon b/rules.neon index 023e11c..84b7149 100644 --- a/rules.neon +++ b/rules.neon @@ -9,6 +9,10 @@ rules: - PHPStan\Rules\PHPUnit\NoMissingSpaceInMethodAnnotationRule - PHPStan\Rules\PHPUnit\ShouldCallParentMethodsRule +conditionalTags: + PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule: + phpstan.rules.rule: [%strictRulesInstalled%, %featureToggles.bleedingEdge%] + services: - class: PHPStan\Rules\PHPUnit\DataProviderDeclarationRule @@ -17,3 +21,6 @@ services: deprecationRulesInstalled: %deprecationRulesInstalled% tags: - phpstan.rules.rule + + - + class: PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule diff --git a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php new file mode 100644 index 0000000..7aea39a --- /dev/null +++ b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php @@ -0,0 +1,64 @@ + + */ +class AssertEqualsIsDiscouragedRule implements Rule +{ + + public function getNodeType(): string + { + return CallLike::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()); + } +}