Skip to content

Add rule that enforces PHPUnit naming conventions #215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rules.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
111 changes: 111 additions & 0 deletions src/Rules/PHPUnit/ClassNamingRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\PHPUnit;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Rules\IdentifierRuleError;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPUnit\Framework\TestCase;
use function sprintf;
use function strlen;
use function substr_compare;

/**
* @implements Rule<Node\Stmt\Class_>
*/
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 [];
}

$class = $this->reflectionProvider->getClass($node->namespacedName->toString());

if (!$class->isSubclassOf(TestCase::class)) {
return [];
}

$errors = [];

if ($class->isAbstract()) {
$this->requireSuffix(
$errors,
$class->getName(),
'TestCase',
'Abstract test case class, \'%s\', should be named ending in \'%s\'.',
);

return $errors;
}

$this->requireSuffix(
$errors,
$class->getName(),
'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.',
$class->getName(),
))->identifier('phpunit.naming')->build();
}
Comment on lines +66 to +71
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this "isFinal" thing is something which should be mixed with a "general naming rule".

whether tests should be final or not is a pretty opinionated decision IMO


return $errors;
}

/**
* @param list<IdentifierRuleError> $errors
* @param class-string $className
* @param non-empty-string $suffix
*/
private function requireSuffix(array &$errors, string $className, string $suffix, string $messageFormat): void
{
if ($this->hasSuffix($className, $suffix)) {
return;
}

$errors[] = RuleErrorBuilder::message(sprintf(
$messageFormat,
$className,
$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;
}

}
2 changes: 1 addition & 1 deletion tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
/**
* @extends RuleTestCase<AssertSameBooleanExpectedRule>
*/
class AssertSameBooleanExpectedRuleTest extends RuleTestCase
final class AssertSameBooleanExpectedRuleTest extends RuleTestCase
{

protected function getRule(): Rule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
/**
* @extends RuleTestCase<ImpossibleCheckTypeMethodCallRule>
*/
class AssertSameMethodDifferentTypesRuleTest extends RuleTestCase
final class AssertSameMethodDifferentTypesRuleTest extends RuleTestCase
{

protected function getRule(): Rule
Expand Down
2 changes: 1 addition & 1 deletion tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
/**
* @extends RuleTestCase<AssertSameNullExpectedRule>
*/
class AssertSameNullExpectedRuleTest extends RuleTestCase
final class AssertSameNullExpectedRuleTest extends RuleTestCase
{

protected function getRule(): Rule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
/**
* @extends RuleTestCase<ImpossibleCheckTypeStaticMethodCallRule>
*/
class AssertSameStaticMethodDifferentTypesRuleTest extends RuleTestCase
final class AssertSameStaticMethodDifferentTypesRuleTest extends RuleTestCase
{

protected function getRule(): Rule
Expand Down
2 changes: 1 addition & 1 deletion tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
/**
* @extends RuleTestCase<AssertSameWithCountRule>
*/
class AssertSameWithCountRuleTest extends RuleTestCase
final class AssertSameWithCountRuleTest extends RuleTestCase
{

protected function getRule(): Rule
Expand Down
2 changes: 1 addition & 1 deletion tests/Rules/PHPUnit/ClassCoversExistsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
/**
* @extends RuleTestCase<ClassCoversExistsRule>
*/
class ClassCoversExistsRuleTest extends RuleTestCase
final class ClassCoversExistsRuleTest extends RuleTestCase
{

protected function getRule(): Rule
Expand Down
2 changes: 1 addition & 1 deletion tests/Rules/PHPUnit/ClassMethodCoversExistsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
/**
* @extends RuleTestCase<ClassMethodCoversExistsRule>
*/
class ClassMethodCoversExistsRuleTest extends RuleTestCase
final class ClassMethodCoversExistsRuleTest extends RuleTestCase
{

protected function getRule(): Rule
Expand Down
41 changes: 41 additions & 0 deletions tests/Rules/PHPUnit/ClassNamingRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\PHPUnit;

use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;

/**
* @extends RuleTestCase<ClassNamingRule>
*/
final 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',
];
}

}
2 changes: 1 addition & 1 deletion tests/Rules/PHPUnit/DataProviderDeclarationRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
/**
* @extends RuleTestCase<DataProviderDeclarationRule>
*/
class DataProviderDeclarationRuleTest extends RuleTestCase
final class DataProviderDeclarationRuleTest extends RuleTestCase
{

protected function getRule(): Rule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
/**
* @extends RuleTestCase<ImpossibleCheckTypeMethodCallRule>
*/
class ImpossibleCheckTypeMethodCallRuleTest extends RuleTestCase
final class ImpossibleCheckTypeMethodCallRuleTest extends RuleTestCase
{

protected function getRule(): Rule
Expand Down
2 changes: 1 addition & 1 deletion tests/Rules/PHPUnit/MockMethodCallRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
/**
* @extends RuleTestCase<MockMethodCallRule>
*/
class MockMethodCallRuleTest extends RuleTestCase
final class MockMethodCallRuleTest extends RuleTestCase
{

protected function getRule(): Rule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
/**
* @extends RuleTestCase<NoMissingSpaceInClassAnnotationRule>
*/
class NoMissingSpaceInClassAnnotationRuleTest extends RuleTestCase
final class NoMissingSpaceInClassAnnotationRuleTest extends RuleTestCase
{

protected function getRule(): Rule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
/**
* @extends RuleTestCase<NoMissingSpaceInMethodAnnotationRule>
*/
class NoMissingSpaceInMethodAnnotationRuleTest extends RuleTestCase
final class NoMissingSpaceInMethodAnnotationRuleTest extends RuleTestCase
{

protected function getRule(): Rule
Expand Down
2 changes: 1 addition & 1 deletion tests/Rules/PHPUnit/ShouldCallParentMethodsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
/**
* @extends RuleTestCase<ShouldCallParentMethodsRule>
*/
class ShouldCallParentMethodsRuleTest extends RuleTestCase
final class ShouldCallParentMethodsRuleTest extends RuleTestCase
{

protected function getRule(): Rule
Expand Down
37 changes: 37 additions & 0 deletions tests/Rules/PHPUnit/data/class-naming.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php declare(strict_types = 1);

namespace ExampleTestCase;

final class IncorrectlyNamedTst extends \PHPUnit\Framework\TestCase
{

}

abstract class IncorrectlyNamedTestCse extends \PHPUnit\Framework\TestCase
{

}

class NotFinalTest extends \PHPUnit\Framework\TestCase
{

}

class NotFinalOrNamedCorrectly extends \PHPUnit\Framework\TestCase
{

}

new class() extends \PHPUnit\Framework\TestCase {

};

final class CorrectlyNamedAndFinalTest extends \PHPUnit\Framework\TestCase
{

}

abstract class CorrectlyNamedTestCase extends \PHPUnit\Framework\TestCase
{

}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use PHPStan\Testing\TypeInferenceTestCase;
use function function_exists;

class AssertFunctionTypeSpecifyingExtensionTest extends TypeInferenceTestCase
final class AssertFunctionTypeSpecifyingExtensionTest extends TypeInferenceTestCase
{

/** @return mixed[] */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use PHPStan\Testing\TypeInferenceTestCase;

class AssertMethodTypeSpecifyingExtensionTest extends TypeInferenceTestCase
final class AssertMethodTypeSpecifyingExtensionTest extends TypeInferenceTestCase
{

/** @return mixed[] */
Expand Down
Loading