From 3ecbd3ccf62f7fbda05bc625f3352b5feb2102c2 Mon Sep 17 00:00:00 2001 From: PrinsFrank <25006490+PrinsFrank@users.noreply.github.com> Date: Wed, 22 Jan 2025 12:59:54 +0100 Subject: [PATCH 1/3] Fix error message for "assertNotEquals" usage referencing "assertSame" and "assertEquals" --- src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php index f4fc89c..3d0820a 100644 --- a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php +++ b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php @@ -55,6 +55,14 @@ public function processNode(Node $node, Scope $scope): array && ($leftType->isSuperTypeOf($rightType)->yes()) && ($rightType->isSuperTypeOf($leftType)->yes()) ) { + if (strtolower($node->name->name) === 'assertnotequals') { + return [ + RuleErrorBuilder::message( + 'You should use assertNotSame() instead of assertNotEquals(), because both values are scalars of the same type', + )->identifier('phpunit.assertEquals')->build(), + ]; + } + return [ RuleErrorBuilder::message( 'You should use assertSame() instead of assertEquals(), because both values are scalars of the same type', From 1a7f3e147267f7fb67b32381dc0e2f5d9143311e Mon Sep 17 00:00:00 2001 From: PrinsFrank <25006490+PrinsFrank@users.noreply.github.com> Date: Wed, 22 Jan 2025 13:20:35 +0100 Subject: [PATCH 2/3] Update readme and test with new error message for assertNotEquals --- README.md | 1 + .../AssertEqualsIsDiscouragedRuleTest.php | 31 ++++++++++--------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 526085b..3469379 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ It also contains this strict framework-specific rules (can be enabled separately * 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) +* Check that you are not using `assertNotEquals()` with same types (`assertNotSame()` should be used) ## How to document mock objects in phpDocs? diff --git a/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php index f1d34d0..568b5b6 100644 --- a/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php +++ b/tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php @@ -11,25 +11,26 @@ 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'; + private const ERROR_MESSAGE_EQUALS = 'You should use assertSame() instead of assertEquals(), because both values are scalars of the same type'; + private const ERROR_MESSAGE_NOT_EQUALS = 'You should use assertNotSame() instead of assertNotEquals(), 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], - [self::ERROR_MESSAGE, 37], - [self::ERROR_MESSAGE, 38], - [self::ERROR_MESSAGE, 39], + [self::ERROR_MESSAGE_EQUALS, 19], + [self::ERROR_MESSAGE_EQUALS, 22], + [self::ERROR_MESSAGE_EQUALS, 23], + [self::ERROR_MESSAGE_EQUALS, 24], + [self::ERROR_MESSAGE_EQUALS, 25], + [self::ERROR_MESSAGE_EQUALS, 26], + [self::ERROR_MESSAGE_EQUALS, 27], + [self::ERROR_MESSAGE_EQUALS, 28], + [self::ERROR_MESSAGE_EQUALS, 29], + [self::ERROR_MESSAGE_EQUALS, 30], + [self::ERROR_MESSAGE_EQUALS, 32], + [self::ERROR_MESSAGE_NOT_EQUALS, 37], + [self::ERROR_MESSAGE_NOT_EQUALS, 38], + [self::ERROR_MESSAGE_NOT_EQUALS, 39], ]); } From ce842908afed98b670025a78f60874b8607d9c51 Mon Sep 17 00:00:00 2001 From: PrinsFrank <25006490+PrinsFrank@users.noreply.github.com> Date: Wed, 22 Jan 2025 13:37:44 +0100 Subject: [PATCH 3/3] Use sprintf instead of new branch to return correct error message --- .../PHPUnit/AssertEqualsIsDiscouragedRule.php | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php index 3d0820a..bd685dd 100644 --- a/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php +++ b/src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php @@ -11,6 +11,7 @@ use PHPStan\Type\TypeCombinator; use function count; use function in_array; +use function sprintf; use function strtolower; /** @@ -55,17 +56,13 @@ public function processNode(Node $node, Scope $scope): array && ($leftType->isSuperTypeOf($rightType)->yes()) && ($rightType->isSuperTypeOf($leftType)->yes()) ) { - if (strtolower($node->name->name) === 'assertnotequals') { - return [ - RuleErrorBuilder::message( - 'You should use assertNotSame() instead of assertNotEquals(), because both values are scalars of the same type', - )->identifier('phpunit.assertEquals')->build(), - ]; - } - return [ RuleErrorBuilder::message( - 'You should use assertSame() instead of assertEquals(), because both values are scalars of the same type', + sprintf( + 'You should use %s() instead of %s(), because both values are scalars of the same type', + strtolower($node->name->name) === 'assertnotequals' ? 'assertNotSame' : 'assertSame', + $node->name->name, + ), )->identifier('phpunit.assertEquals')->build(), ]; }