From 3448c47931ffc46033ca72c9955c5a26b747d139 Mon Sep 17 00:00:00 2001 From: robchett Date: Thu, 2 Nov 2023 18:15:21 +0000 Subject: [PATCH] Warn when an issue handler suppression is unused --- config.xsd | 2 + docs/running_psalm/configuration.md | 5 +++ docs/running_psalm/issues.md | 1 + .../issues/UnusedIssueHandlerSuppression.md | 17 +++++++++ src/Psalm/Config.php | 36 ++++++++++++++++++ src/Psalm/Config/ErrorLevelFileFilter.php | 2 + src/Psalm/Config/IssueHandler.php | 15 +++++++- src/Psalm/Internal/Codebase/Analyzer.php | 6 +++ .../Issue/UnusedIssueHandlerSuppression.php | 11 ++++++ src/Psalm/IssueBuffer.php | 38 +++++++++++++++++++ tests/DocumentationTest.php | 2 + 11 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 docs/running_psalm/issues/UnusedIssueHandlerSuppression.md create mode 100644 src/Psalm/Issue/UnusedIssueHandlerSuppression.php diff --git a/config.xsd b/config.xsd index 72745ea60..88d7b527f 100644 --- a/config.xsd +++ b/config.xsd @@ -47,6 +47,7 @@ + @@ -494,6 +495,7 @@ + diff --git a/docs/running_psalm/configuration.md b/docs/running_psalm/configuration.md index 05f65236f..ac222f951 100644 --- a/docs/running_psalm/configuration.md +++ b/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> diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md index f2655635c..b9e3d8fe2 100644 --- a/docs/running_psalm/issues.md +++ b/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) diff --git a/docs/running_psalm/issues/UnusedIssueHandlerSuppression.md b/docs/running_psalm/issues/UnusedIssueHandlerSuppression.md new file mode 100644 index 000000000..dc796e352 --- /dev/null +++ b/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 + + + + +``` diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index 8759c0ddb..a91336d2c 100644 --- a/src/Psalm/Config.php +++ b/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 @@ final class Config '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 @@ final class Config } } + $config->source_filename = $config_path; if ($config->resolve_from_config_file) { $config->base_dir = $base_dir; } else { @@ -1311,6 +1317,12 @@ final class Config $this->composer_class_loader = $loader; } + /** @return array */ + 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 @@ final class Config 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])) { diff --git a/src/Psalm/Config/ErrorLevelFileFilter.php b/src/Psalm/Config/ErrorLevelFileFilter.php index 1778ccfae..de3ed732c 100644 --- a/src/Psalm/Config/ErrorLevelFileFilter.php +++ b/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, diff --git a/src/Psalm/Config/IssueHandler.php b/src/Psalm/Config/IssueHandler.php index a5af5aefe..aba87f023 100644 --- a/src/Psalm/Config/IssueHandler.php +++ b/src/Psalm/Config/IssueHandler.php @@ -25,7 +25,7 @@ final class IssueHandler private string $error_level = Config::REPORT_ERROR; /** - * @var array + * @var list */ private array $custom_levels = []; @@ -50,6 +50,12 @@ final class IssueHandler return $handler; } + /** @return list */ + public function getFilters(): array + { + return $this->custom_levels; + } + public function setCustomLevels(array $customLevels, string $base_dir): void { /** @var array $customLevel */ @@ -71,6 +77,7 @@ final class IssueHandler { foreach ($this->custom_levels as $custom_level) { if ($custom_level->allows($file_path)) { + $custom_level->suppressions++; return $custom_level->getErrorLevel(); } } @@ -82,6 +89,7 @@ final class IssueHandler { 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 @@ final class IssueHandler { 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 @@ final class IssueHandler { 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 @@ final class IssueHandler { foreach ($this->custom_levels as $custom_level) { if ($custom_level->allowsProperty($property_id)) { + $custom_level->suppressions++; return $custom_level->getErrorLevel(); } } @@ -137,6 +148,7 @@ final class IssueHandler { foreach ($this->custom_levels as $custom_level) { if ($custom_level->allowsClassConstant($constant_id)) { + $custom_level->suppressions++; return $custom_level->getErrorLevel(); } } @@ -148,6 +160,7 @@ final class IssueHandler { foreach ($this->custom_levels as $custom_level) { if ($custom_level->allowsVariable($var_name)) { + $custom_level->suppressions++; return $custom_level->getErrorLevel(); } } diff --git a/src/Psalm/Internal/Codebase/Analyzer.php b/src/Psalm/Internal/Codebase/Analyzer.php index e7eb49e1c..461aae3e1 100644 --- a/src/Psalm/Internal/Codebase/Analyzer.php +++ b/src/Psalm/Internal/Codebase/Analyzer.php @@ -89,6 +89,7 @@ use const PHP_INT_MAX; * used_suppressions: array>, * function_docblock_manipulators: array>, * mutable_classes: array, + * issue_handlers: array{type: string, index: int, count: int}[], * } */ @@ -418,6 +419,10 @@ final class Analyzer 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 @@ final class Analyzer '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 } diff --git a/src/Psalm/Issue/UnusedIssueHandlerSuppression.php b/src/Psalm/Issue/UnusedIssueHandlerSuppression.php new file mode 100644 index 000000000..43699843d --- /dev/null +++ b/src/Psalm/Issue/UnusedIssueHandlerSuppression.php @@ -0,0 +1,11 @@ +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, diff --git a/tests/DocumentationTest.php b/tests/DocumentationTest.php index d86293a82..a8d135e4a 100644 --- a/tests/DocumentationTest.php +++ b/tests/DocumentationTest.php @@ -18,6 +18,7 @@ use Psalm\Internal\Provider\FakeFileProvider; 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 @@ class DocumentationTest extends TestCase case 'TraitMethodSignatureMismatch': case 'UncaughtThrowInGlobalScope': case UnusedBaselineEntry::getIssueType(): + case UnusedIssueHandlerSuppression::getIssueType(): continue 2; /** @todo reinstate this test when the issue is restored */