Skip to content
Permalink
Browse files

Fix #1444 - track unused suppressions

  • Loading branch information...
muglug committed Aug 18, 2019
1 parent 2146f73 commit 2a5e0d8f395017529290dbe821966a0d440141fa
@@ -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>
@@ -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
@@ -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 @@ public function __construct(
$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;
@@ -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 @@ public function getSymbolLocation(string $file_path, string $symbol)
$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 {
@@ -518,7 +518,7 @@ public static function extractFunctionDocblockInfo(PhpParser\Comment\Doc $commen
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 @@ public static function extractClassLikeDocblockInfo(
}
if (isset($parsed_docblock['specials']['psalm-suppress'])) {
foreach ($parsed_docblock['specials']['psalm-suppress'] as $suppress_entry) {
$info->suppressed_issues[] = preg_split('/[\s]+/', $suppress_entry)[0];
foreach ($parsed_docblock['specials']['psalm-suppress'] as $offset => $suppress_entry) {
$info->suppressed_issues[$offset + $comment->getFilePos()] = preg_split('/[\s]+/', $suppress_entry)[0];
}
}
@@ -1016,6 +1016,12 @@ function (FunctionLikeParameter $p) {
}
}
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(
@@ -558,6 +558,14 @@ public function trackTaintedInputs()
$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) {
@@ -297,8 +297,13 @@ function ($line) {
);
if ($suppressed) {
$new_issues = array_diff($suppressed, $this->source->getSuppressedIssues());
/** @psalm-suppress MixedTypeCoercion */
$new_issues = [];
foreach ($suppressed as $offset => $issue_type) {
$new_issues[$offset + $docblock->getFilePos()] = $issue_type;
IssueBuffer::addUnusedSuppression($this->getFilePath(), $offset, $issue_type);
}
$this->addSuppressedIssues($new_issues);
}
}
@@ -69,7 +69,8 @@
* 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 @@ function () use ($rerun) {
'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 @@ function () use ($rerun) {
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 @@ function () use ($rerun) {
$codebase->file_reference_provider->addIssue($issue_data['file_path'], $issue_data);
}
}
if ($codebase->track_unused_suppressions) {
IssueBuffer::processUnusedSuppressions($codebase->file_provider);
}
}
/**
@@ -0,0 +1,6 @@
<?php
namespace Psalm\Issue;
class UnusedPsalmSuppress extends CodeIssue
{
}
@@ -13,6 +13,7 @@
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 @@ public static function accepts(CodeIssue $e, array $suppressed_issues = [])
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 @@ public static function isSuppressed(CodeIssue $e, array $suppressed_issues = [])
$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 @@ public static function isSuppressed(CodeIssue $e, array $suppressed_issues = [])
$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 @@ public static function getIssuesData()
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 @@ public static function clearCache()
self::$recording_level = 0;
self::$recorded_issues = [];
self::$console_issues = [];
self::$unused_suppressions = [];
}
/**
@@ -321,6 +321,9 @@ function getPsalmHelpText(): string
--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
@@ -62,7 +62,8 @@
'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 @@ function ($arg) {
$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);
@@ -143,6 +143,7 @@ public function testInvalidCode($code, $error_message, $error_levels = [], $chec
if ($check_references) {
$this->project_analyzer->getCodebase()->reportUnusedCode();
$this->project_analyzer->trackUnusedSuppressions();
}
foreach ($error_levels as $error_level) {
@@ -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[]}>
*/

0 comments on commit 2a5e0d8

Please sign in to comment.
You can’t perform that action at this time.