feature: NoSuperfluousPhpdocTagsFixer - support untyped and empty ann…

…otations in phpdoc (#5792)
PHP-CS-Fixer · Sep 21, 2022 · 531eaeb · 531eaeb
1 parent 39671cb
commit 531eaeb
Show file tree

Hide file tree

Showing 6 changed files with 213 additions and 21 deletions.
diff --git a/src/DocBlock/TypeExpression.php b/src/DocBlock/TypeExpression.php
@@ -87,7 +87,7 @@ final class TypeExpression
                 (?<constant> # single constant value (case insensitive), e.g.: 1, `\'a\'`
                     (?i)
                     null | true | false
-                    | [\d.]+
+                    | -?(?:\d+(?:\.\d*)?|\.\d+) # all sorts of numbers with or without minus, e.g.: 1, 1.1, 1., .1, -1
                     | \'[^\']+?\' | "[^"]+?"
                     | [@$]?(?:this | self | static)
                     (?-i)

diff --git a/src/Fixer/Phpdoc/NoSuperfluousPhpdocTagsFixer.php b/src/Fixer/Phpdoc/NoSuperfluousPhpdocTagsFixer.php
@@ -17,6 +17,7 @@
 use PhpCsFixer\AbstractFixer;
 use PhpCsFixer\DocBlock\Annotation;
 use PhpCsFixer\DocBlock\DocBlock;
+use PhpCsFixer\DocBlock\TypeExpression;
 use PhpCsFixer\Fixer\ConfigurableFixerInterface;
 use PhpCsFixer\FixerConfiguration\FixerConfigurationResolver;
 use PhpCsFixer\FixerConfiguration\FixerConfigurationResolverInterface;
@@ -33,6 +34,11 @@
 
 final class NoSuperfluousPhpdocTagsFixer extends AbstractFixer implements ConfigurableFixerInterface
 {
+    private const NO_TYPE_INFO = [
+        'types' => [],
+        'allows_null' => true,
+    ];
+
     /**
      * {@inheritdoc}
      */
@@ -320,6 +326,10 @@ private function fixFunctionDocComment(
             $argumentName = $annotation->getVariableName();
 
             if (null === $argumentName) {
+                if ($this->annotationIsSuperfluous($annotation, self::NO_TYPE_INFO, $currentSymbol, $shortNames)) {
+                    $annotation->remove();
+                }
+
                 continue;
             }
 
@@ -364,10 +374,7 @@ private function fixPropertyDocComment(
         if (\count($element['types']) > 0) {
             $propertyTypeInfo = $this->parseTypeHint($tokens, array_key_first($element['types']));
         } else {
-            $propertyTypeInfo = [
-                'types' => [],
-                'allows_null' => true,
-            ];
+            $propertyTypeInfo = self::NO_TYPE_INFO;
         }
 
         $docBlock = new DocBlock($content);
@@ -418,10 +425,7 @@ private function getArgumentsInfo(Tokens $tokens, int $start, int $end): array
             if ($typeIndex !== $index) {
                 $info = $this->parseTypeHint($tokens, $typeIndex);
             } else {
-                $info = [
-                    'types' => [],
-                    'allows_null' => true,
-                ];
+                $info = self::NO_TYPE_INFO;
             }
 
             if (!$info['allows_null']) {
@@ -449,10 +453,7 @@ private function getReturnTypeInfo(Tokens $tokens, int $closingParenthesisIndex)
 
         return $tokens[$colonIndex]->isGivenKind(CT::T_TYPE_COLON)
             ? $this->parseTypeHint($tokens, $tokens->getNextMeaningfulToken($colonIndex))
-            : [
-                'types' => [],
-                'allows_null' => true,
-            ]
+            : self::NO_TYPE_INFO
         ;
     }
 
@@ -519,17 +520,28 @@ private function annotationIsSuperfluous(
         array $symbolShortNames
     ): bool {
         if ('param' === $annotation->getTag()->getName()) {
-            $regex = '/@param\s+[^\$]+\s(?:\&\s*)?(?:\.{3}\s*)?\$\S+\s+\S/';
+            $regex = '{@param(?:\s+'.TypeExpression::REGEX_TYPES.')?(?:\s+(?:\&\s*)?(?:\.{3}\s*)?\$\S+)?(?:\s+(?<description>(?!\*+\/)\S+))?}sx';
         } elseif ('var' === $annotation->getTag()->getName()) {
-            $regex = '/@var\s+\S+(\s+\$\S+)?(\s+)(?!\*+\/)([^$\s]+)/';
+            $regex = '{@var(?:\s+'.TypeExpression::REGEX_TYPES.')?(?:\s+\$\S+)?(?:\s+(?<description>(?!\*\/)\S+))?}sx';
         } else {
-            $regex = '/@return\s+\S+\s+\S/';
+            $regex = '{@return(?:\s+'.TypeExpression::REGEX_TYPES.')?(?:\s+(?<description>(?!\*\/)\S+))?}sx';
         }
 
-        if (Preg::match($regex, $annotation->getContent())) {
+        if (1 !== Preg::match($regex, $annotation->getContent(), $matches)) {
+            // Unable to match the annotation, it must be malformed or has unsupported format.
+            // Either way we don't want to tinker with it.
             return false;
         }
 
+        if (isset($matches['description'])) {
+            return false;
+        }
+
+        if (!isset($matches['types']) || '' === $matches['types']) {
+            // If there's no type info in the annotation, further checks make no sense, exit early.
+            return true;
+        }
+
         $annotationTypes = $this->toComparableNames($annotation->getTypes(), $currentSymbol, $symbolShortNames);
 
         if (['null'] === $annotationTypes) {

diff --git a/tests/DocBlock/AnnotationTest.php b/tests/DocBlock/AnnotationTest.php
@@ -370,8 +370,8 @@ public function provideTypeParsingCases(): array
                 '/** @var array<Foo::A*>|null',
             ],
             [
-                ['null', 'true', 'false', '1', '1.5', "'a'", '"b"'],
-                '/** @var null|true|false|1|1.5|\'a\'|"b"',
+                ['null', 'true', 'false', '1', '-1', '1.5', '-1.5', '.5', '1.', "'a'", '"b"'],
+                '/** @var null|true|false|1|-1|1.5|-1.5|.5|1.|\'a\'|"b"',
             ],
             [
                 ['int', '"a"', 'A<B<C, D>, E<F::*|G[]>>'],

diff --git a/tests/DocBlock/TypeExpressionTest.php b/tests/DocBlock/TypeExpressionTest.php
@@ -93,7 +93,7 @@ public function provideGetTypesCases(): iterable
 
         yield ['array<Foo::A*>|null', ['array<Foo::A*>', 'null']];
 
-        yield ['null|true|false|1|1.5|\'a\'|"b"', ['null', 'true', 'false', '1', '1.5', "'a'", '"b"']];
+        yield ['null|true|false|1|-1|1.5|-1.5|.5|1.|\'a\'|"b"', ['null', 'true', 'false', '1', '-1', '1.5', '-1.5', '.5', '1.', "'a'", '"b"']];
 
         yield ['int | "a" | A<B<C, D>, E<F::*|G[]>>', ['int', '"a"', 'A<B<C, D>, E<F::*|G[]>>']];
 

diff --git a/tests/Fixer/Phpdoc/NoSuperfluousPhpdocTagsFixerTest.php b/tests/Fixer/Phpdoc/NoSuperfluousPhpdocTagsFixerTest.php
@@ -2103,6 +2103,187 @@ public function bar(self $other, int $superfluous): self {}
     public function bar(self $other, int $superfluous): self {}
 };',
         ];
+
+        yield 'remove empty var' => [
+            '<?php
+class Foo {
+    /**
+     */
+    private $foo;
+}',
+            '<?php
+class Foo {
+    /**
+     * @var
+     */
+    private $foo;
+}',
+        ];
+
+        yield 'remove empty var single line' => [
+            '<?php
+class Foo {
+    /**  */
+    private $foo;
+}',
+            '<?php
+class Foo {
+    /** @var */
+    private $foo;
+}',
+        ];
+
+        yield 'dont remove var without a type but with a property name and a description' => [
+            '<?php
+class Foo {
+    /**
+     * @var $foo some description
+     */
+    private $foo;
+}',
+        ];
+
+        yield 'dont remove single line var without a type but with a property name and a description' => [
+            '<?php
+class Foo {
+    /** @var $foo some description */
+    private $foo;
+}',
+        ];
+
+        yield 'remove var without a type but with a property name' => [
+            '<?php
+class Foo {
+    /**
+     */
+    private $foo;
+}',
+            '<?php
+class Foo {
+    /**
+     * @var $foo
+     */
+    private $foo;
+}',
+        ];
+
+        yield 'remove single line var without a type but with a property name' => [
+            '<?php
+class Foo {
+    /**  */
+    private $foo;
+}',
+            '<?php
+class Foo {
+    /** @var $foo */
+    private $foo;
+}',
+        ];
+
+        yield 'remove empty param' => [
+            '<?php
+class Foo {
+    /**
+     */
+    public function foo($foo) {}
+}',
+            '<?php
+class Foo {
+    /**
+     * @param
+     */
+    public function foo($foo) {}
+}',
+        ];
+
+        yield 'remove empty single line param' => [
+            '<?php
+class Foo {
+    /**  */
+    public function foo($foo) {}
+}',
+            '<?php
+class Foo {
+    /** @param */
+    public function foo($foo) {}
+}',
+        ];
+
+        yield 'remove param without a type' => [
+            '<?php
+class Foo {
+    /**
+     */
+    public function foo($foo) {}
+}',
+            '<?php
+class Foo {
+    /**
+     * @param $foo
+     */
+    public function foo($foo) {}
+}',
+        ];
+
+        yield 'remove single line param without a type' => [
+            '<?php
+class Foo {
+    /**  */
+    public function foo($foo) {}
+}',
+            '<?php
+class Foo {
+    /** @param $foo */
+    public function foo($foo) {}
+}',
+        ];
+
+        yield 'dont remove param without a type but with a description' => [
+            '<?php
+class Foo {
+    /**
+     * @param $foo description
+     */
+    public function foo($foo) {}
+}',
+        ];
+
+        yield 'dont remove single line param without a type but with a description' => [
+            '<?php
+class Foo {
+    /** @param $foo description */
+    public function foo($foo) {}
+}',
+        ];
+
+        yield 'remove empty return' => [
+            '<?php
+class Foo {
+    /**
+     */
+    public function foo($foo) {}
+}',
+            '<?php
+class Foo {
+    /**
+     * @return
+     */
+    public function foo($foo) {}
+}',
+        ];
+
+        yield 'remove empty single line return' => [
+            '<?php
+class Foo {
+    /**  */
+    public function foo($foo) {}
+}',
+            '<?php
+class Foo {
+    /** @return */
+    public function foo($foo) {}
+}',
+        ];
     }
 
     /**

diff --git a/tests/Fixtures/Integration/misc/PHP8_0.test b/tests/Fixtures/Integration/misc/PHP8_0.test
@@ -79,7 +79,6 @@ class T
     {
     }
 
-    /** @return static */
     public function something(): static
     {
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -79,7 +79,6 @@ class T @@
         {
         }
-        /** @return static */
         public function something(): static
         {
         }
@@ Expand Down @@