Repository: PHP-CS-Fixer/PHP-CS-Fixer (php)

Issue:
Fixer breaks PhpDoc before return
## Bug report

<!--
  Before reporting a bug, make sure PHP CS Fixer is up-to-date.

  Check that the issue is NOT regarded to an applied rule by using the `-vvv` option (will list applied rules) on a minimal code snippet.
-->

### Description

<!--
  Please describe the problem.
-->
`phpdoc_to_comment` rule breaks PhpDoc that is used to declare returned value from within a function.

### Runtime version

<!--
   Provide PHP CS Fixer version and PHP runtime by sharing result of this command:
   $ php-cs-fixer -V
-->
PHP CS Fixer 3.28.0 Jump & Run by Fabien Potencier and Dariusz Ruminski.
PHP runtime: 8.2.11

### Used command

<!--
   Provide the command used to run PHP CS Fixer (run with `-vvv`).
-->
`php ./vendor/bin/php-cs-fixer fix --allow-risky=yes --config=.php-cs-fixer.dist.php`

```
Loaded config default from ".php-cs-fixer.dist.php".
Using cache file "/srv/app/var/tools/.php-cs-fixer.cache".
.........F................................................                                                                                                                                                                  58 / 58 (100%)
Legend: .-no changes, F-fixed, S-skipped (cached or empty file), I-invalid file syntax (file ignored), E-error
   1) /srv/app/src/Infrastructure/State/Order/OrderGetCollectionProvider.php (phpdoc_to_comment)

Fixed 1 of 58 files in 0.743 seconds, 16.000 MB memory used
```

### Configuration file

<!--
   Provide content of configuration file you used (.php-cs-fixer[.dist].php).
-->

```php
<?php

$finder = (new PhpCsFixer\Finder())
    ->in(__DIR__.'/src')
    ->in(__DIR__.'/tests')
    ->in(__DIR__.'/migrations')
;

return (new PhpCsFixer\Config())
    ->setRiskyAllowed(true)
    ->setRules([
        '@Symfony' => true,
        '@Symfony:risky' => true,
        '@DoctrineAnnotation' => true,
    ])
    ->setFinder($finder)
    ->setCacheFile(__DIR__.'/var/tools/.php-cs-fixer.cache')
;
```

### Code snippet that reproduces the problem

<!--
  Provide a minimal code snippet before running PHP CS Fixer. If relevant,
  also provide the expected output, the actual output and/or the error that
  occurred, if any.

  Please use markdown syntax for each code snippet, e.g.:

  ```php
  <?php
  $var = 'foo';
  ```
-->
Expected:
```php
<?php

function something(): object
{
    /** @var object */
    return $this->get();
}
```
Actual:
```php
<?php

function something(): object
{
    /* @var object */
    return $this->get();
}
```

Pull request description:
feat(PhpdocToCommentFixer): Add option to handle return as valid docblock usage (#7401)
Fixes #7401
Supersedes #5109 

Patch:
diff --git a/doc/rules/phpdoc/phpdoc_to_comment.rst b/doc/rules/phpdoc/phpdoc_to_comment.rst
index f765aab99..ed2434b25 100644
--- a/doc/rules/phpdoc/phpdoc_to_comment.rst
+++ b/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
 ---------
 
diff --git a/src/Fixer/Phpdoc/PhpdocToCommentFixer.php b/src/Fixer/Phpdoc/PhpdocToCommentFixer.php
index 402ca5fc4..4375330ef 100644
--- a/src/Fixer/Phpdoc/PhpdocToCommentFixer.php
+++ b/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 @@ public function getDefinition(): FixerDefinitionInterface
 ',
                     ['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 @@ public function configure(array $configuration = null): void
             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 @@ protected function createConfigurationDefinition(): FixerConfigurationResolverIn
                 ->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 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens): void
                 continue;
             }
 
+            if ($this->allowBeforeReturnStatement && $commentsAnalyzer->isBeforeReturn($tokens, $index)) {
+                continue;
+            }
+
             if ($commentsAnalyzer->isBeforeStructuralElement($tokens, $index)) {
                 continue;
             }
diff --git a/src/Tokenizer/Analyzer/CommentsAnalyzer.php b/src/Tokenizer/Analyzer/CommentsAnalyzer.php
index d0d87ef2c..ad9b05a22 100644
--- a/src/Tokenizer/Analyzer/CommentsAnalyzer.php
+++ b/src/Tokenizer/Analyzer/CommentsAnalyzer.php
@@ -73,18 +73,7 @@ public function isBeforeStructuralElement(Tokens $tokens, int $index): bool
             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 @@ public function isBeforeStructuralElement(Tokens $tokens, int $index): bool
             return true;
         }
 
-        if ($this->isValidLanguageConstruct($tokens, $token, $nextIndex)) {
+        if ($this->isValidVariableAssignment($tokens, $token, $nextIndex)) {
             return true;
         }
 
@@ -113,6 +102,24 @@ public function isBeforeStructuralElement(Tokens $tokens, int $index): bool
         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 @@ private function isValidControl(Tokens $tokens, Token $docsToken, int $controlIn
      * @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 @@ private function getLineBreakCount(Tokens $tokens, int $whiteStart, int $whiteEn
 
         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;
+    }
 }


Test patch:
diff --git a/tests/Fixer/Phpdoc/PhpdocToCommentFixerTest.php b/tests/Fixer/Phpdoc/PhpdocToCommentFixerTest.php
index 578cc3d34..1b18bfffa 100644
--- a/tests/Fixer/Phpdoc/PhpdocToCommentFixerTest.php
+++ b/tests/Fixer/Phpdoc/PhpdocToCommentFixerTest.php
@@ -685,6 +685,36 @@ public function b()
             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
diff --git a/tests/Tokenizer/Analyzer/CommentsAnalyzerTest.php b/tests/Tokenizer/Analyzer/CommentsAnalyzerTest.php
index 77f0a0a7a..d0549d44d 100644
--- a/tests/Tokenizer/Analyzer/CommentsAnalyzerTest.php
+++ b/tests/Tokenizer/Analyzer/CommentsAnalyzerTest.php
@@ -465,4 +465,70 @@ enum E {}
             ',
         ];
     }
+
+    /**
+     * @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,
+        ];
+    }
 }