From bc5eefdb76cb44636fe9f01ad2a4eec5b5fcec4a Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Tue, 11 Feb 2020 14:40:59 +0100 Subject: [PATCH] improve AddSeeTestAnnotationRector --- ecs.yaml | 2 +- .../PhpDocInfo/PhpDocInfo/PhpDocInfoTest.php | 6 +- .../ComposerAutoloadedDirectoryProvider.php | 2 +- .../Class_/AddSeeTestAnnotationRector.php | 67 ++++++++++--------- .../psr4/tests/FileRelocationResolverTest.php | 2 +- sonar-project.properties | 6 +- src/Testing/Finder/RectorsFinder.php | 11 ++- 7 files changed, 56 insertions(+), 40 deletions(-) diff --git a/ecs.yaml b/ecs.yaml index 57988d4e675..2706789f95e 100644 --- a/ecs.yaml +++ b/ecs.yaml @@ -124,7 +124,7 @@ parameters: - 'rules/cakephp-to-symfony/tests/Rector/Class_/CakePHPModelToDoctrineRepositoryRector/CakePHPModelToDoctrineRepositoryRectorTest.php' PhpCsFixer\Fixer\PhpUnit\PhpUnitStrictFixer: - - 'rules/better-php-doc-parser/tests/PhpDocInfo/PhpDocInfo/PhpDocInfoTest.php' + - 'packages/better-php-doc-parser/tests/PhpDocInfo/PhpDocInfo/PhpDocInfoTest.php' # intentional "assertEquals()" - 'tests/PhpParser/Node/NodeFactoryTest.php' - '*TypeResolverTest.php' diff --git a/packages/better-php-doc-parser/tests/PhpDocInfo/PhpDocInfo/PhpDocInfoTest.php b/packages/better-php-doc-parser/tests/PhpDocInfo/PhpDocInfo/PhpDocInfoTest.php index a0ee89cbe7b..96f2d851a9d 100644 --- a/packages/better-php-doc-parser/tests/PhpDocInfo/PhpDocInfo/PhpDocInfoTest.php +++ b/packages/better-php-doc-parser/tests/PhpDocInfo/PhpDocInfo/PhpDocInfoTest.php @@ -73,19 +73,19 @@ final class PhpDocInfoTest extends AbstractKernelTestCase new PreSlashStringType(), ]); - $this->assertSame($expectedUnionType, $paramType); + $this->assertEquals($expectedUnionType, $paramType); } public function testGetVarType(): void { $expectedObjectType = new ObjectType('SomeType'); - $this->assertSame($expectedObjectType, $this->phpDocInfo->getVarType()); + $this->assertEquals($expectedObjectType, $this->phpDocInfo->getVarType()); } public function testGetReturnType(): void { $expectedObjectType = new ObjectType('SomeType'); - $this->assertSame($expectedObjectType, $this->phpDocInfo->getReturnType()); + $this->assertEquals($expectedObjectType, $this->phpDocInfo->getReturnType()); } public function testReplaceTagByAnother(): void diff --git a/rules/phpunit/src/Composer/ComposerAutoloadedDirectoryProvider.php b/rules/phpunit/src/Composer/ComposerAutoloadedDirectoryProvider.php index c3ae2d3b928..44cd75a3b1f 100644 --- a/rules/phpunit/src/Composer/ComposerAutoloadedDirectoryProvider.php +++ b/rules/phpunit/src/Composer/ComposerAutoloadedDirectoryProvider.php @@ -31,7 +31,7 @@ final class ComposerAutoloadedDirectoryProvider public function provide(): array { if (PHPUnitEnvironment::isPHPUnitRun()) { - return [getcwd() . '/src', getcwd() . '/tests', getcwd() . '/packages']; + return [getcwd() . '/src', getcwd() . '/tests', getcwd() . '/packages', getcwd() . '/rules']; } $composerJson = $this->loadComposerJsonArray(); diff --git a/rules/phpunit/src/Rector/Class_/AddSeeTestAnnotationRector.php b/rules/phpunit/src/Rector/Class_/AddSeeTestAnnotationRector.php index 7b7e02d08bc..9c389c70a8e 100644 --- a/rules/phpunit/src/Rector/Class_/AddSeeTestAnnotationRector.php +++ b/rules/phpunit/src/Rector/Class_/AddSeeTestAnnotationRector.php @@ -8,6 +8,7 @@ use Nette\Loaders\RobotLoader; use Nette\Utils\Strings; use PhpParser\Node; use PhpParser\Node\Stmt\Class_; +use PHPStan\PhpDocParser\Ast\PhpDoc\GenericTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode; use Rector\AttributeAwarePhpDoc\Ast\PhpDoc\AttributeAwareGenericTagValueNode; use Rector\AttributeAwarePhpDoc\Ast\PhpDoc\AttributeAwarePhpDocTagNode; @@ -49,7 +50,9 @@ class SomeService { } -class SomeServiceTest extends \PHPUnit\Framework\TestCase +use PHPUnit\Framework\TestCase; + +class SomeServiceTest extends TestCase { } PHP @@ -62,7 +65,9 @@ class SomeService { } -class SomeServiceTest extends \PHPUnit\Framework\TestCase +use PHPUnit\Framework\TestCase; + +class SomeServiceTest extends TestCase { } PHP @@ -84,53 +89,44 @@ PHP */ public function refactor(Node $node): ?Node { - if ($this->shouldSkipClass($node)) { + $testCaseClassName = $this->resolveTestCaseClassName($node); + if ($testCaseClassName === null) { + return null; + } + + if ($this->shouldSkipClass($node, $testCaseClassName)) { return null; } /** @var PhpDocInfo $phpDocInfo */ $phpDocInfo = $node->getAttribute(AttributeKey::PHP_DOC_INFO); - /** @var string $className */ - $className = $this->getName($node); - - $testCaseClassName = $this->resolveTestCaseClassName($className); - if ($testCaseClassName === null) { - return null; - } - $seeTagNode = $this->createSeePhpDocTagNode($testCaseClassName); $phpDocInfo->addPhpDocTagNode($seeTagNode); return $node; } - private function shouldSkipClass(Class_ $class): bool + private function shouldSkipClass(Class_ $class, string $testCaseClassName): bool { - if ($class->isAnonymous()) { + // we are in the test case + if ($this->isName($class, '*Test')) { return true; } - $className = $this->getName($class); - if ($className === null) { - return true; - } + /** @var PhpDocInfo $phpDocInfo */ + $phpDocInfo = $class->getAttribute(AttributeKey::PHP_DOC_INFO); - // is a test case - if (Strings::endsWith($className, 'Test')) { - return true; - } + $seeTags = $phpDocInfo->getTagsByName('see'); // is the @see annotation already added - if ($class->getDocComment() !== null) { - /** @var string $docCommentText */ - $docCommentText = $class->getDocComment()->getText(); + foreach ($seeTags as $seeTag) { + if (! $seeTag->value instanceof GenericTagValueNode) { + continue; + } - /** @var string $shortClassName */ - $shortClassName = Strings::after($className, '\\', -1); - $seeClassPattern = '#@see (.*?)' . preg_quote($shortClassName, '#') . 'Test#m'; - - if (Strings::match($docCommentText, $seeClassPattern)) { + $seeTagClass = ltrim($seeTag->value->value, '\\'); + if ($seeTagClass === $testCaseClassName) { return true; } } @@ -138,8 +134,18 @@ PHP return false; } - private function resolveTestCaseClassName(string $className): ?string + private function resolveTestCaseClassName(Class_ $class): ?string { + if ($this->isAnonymousClass($class)) { + return null; + } + + $className = $this->getName($class); + if ($className === null) { + return null; + } + + // fallback for unit tests that only have extra "Test" suffix if (class_exists($className . 'Test')) { return $className . 'Test'; } @@ -182,6 +188,7 @@ PHP private function createRobotLoadForDirectories(): RobotLoader { $robotLoader = new RobotLoader(); + $robotLoader->setTempDirectory(sys_get_temp_dir() . '/tests_add_see_rector_tests'); $directories = $this->composerAutoloadedDirectoryProvider->provide(); foreach ($directories as $directory) { diff --git a/rules/psr4/tests/FileRelocationResolverTest.php b/rules/psr4/tests/FileRelocationResolverTest.php index e039cafb31d..8f4e74b7cc9 100644 --- a/rules/psr4/tests/FileRelocationResolverTest.php +++ b/rules/psr4/tests/FileRelocationResolverTest.php @@ -47,7 +47,7 @@ final class FileRelocationResolverTest extends AbstractKernelTestCase __DIR__ . '/Source/SomeFile.php', SomeFile::class, 'Rector\PSR10\Tests\Source\SomeFile', - 'packages/psr4/tests/Source/SomeFile.php', + 'rules/psr4/tests/Source/SomeFile.php', ]; } } diff --git a/sonar-project.properties b/sonar-project.properties index 7b32942ab34..72d7c8b94a3 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -4,9 +4,9 @@ sonar.projectKey=rectorphp_rector # relative paths to source # wildcards don't work :( -sonar.sources=compiler/src,src,packages/architecture/src,packages/attribute-aware-php-doc/src,packages/autodiscovery/src,packages/better-php-doc-parser/src,packages/cakephp/src,packages/cakephp-to-symfony/src,packages/celebrity/src,packages/code-quality/src,packages/coding-style/src,packages/console-differ/src,packages/dead-code/src,packages/doctrine/src,packages/doctrine-code-quality/src,packages/doctrine-gedmo-to-knplabs/src,packages/dynamic-type-analysis/src,packages/elastic-search-dsl/src,packages/file-system-rector/src,packages/framework-migration/src,packages/guzzle/src,packages/laravel/src,packages/legacy/src,packages/minimal-scope/src,packages/mysql-to-mysqli/src,packages/nette/src,packages/nette-tester-to-phpunit/src,packages/nette-to-symfony/src,packages/node-type-resolver/src,packages/phalcon/src,packages/php-52/src,packages/php-53/src,packages/php-54/src,packages/php-55/src,packages/php-56/src,packages/php-70/src,packages/php-71/src,packages/php-72/src,packages/php-73/src,packages/php-74/src,packages/php-80/src,packages/php-deglobalize/src,packages/php-spec-to-phpunit/src,packages/phpstan/src,packages/phpstan-static-type-mapper/src,packages/phpunit/src,packages/phpunit-symfony/src,packages/polyfill/src,packages/psr4/src,packages/rector-generator/src,packages/refactoring/src,packages/removing-static/src,packages/renaming/src,packages/restoration/src,packages/sensio/src,packages/shopware/src,packages/silverstripe/src,packages/solid/src,packages/strict-code-quality/src,packages/sylius/src,packages/symfony/src,packages/symfony-code-quality/src,packages/symfony-phpunit/src,packages/twig/src,packages/type-declaration/src,packages/zend-to-symfony/src +sonar.sources=compiler/src,src,rules/autodiscovery/src,rules/architecture/src,packages/attribute-aware-php-doc/src,packages/better-php-doc-parser/src,rules/cakephp/src,rules/celebrity/src,rules/code-quality/src,rules/coding-style/src,packages/console-differ/src,rules/dead-code/src,rules/doctrine/src,rules/doctrine-code-quality/src,rules/framework-migration/src,packages/file-system-rector/src,rules/elastic-search-dsl/src,rules/guzzle/src,rules/laravel/src,packages/legacy/src,rules/mysql-to-mysqli/src,rules/nette-tester-to-phpunit/src,rules/nette-to-symfony/src,packages/nette/src,packages/node-collector/src,packages/node-type-resolver/src,packages/node-name-resolver/src,rules/phpstan/src,packages/phpstan-static-type-mapper/src,rules/phpunit-symfony/src,rules/phpunit/src,rules/psr4/src,rules/php-spec-to-phpunit/src,rules/php-52/src,rules/php-53/src,rules/php-54/src,rules/php-55/src,rules/php-56/src,rules/php-70/src,rules/php-71/src,rules/php-72/src,rules/php-73/src,rules/php-74/src,rules/php-80/src,rules/removing-static/src,rules/renaming/src,rules/restoration/src,packages/refactoring/src,rules/solid/src,rules/sensio/src,rules/shopware/src,rules/silverstripe/src,packages/static-type-mapper/src,rules/sylius/src,rules/symfony-code-quality/src,rules/symfony-phpunit/src,rules/symfony/src,rules/twig/src,rules/type-declaration/src,packages/vendor-locker/src,rules/zend-to-symfony/src,packages/rector-generator/src,rules/strict-code-quality/src,packages/dynamic-type-analysis/src,rules/php-deglobalize/src,rules/phalcon/src,rules/doctrine-gedmo-to-knplabs/src,rules/minimal-scope/src,packages/polyfill/src,rules/cakephp-to-symfony/src -sonar.tests=tests,packages/architecture/tests,packages/autodiscovery/tests,packages/better-php-doc-parser/tests,packages/cakephp/tests,packages/cakephp-to-symfony/tests,packages/celebrity/tests,packages/code-quality/tests,packages/coding-style/tests,packages/dead-code/tests,packages/doctrine/tests,packages/doctrine-code-quality/tests,packages/doctrine-gedmo-to-knplabs/tests,packages/dynamic-type-analysis/tests,packages/elastic-search-dsl/tests,packages/guzzle/tests,packages/laravel/tests,packages/legacy/tests,packages/minimal-scope/tests,packages/mysql-to-mysqli/tests,packages/nette/tests,packages/nette-tester-to-phpunit/tests,packages/nette-to-symfony/tests,packages/node-type-resolver/tests,packages/phalcon/tests,packages/php-52/tests,packages/php-53/tests,packages/php-54/tests,packages/php-55/tests,packages/php-56/tests,packages/php-70/tests,packages/php-71/tests,packages/php-72/tests,packages/php-73/tests,packages/php-74/tests,packages/php-80/tests,packages/php-deglobalize/tests,packages/php-spec-to-phpunit/tests,packages/phpstan/tests,packages/phpunit/tests,packages/phpunit-symfony/tests,packages/polyfill/tests,packages/psr4/tests,packages/removing-static/tests,packages/renaming/tests,packages/restoration/tests,packages/sensio/tests,packages/shopware/tests,packages/silverstripe/tests,packages/solid/tests,packages/strict-code-quality/tests,packages/sylius/tests,packages/symfony/tests,packages/symfony-code-quality/tests,packages/symfony-phpunit/tests,packages/twig/tests,packages/type-declaration/tests,packages/zend-to-symfony/tests +sonar.tests=tests,rules/architecture/tests,packages/autodiscovery/tests,packages/better-php-doc-parser/tests,packages/cakephp/tests,packages/cakephp-to-symfony/tests,packages/celebrity/tests,packages/code-quality/tests,packages/coding-style/tests,packages/dead-code/tests,packages/doctrine/tests,packages/doctrine-code-quality/tests,packages/doctrine-gedmo-to-knplabs/tests,packages/dynamic-type-analysis/tests,packages/elastic-search-dsl/tests,packages/guzzle/tests,packages/laravel/tests,packages/legacy/tests,packages/minimal-scope/tests,packages/mysql-to-mysqli/tests,packages/nette/tests,packages/nette-tester-to-phpunit/tests,packages/nette-to-symfony/tests,packages/node-type-resolver/tests,packages/phalcon/tests,packages/php-52/tests,packages/php-53/tests,packages/php-54/tests,packages/php-55/tests,packages/php-56/tests,packages/php-70/tests,packages/php-71/tests,packages/php-72/tests,packages/php-73/tests,packages/php-74/tests,packages/php-80/tests,packages/php-deglobalize/tests,packages/php-spec-to-phpunit/tests,packages/phpstan/tests,packages/phpunit/tests,packages/phpunit-symfony/tests,packages/polyfill/tests,packages/psr4/tests,packages/removing-static/tests,packages/renaming/tests,packages/restoration/tests,packages/sensio/tests,packages/shopware/tests,packages/silverstripe/tests,packages/solid/tests,packages/strict-code-quality/tests,packages/sylius/tests,packages/symfony/tests,packages/symfony-code-quality/tests,packages/symfony-phpunit/tests,packages/twig/tests,packages/type-declaration/tests,packages/zend-to-symfony/tests # see https://docs.sonarqube.org/latest/project-administration/narrowing-the-focus/#NarrowingtheFocus-patterns -sonar.exclusions=src/**/*.php.inc,packages/**/*.php.inc,packages/**/Fixture/**/*,tests/**/Source/**/* +sonar.exclusions=src/**/*.php.inc,rules/**/*.php.inc,packages/**/*.php.inc,packages/**/Fixture/**/*,rules/**/Fixture/**/*,tests/**/Source/**/* diff --git a/src/Testing/Finder/RectorsFinder.php b/src/Testing/Finder/RectorsFinder.php index 6ca4d1eb87a..f1a6c477e97 100644 --- a/src/Testing/Finder/RectorsFinder.php +++ b/src/Testing/Finder/RectorsFinder.php @@ -13,12 +13,21 @@ use ReflectionClass; final class RectorsFinder { + /** + * @var string[] + */ + private const RECTOR_PATHS = [ + __DIR__ . '/../../../rules', + __DIR__ . '/../../../packages', + __DIR__ . '/../../../src', + ]; + /** * @return string[] */ public function findCoreRectorClasses(): array { - $allRectors = $this->findInDirectories([__DIR__ . '/../../../packages', __DIR__ . '/../../../src']); + $allRectors = $this->findInDirectories(self::RECTOR_PATHS); return array_map(function (RectorInterface $rector): string { return get_class($rector);