Skip to content

Commit

Permalink
POC: Autofixer for unused suppressions
Browse files Browse the repository at this point in the history
  • Loading branch information
Daimona committed Oct 9, 2022
1 parent eedf5fa commit 0a15585
Show file tree
Hide file tree
Showing 3 changed files with 283 additions and 11 deletions.
29 changes: 23 additions & 6 deletions .phan/plugins/UnusedSuppressionPlugin.php
Expand Up @@ -17,9 +17,11 @@
use Phan\PluginV3\AnalyzeFunctionCapability;
use Phan\PluginV3\AnalyzeMethodCapability;
use Phan\PluginV3\AnalyzePropertyCapability;
use Phan\PluginV3\AutomaticFixCapability;
use Phan\PluginV3\BeforeAnalyzeFileCapability;
use Phan\PluginV3\FinalizeProcessCapability;
use Phan\PluginV3\SuppressionCapability;
use UnusedSuppressionPlugin\Fixers;

/**
* Check for unused (at)suppress annotations.
Expand All @@ -33,8 +35,12 @@ class UnusedSuppressionPlugin extends PluginV3 implements
AnalyzeFunctionCapability,
AnalyzeMethodCapability,
AnalyzePropertyCapability,
FinalizeProcessCapability
FinalizeProcessCapability,
AutomaticFixCapability
{
private const UnusedSuppression = 'UnusedSuppression';
private const UnusedPluginSuppression = 'UnusedPluginSuppression';
private const UnusedPluginFileSuppression = 'UnusedPluginFileSuppression';

/**
* @var AddressableElement[] - Analysis is postponed until finalizeProcess.
Expand Down Expand Up @@ -78,7 +84,7 @@ private static function analyzeAddressableElement(
$suppress_issue_list =
$element->getSuppressIssueList();

if (\array_key_exists('UnusedSuppression', $suppress_issue_list)) {
if (\array_key_exists(self::UnusedSuppression, $suppress_issue_list)) {
// The element's doc comment is suppressing everything emitted by this plugin.
return;
}
Expand All @@ -94,7 +100,7 @@ private static function analyzeAddressableElement(
self::emitIssue(
$code_base,
$element->getContext(),
'UnusedSuppression',
self::UnusedSuppression,
"Element {FUNCTIONLIKE} suppresses issue {ISSUETYPE} but does not use it",
[(string)$element->getFQSEN(), $issue_type]
);
Expand Down Expand Up @@ -249,13 +255,13 @@ private function analyzePluginSuppressionsForFile(CodeBase $code_base, Suppressi
continue;
}
// TODO: finish letting plugins suppress UnusedSuppression on other plugins
$issue_kind = 'UnusedPluginSuppression';
$issue_kind = self::UnusedPluginSuppression;
$message = 'Plugin {STRING_LITERAL} suppresses issue {ISSUETYPE} on this line but this suppression is unused or suppressed elsewhere';
if ($lineno === 0) {
$issue_kind = 'UnusedPluginFileSuppression';
$issue_kind = self::UnusedPluginFileSuppression;
$message = 'Plugin {STRING_LITERAL} suppresses issue {ISSUETYPE} in this file but this suppression is unused or suppressed elsewhere';
}
if (isset($plugin_suppressions['UnusedSuppression'][$lineno_of_comment])) {
if (isset($plugin_suppressions[self::UnusedSuppression][$lineno_of_comment])) {
continue;
}
if (isset($plugin_suppressions[$issue_kind][$lineno_of_comment])) {
Expand Down Expand Up @@ -305,6 +311,17 @@ public function recordPluginSuppression(
$plugin_class = \get_class($plugin);
$this->plugin_active_suppression_list[$plugin_class][$file_name][$issue_type][$line] = $line;
}

public function getAutomaticFixers(): array
{
require_once __DIR__ . '/UnusedSuppressionPlugin/Fixers.php';
$function_like_fixer = Closure::fromCallable([Fixers::class, 'fixUnusedSuppression']);
return [
self::UnusedSuppression => $function_like_fixer,
self::UnusedPluginSuppression => $function_like_fixer,
self::UnusedPluginFileSuppression => $function_like_fixer,
];
}
}

// Every plugin needs to return an instance of itself at the
Expand Down
251 changes: 251 additions & 0 deletions .phan/plugins/UnusedSuppressionPlugin/Fixers.php
@@ -0,0 +1,251 @@
<?php

declare(strict_types=1);

namespace UnusedSuppressionPlugin;

use Microsoft\PhpParser;
use Microsoft\PhpParser\Node\Statement\ClassDeclaration;
use Microsoft\PhpParser\Node\Expression\AnonymousFunctionCreationExpression;
use Microsoft\PhpParser\Node\MethodDeclaration;
use Microsoft\PhpParser\Node\PropertyDeclaration;
use Microsoft\PhpParser\Node\Statement\FunctionDeclaration;
use Microsoft\PhpParser\ParseContext;
use Microsoft\PhpParser\PhpTokenizer;
use Microsoft\PhpParser\Token;
use Microsoft\PhpParser\TokenKind;
use Phan\CodeBase;
use Phan\IssueInstance;
use Phan\Language\Element\Comment\Builder;
use Phan\Library\FileCacheEntry;
use Phan\Plugin\Internal\BuiltinSuppressionPlugin;
use Phan\Plugin\Internal\IssueFixingPlugin\FileEdit;
use Phan\Plugin\Internal\IssueFixingPlugin\FileEditSet;

/**
* This plugin implements --automatic-fix for UnusedSuppressionPlugin
*/
class Fixers
{
/**
* Remove an unused suppression
* @param CodeBase $code_base @unused-param
*/
public static function fixUnusedSuppression(
CodeBase $code_base,
FileCacheEntry $contents,
IssueInstance $instance
): ?FileEditSet {
$line_number = $instance->getLine();
$line_content = $contents->getLine($line_number);
if ($line_content === null) {
// Impossible?!
return null;
}

$suppressed_issue_name = $instance->getTemplateParameters()[1];
$line_nodes = $contents->getNodesAtLine($instance->getLine());
if ($line_nodes) {
// Could be a block-level suppression, or a single-line comment in the same line as other statements.
// We only handle the first case for now.
return self::getBlockLevelSuppressionFix($suppressed_issue_name, $line_nodes, $line_number, $contents);
}
if (preg_match('!\s+//!',$line_content)) {
// This line contains only a single-line comment, probably where the suppression is from.
return self::getInlineSuppressionFix($suppressed_issue_name, $line_content, $line_number, $contents);
}
// Impossible?
return null;
}

/**
* Handles suppressions in single-line comments, when there's nothing but the comment on that line.
*/
private static function getInlineSuppressionFix(
string $suppressed_issue_name,
string $line_content,
int $line_number,
FileCacheEntry $contents
): ?FileEditSet {
$suppressions = BuiltinSuppressionPlugin::yieldSuppressionCommentsFromTokenContents($line_content, $line_number);
if (count($suppressions) !== 1) {
// Probably possible in theory, but skip for now.
return null;
}
$suppression = $suppressions[0];
$suppression_type = $suppression[3];
if ($suppression_type === 'suppress-next-next-line' || $suppression_type === 'suppress-previous-line') {
$next_line_content = $contents->getLine($line_number + 1);
if ($next_line_content && preg_match('!\s+//!', $next_line_content)) {
// Skip, since there could be a multi-line explanation of the suppression and we don't
// want to remove only a part of it.
return null;
}
}

$kind_replace_start_and_end = self::getReplacementStartAndEndInKindList($suppressed_issue_name, $suppression[4]);
if ($kind_replace_start_and_end === null) {
return null;
}
$kind_list_start_offset = $suppression[5];
$edit = new FileEdit(
$contents->getLineOffset($line_number) + $kind_list_start_offset + $kind_replace_start_and_end[0],
$contents->getLineOffset($line_number) + $kind_list_start_offset + $kind_replace_start_and_end[1],
''
);
// XXX: Here we would check if the kind list is now empty, and remove the whole comment if that's the case.
return new FileEditSet( [ $edit ] );
}

/**
* @param list<PhpParser\Node> $nodes
*/
private static function getBlockLevelSuppressionFix(
string $suppressed_issue_name,
array $nodes,
int $line_number,
FileCacheEntry $contents
): ?FileEditSet {
$doc_block = self::getRelevantDocBlock($nodes);
if (!$doc_block) {
return null;
}

$edits = array_merge(
self::getBlockEditsForNativeSuppression($doc_block, $suppressed_issue_name, $contents),
self::getBlockEditsForPluginSuppression($doc_block, $suppressed_issue_name, $line_number, $contents)
);
// XXX: Here we would check if the doc block is now empty, and remove it if that's the case.
return $edits ? new FileEditSet($edits) : null;
}

/**
* @return FileEdit[]
*/
private static function getBlockEditsForNativeSuppression(
Token $doc_block,
string $suppressed_issue_name,
FileCacheEntry $contents
): array
{
$doc_block_text = $doc_block->getText($contents->getContents());
$suppression_matches = [];
$match_count = preg_match_all(Builder::PHAN_SUPPRESS_REGEX, $doc_block_text, $suppression_matches, PREG_OFFSET_CAPTURE);
if (!$match_count) {
return [];
}

$edits = [];
for ($i = 0; $i < $match_count; $i++) {
[ $kind_list_text, $kind_list_offset ] = $suppression_matches[1][$i];

$kind_replace_start_and_end = self::getReplacementStartAndEndInKindList($suppressed_issue_name, $kind_list_text);
if ($kind_replace_start_and_end === null) {
continue;
}
$edits[] = new FileEdit(
$doc_block->getStartPosition() + $kind_list_offset + $kind_replace_start_and_end[0],
$doc_block->getStartPosition() + $kind_list_offset + $kind_replace_start_and_end[1],
''
);
}
// XXX: Here we would check if the kind list is now empty, and remove the whole comment if that's the case.
return $edits;
}

/**
* @return FileEdit[]
*/
private static function getBlockEditsForPluginSuppression(
Token $doc_block,
string $suppressed_issue_name,
int $line_number,
FileCacheEntry $contents
): array {
$suppressions = BuiltinSuppressionPlugin::yieldSuppressionCommentsFromTokenContents(
$doc_block->getText($contents->getContents()),
$line_number
);
$edits = [];
foreach ($suppressions as $suppression) {
$next_line_content = $contents->getLine($line_number + 1);
if ($next_line_content && preg_match('!\s*\*\s*[^\s/@]!', $next_line_content)) {
// Next line is not comment end or an @-line. Could be an explanation of the suppression, so skip.
continue;
}

$kind_replace_start_and_end = self::getReplacementStartAndEndInKindList($suppressed_issue_name, $suppression[4]);
if ($kind_replace_start_and_end === null) {
continue;
}
$kind_list_start_offset = $suppression[5];
$edits[] = new FileEdit(
$doc_block->getStartPosition() + $kind_list_start_offset + $kind_replace_start_and_end[0],
$doc_block->getStartPosition() + $kind_list_start_offset + $kind_replace_start_and_end[0],
''
);
}
// XXX: Here we would check if the kind list is now empty, and remove the whole comment if that's the case.
return $edits;
}


/**
* @return array{0:int,1:int}|null
*/
private static function getReplacementStartAndEndInKindList(string $suppressed_issue_name, string $kind_list_text): ?array
{
$kind_list_replace_start = strpos($kind_list_text, $suppressed_issue_name);
if ($kind_list_replace_start === false) {
return null;
}
$kind_list_replace_end = $kind_list_replace_start + strlen($suppressed_issue_name);
if (($kind_list_text[$kind_list_replace_end] ?? '') === ',') {
// Remove trailing comma if there are more issues being suppressed here.
++$kind_list_replace_end;
}
return [ $kind_list_replace_start, $kind_list_replace_end ];
}

/**
* @param list<PhpParser\Node> $nodes
* @return Token|null
*/
private static function getRelevantDocBlock(array $nodes): ?Token
{
foreach ($nodes as $node) {
if (
$node instanceof FunctionDeclaration || $node instanceof MethodDeclaration ||
$node instanceof AnonymousFunctionCreationExpression ||
$node instanceof PropertyDeclaration || $node instanceof ClassDeclaration
) {
return self::getDocCommentToken($node);
}
}
return null;
}

/**
* Copied from PHPDocRedundantPlugin\Fixers et al., adapted from Node::getDocCommentText().
* @suppress PhanThrowTypeAbsentForCall
* @suppress PhanUndeclaredClassMethod
* @suppress UnusedSuppression false positive for PhpTokenizer with polyfill due to https://github.com/Microsoft/tolerant-php-parser/issues/292
*/
private static function getDocCommentToken(PhpParser\Node $node): ?Token
{
$leadingTriviaText = $node->getLeadingCommentAndWhitespaceText();
$leadingTriviaTokens = PhpTokenizer::getTokensArrayFromContent(
$leadingTriviaText,
ParseContext::SourceElements,
$node->getFullStartPosition(),
false
);
for ($i = \count($leadingTriviaTokens) - 1; $i >= 0; $i--) {
$token = $leadingTriviaTokens[$i];
if ($token->kind === TokenKind::DocCommentToken) {
return $token;
}
}
return null;
}
}
14 changes: 9 additions & 5 deletions src/Phan/Plugin/Internal/BuiltinSuppressionPlugin.php
Expand Up @@ -199,8 +199,11 @@ private static function computeIssueSuppressionList(
return $suppression_list;
}

// @phan-suppress-next-line PhanAccessClassConstantInternal
private const SUPPRESS_ISSUE_REGEX = '/@phan-(suppress-(next(?:-next)?|current|previous)-line|file-suppress)\s+' . Builder::SUPPRESS_ISSUE_LIST . '/';
/**
* @phan-suppress-next-line PhanAccessClassConstantInternal
* @internal
*/
public const SUPPRESS_ISSUE_REGEX = '/@phan-(suppress-(next(?:-next)?|current|previous)-line|file-suppress)\s+' . Builder::SUPPRESS_ISSUE_LIST . '/';

/**
* @return Generator<array{0:string,1:int,2:int,3:string,4:string}>
Expand Down Expand Up @@ -271,8 +274,9 @@ private static function yieldSuppressionCommentsOld(
/**
* @return list<array{0:string,1:int,2:int,3:string,4:string}>
* returns list of [$comment_text, $comment_start_line, $comment_start_offset, $comment_name, $kind_list_text];
* @internal
*/
private static function yieldSuppressionCommentsFromTokenContents(
public static function yieldSuppressionCommentsFromTokenContents(
string $comment_text,
int $comment_start_line
): array {
Expand All @@ -289,9 +293,9 @@ private static function yieldSuppressionCommentsFromTokenContents(
for ($i = 0; $i < $match_count; $i++) {
$comment_start_offset = $matches[0][$i][1]; // byte offset
$comment_name = $matches[1][$i][0];
$kind_list_text = $matches[3][$i][0]; // byte offset
[ $kind_list_text, $kind_list_offset ] = $matches[3][$i];

$result[] = [$comment_text, $comment_start_line, $comment_start_offset, $comment_name, $kind_list_text];
$result[] = [$comment_text, $comment_start_line, $comment_start_offset, $comment_name, $kind_list_text, $kind_list_offset];
}
return $result;
}
Expand Down

0 comments on commit 0a15585

Please sign in to comment.