feat(PhpdocToCommentFixer): Add option to handle return as valid docblock usage (#7401) (#7402)

doc/rules/phpdoc/phpdoc_to_comment.rst

@@ -7,6 +7,15 @@ Docblocks should only be used on structural elements.
 Configuration
 -------------
 
+``allow_before_return_statement``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Whether to allow PHPDoc before return statement.
+
+Allowed types: ``bool``
+
+Default value: ``false``
+
 ``ignored_tags``
 ~~~~~~~~~~~~~~~~
 
@@ -60,6 +69,29 @@ With configuration: ``['ignored_tags' => ['todo']]``.
         $sqlite->open($path);
     }
 
+Example #3
+~~~~~~~~~~
+
+With configuration: ``['allow_before_return_statement' => true]``.
+
+.. code-block:: diff
+
+   --- Original
+   +++ New
+    <?php
+    $first = true;// needed because by default first docblock is never fixed.
+
+   -/** This should be a comment */
+   +/* This should be a comment */
+    foreach($connections as $key => $sqlite) {
+        $sqlite->open($path);
+    }
+
+    function returnClassName() {
+        /** @var class-string */
+        return \StdClass::class;
+    }
+
 Rule sets
 ---------
 

src/Fixer/Phpdoc/PhpdocToCommentFixer.php

@@ -37,6 +37,7 @@ final class PhpdocToCommentFixer extends AbstractFixer implements ConfigurableFi
      * @var string[]
      */
     private array $ignoredTags = [];
+    private bool $allowBeforeReturnStatement = false;
 
     public function isCandidate(Tokens $tokens): bool
     {
@@ -90,6 +91,22 @@ foreach($connections as $key => $sqlite) {
 ',
                     ['ignored_tags' => ['todo']]
                 ),
+                new CodeSample(
+                    '<?php
+$first = true;// needed because by default first docblock is never fixed.
+
+/** This should be a comment */
+foreach($connections as $key => $sqlite) {
+    $sqlite->open($path);
+}
+
+function returnClassName() {
+    /** @var class-string */
+    return \StdClass::class;
+}
+',
+                    ['allow_before_return_statement' => true]
+                ),
             ]
         );
     }
@@ -102,6 +119,8 @@ foreach($connections as $key => $sqlite) {
             static fn (string $tag): string => strtolower($tag),
             $this->configuration['ignored_tags']
         );
+
+        $this->allowBeforeReturnStatement = true === $this->configuration['allow_before_return_statement'];
     }
 
     protected function createConfigurationDefinition(): FixerConfigurationResolverInterface
@@ -111,6 +130,10 @@ foreach($connections as $key => $sqlite) {
                 ->setAllowedTypes(['array'])
                 ->setDefault([])
                 ->getOption(),
+            (new FixerOptionBuilder('allow_before_return_statement', 'Whether to allow PHPDoc before return statement.'))
+                ->setAllowedTypes(['bool'])
+                ->setDefault(false) // @TODO 4.0: set to `true`
+                ->getOption(),
         ]);
     }
 
@@ -127,6 +150,10 @@ foreach($connections as $key => $sqlite) {
                 continue;
             }
 
+            if ($this->allowBeforeReturnStatement && $commentsAnalyzer->isBeforeReturn($tokens, $index)) {
+                continue;
+            }
+
             if ($commentsAnalyzer->isBeforeStructuralElement($tokens, $index)) {
                 continue;
             }

src/Tokenizer/Analyzer/CommentsAnalyzer.php

@@ -73,18 +73,7 @@ final class CommentsAnalyzer
             throw new \InvalidArgumentException('Given index must point to a comment.');
         }
 
-        $nextIndex = $index;
-        do {
-            $nextIndex = $tokens->getNextMeaningfulToken($nextIndex);
-
-            // @TODO: drop condition when PHP 8.0+ is required
-            if (\defined('T_ATTRIBUTE')) {
-                while (null !== $nextIndex && $tokens[$nextIndex]->isGivenKind(T_ATTRIBUTE)) {
-                    $nextIndex = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_ATTRIBUTE, $nextIndex);
-                    $nextIndex = $tokens->getNextMeaningfulToken($nextIndex);
-                }
-            }
-        } while (null !== $nextIndex && $tokens[$nextIndex]->equals('('));
+        $nextIndex = $this->getNextTokenIndex($tokens, $index);
 
         if (null === $nextIndex || $tokens[$nextIndex]->equals('}')) {
             return false;
@@ -102,7 +91,7 @@ final class CommentsAnalyzer
             return true;
         }
 
-        if ($this->isValidLanguageConstruct($tokens, $token, $nextIndex)) {
+        if ($this->isValidVariableAssignment($tokens, $token, $nextIndex)) {
             return true;
         }
 
@@ -113,6 +102,24 @@ final class CommentsAnalyzer
         return false;
     }
 
+    /**
+     * Check if comment at given index precedes return statement.
+     */
+    public function isBeforeReturn(Tokens $tokens, int $index): bool
+    {
+        if (!$tokens[$index]->isGivenKind([T_COMMENT, T_DOC_COMMENT])) {
+            throw new \InvalidArgumentException('Given index must point to a comment.');
+        }
+
+        $nextIndex = $this->getNextTokenIndex($tokens, $index);
+
+        if (null === $nextIndex || $tokens[$nextIndex]->equals('}')) {
+            return false;
+        }
+
+        return $tokens[$nextIndex]->isGivenKind(T_RETURN);
+    }
+
     /**
      * Return array of indices that are part of a comment started at given index.
      *
@@ -251,7 +258,7 @@ final class CommentsAnalyzer
      * @param Token $docsToken              docs Token
      * @param int   $languageConstructIndex index of variable Token
      */
-    private function isValidLanguageConstruct(Tokens $tokens, Token $docsToken, int $languageConstructIndex): bool
+    private function isValidVariableAssignment(Tokens $tokens, Token $docsToken, int $languageConstructIndex): bool
     {
         static $languageStructures = [
             T_LIST,
@@ -321,4 +328,23 @@ final class CommentsAnalyzer
 
         return $lineCount;
     }
+
+    private function getNextTokenIndex(Tokens $tokens, int $startIndex): ?int
+    {
+        $nextIndex = $startIndex;
+
+        do {
+            $nextIndex = $tokens->getNextMeaningfulToken($nextIndex);
+
+            // @TODO: drop condition when PHP 8.0+ is required
+            if (\defined('T_ATTRIBUTE')) {
+                while (null !== $nextIndex && $tokens[$nextIndex]->isGivenKind(T_ATTRIBUTE)) {
+                    $nextIndex = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_ATTRIBUTE, $nextIndex);
+                    $nextIndex = $tokens->getNextMeaningfulToken($nextIndex);
+                }
+            }
+        } while (null !== $nextIndex && $tokens[$nextIndex]->equals('('));
+
+        return $nextIndex;
+    }
 }

tests/Fixer/Phpdoc/PhpdocToCommentFixerTest.php

@@ -685,6 +685,36 @@ foreach($connections as $key => $sqlite) {
             fn ($x) => $x + 42;
             ',
         ];
+
+        yield 'convert before return without option' => [
+            '<?php
+function doSomething()
+{
+    /* @var void */
+    return;
+}
+',
+            '<?php
+function doSomething()
+{
+    /** @var void */
+    return;
+}
+',
+            ['allow_before_return_statement' => false],
+        ];
+
+        yield 'do not convert before return with option' => [
+            '<?php
+function doSomething()
+{
+    /** @var void */
+    return;
+}
+',
+            null,
+            ['allow_before_return_statement' => true],
+        ];
     }
 
     public static function provideTraitsCases(): iterable

tests/Tokenizer/Analyzer/CommentsAnalyzerTest.php

@@ -465,4 +465,70 @@ enum Foo: int {
             ',
         ];
     }
+
+    /**
+     * @dataProvider provideReturnStatementCases
+     */
+    public function testReturnStatement(string $code, bool $expected): void
+    {
+        $tokens = Tokens::fromCode($code);
+        $index = $tokens->getNextTokenOfKind(0, [[T_COMMENT], [T_DOC_COMMENT]]);
+        $analyzer = new CommentsAnalyzer();
+
+        self::assertSame($expected, $analyzer->isBeforeReturn($tokens, $index));
+    }
+
+    /**
+     * @return iterable<string, array{string, bool}>
+     */
+    public static function provideReturnStatementCases(): iterable
+    {
+        yield 'docblock before var' => [
+            '<?php
+            function returnClassName()
+            {
+                /** @todo something */
+                $var = 123;
+
+                return;
+            }
+            ',
+            false,
+        ];
+
+        yield 'comment before var' => [
+            '<?php
+            function returnClassName()
+            {
+                // @todo something
+                $var = 123;
+
+                return;
+            }
+            ',
+            false,
+        ];
+
+        yield 'docblock return' => [
+            '<?php
+            function returnClassName()
+            {
+                /** @todo something */
+                return;
+            }
+            ',
+            true,
+        ];
+
+        yield 'comment return' => [
+            '<?php
+            function returnClassName()
+            {
+                // @todo something
+                return;
+            }
+            ',
+            true,
+        ];
+    }
 }