Merge pull request #9314 from weirdan/flag-unused-docblock-params

Flag docblock parameters that have no counterparts in function signature
vimeo · Feb 16, 2023 · f70f651 · f70f651
2 parents 1e71da3 + cd5b743
commit f70f651
Show file tree

Hide file tree

Showing 9 changed files with 53 additions and 10 deletions.
diff --git a/config.xsd b/config.xsd
@@ -484,6 +484,7 @@
             <xs:element name="UnusedClass" type="ClassIssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedClosureParam" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedConstructor" type="MethodIssueHandlerType" minOccurs="0" />
+            <xs:element name="UnusedDocblockParam" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedForeachValue" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedFunctionCall" type="FunctionIssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedMethod" type="MethodIssueHandlerType" minOccurs="0" />

diff --git a/docs/running_psalm/error_levels.md b/docs/running_psalm/error_levels.md
@@ -302,6 +302,7 @@ Level 5 and above allows a more non-verifiable code, and higher levels are even
  - [UnusedClass](issues/UnusedClass.md)
  - [UnusedClosureParam](issues/UnusedClosureParam.md)
  - [UnusedConstructor](issues/UnusedConstructor.md)
+ - [UnusedDocblockParam](issues/UnusedDocblockParam.md)
  - [UnusedForeachValue](issues/UnusedForeachValue.md)
  - [UnusedMethod](issues/UnusedMethod.md)
  - [UnusedParam](issues/UnusedParam.md)

diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md
@@ -288,6 +288,7 @@
  - [UnusedClass](issues/UnusedClass.md)
  - [UnusedClosureParam](issues/UnusedClosureParam.md)
  - [UnusedConstructor](issues/UnusedConstructor.md)
+ - [UnusedDocblockParam](issues/UnusedDocblockParam.md)
  - [UnusedForeachValue](issues/UnusedForeachValue.md)
  - [UnusedFunctionCall](issues/UnusedFunctionCall.md)
  - [UnusedMethod](issues/UnusedMethod.md)

diff --git a/docs/running_psalm/issues/UnusedDocblockParam.md b/docs/running_psalm/issues/UnusedDocblockParam.md
@@ -0,0 +1,14 @@
+# UnusedDocblockParam
+
+Emitted when `--find-dead-code` is turned on and a parameter specified in docblock does not have a corresponding parameter in function / method signature.
+
+```php
+<?php
+
+/**
+ * @param string $legacy_param was renamed to $newParam
+ */
+function f(string $newParam): string {
+    return strtolower($newParam);
+}
+```
diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
@@ -42,6 +42,7 @@
 use Psalm\Issue\ReservedWord;
 use Psalm\Issue\UnresolvableConstant;
 use Psalm\Issue\UnusedClosureParam;
+use Psalm\Issue\UnusedDocblockParam;
 use Psalm\Issue\UnusedParam;
 use Psalm\IssueBuffer;
 use Psalm\Plugin\EventHandler\Event\AfterFunctionLikeAnalysisEvent;
@@ -361,14 +362,22 @@ public function analyze(
             $context->external_mutation_free = true;
         }
 
-        if ($storage->has_undertyped_native_parameters) {
-            foreach ($storage->unused_docblock_parameters as $param_name => $param_location) {
+        foreach ($storage->unused_docblock_parameters as $param_name => $param_location) {
+            if ($storage->has_undertyped_native_parameters) {
                 IssueBuffer::maybeAdd(
                     new InvalidDocblockParamName(
                         'Incorrect param name $' . $param_name . ' in docblock for ' . $cased_method_id,
                         $param_location,
                     ),
                 );
+            } elseif ($codebase->find_unused_code) {
+                 IssueBuffer::maybeAdd(
+                     new UnusedDocblockParam(
+                         'Docblock parameter $' . $param_name . ' in docblock for ' . $cased_method_id
+                         . ' does not have a counterpart in signature parameter list',
+                         $param_location,
+                     ),
+                 );
             }
         }
 

diff --git a/src/Psalm/Internal/Type/TypeParser.php b/src/Psalm/Internal/Type/TypeParser.php
@@ -104,7 +104,6 @@ class TypeParser
      * Parses a string type representation
      *
      * @param  list<array{0: string, 1: int, 2?: string}> $type_tokens
-     * @param  array{int,int}|null   $php_version
      * @param  array<string, array<string, Union>> $template_type_map
      * @param  array<string, TypeAlias> $type_aliases
      */
@@ -716,7 +715,7 @@ private static function getTypeFromGenericTree(
                     throw new TypeParseTreeException('Class string param should be a named object');
                 }
 
-                $types []= new TClassString($type->value, $type, false, false, false, $from_docblock);
+                $types[] = new TClassString($type->value, $type, false, false, false, $from_docblock);
             }
 
             return new Union($types);
@@ -1452,7 +1451,7 @@ private static function getTypeFromKeyedArrayTree(
                         || ($had_optional && !$property_maybe_undefined)
                         || $type === 'array'
                         || $type === 'callable-array'
-                        || $previous_property_key != ($property_key-1)
+                        || $previous_property_key != ($property_key - 1)
                     )
                 ) {
                     $is_list = false;

diff --git a/src/Psalm/Issue/UnusedDocblockParam.php b/src/Psalm/Issue/UnusedDocblockParam.php
@@ -0,0 +1,9 @@
+<?php
+
+namespace Psalm\Issue;
+
+final class UnusedDocblockParam extends CodeIssue
+{
+    public const ERROR_LEVEL = -2;
+    public const SHORTCODE = 319;
+}
diff --git a/src/Psalm/Type.php b/src/Psalm/Type.php
@@ -70,7 +70,6 @@ abstract class Type
     /**
      * Parses a string type representation
      *
-     * @param  array{int,int}|null   $php_version
      * @param  array<string, array<string, Union>> $template_type_map
      */
     public static function parseString(
@@ -905,16 +904,16 @@ private static function intersectAtomicTypes(
             if ($intersection_atomic === null || $wider_type === null) {
                 throw new LogicException(
                     '$intersection_atomic and $wider_type should be both set or null.'
-                    .' Check the preceding code for errors.'
-                    .' Did you forget to assign one of the variables?',
+                    . ' Check the preceding code for errors.'
+                    . ' Did you forget to assign one of the variables?',
                 );
             }
             if (!self::mayHaveIntersection($intersection_atomic, $codebase)
                 || !self::mayHaveIntersection($wider_type, $codebase)
             ) {
                 throw new LogicException(
                     '$intersection_atomic and $wider_type should be both support intersection.'
-                    .' Check the preceding code for errors.',
+                    . ' Check the preceding code for errors.',
                 );
             }
 

diff --git a/tests/UnusedCodeTest.php b/tests/UnusedCodeTest.php
@@ -1757,7 +1757,7 @@ function f(): void {
             'exitInlineHtml' => [
                 'code' => '<?php
                     exit(0);
-                    ?'.'>foo
+                    ?' . '>foo
                 ',
                 'error_message' => 'UnevaluatedCode',
             ],
@@ -1836,6 +1836,16 @@ public function b(int $c): void {}
                     PHP,
                 'error_message' => 'PossiblyUnusedParam',
             ],
+            'unused param tag' => [
+                'code' => <<<'PHP'
+                    <?php
+                    /**
+                     * @param string $param
+                     */
+                    function f(): void {}
+                    PHP,
+                'error_message' => 'UnusedDocblockParam',
+            ],
         ];
     }
 }