From c2386c7d0b1d5ce61638511d6ba83f11adf2634c Mon Sep 17 00:00:00 2001 From: johan Date: Thu, 5 Jan 2023 16:29:41 +0100 Subject: [PATCH 1/2] [RemoveTestSuffixFromAbstractTestClassesRector] Add new rule (#132) --- ...estSuffixFromAbstractTestClassesRector.php | 90 +++++++++++++++++++ ...ce_abstract_class_with_suffix_test.php.inc | 8 ++ ...skip_abstract_class_without_suffix.php.inc | 8 ++ ...uffixFromAbstractTestClassesRectorTest.php | 47 ++++++++++ .../config/configured_rule.php | 10 +++ 5 files changed, 163 insertions(+) create mode 100644 src/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector.php create mode 100644 tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/Fixture/replace_abstract_class_with_suffix_test.php.inc create mode 100644 tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/Fixture/skip_abstract_class_without_suffix.php.inc create mode 100644 tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/RemoveTestSuffixFromAbstractTestClassesRectorTest.php create mode 100644 tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/config/configured_rule.php diff --git a/src/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector.php b/src/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector.php new file mode 100644 index 00000000..af762465 --- /dev/null +++ b/src/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector.php @@ -0,0 +1,90 @@ +> + */ + public function getNodeTypes(): array + { + return [ClassLike::class]; + } + + /** + * @param ClassLike $node + */ + public function refactor(Node $node): ?Node + { + if (!$node->isAbstract()) { + return null; + } + $directorySuffix = $this->getPhpUnitDirectorySuffix(); + + $filePath = $this->file->getFilePath(); + $basename = pathinfo($filePath, PATHINFO_FILENAME); + + if (!str_ends_with($basename, $directorySuffix)) { + return null; + } + + $className = $this->getName($node); + if ($className === null) { + return null; + } + + $classShortName = $this->nodeNameResolver->getShortName($className); + // no match → rename file + $newFileLocation = dirname($filePath) . DIRECTORY_SEPARATOR . $classShortName . 'Case.php'; + $this->removedAndAddedFilesCollector->addMovedFile($this->file, $newFileLocation); + + + return null; + } + + private function getPhpUnitDirectorySuffix(): string + { + // TODO check in phpunit config if this is different. + return 'Test'; + } +} diff --git a/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/Fixture/replace_abstract_class_with_suffix_test.php.inc b/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/Fixture/replace_abstract_class_with_suffix_test.php.inc new file mode 100644 index 00000000..844fa735 --- /dev/null +++ b/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/Fixture/replace_abstract_class_with_suffix_test.php.inc @@ -0,0 +1,8 @@ +doTestFile($filePath); + + Assert::string($this->originalTempFilePath); + $originalDirectory = dirname($this->originalTempFilePath); + + $expectedAddedFileWithContent = new AddedFileWithContent( + $originalDirectory . '/ReplaceAbstractClassWithSuffixTestCase.php', + FileSystem::read(__DIR__ . '/Fixture/replace_abstract_class_with_suffix_test.php.inc') + ); + $this->assertFileWasAdded($expectedAddedFileWithContent); + +// $expectedAddedFileWithContent = new AddedFileWithContent( +// $originalDirectory . '/SkipAbstractClassWithoutSuffix.php', +// FileSystem::read(__DIR__ . '/Fixture/skip_abstract_class_without_suffix.php.inc') +// ); +// $this->assertFileWasAdded($expectedAddedFileWithContent); + } + + public function provideData(): Iterator + { + return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/config/configured_rule.php b/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/config/configured_rule.php new file mode 100644 index 00000000..b1ae9ae0 --- /dev/null +++ b/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/config/configured_rule.php @@ -0,0 +1,10 @@ +rule(RemoveTestSuffixFromAbstractTestClassesRector::class); +}; From e4f92c60351ee4ef93bfd2619ae3fb611266c422 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 6 Feb 2023 16:10:44 +0000 Subject: [PATCH 2/2] Make use of new file asserts --- composer.json | 2 +- ecs.php | 2 +- phpstan.neon | 8 +- rector.php | 4 +- ...estSuffixFromAbstractTestClassesRector.php | 86 +++++++++++-------- .../ExtendsTestCase.php} | 3 +- .../Fixture/extends_test.php.inc | 23 +++++ ...uffixFromAbstractTestClassesRectorTest.php | 32 +++---- .../config/configured_rule.php | 2 + 9 files changed, 95 insertions(+), 67 deletions(-) rename tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/{Fixture/replace_abstract_class_with_suffix_test.php.inc => Expected/ExtendsTestCase.php} (60%) create mode 100644 tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/Fixture/extends_test.php.inc diff --git a/composer.json b/composer.json index df1cabe1..935fcb4c 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,7 @@ "symplify/phpstan-extensions": "^11.1", "symplify/easy-coding-standard": "^11.2", "symplify/rule-doc-generator": "^11.2", - "rector/phpstan-rules": "^0.6.5", + "rector/phpstan-rules": "^0.6", "phpstan/extension-installer": "^1.2", "phpstan/phpstan-strict-rules": "^1.4.5", "phpstan/phpstan-webmozart-assert": "^1.2.2", diff --git a/ecs.php b/ecs.php index 05b44fa8..0f3672ff 100644 --- a/ecs.php +++ b/ecs.php @@ -16,7 +16,7 @@ __DIR__ . '/rector.php', ]); - $ecsConfig->skip(['*/Source/*', '*/Fixture/*']); + $ecsConfig->skip(['*/Source/*', '*/Fixture/*', '*/Expected/*']); $ecsConfig->lineEnding("\n"); }; diff --git a/phpstan.neon b/phpstan.neon index 489366a2..c2828a0e 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -13,6 +13,7 @@ parameters: - */Source/* - *Source/* - */Fixture/* + - */Expected/* reportUnmatchedIgnoredErrors: false @@ -30,10 +31,3 @@ parameters: - '#Parameter \#1 \$node (.*?) of method Rector\\(.*?)\(\) should be contravariant with parameter \$node \(PhpParser\\Node\) of method Rector\\Core\\Contract\\Rector\\PhpRectorInterface\:\:refactor\(\)#' - '#Cognitive complexity for "Rector\\PHPUnit\\Rector\\MethodCall\\DelegateExceptionArgumentsRector\:\:refactor\(\)" is 12, keep it under 10#' - - '#Class "Rector\\PHPUnit\\Rector\\MethodCall\\DelegateExceptionArgumentsRector" has invalid namespace category "MethodCall"\. Pick one of\: "StmtsAwareInterface"#' - - '#Parameter \#2 \$callable of method Rector\\Core\\NodeManipulator\\ForeachManipulator\:\:matchOnlyStmt\(\) expects callable\(PhpParser\\Node, PhpParser\\Node\\Stmt\\Foreach_\=\)\: PhpParser\\Node\|null, Closure\(PhpParser\\Node, PhpParser\\Node\\Stmt\\Foreach_\)\: PhpParser\\Node\\Expr\\MethodCall\|PhpParser\\Node\\Expr\\StaticCall\|null given#' - - # solve later -# - '#Call to an undefined method PhpParser\\Node\\Expr\:\:getArgs\(\)#' - - '#Cognitive complexity for "Rector\\PHPUnit\\Rector\\ClassMethod\\CreateMockToAnonymousClassRector\:\:refactor\(\)" is \d+, keep it under 10#' - diff --git a/rector.php b/rector.php index 1a58d92a..7c6d7eee 100644 --- a/rector.php +++ b/rector.php @@ -4,6 +4,7 @@ use Rector\Config\RectorConfig; use Rector\Php55\Rector\String_\StringClassNameToClassConstantRector; +use Rector\PHPUnit\Set\PHPUnitSetList; use Rector\Set\ValueObject\LevelSetList; use Rector\Set\ValueObject\SetList; @@ -16,6 +17,7 @@ // for tests '*/Source/*', '*/Fixture/*', + '*/Expected/*', // object types StringClassNameToClassConstantRector::class => [ @@ -33,7 +35,7 @@ __DIR__ . '/config/config.php', LevelSetList::UP_TO_PHP_81, SetList::DEAD_CODE, - \Rector\PHPUnit\Set\PHPUnitSetList::PHPUNIT_100, + PHPUnitSetList::PHPUNIT_100, SetList::CODE_QUALITY, SetList::CODING_STYLE, SetList::EARLY_RETURN, diff --git a/src/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector.php b/src/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector.php index af762465..f7567881 100644 --- a/src/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector.php +++ b/src/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector.php @@ -5,42 +5,55 @@ namespace Rector\PHPUnit\Rector\ClassLike; use PhpParser\Node; -use PhpParser\Node\Stmt\ClassLike; -use Rector\Core\Application\FileSystem\RemovedAndAddedFilesCollector; +use PhpParser\Node\Identifier; +use PhpParser\Node\Stmt\Class_; +use PhpParser\Node\Stmt\Namespace_; +use Rector\Core\Exception\ShouldNotHappenException; use Rector\Core\Rector\AbstractRector; +use Rector\NodeTypeResolver\Node\AttributeKey; +use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; +use Rector\Symfony\Printer\NeighbourClassLikePrinter; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @see Rector\PHPUnit\Tests\Rector\ClassLike\RemoveTestSuffixFromAbstractTestClassesRector\RemoveTestSuffixFromAbstractTestClassesRectorTest + * @see \Rector\PHPUnit\Tests\Rector\ClassLike\RemoveTestSuffixFromAbstractTestClassesRector\RemoveTestSuffixFromAbstractTestClassesRectorTest */ final class RemoveTestSuffixFromAbstractTestClassesRector extends AbstractRector { public function __construct( - private readonly RemovedAndAddedFilesCollector $removedAndAddedFilesCollector, - ) - { + private readonly NeighbourClassLikePrinter $neighbourClassLikePrinter, + private readonly TestsNodeAnalyzer $testsNodeAnalyzer, + ) { } public function getRuleDefinition(): RuleDefinition { - return new RuleDefinition('Rename Suffix for abstract test class as it may not end with directory suffix (default is Test)', [ - new CodeSample( - <<<'CODE_SAMPLE' -// app/ReplaceAbstractClassWithSuffixTest.php -abstract class ReplaceAbstractClassWithSuffixTest + return new RuleDefinition( + 'Rename abstract test class suffix from "*Test" to "*TestCase"', + [ + new CodeSample( + <<<'CODE_SAMPLE' +// tests/AbstractTest.php +use PHPUnit\Framework\TestCase; + +abstract class AbstractTest extends TestCase { } CODE_SAMPLE - , - <<<'CODE_SAMPLE' -// app/ReplaceAbstractClassWithSuffixTestCase.php -abstract class ReplaceAbstractClassWithSuffixTestCase + , + <<<'CODE_SAMPLE' +// tests/AbstractTestCase.php +use PHPUnit\Framework\TestCase; + +abstract class AbstractTestCase extends TestCase { } CODE_SAMPLE - ), - ]); + ), + + ] + ); } /** @@ -48,43 +61,48 @@ abstract class ReplaceAbstractClassWithSuffixTestCase */ public function getNodeTypes(): array { - return [ClassLike::class]; + return [Class_::class]; } /** - * @param ClassLike $node + * @param Class_ $node */ public function refactor(Node $node): ?Node { - if (!$node->isAbstract()) { + if (! $node->isAbstract()) { return null; } - $directorySuffix = $this->getPhpUnitDirectorySuffix(); - $filePath = $this->file->getFilePath(); - $basename = pathinfo($filePath, PATHINFO_FILENAME); + if (! $this->testsNodeAnalyzer->isInTestClass($node)) { + return null; + } - if (!str_ends_with($basename, $directorySuffix)) { + if (! $node->name instanceof Identifier) { return null; } - $className = $this->getName($node); - if ($className === null) { + if (! $this->isName($node->name, '*Test')) { return null; } - $classShortName = $this->nodeNameResolver->getShortName($className); - // no match → rename file - $newFileLocation = dirname($filePath) . DIRECTORY_SEPARATOR . $classShortName . 'Case.php'; - $this->removedAndAddedFilesCollector->addMovedFile($this->file, $newFileLocation); + // rename class + $testCaseClassName = $node->name->toString() . 'Case'; + $node->name = new Identifier($testCaseClassName); + $this->printNewNodes($node); - return null; + return $node; } - private function getPhpUnitDirectorySuffix(): string + private function printNewNodes(Class_ $class): void { - // TODO check in phpunit config if this is different. - return 'Test'; + $filePath = $this->file->getFilePath(); + + $parentNode = $class->getAttribute(AttributeKey::PARENT_NODE); + if (! $parentNode instanceof Namespace_) { + throw new ShouldNotHappenException(); + } + + $this->neighbourClassLikePrinter->printClassLike($class, $parentNode, $filePath, $this->file); } } diff --git a/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/Fixture/replace_abstract_class_with_suffix_test.php.inc b/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/Expected/ExtendsTestCase.php similarity index 60% rename from tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/Fixture/replace_abstract_class_with_suffix_test.php.inc rename to tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/Expected/ExtendsTestCase.php index 844fa735..ff1affe1 100644 --- a/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/Fixture/replace_abstract_class_with_suffix_test.php.inc +++ b/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/Expected/ExtendsTestCase.php @@ -1,8 +1,7 @@ +----- + diff --git a/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/RemoveTestSuffixFromAbstractTestClassesRectorTest.php b/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/RemoveTestSuffixFromAbstractTestClassesRectorTest.php index ef9cb9ab..483a3d07 100644 --- a/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/RemoveTestSuffixFromAbstractTestClassesRectorTest.php +++ b/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/RemoveTestSuffixFromAbstractTestClassesRectorTest.php @@ -6,38 +6,28 @@ use Iterator; use Nette\Utils\FileSystem; -use Rector\FileSystemRector\ValueObject\AddedFileWithContent; use Rector\Testing\PHPUnit\AbstractRectorTestCase; -use Webmozart\Assert\Assert; final class RemoveTestSuffixFromAbstractTestClassesRectorTest extends AbstractRectorTestCase { - /** - * @dataProvider provideData() - */ - public function test(string $filePath): void + public function testNoChanges(): void { - $this->doTestFile($filePath); + $this->doTestFile(__DIR__ . '/Fixture/skip_abstract_class_without_suffix.php.inc'); + } - Assert::string($this->originalTempFilePath); - $originalDirectory = dirname($this->originalTempFilePath); + public function testChanges(): void + { + $this->doTestFile(__DIR__ . '/Fixture/extends_test.php.inc'); - $expectedAddedFileWithContent = new AddedFileWithContent( - $originalDirectory . '/ReplaceAbstractClassWithSuffixTestCase.php', - FileSystem::read(__DIR__ . '/Fixture/replace_abstract_class_with_suffix_test.php.inc') + $this->assertFileWasAdded( + __DIR__ . '/Fixture/ExtendsTestCase.php', + FileSystem::read(__DIR__ . '/Expected/ExtendsTestCase.php') ); - $this->assertFileWasAdded($expectedAddedFileWithContent); - -// $expectedAddedFileWithContent = new AddedFileWithContent( -// $originalDirectory . '/SkipAbstractClassWithoutSuffix.php', -// FileSystem::read(__DIR__ . '/Fixture/skip_abstract_class_without_suffix.php.inc') -// ); -// $this->assertFileWasAdded($expectedAddedFileWithContent); } - public function provideData(): Iterator + public static function provideData(): Iterator { - return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture'); + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); } public function provideConfigFilePath(): string diff --git a/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/config/configured_rule.php b/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/config/configured_rule.php index b1ae9ae0..58647b9c 100644 --- a/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/config/configured_rule.php +++ b/tests/Rector/ClassLike/RemoveTestSuffixFromAbstractTestClassesRector/config/configured_rule.php @@ -6,5 +6,7 @@ use Rector\PHPUnit\Rector\ClassLike\RemoveTestSuffixFromAbstractTestClassesRector; return static function (RectorConfig $rectorConfig): void { + $rectorConfig->import(__DIR__ . '/../../../../../config/config.php'); + $rectorConfig->rule(RemoveTestSuffixFromAbstractTestClassesRector::class); };