Fix #1444 - track unused suppressions

config.xsd

@@ -344,6 +344,7 @@
             <xs:element name="UnusedVariable" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedParam" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedProperty" type="PropertyIssueHandlerType" minOccurs="0" />
+            <xs:element name="UnusedPsalmSuppress" type="FunctionIssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedClass" type="ClassIssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedMethod" type="MethodIssueHandlerType" minOccurs="0" />
         </xs:choice>

docs/running_psalm/issues.md

@@ -2431,6 +2431,15 @@ $a = new A();
 echo $a->getFoo();
 ```
 
+### UnusedPsalmSuppress
+
+Emitted when `--find-unused-psalm-suppress` is turned on and Psalm cannot find any uses of a given `@psalm-suppress` annotation
+
+```php
+/** @psalm-suppress InvalidArgument */
+echo strpos("hello", "e");
+```
+
 ### UnusedVariable
 
 Emitted when `--find-dead-code` is turned on and Psalm cannot find any references to a variable, once instantiated

src/Psalm/CodeLocation/Raw.php

@@ -14,6 +14,7 @@ class Raw extends \Psalm\CodeLocation
     public function __construct(
         string $file_contents,
         string $file_path,
+        string $file_name,
         int $file_start,
         int $file_end
     ) {
@@ -22,7 +23,7 @@ class Raw extends \Psalm\CodeLocation
         $this->raw_file_start = $this->file_start;
         $this->raw_file_end = $this->file_end;
         $this->file_path = $file_path;
-        $this->file_name = $file_path;
+        $this->file_name = $file_name;
         $this->single_line = false;
 
         $this->preview_start = $this->file_start;

src/Psalm/Codebase.php

@@ -272,7 +272,10 @@ class Codebase
      */
     public $php_minor_version = PHP_MINOR_VERSION;
 
-
+    /**
+     * @var bool
+     */
+    public $track_unused_suppressions = false;
 
     public function __construct(
         Config $config,
@@ -1032,7 +1035,13 @@ class Codebase
 
             $file_contents = $this->getFileContents($file_path);
 
-            return new CodeLocation\Raw($file_contents, $file_path, (int) $symbol_parts[0], (int) $symbol_parts[1]);
+            return new CodeLocation\Raw(
+                $file_contents,
+                $file_path,
+                $this->config->shortenFileName($file_path),
+                (int) $symbol_parts[0],
+                (int) $symbol_parts[1]
+            );
         }
 
         try {

src/Psalm/Internal/Analyzer/CommentAnalyzer.php

@@ -518,7 +518,7 @@ class CommentAnalyzer
 
         if (isset($parsed_docblock['specials']['psalm-suppress'])) {
             foreach ($parsed_docblock['specials']['psalm-suppress'] as $offset => $suppress_entry) {
-                $info->suppressed_issues[$offset] = preg_split('/[\s]+/', $suppress_entry)[0];
+                $info->suppressed_issues[$offset + $comment->getFilePos()] = preg_split('/[\s]+/', $suppress_entry)[0];
             }
         }
 
@@ -866,8 +866,8 @@ class CommentAnalyzer
         }
 
         if (isset($parsed_docblock['specials']['psalm-suppress'])) {
-            foreach ($parsed_docblock['specials']['psalm-suppress'] as $suppress_entry) {
+            foreach ($parsed_docblock['specials']['psalm-suppress'] as $offset => $suppress_entry) {
-                $info->suppressed_issues[] = preg_split('/[\s]+/', $suppress_entry)[0];
+                $info->suppressed_issues[$offset + $comment->getFilePos()] = preg_split('/[\s]+/', $suppress_entry)[0];
             }
         }
 

src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php

@@ -1016,6 +1016,12 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer implements Statements
             }
         }
 
+        if ($codebase->track_unused_suppressions) {
+            foreach ($storage->suppressed_issues as $offset => $issue_name) {
+                IssueBuffer::addUnusedSuppression($this->getFilePath(), $offset, $issue_name);
+            }
+        }
+
         foreach ($storage->throws as $expected_exception => $_) {
             if (isset($storage->throw_locations[$expected_exception])) {
                 if (ClassLikeAnalyzer::checkFullyQualifiedClassLikeName(

src/Psalm/Internal/Analyzer/ProjectAnalyzer.php

@@ -558,6 +558,14 @@ class ProjectAnalyzer
         $this->codebase->taint = new Taint();
     }
 
+    /**
+     * @return void
+     */
+    public function trackUnusedSuppressions()
+    {
+        $this->codebase->track_unused_suppressions = true;
+    }
+
     public function interpretRefactors() : void
     {
         if (!$this->codebase->alter_code) {

src/Psalm/Internal/Analyzer/StatementsAnalyzer.php

@@ -297,8 +297,13 @@ class StatementsAnalyzer extends SourceAnalyzer implements StatementsSource
                     );
 
                     if ($suppressed) {
-                        $new_issues = array_diff($suppressed, $this->source->getSuppressedIssues());
+                        $new_issues = [];
-                        /** @psalm-suppress MixedTypeCoercion */
+
+                        foreach ($suppressed as $offset => $issue_type) {
+                            $new_issues[$offset + $docblock->getFilePos()] = $issue_type;
+                            IssueBuffer::addUnusedSuppression($this->getFilePath(), $offset, $issue_type);
+                        }
+
                         $this->addSuppressedIssues($new_issues);
                     }
                 }

src/Psalm/Internal/Codebase/Analyzer.php

@@ -69,7 +69,8 @@ use function usort;
  *      class_method_locations: array<string, array<int, \Psalm\CodeLocation>>,
  *      class_property_locations: array<string, array<int, \Psalm\CodeLocation>>,
  *      possible_method_param_types: array<string, array<int, \Psalm\Type\Union>>,
- *      taint_data: ?\Psalm\Internal\Codebase\Taint
+ *      taint_data: ?\Psalm\Internal\Codebase\Taint,
+ *      unused_suppressions: array<string, array<int, int>>
  * }
  */
 
@@ -408,6 +409,7 @@ class Analyzer
                         'class_property_locations' => $rerun ? [] : $file_reference_provider->getAllClassPropertyLocations(),
                         'possible_method_param_types' => $rerun ? [] : $analyzer->getPossibleMethodParamTypes(),
                         'taint_data' => $codebase->taint,
+                        'unused_suppressions' => $codebase->track_unused_suppressions ? IssueBuffer::getUnusedSuppressions() : [],
                     ];
                     // @codingStandardsIgnoreEnd
                 },
@@ -427,6 +429,10 @@ class Analyzer
             foreach ($forked_pool_data as $pool_data) {
                 IssueBuffer::addIssues($pool_data['issues']);
 
+                if ($codebase->track_unused_suppressions) {
+                    IssueBuffer::addUnusedSuppressions($pool_data['unused_suppressions']);
+                }
+
                 if ($codebase->taint && $pool_data['taint_data']) {
                     $codebase->taint->addThreadData($pool_data['taint_data']);
                 }
@@ -531,6 +537,10 @@ class Analyzer
                 $codebase->file_reference_provider->addIssue($issue_data['file_path'], $issue_data);
             }
         }
+
+        if ($codebase->track_unused_suppressions) {
+            IssueBuffer::processUnusedSuppressions($codebase->file_provider);
+        }
     }
 
     /**

src/Psalm/Issue/UnusedPsalmSuppress.php

@@ -0,0 +1,6 @@
+<?php
+namespace Psalm\Issue;
+
+class UnusedPsalmSuppress extends CodeIssue
+{
+}

src/Psalm/IssueBuffer.php

@@ -13,6 +13,7 @@ use function microtime;
 use function number_format;
 use Psalm\Internal\Analyzer\ProjectAnalyzer;
 use Psalm\Issue\CodeIssue;
+use Psalm\Issue\UnusedPsalmSuppress;
 use Psalm\Report\CheckstyleReport;
 use Psalm\Report\CompactReport;
 use Psalm\Report\ConsoleReport;
@@ -58,6 +59,11 @@ class IssueBuffer
     /** @var array<int, array<int, CodeIssue>> */
     protected static $recorded_issues = [];
 
+    /**
+     * @var array<string, array<int, int>>
+     */
+    protected static $unused_suppressions = [];
+
     /**
      * @param   CodeIssue $e
      * @param   string[]  $suppressed_issues
@@ -73,6 +79,15 @@ class IssueBuffer
         return self::add($e);
     }
 
+    public static function addUnusedSuppression(string $file_path, int $offset, string $issue_type) : void
+    {
+        if (!isset(self::$unused_suppressions[$file_path])) {
+            self::$unused_suppressions[$file_path] = [];
+        }
+
+        self::$unused_suppressions[$file_path][$offset] = $offset + \strlen($issue_type) - 1;
+    }
+
     /**
      * @param   CodeIssue $e
      * @param   string[]  $suppressed_issues
@@ -85,14 +100,17 @@ class IssueBuffer
 
         $fqcn_parts = explode('\\', get_class($e));
         $issue_type = array_pop($fqcn_parts);
+        $file_path = $e->getFilePath();
 
-        if (!$config->reportIssueInFile($issue_type, $e->getFilePath())) {
+        if (!$config->reportIssueInFile($issue_type, $file_path)) {
             return true;
         }
 
         $suppressed_issue_position = array_search($issue_type, $suppressed_issues);
 
         if ($suppressed_issue_position !== false) {
+            /** @psalm-suppress MixedArrayTypeCoercion */
+            unset(self::$unused_suppressions[$file_path][$suppressed_issue_position]);
             return true;
         }
 
@@ -102,6 +120,8 @@ class IssueBuffer
             $suppressed_issue_position = array_search($parent_issue_type, $suppressed_issues);
 
             if ($suppressed_issue_position !== false) {
+                /** @psalm-suppress MixedArrayTypeCoercion */
+                unset(self::$unused_suppressions[$file_path][$suppressed_issue_position]);
                 return true;
             }
         }
@@ -190,6 +210,47 @@ class IssueBuffer
         return self::$issues_data;
     }
 
+    /**
+     * @return array<string, array<int, int>>
+     */
+    public static function getUnusedSuppressions() : array
+    {
+        return self::$unused_suppressions;
+    }
+
+    public static function addUnusedSuppressions(array $unused_suppressions) : void
+    {
+        self::$unused_suppressions += $unused_suppressions;
+    }
+
+    public static function processUnusedSuppressions(\Psalm\Internal\Provider\FileProvider $file_provider) : void
+    {
+        $config = Config::getInstance();
+
+        foreach (self::$unused_suppressions as $file_path => $offsets) {
+            if (!$offsets) {
+                continue;
+            }
+
+            $file_contents = $file_provider->getContents($file_path);
+
+            foreach ($offsets as $start => $end) {
+                self::add(
+                    new UnusedPsalmSuppress(
+                        'This suppression is never used',
+                        new CodeLocation\Raw(
+                            $file_contents,
+                            $file_path,
+                            $config->shortenFileName($file_path),
+                            $start,
+                            $end
+                        )
+                    )
+                );
+            }
+        }
+    }
+
     /**
      * @return int
      */
@@ -492,6 +553,7 @@ class IssueBuffer
         self::$recording_level = 0;
         self::$recorded_issues = [];
         self::$console_issues = [];
+        self::$unused_suppressions = [];
     }
 
     /**

src/command_functions.php

@@ -321,6 +321,9 @@ Options:
     --find-unused-code[=auto]
         Look for unused code. Options are 'auto' or 'always'. If no value is specified, default is 'auto'
 
+    --find-unused-psalm-suppress
+        Finds all @psalm-suppress annotations that aren’t used
+
     --find-references-to=[class|method|property]
         Searches the codebase for references to the given fully-qualified class or method,
         where method is in the format class::methodName

src/psalm.php

@@ -62,7 +62,8 @@ $valid_long_options = [
     'shepherd::',
     'no-progress',
     'include-php-versions', // used for baseline
-    'track-tainted-input'
+    'track-tainted-input',
+    'find-unused-psalm-suppress',
 ];
 
 gc_collect_cycles();
@@ -511,6 +512,10 @@ if (isset($options['track-tainted-input'])) {
     $project_analyzer->trackTaintedInputs();
 }
 
+if (isset($options['find-unused-psalm-suppress'])) {
+    $project_analyzer->trackUnusedSuppressions();
+}
+
 /** @var string $plugin_path */
 foreach ($plugins as $plugin_path) {
     $config->addPluginPath($plugin_path);

tests/DocumentationTest.php

@@ -143,6 +143,7 @@ class DocumentationTest extends TestCase
 
         if ($check_references) {
             $this->project_analyzer->getCodebase()->reportUnusedCode();
+            $this->project_analyzer->trackUnusedSuppressions();
         }
 
         foreach ($error_levels as $error_level) {

tests/IssueSuppressionTest.php

@@ -8,6 +8,55 @@ class IssueSuppressionTest extends TestCase
     use Traits\ValidCodeAnalysisTestTrait;
     use Traits\InvalidCodeAnalysisTestTrait;
 
+    /**
+     * @return void
+     */
+    public function testIssueSuppressedOnFunction()
+    {
+        $this->expectException(\Psalm\Exception\CodeException::class);
+        $this->expectExceptionMessage('UnusedPsalmSuppress');
+
+        $this->project_analyzer->trackUnusedSuppressions();
+
+        $this->addFile(
+            'somefile.php',
+            '<?php
+                class A {
+                    /**
+                     * @psalm-suppress UndefinedClass
+                     * @psalm-suppress MixedMethodCall
+                     * @psalm-suppress MissingReturnType
+                     * @psalm-suppress UnusedVariable
+                     */
+                    public function b() {
+                        B::fooFoo()->barBar()->bat()->baz()->bam()->bas()->bee()->bet()->bes()->bis();
+                    }
+                }'
+        );
+
+        $this->analyzeFile('somefile.php', new \Psalm\Context());
+    }
+
+    /**
+     * @return void
+     */
+    public function testIssueSuppressedOnStatement()
+    {
+        $this->expectException(\Psalm\Exception\CodeException::class);
+        $this->expectExceptionMessage('UnusedPsalmSuppress');
+
+        $this->project_analyzer->trackUnusedSuppressions();
+
+        $this->addFile(
+            'somefile.php',
+            '<?php
+                /** @psalm-suppress InvalidArgument */
+                echo strpos("hello", "e");'
+        );
+
+        $this->analyzeFile('somefile.php', new \Psalm\Context());
+    }
+
     /**
      * @return iterable<string,array{string,assertions?:array<string,string>,error_levels?:string[]}>
      */

tests/TestCase.php

@@ -119,6 +119,10 @@ class TestCase extends BaseTestCase
                 $file_analyzer->analyze($context);
             }
         }
+
+        if ($codebase->track_unused_suppressions) {
+            \Psalm\IssueBuffer::processUnusedSuppressions($codebase->file_provider);
+        }
     }
 
     /**