From dbfe30d83115d8516dcf30ac930de072a0abc4f4 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Wed, 27 Nov 2024 00:11:01 +0000 Subject: [PATCH 1/4] Add rule that enforces PHPUnit naming conventions --- src/Rules/PHPUnit/ClassNamingRule.php | 93 +++++++++++++++++++++ tests/Rules/PHPUnit/ClassNamingRuleTest.php | 41 +++++++++ tests/Rules/PHPUnit/data/class-naming.php | 37 ++++++++ 3 files changed, 171 insertions(+) create mode 100644 src/Rules/PHPUnit/ClassNamingRule.php create mode 100644 tests/Rules/PHPUnit/ClassNamingRuleTest.php create mode 100644 tests/Rules/PHPUnit/data/class-naming.php diff --git a/src/Rules/PHPUnit/ClassNamingRule.php b/src/Rules/PHPUnit/ClassNamingRule.php new file mode 100644 index 0000000..d445249 --- /dev/null +++ b/src/Rules/PHPUnit/ClassNamingRule.php @@ -0,0 +1,93 @@ + + */ +class ClassNamingRule implements Rule +{ + + private ReflectionProvider $reflectionProvider; + + public function __construct(ReflectionProvider $reflectionProvider) + { + $this->reflectionProvider = $reflectionProvider; + } + + public function getNodeType(): string + { + return Node\Stmt\Class_::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!isset($node->namespacedName)) { + return []; + } + + $className = $node->namespacedName->name; + $class = $this->reflectionProvider->getClass($className); + + if (!$class->isSubclassOf(TestCase::class)) { + return []; + } + + $errors = []; + + if ($class->isAbstract()) { + $this->requireSuffix( + $errors, + $className, + 'TestCase', + 'Abstract test case class, \'%s\', should be named ending in \'%s\'.', + ); + + return $errors; + } + + $this->requireSuffix( + $errors, + $className, + 'Test', + 'Concrete test class, \'%s\', should be named ending in \'%s\'.', + ); + + if (!$class->isFinal()) { + $errors[] = RuleErrorBuilder::message(sprintf( + 'Concrete test class, \'%s\', should be declared final.', + $className, + ))->identifier('phpunit.naming')->build(); + } + + return $errors; + } + + /** + * @param list $errors + */ + private function requireSuffix(array &$errors, string $className, string $suffix, string $messageFormat): void + { + if (str_ends_with($className, $suffix)) { + return; + } + + // @todo Case sensitivity?? + $errors[] = RuleErrorBuilder::message(sprintf( + $messageFormat, + $className, + $suffix, + ))->identifier('phpunit.naming')->build(); + } + +} diff --git a/tests/Rules/PHPUnit/ClassNamingRuleTest.php b/tests/Rules/PHPUnit/ClassNamingRuleTest.php new file mode 100644 index 0000000..77a5d9c --- /dev/null +++ b/tests/Rules/PHPUnit/ClassNamingRuleTest.php @@ -0,0 +1,41 @@ + + */ +class ClassNamingRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new ClassNamingRule(self::getContainer()->getByType(ReflectionProvider::class)); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/class-naming.php'], [ + ['Concrete test class, \'ExampleTestCase\\IncorrectlyNamedTst\', should be named ending in \'Test\'.', 5], + ['Abstract test case class, \'ExampleTestCase\\IncorrectlyNamedTestCse\', should be named ending in \'TestCase\'.', 10], + ['Concrete test class, \'ExampleTestCase\\NotFinalTest\', should be declared final.', 15], + ['Concrete test class, \'ExampleTestCase\\NotFinalOrNamedCorrectly\', should be named ending in \'Test\'.', 20], + ['Concrete test class, \'ExampleTestCase\\NotFinalOrNamedCorrectly\', should be declared final.', 20], + ]); + } + + /** + * @return string[] + */ + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/../../../extension.neon', + ]; + } + +} diff --git a/tests/Rules/PHPUnit/data/class-naming.php b/tests/Rules/PHPUnit/data/class-naming.php new file mode 100644 index 0000000..1559e2c --- /dev/null +++ b/tests/Rules/PHPUnit/data/class-naming.php @@ -0,0 +1,37 @@ + Date: Wed, 27 Nov 2024 00:20:39 +0000 Subject: [PATCH 2/4] Add to extension rules list --- rules.neon | 1 + 1 file changed, 1 insertion(+) diff --git a/rules.neon b/rules.neon index 023e11c..70f2628 100644 --- a/rules.neon +++ b/rules.neon @@ -4,6 +4,7 @@ rules: - PHPStan\Rules\PHPUnit\AssertSameWithCountRule - PHPStan\Rules\PHPUnit\ClassCoversExistsRule - PHPStan\Rules\PHPUnit\ClassMethodCoversExistsRule + - PHPStan\Rules\PHPUnit\ClassNamingRule - PHPStan\Rules\PHPUnit\MockMethodCallRule - PHPStan\Rules\PHPUnit\NoMissingSpaceInClassAnnotationRule - PHPStan\Rules\PHPUnit\NoMissingSpaceInMethodAnnotationRule From bdb8e81cb48f10b230464eba4ae3a347b2fd6208 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Thu, 5 Dec 2024 10:23:20 +0000 Subject: [PATCH 3/4] Make tests final --- tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php | 2 +- tests/Rules/PHPUnit/AssertSameMethodDifferentTypesRuleTest.php | 2 +- tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php | 2 +- .../PHPUnit/AssertSameStaticMethodDifferentTypesRuleTest.php | 2 +- tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php | 2 +- tests/Rules/PHPUnit/ClassCoversExistsRuleTest.php | 2 +- tests/Rules/PHPUnit/ClassMethodCoversExistsRuleTest.php | 2 +- tests/Rules/PHPUnit/ClassNamingRuleTest.php | 2 +- tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php | 2 +- tests/Rules/PHPUnit/ImpossibleCheckTypeMethodCallRuleTest.php | 2 +- tests/Rules/PHPUnit/MockMethodCallRuleTest.php | 2 +- tests/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRuleTest.php | 2 +- .../Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRuleTest.php | 2 +- tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php | 2 +- .../Type/PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php | 2 +- tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php | 2 +- 16 files changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php b/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php index 1fe31df..acddc09 100644 --- a/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php @@ -8,7 +8,7 @@ /** * @extends RuleTestCase */ -class AssertSameBooleanExpectedRuleTest extends RuleTestCase +final class AssertSameBooleanExpectedRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/PHPUnit/AssertSameMethodDifferentTypesRuleTest.php b/tests/Rules/PHPUnit/AssertSameMethodDifferentTypesRuleTest.php index 0898640..00076a6 100644 --- a/tests/Rules/PHPUnit/AssertSameMethodDifferentTypesRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameMethodDifferentTypesRuleTest.php @@ -9,7 +9,7 @@ /** * @extends RuleTestCase */ -class AssertSameMethodDifferentTypesRuleTest extends RuleTestCase +final class AssertSameMethodDifferentTypesRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php b/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php index 1e802dc..2b7f42f 100644 --- a/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php @@ -8,7 +8,7 @@ /** * @extends RuleTestCase */ -class AssertSameNullExpectedRuleTest extends RuleTestCase +final class AssertSameNullExpectedRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/PHPUnit/AssertSameStaticMethodDifferentTypesRuleTest.php b/tests/Rules/PHPUnit/AssertSameStaticMethodDifferentTypesRuleTest.php index cc65b27..ffbe0e4 100644 --- a/tests/Rules/PHPUnit/AssertSameStaticMethodDifferentTypesRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameStaticMethodDifferentTypesRuleTest.php @@ -9,7 +9,7 @@ /** * @extends RuleTestCase */ -class AssertSameStaticMethodDifferentTypesRuleTest extends RuleTestCase +final class AssertSameStaticMethodDifferentTypesRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php index 32f564d..65ec0d4 100644 --- a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php @@ -8,7 +8,7 @@ /** * @extends RuleTestCase */ -class AssertSameWithCountRuleTest extends RuleTestCase +final class AssertSameWithCountRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/PHPUnit/ClassCoversExistsRuleTest.php b/tests/Rules/PHPUnit/ClassCoversExistsRuleTest.php index 2a835ea..b837379 100644 --- a/tests/Rules/PHPUnit/ClassCoversExistsRuleTest.php +++ b/tests/Rules/PHPUnit/ClassCoversExistsRuleTest.php @@ -8,7 +8,7 @@ /** * @extends RuleTestCase */ -class ClassCoversExistsRuleTest extends RuleTestCase +final class ClassCoversExistsRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/PHPUnit/ClassMethodCoversExistsRuleTest.php b/tests/Rules/PHPUnit/ClassMethodCoversExistsRuleTest.php index 45e8b1f..fbb8a51 100644 --- a/tests/Rules/PHPUnit/ClassMethodCoversExistsRuleTest.php +++ b/tests/Rules/PHPUnit/ClassMethodCoversExistsRuleTest.php @@ -9,7 +9,7 @@ /** * @extends RuleTestCase */ -class ClassMethodCoversExistsRuleTest extends RuleTestCase +final class ClassMethodCoversExistsRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/PHPUnit/ClassNamingRuleTest.php b/tests/Rules/PHPUnit/ClassNamingRuleTest.php index 77a5d9c..28cb4a4 100644 --- a/tests/Rules/PHPUnit/ClassNamingRuleTest.php +++ b/tests/Rules/PHPUnit/ClassNamingRuleTest.php @@ -9,7 +9,7 @@ /** * @extends RuleTestCase */ -class ClassNamingRuleTest extends RuleTestCase +final class ClassNamingRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php index 18cc12b..c1d64ab 100644 --- a/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php +++ b/tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php @@ -9,7 +9,7 @@ /** * @extends RuleTestCase */ -class DataProviderDeclarationRuleTest extends RuleTestCase +final class DataProviderDeclarationRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/PHPUnit/ImpossibleCheckTypeMethodCallRuleTest.php b/tests/Rules/PHPUnit/ImpossibleCheckTypeMethodCallRuleTest.php index 4c06546..683738c 100644 --- a/tests/Rules/PHPUnit/ImpossibleCheckTypeMethodCallRuleTest.php +++ b/tests/Rules/PHPUnit/ImpossibleCheckTypeMethodCallRuleTest.php @@ -9,7 +9,7 @@ /** * @extends RuleTestCase */ -class ImpossibleCheckTypeMethodCallRuleTest extends RuleTestCase +final class ImpossibleCheckTypeMethodCallRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/PHPUnit/MockMethodCallRuleTest.php b/tests/Rules/PHPUnit/MockMethodCallRuleTest.php index c9c33e6..3cc80de 100644 --- a/tests/Rules/PHPUnit/MockMethodCallRuleTest.php +++ b/tests/Rules/PHPUnit/MockMethodCallRuleTest.php @@ -9,7 +9,7 @@ /** * @extends RuleTestCase */ -class MockMethodCallRuleTest extends RuleTestCase +final class MockMethodCallRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRuleTest.php b/tests/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRuleTest.php index e28fde1..1a68ce1 100644 --- a/tests/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRuleTest.php +++ b/tests/Rules/PHPUnit/NoMissingSpaceInClassAnnotationRuleTest.php @@ -8,7 +8,7 @@ /** * @extends RuleTestCase */ -class NoMissingSpaceInClassAnnotationRuleTest extends RuleTestCase +final class NoMissingSpaceInClassAnnotationRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRuleTest.php b/tests/Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRuleTest.php index 2926ec9..db07798 100644 --- a/tests/Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRuleTest.php +++ b/tests/Rules/PHPUnit/NoMissingSpaceInMethodAnnotationRuleTest.php @@ -8,7 +8,7 @@ /** * @extends RuleTestCase */ -class NoMissingSpaceInMethodAnnotationRuleTest extends RuleTestCase +final class NoMissingSpaceInMethodAnnotationRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php b/tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php index b378c67..512287e 100644 --- a/tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php +++ b/tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php @@ -8,7 +8,7 @@ /** * @extends RuleTestCase */ -class ShouldCallParentMethodsRuleTest extends RuleTestCase +final class ShouldCallParentMethodsRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Type/PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php b/tests/Type/PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php index 2d43f8b..804a27b 100644 --- a/tests/Type/PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php +++ b/tests/Type/PHPUnit/AssertFunctionTypeSpecifyingExtensionTest.php @@ -5,7 +5,7 @@ use PHPStan\Testing\TypeInferenceTestCase; use function function_exists; -class AssertFunctionTypeSpecifyingExtensionTest extends TypeInferenceTestCase +final class AssertFunctionTypeSpecifyingExtensionTest extends TypeInferenceTestCase { /** @return mixed[] */ diff --git a/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php b/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php index e1841e0..953f36a 100644 --- a/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php +++ b/tests/Type/PHPUnit/AssertMethodTypeSpecifyingExtensionTest.php @@ -4,7 +4,7 @@ use PHPStan\Testing\TypeInferenceTestCase; -class AssertMethodTypeSpecifyingExtensionTest extends TypeInferenceTestCase +final class AssertMethodTypeSpecifyingExtensionTest extends TypeInferenceTestCase { /** @return mixed[] */ From 2503a3e6fcb5af8e7df154ec3b3c6f36894b2b68 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Fri, 6 Dec 2024 11:15:52 +0000 Subject: [PATCH 4/4] Perform case insensitive comparison of class name --- src/Rules/PHPUnit/ClassNamingRule.php | 34 ++++++++++++++++++++------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/Rules/PHPUnit/ClassNamingRule.php b/src/Rules/PHPUnit/ClassNamingRule.php index d445249..5550f7b 100644 --- a/src/Rules/PHPUnit/ClassNamingRule.php +++ b/src/Rules/PHPUnit/ClassNamingRule.php @@ -10,7 +10,8 @@ use PHPStan\Rules\RuleErrorBuilder; use PHPUnit\Framework\TestCase; use function sprintf; -use function str_ends_with; +use function strlen; +use function substr_compare; /** * @implements Rule @@ -36,8 +37,7 @@ public function processNode(Node $node, Scope $scope): array return []; } - $className = $node->namespacedName->name; - $class = $this->reflectionProvider->getClass($className); + $class = $this->reflectionProvider->getClass($node->namespacedName->toString()); if (!$class->isSubclassOf(TestCase::class)) { return []; @@ -48,7 +48,7 @@ public function processNode(Node $node, Scope $scope): array if ($class->isAbstract()) { $this->requireSuffix( $errors, - $className, + $class->getName(), 'TestCase', 'Abstract test case class, \'%s\', should be named ending in \'%s\'.', ); @@ -58,7 +58,7 @@ public function processNode(Node $node, Scope $scope): array $this->requireSuffix( $errors, - $className, + $class->getName(), 'Test', 'Concrete test class, \'%s\', should be named ending in \'%s\'.', ); @@ -66,7 +66,7 @@ public function processNode(Node $node, Scope $scope): array if (!$class->isFinal()) { $errors[] = RuleErrorBuilder::message(sprintf( 'Concrete test class, \'%s\', should be declared final.', - $className, + $class->getName(), ))->identifier('phpunit.naming')->build(); } @@ -75,14 +75,15 @@ public function processNode(Node $node, Scope $scope): array /** * @param list $errors + * @param class-string $className + * @param non-empty-string $suffix */ private function requireSuffix(array &$errors, string $className, string $suffix, string $messageFormat): void { - if (str_ends_with($className, $suffix)) { + if ($this->hasSuffix($className, $suffix)) { return; } - // @todo Case sensitivity?? $errors[] = RuleErrorBuilder::message(sprintf( $messageFormat, $className, @@ -90,4 +91,21 @@ private function requireSuffix(array &$errors, string $className, string $suffix ))->identifier('phpunit.naming')->build(); } + /** + * Checks if class name has the given suffix. + * + * Comparison is case insensitive. + * + * @param class-string $className + * @param non-empty-string $suffix + */ + private function hasSuffix(string $className, string $suffix): bool + { + $classNameLen = strlen($className); + $suffixLen = strlen($suffix); + + return $suffixLen < $classNameLen + && substr_compare($className, $suffix, -$suffixLen, $suffixLen, true) === 0; + } + }