From 16df9f40704a08ab0b21f00d76b7b37102c8d6c4 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 10 Jul 2021 02:49:32 +0200 Subject: [PATCH] [CodingStandard] Add DoctrineAnnotationNestedBracketsFixer (#3396) --- composer.json | 2 +- ecs.php | 6 + .../SimpleCallableNodeTraverser.php | 2 +- packages/coding-standard/config/config.php | 2 + .../DoctrineAnnotationNestedBracketsFixer.php | 183 ++++++++++++++++++ .../Commenting/RemoveCommentedCodeFixer.php | 3 +- .../LineLength/DocBlockLineLengthFixer.php | 3 + .../DoctrineAnnotationElementAnalyzer.php | 79 ++++++++ .../DoctrineAnnotationNameResolver.php | 45 +++++ ...trineAnnotationNestedBracketsFixerTest.php | 34 ++++ .../Fixture/skip_already_wrapped.php.inc | 18 ++ .../skip_not_configured_annotation.php.inc | 16 ++ ...nested_annotation_without_brackets.php.inc | 37 ++++ .../Source/MissingYou.php | 9 + .../Source/SomeAnnotation.php | 9 + .../Source/SomeAnnotations.php | 9 + .../config/configured_rule.php | 15 ++ .../ClassReferencePhpDocNodeVisitor.php | 5 +- .../src/Bundle/SimplePhpDocParserBundle.php | 3 +- .../CloningPhpDocNodeVisitor.php | 5 +- .../ParentConnectingPhpDocNodeVisitor.php | 5 +- phpstan.neon | 3 + rector.php | 5 + 23 files changed, 481 insertions(+), 17 deletions(-) create mode 100644 packages/coding-standard/src/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer.php create mode 100644 packages/coding-standard/src/TokenAnalyzer/DoctrineAnnotationElementAnalyzer.php create mode 100644 packages/coding-standard/src/TokenAnalyzer/DoctrineAnnotationNameResolver.php create mode 100644 packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/DoctrineAnnotationNestedBracketsFixerTest.php create mode 100644 packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/Fixture/skip_already_wrapped.php.inc create mode 100644 packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/Fixture/skip_not_configured_annotation.php.inc create mode 100644 packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/Fixture/some_nested_annotation_without_brackets.php.inc create mode 100644 packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/Source/MissingYou.php create mode 100644 packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/Source/SomeAnnotation.php create mode 100644 packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/Source/SomeAnnotations.php create mode 100644 packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/config/configured_rule.php diff --git a/composer.json b/composer.json index 656a3fa188..fcd54589a8 100644 --- a/composer.json +++ b/composer.json @@ -56,7 +56,7 @@ "ondram/ci-detector": "^4.1", "phpunit/phpunit": "^9.5", "psr/log": "^1.1", - "rector/rector": "dev-main#ecda10a", + "rector/rector": "dev-main#54dcc7b", "symfony/doctrine-bridge": "^5.3", "symfony/framework-bundle": "^5.3", "symfony/security-bundle": "^5.3", diff --git a/ecs.php b/ecs.php index 31c24a1720..99ac3fd711 100644 --- a/ecs.php +++ b/ecs.php @@ -4,6 +4,7 @@ use PhpCsFixer\Fixer\PhpUnit\PhpUnitStrictFixer; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; +use Symplify\CodingStandard\Fixer\Annotation\DoctrineAnnotationNestedBracketsFixer; use Symplify\CodingStandard\Fixer\LineLength\LineLengthFixer; use Symplify\EasyCodingStandard\ValueObject\Option; use Symplify\EasyCodingStandard\ValueObject\Set\SetList; @@ -12,6 +13,11 @@ $services = $containerConfigurator->services(); $services->set(LineLengthFixer::class); + $services->set(DoctrineAnnotationNestedBracketsFixer::class) + ->call('configure', [[ + DoctrineAnnotationNestedBracketsFixer::ANNOTATION_CLASSES => ['Doctrine\ORM\JoinColumns'], + ]]); + $containerConfigurator->import(SetList::CLEAN_CODE); $containerConfigurator->import(SetList::SYMPLIFY); $containerConfigurator->import(SetList::COMMON); diff --git a/packages/astral/src/NodeTraverser/SimpleCallableNodeTraverser.php b/packages/astral/src/NodeTraverser/SimpleCallableNodeTraverser.php index 779f9e9597..fd8470d9ee 100644 --- a/packages/astral/src/NodeTraverser/SimpleCallableNodeTraverser.php +++ b/packages/astral/src/NodeTraverser/SimpleCallableNodeTraverser.php @@ -17,7 +17,7 @@ final class SimpleCallableNodeTraverser /** * @param Node|Node[]|null $nodes */ - public function traverseNodesWithCallable($nodes, callable $callable): void + public function traverseNodesWithCallable(Node | array | null $nodes, callable $callable): void { if ($nodes === null) { return; diff --git a/packages/coding-standard/config/config.php b/packages/coding-standard/config/config.php index 0e99225f44..bbdf4bf508 100644 --- a/packages/coding-standard/config/config.php +++ b/packages/coding-standard/config/config.php @@ -3,6 +3,7 @@ declare(strict_types=1); use PhpCsFixer\Tokenizer\Analyzer\FunctionsAnalyzer; +use PhpCsFixer\Tokenizer\Analyzer\NamespaceUsesAnalyzer; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; use Symplify\EasyCodingStandard\Caching\ChangedFilesDetector; use Symplify\PackageBuilder\Reflection\PrivatesAccessor; @@ -24,6 +25,7 @@ __DIR__ . '/../src/ValueObject', ]); + $services->set(NamespaceUsesAnalyzer::class); $services->set(FunctionsAnalyzer::class); $services->set(PrivatesAccessor::class); diff --git a/packages/coding-standard/src/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer.php b/packages/coding-standard/src/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer.php new file mode 100644 index 0000000000..6d08574474 --- /dev/null +++ b/packages/coding-standard/src/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer.php @@ -0,0 +1,183 @@ + ['MainAnnotation'], + ] + ), + ]); + } + + /** + * @param array $configuration + */ + public function configure(array $configuration): void + { + $annotationsClasses = $configuration[self::ANNOTATION_CLASSES] ?? []; + Assert::isArray($annotationsClasses); + Assert::allString($annotationsClasses); + + $this->annotationClasses = $annotationsClasses; + } + + /** + * @param Tokens $tokens + */ + public function isCandidate(Tokens $tokens): bool + { + return $tokens->isTokenKindFound(T_DOC_COMMENT); + } + + /** + * @param Tokens $tokens + */ + public function fix(SplFileInfo $fileInfo, Tokens $tokens): void + { + $useDeclarations = $this->namespaceUsesAnalyzer->getDeclarationsFromTokens($tokens); + + // fetch indexes one time, this is safe as we never add or remove a token during fixing + + /** @var Token[] $docCommentTokens */ + $docCommentTokens = $tokens->findGivenKind(T_DOC_COMMENT); + foreach ($docCommentTokens as $index => $docCommentToken) { + if (! $this->doctrineAnnotationElementAnalyzer->detect($tokens, $index)) { + continue; + } + + $doctrineAnnotationTokens = DoctrineAnnotationTokens::createFromDocComment($docCommentToken, []); + $this->fixAnnotations($doctrineAnnotationTokens, $useDeclarations); + + $tokens[$index] = new Token([T_DOC_COMMENT, $doctrineAnnotationTokens->getCode()]); + } + } + + /** + * @param DoctrineAnnotationTokens $doctrineAnnotationTokens + */ + private function fixAnnotations(DoctrineAnnotationTokens $doctrineAnnotationTokens, $useDeclarations): void + { + foreach ($doctrineAnnotationTokens as $index => $token) { + $isAtToken = $doctrineAnnotationTokens[$index]->isType(DocLexer::T_AT); + if (! $isAtToken) { + continue; + } + + $annotationName = $this->doctrineAnnotationNameResolver->resolveName( + $doctrineAnnotationTokens, + $index, + $useDeclarations + ); + if ($annotationName === null) { + continue; + } + + if (! in_array($annotationName, $this->annotationClasses, true)) { + continue; + } + + $closingBraceIndex = $doctrineAnnotationTokens->getAnnotationEnd($index); + if ($closingBraceIndex === null) { + continue; + } + + $braceIndex = $doctrineAnnotationTokens->getNextMeaningfulToken($index + 1); + if ($braceIndex === null) { + continue; + } + + /** @var DoctrineAnnotationToken $braceToken */ + $braceToken = $doctrineAnnotationTokens[$braceIndex]; + if (! $this->doctrineAnnotationElementAnalyzer->isOpeningBracketFollowedByAnnotation( + $braceToken, + $doctrineAnnotationTokens, + $braceIndex + )) { + continue; + } + + // add closing brace + $doctrineAnnotationTokens->insertAt( + $closingBraceIndex, + new DoctrineAnnotationToken(DocLexer::T_OPEN_CURLY_BRACES, '}') + ); + + // add opening brace + $doctrineAnnotationTokens->insertAt( + $braceIndex + 1, + new DoctrineAnnotationToken(DocLexer::T_OPEN_CURLY_BRACES, '{') + ); + } + } +} diff --git a/packages/coding-standard/src/Fixer/Commenting/RemoveCommentedCodeFixer.php b/packages/coding-standard/src/Fixer/Commenting/RemoveCommentedCodeFixer.php index b49599accc..9e3f68e9d0 100644 --- a/packages/coding-standard/src/Fixer/Commenting/RemoveCommentedCodeFixer.php +++ b/packages/coding-standard/src/Fixer/Commenting/RemoveCommentedCodeFixer.php @@ -19,7 +19,8 @@ use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @see https://softwareengineering.stackexchange.com/a/394288/148956 + * @deprecated This rule is seriously buggy. Don't use it. We're working on a better replacement + * @see https://github.com/symplify/symplify/issues/3395 * * @see \Symplify\CodingStandard\Tests\Fixer\Commenting\RemoveCommentedCodeFixer\RemoveCommentedCodeFixerTest */ diff --git a/packages/coding-standard/src/Fixer/LineLength/DocBlockLineLengthFixer.php b/packages/coding-standard/src/Fixer/LineLength/DocBlockLineLengthFixer.php index b4ee86f629..fdf96103ee 100644 --- a/packages/coding-standard/src/Fixer/LineLength/DocBlockLineLengthFixer.php +++ b/packages/coding-standard/src/Fixer/LineLength/DocBlockLineLengthFixer.php @@ -122,6 +122,9 @@ public function fix(SplFileInfo $file, Tokens $tokens): void } } + /** + * @param array $configuration + */ public function configure(array $configuration): void { $this->lineLength = $configuration[self::LINE_LENGTH] ?? self::DEFAULT_LINE_LENGHT; diff --git a/packages/coding-standard/src/TokenAnalyzer/DoctrineAnnotationElementAnalyzer.php b/packages/coding-standard/src/TokenAnalyzer/DoctrineAnnotationElementAnalyzer.php new file mode 100644 index 0000000000..754d498494 --- /dev/null +++ b/packages/coding-standard/src/TokenAnalyzer/DoctrineAnnotationElementAnalyzer.php @@ -0,0 +1,79 @@ + $tokens + */ + public function detect(Tokens $tokens, int $index): bool + { + $tokensAnalyzer = new TokensAnalyzer($tokens); + $classyElements = $tokensAnalyzer->getClassyElements(); + + do { + $index = $tokens->getNextMeaningfulToken($index); + + if ($index === null) { + return false; + } + } while ($tokens[$index]->isGivenKind([T_ABSTRACT, T_FINAL])); + + if ($tokens[$index]->isClassy()) { + return true; + } + + while ($tokens[$index]->isGivenKind( + [T_PUBLIC, T_PROTECTED, T_PRIVATE, T_FINAL, T_ABSTRACT, T_NS_SEPARATOR, T_STRING, CT::T_NULLABLE_TYPE] + )) { + $index = $tokens->getNextMeaningfulToken($index); + if (! is_int($index)) { + return false; + } + } + + return isset($classyElements[$index]); + } + + /** + * We look for "(@SomeAnnotation" + * + * @param DoctrineAnnotationTokens $doctrineAnnotationTokens + */ + public function isOpeningBracketFollowedByAnnotation( + Token $token, + DoctrineAnnotationTokens $doctrineAnnotationTokens, + int $braceIndex + ): bool { + // should be "(" + $isNextOpenParenthesis = $token->isType(DocLexer::T_OPEN_PARENTHESIS); + if (! $isNextOpenParenthesis) { + return false; + } + + $nextTokenIndex = $doctrineAnnotationTokens->getNextMeaningfulToken($braceIndex); + if ($nextTokenIndex === null) { + return false; + } + + /** @var Token $nextToken */ + $nextToken = $doctrineAnnotationTokens[$nextTokenIndex]; + + // next token must be nested annotation, we don't care otherwise + return $nextToken->isType(DocLexer::T_AT); + } +} diff --git a/packages/coding-standard/src/TokenAnalyzer/DoctrineAnnotationNameResolver.php b/packages/coding-standard/src/TokenAnalyzer/DoctrineAnnotationNameResolver.php new file mode 100644 index 0000000000..cfef4a9edd --- /dev/null +++ b/packages/coding-standard/src/TokenAnalyzer/DoctrineAnnotationNameResolver.php @@ -0,0 +1,45 @@ + $tokens + * @param NamespaceUseAnalysis[] $namespaceUseAnalyses + */ + public function resolveName(Tokens $tokens, int $index, array $namespaceUseAnalyses): ?string + { + $openParenthesisPosition = $tokens->getNextTokenOfType(DocLexer::T_OPEN_PARENTHESIS, $index); + if ($openParenthesisPosition === null) { + return null; + } + + $annotationShortName = ''; + + for ($i = $index + 1; $i < $openParenthesisPosition; ++$i) { + /** @var Token $currentToken */ + $currentToken = $tokens[$i]; + $annotationShortName .= $currentToken->getContent(); + } + + if ($annotationShortName === '') { + return null; + } + + foreach ($namespaceUseAnalyses as $namespaceUseAnalysis) { + if ($namespaceUseAnalysis->getShortName() === $annotationShortName) { + return $namespaceUseAnalysis->getFullName(); + } + } + + return $annotationShortName; + } +} diff --git a/packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/DoctrineAnnotationNestedBracketsFixerTest.php b/packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/DoctrineAnnotationNestedBracketsFixerTest.php new file mode 100644 index 0000000000..3c7cd23e65 --- /dev/null +++ b/packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/DoctrineAnnotationNestedBracketsFixerTest.php @@ -0,0 +1,34 @@ +doTestFileInfo($fileInfo); + } + + /** + * @return Iterator + */ + public function provideData(): Iterator + { + return StaticFixtureFinder::yieldDirectoryExclusively(__DIR__ . '/Fixture'); + } + + public function provideConfig(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/Fixture/skip_already_wrapped.php.inc b/packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/Fixture/skip_already_wrapped.php.inc new file mode 100644 index 0000000000..44fd38ba91 --- /dev/null +++ b/packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/Fixture/skip_already_wrapped.php.inc @@ -0,0 +1,18 @@ + +----- + diff --git a/packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/Source/MissingYou.php b/packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/Source/MissingYou.php new file mode 100644 index 0000000000..10ef8634ab --- /dev/null +++ b/packages/coding-standard/tests/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer/Source/MissingYou.php @@ -0,0 +1,9 @@ +services(); + $services->set(DoctrineAnnotationNestedBracketsFixer::class) + ->call('configure', [[ + DoctrineAnnotationNestedBracketsFixer::ANNOTATION_CLASSES => [SomeAnnotations::class], + ]]); +}; diff --git a/packages/phpstan-rules/src/PhpDoc/ClassReferencePhpDocNodeVisitor.php b/packages/phpstan-rules/src/PhpDoc/ClassReferencePhpDocNodeVisitor.php index 86f0f4ac3a..5fec81c748 100644 --- a/packages/phpstan-rules/src/PhpDoc/ClassReferencePhpDocNodeVisitor.php +++ b/packages/phpstan-rules/src/PhpDoc/ClassReferencePhpDocNodeVisitor.php @@ -46,10 +46,7 @@ public function configureClassName(string $className): void $this->className = $className; } - /** - * @return int|Node|null - */ - public function enterNode(Node $node) + public function enterNode(Node $node): Node { if ($node instanceof PhpDocTagNode) { $this->processPhpDocTagNode($node); diff --git a/packages/simple-php-doc-parser/src/Bundle/SimplePhpDocParserBundle.php b/packages/simple-php-doc-parser/src/Bundle/SimplePhpDocParserBundle.php index 4dff0a29bb..9be28850c4 100644 --- a/packages/simple-php-doc-parser/src/Bundle/SimplePhpDocParserBundle.php +++ b/packages/simple-php-doc-parser/src/Bundle/SimplePhpDocParserBundle.php @@ -4,13 +4,12 @@ namespace Symplify\SimplePhpDocParser\Bundle; -use Symfony\Component\DependencyInjection\Extension\ExtensionInterface; use Symfony\Component\HttpKernel\Bundle\Bundle; use Symplify\SimplePhpDocParser\Bundle\DependencyInjection\Extension\SimplePhpDocParserExtension; final class SimplePhpDocParserBundle extends Bundle { - public function getContainerExtension(): ?ExtensionInterface + public function getContainerExtension(): SimplePhpDocParserExtension { return new SimplePhpDocParserExtension(); } diff --git a/packages/simple-php-doc-parser/src/PhpDocNodeVisitor/CloningPhpDocNodeVisitor.php b/packages/simple-php-doc-parser/src/PhpDocNodeVisitor/CloningPhpDocNodeVisitor.php index ecf5304078..12571642c6 100644 --- a/packages/simple-php-doc-parser/src/PhpDocNodeVisitor/CloningPhpDocNodeVisitor.php +++ b/packages/simple-php-doc-parser/src/PhpDocNodeVisitor/CloningPhpDocNodeVisitor.php @@ -13,10 +13,7 @@ */ final class CloningPhpDocNodeVisitor extends AbstractPhpDocNodeVisitor { - /** - * @return int|Node|null - */ - public function enterNode(Node $origNode) + public function enterNode(Node $origNode): Node { $node = clone $origNode; $node->setAttribute(PhpDocAttributeKey::ORIG_NODE, $origNode); diff --git a/packages/simple-php-doc-parser/src/PhpDocNodeVisitor/ParentConnectingPhpDocNodeVisitor.php b/packages/simple-php-doc-parser/src/PhpDocNodeVisitor/ParentConnectingPhpDocNodeVisitor.php index 7ee5127de9..c391a51706 100644 --- a/packages/simple-php-doc-parser/src/PhpDocNodeVisitor/ParentConnectingPhpDocNodeVisitor.php +++ b/packages/simple-php-doc-parser/src/PhpDocNodeVisitor/ParentConnectingPhpDocNodeVisitor.php @@ -24,10 +24,7 @@ public function beforeTraverse(Node $node): void $this->stack = [$node]; } - /** - * @return int|Node|null - */ - public function enterNode(Node $node) + public function enterNode(Node $node): Node { if ($this->stack !== []) { $parentNode = $this->stack[count($this->stack) - 1]; diff --git a/phpstan.neon b/phpstan.neon index ddd46aa9e7..d5f5234a19 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -524,3 +524,6 @@ parameters: - message: '#Cognitive complexity for "Symplify\\SimplePhpDocParser\\PhpDocNodeTraverser\:\:(.*?)\(\)" is \d+, keep it under \d+#' path: packages/simple-php-doc-parser/src/PhpDocNodeTraverser.php + + # wrong php-cs-fixer doc types + - '#Parameter \#1 \$type of method PhpCsFixer\\Doctrine\\Annotation\\Tokens\:\:getNextTokenOfType\(\) expects array\|string, int given#' diff --git a/rector.php b/rector.php index 74e1fe4aa5..57ba63e88b 100644 --- a/rector.php +++ b/rector.php @@ -98,6 +98,11 @@ // many false postivies RenameForeachValueVariableToMatchExprVariableRector::class, + // buggy on array access object + \Rector\CodeQuality\Rector\Foreach_\UnusedForeachValueToArrayKeysRector::class => [ + __DIR__ . '/packages/coding-standard/src/Fixer/Annotation/DoctrineAnnotationNestedBracketsFixer.php', + ], + // buggy with parent interface contract ParamTypeDeclarationRector::class => [__DIR__ . '/packages/skipper/src/SkipVoter/*SkipVoter.php'], UnSpreadOperatorRector::class => [__DIR__ . '/packages/git-wrapper'],