Skip to content

Commit

Permalink
Merge pull request #8774 from bdsl/report-by-issue-type-severity
Browse files Browse the repository at this point in the history
  • Loading branch information
weirdan authored Dec 2, 2022
2 parents 8e43460 + 1dbdf78 commit 4defa17
Show file tree
Hide file tree
Showing 7 changed files with 452 additions and 172 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
keys:
- composer-v4

- run: composer update
- run: COMPOSER_ROOT_VERSION=dev-master composer update

- save_cache:
key: composer-v4
Expand Down
327 changes: 157 additions & 170 deletions docs/running_psalm/error_levels.md

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/Psalm/Internal/Cli/Psalm.php
Original file line number Diff line number Diff line change
Expand Up @@ -1262,7 +1262,7 @@ private static function getHelpText(): string
--output-format=console
Changes the output format.
Available formats: compact, console, text, emacs, json, pylint, xml, checkstyle, junit, sonarqube,
github, phpstorm, codeclimate
github, phpstorm, codeclimate, by-issue-level
--no-progress
Disable the progress indicator
Expand Down
5 changes: 5 additions & 0 deletions src/Psalm/IssueBuffer.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Psalm\Issue\UnusedPsalmSuppress;
use Psalm\Plugin\EventHandler\Event\AfterAnalysisEvent;
use Psalm\Plugin\EventHandler\Event\BeforeAddIssueEvent;
use Psalm\Report\ByIssueLevelAndTypeReport;
use Psalm\Report\CheckstyleReport;
use Psalm\Report\CodeClimateReport;
use Psalm\Report\CompactReport;
Expand Down Expand Up @@ -854,6 +855,10 @@ public static function getOutput(
$output = new JsonReport($normalized_data, self::$fixable_issue_counts, $report_options);
break;

case Report::TYPE_BY_ISSUE_LEVEL:
$output = new ByIssueLevelAndTypeReport($normalized_data, self::$fixable_issue_counts, $report_options);
break;

case Report::TYPE_JSON_SUMMARY:
$output = new JsonSummaryReport(
$normalized_data,
Expand Down
1 change: 1 addition & 0 deletions src/Psalm/Report.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ abstract class Report
public const TYPE_SARIF = 'sarif';
public const TYPE_CODECLIMATE = 'codeclimate';
public const TYPE_COUNT = 'count';
public const TYPE_BY_ISSUE_LEVEL = 'by-issue-level';

/**
* @var array<int, IssueData>
Expand Down
190 changes: 190 additions & 0 deletions src/Psalm/Report/ByIssueLevelAndTypeReport.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
<?php

namespace Psalm\Report;

use Psalm\Config;
use Psalm\Internal\Analyzer\DataFlowNodeData;
use Psalm\Internal\Analyzer\IssueData;
use Psalm\Report;

use function basename;
use function get_cfg_var;
use function ini_get;
use function strlen;
use function strtr;
use function substr;
use function usort;

final class ByIssueLevelAndTypeReport extends Report
{
/** @var string|null */
private $link_format;

public function create(): string
{
$this->sortIssuesByLevelAndType();

$output = <<<HEADING
|----------------------------------------------------------------------------------------|
| Issues have been sorted by level and type. Feature-specific issues and the |
| most serious issues that will always be reported are listed first, with |
| remaining issues in level order. Issues near the top are usually the most serious. |
| Reducing the errorLevel in psalm.xml will suppress output of issues further down |
| this report. |
| |
| The level at which issue is reported as an error is given in brackets - e.g. |
| `ERROR (2): MissingReturnType` indicates that MissingReturnType is only reported |
| as an error when Psalm's level is set to 2 or below. |
| |
| Issues are shown or hidden in this report according to current settings. For |
| the most complete report set Psalm's error level to 0 or use --show-info=true |
| See https://psalm.dev/docs/running_psalm/error_levels/ |
|----------------------------------------------------------------------------------------|
HEADING;

foreach ($this->issues_data as $issue_data) {
$output .= $this->format($issue_data) . "\n" . "\n";
}

return $output;
}

/**
* Copied from ConsoleReport with only very minor changes.
*/
private function format(IssueData $issue_data): string
{
$issue_string = '';

$is_error = $issue_data->severity === Config::REPORT_ERROR;

if ($is_error) {
$issue_string .= ($this->use_color ? "\e[0;31mERROR\e[0m" : 'ERROR');
} else {
$issue_string .= 'INFO';
}

$issue_reference = $issue_data->link ? ' (see ' . $issue_data->link . ')' : '';

$level = $issue_data->error_level;
$issue_string .= ($level > 0 ? " ($level): " : ": ")
. $issue_data->type
. ' - ' . $this->getFileReference($issue_data)
. ' - ' . $issue_data->message . $issue_reference . "\n";


if ($issue_data->taint_trace) {
$issue_string .= $this->getTaintSnippets($issue_data->taint_trace);
} elseif ($this->show_snippet) {
$snippet = $issue_data->snippet;

if (!$this->use_color) {
$issue_string .= $snippet;
} else {
$selection_start = $issue_data->from - $issue_data->snippet_from;
$selection_length = $issue_data->to - $issue_data->from;

$issue_string .= substr($snippet, 0, $selection_start)
. ($is_error ? "\e[97;41m" : "\e[30;47m") . substr($snippet, $selection_start, $selection_length)
. "\e[0m" . substr($snippet, $selection_length + $selection_start) . "\n";
}
}

if ($issue_data->other_references) {
if ($this->show_snippet) {
$issue_string .= "\n";
}

$issue_string .= $this->getTaintSnippets($issue_data->other_references);
}

return $issue_string;
}

/**
* Copied from ConsoleReport unchanged. We could consider moving to another class to reduce duplication.
* @param non-empty-list<DataFlowNodeData|array{label: string, entry_path_type: string}> $taint_trace
*/
private function getTaintSnippets(array $taint_trace): string
{
$snippets = '';

foreach ($taint_trace as $node_data) {
if ($node_data instanceof DataFlowNodeData) {
$snippets .= ' ' . $node_data->label . ' - ' . $this->getFileReference($node_data) . "\n";

if ($this->show_snippet) {
$snippet = $node_data->snippet;

if (!$this->use_color) {
$snippets .= $snippet . "\n\n";
} else {
$selection_start = $node_data->from - $node_data->snippet_from;
$selection_length = $node_data->to - $node_data->from;

$snippets .= substr($snippet, 0, $selection_start)
. "\e[30;47m" . substr($snippet, $selection_start, $selection_length)
. "\e[0m" . substr($snippet, $selection_length + $selection_start) . "\n\n";
}
}
} else {
$snippets .= ' ' . $node_data['label'] . "\n";
$snippets .= ' <no known location>' . "\n\n";
}
}

return $snippets;
}

/**
* Copied from ConsoleReport unchanged. We could consider moving to another class to reduce duplication.
* @param IssueData|DataFlowNodeData $data
*/
private function getFileReference($data): string
{
$reference = $data->file_name . ':' . $data->line_from . ':' . $data->column_from;

if (!$this->use_color) {
return $reference;
}

$file_basename = basename($data->file_name);
$file_path = substr($data->file_name, 0, -strlen($file_basename));

$reference = $file_path
. "\033[1;31m"
. $file_basename . ':' . $data->line_from . ':' . $data->column_from
. "\033[0m"
;

if ($this->in_ci) {
return $reference;
}

if (null === $this->link_format) {
// if xdebug is not enabled, use `get_cfg_var` to get the value directly from php.ini
$this->link_format = ini_get('xdebug.file_link_format') ?: get_cfg_var('xdebug.file_link_format')
?: 'file://%f#L%l';
}

$link = strtr($this->link_format, ['%f' => $data->file_path, '%l' => $data->line_from]);
// $reference = $data->file_name . ':' . $data->line_from . ':' . $data->column_from;


return "\033]8;;" . $link . "\033\\" . $reference . "\033]8;;\033\\";
}

private function sortIssuesByLevelAndType(): void
{
usort($this->issues_data, function (IssueData $left, IssueData $right): int {
// negative error levels go to the top, followed by large positive levels, with level 1 at the bottom.
return [$left->error_level > 0, -$left->error_level, $left->type,
$left->file_path, $left->file_name, $left->line_from]
<=>
[$right->error_level > 0, -$right->error_level, $right->type,
$right->file_path, $right->file_name, $right->line_from];
});
}
}
97 changes: 97 additions & 0 deletions tests/ByIssueLevelAndTypeReportTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
<?php

namespace Psalm\Tests;

use PHPUnit\Framework\TestCase;
use Psalm\Internal\Analyzer\IssueData;
use Psalm\Report\ByIssueLevelAndTypeReport;
use Psalm\Report\ReportOptions;

class ByIssueLevelAndTypeReportTest extends TestCase
{
public function testItGeneratesReport(): void
{
$issuesData = [
$this->issueData(2, 'SomeLevel2IssueType'),
$this->issueData(-1, 'SomeAlwaysReportedIssueType'),
$this->issueData(4, 'SomeLevel4IssueType'),
$this->issueData(7, 'SomeLevel7IssueType'),
$this->issueData(4, 'AnotherLevel4IssueType'),
$this->issueData(4, 'SomeLevel4IssueType'), // same issue type as above, will be sorted together
$this->issueData(1, 'SomeIssueType'),
$this->issueData(-2, 'SomeFeatureSpecificIssueType'),
];

$reportOptions = new ReportOptions();
$reportOptions->use_color = false;

$report = new ByIssueLevelAndTypeReport($issuesData, [], $reportOptions);

$this->assertSame(<<<EXPECTED
|----------------------------------------------------------------------------------------|
| Issues have been sorted by level and type. Feature-specific issues and the |
| most serious issues that will always be reported are listed first, with |
| remaining issues in level order. Issues near the top are usually the most serious. |
| Reducing the errorLevel in psalm.xml will suppress output of issues further down |
| this report. |
| |
| The level at which issue is reported as an error is given in brackets - e.g. |
| `ERROR (2): MissingReturnType` indicates that MissingReturnType is only reported |
| as an error when Psalm's level is set to 2 or below. |
| |
| Issues are shown or hidden in this report according to current settings. For |
| the most complete report set Psalm's error level to 0 or use --show-info=true |
| See https://psalm.dev/docs/running_psalm/error_levels/ |
|----------------------------------------------------------------------------------------|
ERROR: SomeAlwaysReportedIssueType - file.php:1:1 - message
ERROR: SomeFeatureSpecificIssueType - file.php:1:1 - message
ERROR (7): SomeLevel7IssueType - file.php:1:1 - message
ERROR (4): AnotherLevel4IssueType - file.php:1:1 - message
ERROR (4): SomeLevel4IssueType - file.php:1:1 - message
ERROR (4): SomeLevel4IssueType - file.php:1:1 - message
ERROR (2): SomeLevel2IssueType - file.php:1:1 - message
ERROR (1): SomeIssueType - file.php:1:1 - message
EXPECTED, $report->create());
}

private function issueData(int $errorLevel, string $type): IssueData
{
return new IssueData(
'error',
1,
1,
$type,
'message',
'file.php',
'/',
'',
'',
1,
1,
1,
1,
1,
1,
0,
$errorLevel
);
}
}

0 comments on commit 4defa17

Please sign in to comment.