From b27a233cdd425fc65515f27f7d066e1ba475533e Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Sun, 13 Sep 2020 23:41:14 +0300 Subject: [PATCH] Support multiple issue types in `@psalm-suppress` (#4179) * Accept multiple issue names in `@psalm-suppress` Fixes vimeo/psalm#1575 * Accept multiple issue types on statement docblocks as well * Proper highlighting of individual issues in compound suppressions --- src/Psalm/DocComment.php | 37 +++++++++++++++++ .../Internal/Analyzer/CommentAnalyzer.php | 11 ++--- .../Internal/Analyzer/StatementsAnalyzer.php | 40 +++++++------------ tests/IssueSuppressionTest.php | 11 +++++ 4 files changed, 68 insertions(+), 31 deletions(-) diff --git a/src/Psalm/DocComment.php b/src/Psalm/DocComment.php index 8546f4538..7c4a29986 100644 --- a/src/Psalm/DocComment.php +++ b/src/Psalm/DocComment.php @@ -197,4 +197,41 @@ class DocComment return $parsed_docblock; } + + /** + * @psalm-pure + * @return array + */ + public static function parseSuppressList(string $suppress_entry): array + { + preg_match( + '/ + (?(DEFINE) + # either a single issue or comma separated list of issues + (? (?&issue) \s* , \s* (?&issue_list) | (?&issue) ) + + # definition of a single issue + (? [A-Za-z0-9_-]+ ) + ) + ^ (?P (?&issue_list) ) (?P .* ) $ + /xm', + $suppress_entry, + $matches + ); + + if (!isset($matches['issues'])) { + return []; + } + + $issue_offset = 0; + $ret = []; + + foreach (explode(',', $matches['issues']) as $suppressed_issue) { + $issue_offset += strspn($suppressed_issue, "\t\n\f\r "); + $ret[$issue_offset] = trim($suppressed_issue); + $issue_offset += strlen($suppressed_issue) + 1; + } + + return $ret; + } } diff --git a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php index 2c0719fd9..cd179da21 100644 --- a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php @@ -357,10 +357,7 @@ class CommentAnalyzer } /** - * @param int $line_number - * * @throws DocblockParseException if there was a problem parsing the docblock - * */ public static function extractFunctionDocblockInfo(PhpParser\Comment\Doc $comment): FunctionDocblockComment { @@ -644,7 +641,9 @@ class CommentAnalyzer if (isset($parsed_docblock->tags['psalm-suppress'])) { foreach ($parsed_docblock->tags['psalm-suppress'] as $offset => $suppress_entry) { - $info->suppressed_issues[$offset + $comment->getFilePos()] = preg_split('/[\s]+/', $suppress_entry)[0]; + foreach (DocComment::parseSuppressList($suppress_entry) as $issue_offset => $suppressed_issue) { + $info->suppressed_issues[$issue_offset + $offset + $comment->getFilePos()] = $suppressed_issue; + } } } @@ -985,7 +984,9 @@ class CommentAnalyzer if (isset($parsed_docblock->tags['psalm-suppress'])) { foreach ($parsed_docblock->tags['psalm-suppress'] as $offset => $suppress_entry) { - $info->suppressed_issues[$offset + $comment->getFilePos()] = preg_split('/[\s]+/', $suppress_entry)[0]; + foreach (DocComment::parseSuppressList($suppress_entry) as $issue_offset => $suppressed_issue) { + $info->suppressed_issues[$issue_offset + $offset + $comment->getFilePos()] = $suppressed_issue; + } } } diff --git a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php index 00274b3ee..2187e59a7 100644 --- a/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php @@ -342,37 +342,25 @@ class StatementsAnalyzer extends SourceAnalyzer implements StatementsSource } if (isset($statements_analyzer->parsed_docblock->tags['psalm-suppress'])) { - $suppressed = array_filter( - array_map( - /** - * @param string $line - * - * @return string - */ - function ($line): string { - return preg_split('/[\s]+/', $line)[0]; - }, - $statements_analyzer->parsed_docblock->tags['psalm-suppress'] - ) - ); - + $suppressed = $statements_analyzer->parsed_docblock->tags['psalm-suppress']; if ($suppressed) { $new_issues = []; - foreach ($suppressed as $offset => $issue_type) { - $offset += $docblock->getFilePos(); - $new_issues[$offset] = $issue_type; + foreach ($suppressed as $offset => $suppress_entry) { + foreach (DocComment::parseSuppressList($suppress_entry) as $issue_offset => $issue_type) { + $new_issues[$issue_offset + $offset + $docblock->getFilePos()] = $issue_type; - if ($issue_type === 'InaccessibleMethod') { - continue; - } + if ($issue_type === 'InaccessibleMethod') { + continue; + } - if ($codebase->track_unused_suppressions) { - IssueBuffer::addUnusedSuppression( - $statements_analyzer->getFilePath(), - $offset, - $issue_type - ); + if ($codebase->track_unused_suppressions) { + IssueBuffer::addUnusedSuppression( + $statements_analyzer->getFilePath(), + $issue_offset + $offset + $docblock->getFilePos(), + $issue_type + ); + } } } diff --git a/tests/IssueSuppressionTest.php b/tests/IssueSuppressionTest.php index 8a482859f..742375cc0 100644 --- a/tests/IssueSuppressionTest.php +++ b/tests/IssueSuppressionTest.php @@ -210,6 +210,17 @@ class IssueSuppressionTest extends TestCase } }', ], + 'multipleIssues' => [ + 'barBar()->bat()->baz()->bam()->bas()->bee()->bet()->bes()->bis(); + } + }', + ], 'undefinedClassOneLine' => [ '