Merge pull request #10341 from robchett/report_unused_issue_handler_s…

…uppressions

Report unused issue handler suppressions

config.xsd

@@ -47,6 +47,7 @@
         <xs:attribute name="findUnusedPsalmSuppress" type="xs:boolean" default="false" />
         <!-- TODO: Update default to true in Psalm 6 -->
         <xs:attribute name="findUnusedBaselineEntry" type="xs:boolean" default="false" />
+        <xs:attribute name="findUnusedIssueHandlerSuppression" type="xs:boolean" default="true" />
         <xs:attribute name="hideExternalErrors" type="xs:boolean" default="false" />
         <xs:attribute name="hoistConstants" type="xs:boolean" default="false" />
         <xs:attribute name="ignoreInternalFunctionFalseReturn" type="xs:boolean" default="false" />
@@ -494,6 +495,7 @@
             <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="UnusedIssueHandlerSuppression" 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" />

docs/running_psalm/configuration.md

@@ -513,6 +513,11 @@ class PremiumCar extends StandardCar {
 Emits [UnusedBaselineEntry](issues/UnusedBaselineEntry.md) when a baseline entry
 is not being used to suppress an issue.
 
+#### findUnusedIssueHandlerSuppression
+
+Emits [UnusedIssueHandlerSuppression](issues/UnusedIssueHandlerSuppression.md) when a suppressed issue handler
+is not being used to suppress an issue.
+
 ## Project settings
 
 #### <projectFiles>

docs/running_psalm/issues.md

@@ -298,6 +298,7 @@
  - [UnusedDocblockParam](issues/UnusedDocblockParam.md)
  - [UnusedForeachValue](issues/UnusedForeachValue.md)
  - [UnusedFunctionCall](issues/UnusedFunctionCall.md)
+ - [UnusedIssueHandlerSuppression](issues/UnusedIssueHandlerSuppression.md)
  - [UnusedMethod](issues/UnusedMethod.md)
  - [UnusedMethodCall](issues/UnusedMethodCall.md)
  - [UnusedParam](issues/UnusedParam.md)

docs/running_psalm/issues/UnusedIssueHandlerSuppression.md

@@ -0,0 +1,17 @@
+# UnusedIssueHandlerSuppression
+
+Emitted when an issue type suppression in the configuration file is not being used to suppress an issue.
+
+Enabled by [findUnusedIssueHandlerSuppression](../configuration.md#findunusedissuehandlersuppression) 
+
+```php
+<?php
+$a = 'Hello, World!';
+echo $a;
+```
+```xml
+<?xml version="1.0" encoding="UTF-8"?>
+<issueHandlers>
+    <PossiblyNullOperand errorLevel="suppress"/>
+</issueHandlers>
+```

psalm.xml.dist

@@ -13,6 +13,7 @@
     errorBaseline="psalm-baseline.xml"
     findUnusedPsalmSuppress="true"
     findUnusedBaselineEntry="true"
+    findUnusedIssueHandlerSuppression="true"
 >
     <stubs>
         <file name="stubs/phpparser.phpstub"/>
@@ -63,24 +64,6 @@
             </errorLevel>
         </DeprecatedMethod>
 
-        <DeprecatedClass>
-            <errorLevel type="suppress">
-                <referencedClass name="PackageVersions\Versions"/>
-            </errorLevel>
-        </DeprecatedClass>
-
-        <UnusedParam>
-            <errorLevel type="suppress">
-                <directory name="examples"/>
-            </errorLevel>
-        </UnusedParam>
-
-        <PossiblyUnusedParam>
-            <errorLevel type="suppress">
-                <directory name="examples"/>
-            </errorLevel>
-        </PossiblyUnusedParam>
-
         <UnusedClass>
             <errorLevel type="suppress">
                 <directory name="examples"/>
@@ -104,12 +87,6 @@
             </errorLevel>
         </PossiblyUndefinedIntArrayOffset>
 
-        <ImpureMethodCall>
-            <errorLevel type="suppress">
-                <directory name="src/Psalm/Storage/Assertion"/>
-            </errorLevel>
-        </ImpureMethodCall>
-
         <MissingThrowsDocblock errorLevel="info"/>
 
         <PossiblyUnusedProperty>

src/Psalm/Config.php

@@ -230,6 +230,8 @@ final class Config
      */
     public string $base_dir;
 
+    public ?string $source_filename = null;
+
     /**
      * The PHP version to assume as declared in the config file
      */
@@ -369,6 +371,8 @@ final class Config
 
     public bool $find_unused_baseline_entry = true;
 
+    public bool $find_unused_issue_handler_suppression = true;
+
     public bool $run_taint_analysis = false;
 
     public bool $use_phpstorm_meta_path = true;
@@ -935,6 +939,7 @@ private static function fromXmlAndPaths(
             'allowNamedArgumentCalls' => 'allow_named_arg_calls',
             'findUnusedPsalmSuppress' => 'find_unused_psalm_suppress',
             'findUnusedBaselineEntry' => 'find_unused_baseline_entry',
+            'findUnusedIssueHandlerSuppression' => 'find_unused_issue_handler_suppression',
             'reportInfo' => 'report_info',
             'restrictReturnTypes' => 'restrict_return_types',
             'limitMethodComplexity' => 'limit_method_complexity',
@@ -950,6 +955,7 @@ private static function fromXmlAndPaths(
             }
         }
 
+        $config->source_filename = $config_path;
         if ($config->resolve_from_config_file) {
             $config->base_dir = $base_dir;
         } else {
@@ -1311,6 +1317,12 @@ public function setComposerClassLoader(?ClassLoader $loader = null): void
         $this->composer_class_loader = $loader;
     }
 
+    /** @return array<string, IssueHandler> */
+    public function getIssueHandlers(): array
+    {
+        return $this->issue_handlers;
+    }
+
     public function setAdvancedErrorLevel(string $issue_key, array $config, ?string $default_error_level = null): void
     {
         $this->issue_handlers[$issue_key] = new IssueHandler();
@@ -1858,6 +1870,30 @@ public static function getParentIssueType(string $issue_type): ?string
         return null;
     }
 
+    /** @return array{type: string, index: int, count: int}[] */
+    public function getIssueHandlerSuppressions(): array
+    {
+        $suppressions = [];
+        foreach ($this->issue_handlers as $key => $handler) {
+            foreach ($handler->getFilters() as $index => $filter) {
+                $suppressions[] = [
+                    'type' => $key,
+                    'index' => $index,
+                    'count' => $filter->suppressions,
+                ];
+            }
+        }
+        return $suppressions;
+    }
+
+    /** @param array{type: string, index: int, count: int}[] $filters */
+    public function combineIssueHandlerSuppressions(array $filters): void
+    {
+        foreach ($filters as $filter) {
+            $this->issue_handlers[$filter['type']]->getFilters()[$filter['index']]->suppressions += $filter['count'];
+        }
+    }
+
     public function getReportingLevelForFile(string $issue_type, string $file_path): string
     {
         if (isset($this->issue_handlers[$issue_type])) {

src/Psalm/Config/ErrorLevelFileFilter.php

@@ -15,6 +15,8 @@ final class ErrorLevelFileFilter extends FileFilter
 {
     private string $error_level = '';
 
+    public int $suppressions = 0;
+
     public static function loadFromArray(
         array $config,
         string $base_dir,

src/Psalm/Config/IssueHandler.php

@@ -25,7 +25,7 @@ final class IssueHandler
     private string $error_level = Config::REPORT_ERROR;
 
     /**
-     * @var array<ErrorLevelFileFilter>
+     * @var list<ErrorLevelFileFilter>
      */
     private array $custom_levels = [];
 
@@ -50,6 +50,12 @@ public static function loadFromXMLElement(SimpleXMLElement $e, string $base_dir)
         return $handler;
     }
 
+    /** @return list<ErrorLevelFileFilter> */
+    public function getFilters(): array
+    {
+        return $this->custom_levels;
+    }
+
     public function setCustomLevels(array $customLevels, string $base_dir): void
     {
         /** @var array $customLevel */
@@ -71,6 +77,7 @@ public function getReportingLevelForFile(string $file_path): string
     {
         foreach ($this->custom_levels as $custom_level) {
             if ($custom_level->allows($file_path)) {
+                $custom_level->suppressions++;
                 return $custom_level->getErrorLevel();
             }
         }
@@ -82,6 +89,7 @@ public function getReportingLevelForClass(string $fq_classlike_name): ?string
     {
         foreach ($this->custom_levels as $custom_level) {
             if ($custom_level->allowsClass($fq_classlike_name)) {
+                $custom_level->suppressions++;
                 return $custom_level->getErrorLevel();
             }
         }
@@ -93,6 +101,7 @@ public function getReportingLevelForMethod(string $method_id): ?string
     {
         foreach ($this->custom_levels as $custom_level) {
             if ($custom_level->allowsMethod(strtolower($method_id))) {
+                $custom_level->suppressions++;
                 return $custom_level->getErrorLevel();
             }
         }
@@ -115,6 +124,7 @@ public function getReportingLevelForArgument(string $function_id): ?string
     {
         foreach ($this->custom_levels as $custom_level) {
             if ($custom_level->allowsMethod(strtolower($function_id))) {
+                $custom_level->suppressions++;
                 return $custom_level->getErrorLevel();
             }
         }
@@ -126,6 +136,7 @@ public function getReportingLevelForProperty(string $property_id): ?string
     {
         foreach ($this->custom_levels as $custom_level) {
             if ($custom_level->allowsProperty($property_id)) {
+                $custom_level->suppressions++;
                 return $custom_level->getErrorLevel();
             }
         }
@@ -137,6 +148,7 @@ public function getReportingLevelForClassConstant(string $constant_id): ?string
     {
         foreach ($this->custom_levels as $custom_level) {
             if ($custom_level->allowsClassConstant($constant_id)) {
+                $custom_level->suppressions++;
                 return $custom_level->getErrorLevel();
             }
         }
@@ -148,6 +160,7 @@ public function getReportingLevelForVariable(string $var_name): ?string
     {
         foreach ($this->custom_levels as $custom_level) {
             if ($custom_level->allowsVariable($var_name)) {
+                $custom_level->suppressions++;
                 return $custom_level->getErrorLevel();
             }
         }

src/Psalm/Internal/Codebase/Analyzer.php

@@ -89,6 +89,7 @@
  *      used_suppressions: array<string, array<int, bool>>,
  *      function_docblock_manipulators: array<string, array<int, FunctionDocblockManipulator>>,
  *      mutable_classes: array<string, bool>,
+ *      issue_handlers: array{type: string, index: int, count: int}[],
  * }
  */
 
@@ -418,6 +419,10 @@ static function (): void {
                     IssueBuffer::addUsedSuppressions($pool_data['used_suppressions']);
                 }
 
+                if ($codebase->config->find_unused_issue_handler_suppression) {
+                    $codebase->config->combineIssueHandlerSuppressions($pool_data['issue_handlers']);
+                }
+
                 if ($codebase->taint_flow_graph && $pool_data['taint_data']) {
                     $codebase->taint_flow_graph->addGraph($pool_data['taint_data']);
                 }
@@ -1639,6 +1644,7 @@ private function getWorkerData(): array
             'used_suppressions'                          => $codebase->track_unused_suppressions ? IssueBuffer::getUsedSuppressions() : [],
             'function_docblock_manipulators'             => FunctionDocblockManipulator::getManipulators(),
             'mutable_classes'                            => $codebase->analyzer->mutable_classes,
+            'issue_handlers'                             => $this->config->getIssueHandlerSuppressions()
         ];
         // @codingStandardsIgnoreEnd
     }

src/Psalm/Issue/UnusedIssueHandlerSuppression.php

@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Psalm\Issue;
+
+class UnusedIssueHandlerSuppression extends CodeIssue
+{
+    public const ERROR_LEVEL = -1;
+    public const SHORTCODE = 326;
+}

src/Psalm/IssueBuffer.php

@@ -17,6 +17,7 @@
 use Psalm\Issue\MixedIssue;
 use Psalm\Issue\TaintedInput;
 use Psalm\Issue\UnusedBaselineEntry;
+use Psalm\Issue\UnusedIssueHandlerSuppression;
 use Psalm\Issue\UnusedPsalmSuppress;
 use Psalm\Plugin\EventHandler\Event\AfterAnalysisEvent;
 use Psalm\Plugin\EventHandler\Event\BeforeAddIssueEvent;
@@ -645,6 +646,43 @@ public static function finish(
             }
         }
 
+        if ($codebase->config->find_unused_issue_handler_suppression) {
+            foreach ($codebase->config->getIssueHandlers() as $type => $handler) {
+                foreach ($handler->getFilters() as $filter) {
+                    if ($filter->suppressions > 0 && $filter->getErrorLevel() == Config::REPORT_SUPPRESS) {
+                        continue;
+                    }
+                    $issues_data['config'][] = new IssueData(
+                        IssueData::SEVERITY_ERROR,
+                        0,
+                        0,
+                        UnusedIssueHandlerSuppression::getIssueType(),
+                        sprintf(
+                            'Suppressed issue type "%s" for %s was not thrown.',
+                            $type,
+                            str_replace(
+                                $codebase->config->base_dir,
+                                '',
+                                implode(', ', [...$filter->getFiles(), ...$filter->getDirectories()]),
+                            ),
+                        ),
+                        $codebase->config->source_filename ?? '',
+                        '',
+                        '',
+                        '',
+                        0,
+                        0,
+                        0,
+                        0,
+                        0,
+                        0,
+                        UnusedIssueHandlerSuppression::SHORTCODE,
+                        UnusedIssueHandlerSuppression::ERROR_LEVEL,
+                    );
+                }
+            }
+        }
+
         echo self::getOutput(
             $issues_data,
             $project_analyzer->stdout_report_options,

tests/DocumentationTest.php

@@ -18,6 +18,7 @@
 use Psalm\Internal\Provider\Providers;
 use Psalm\Internal\RuntimeCaches;
 use Psalm\Issue\UnusedBaselineEntry;
+use Psalm\Issue\UnusedIssueHandlerSuppression;
 use Psalm\Tests\Internal\Provider\FakeParserCacheProvider;
 use UnexpectedValueException;
 
@@ -270,6 +271,7 @@ public function providerInvalidCodeParse(): array
                 case 'TraitMethodSignatureMismatch':
                 case 'UncaughtThrowInGlobalScope':
                 case UnusedBaselineEntry::getIssueType():
+                case UnusedIssueHandlerSuppression::getIssueType():
                     continue 2;
 
                 /** @todo reinstate this test when the issue is restored */