From b70d3e228e885ee873514d6c5255ac52c4514658 Mon Sep 17 00:00:00 2001 From: Barney Laurance Date: Sat, 26 Nov 2022 22:31:12 +0000 Subject: [PATCH 01/18] WIP: Add new option --output-format=by-issue-severity to sort issues by level and type --- src/Psalm/IssueBuffer.php | 5 + src/Psalm/Report.php | 1 + src/Psalm/Report/ByIssueSeverityReport.php | 192 +++++++++++++++++++++ 3 files changed, 198 insertions(+) create mode 100644 src/Psalm/Report/ByIssueSeverityReport.php diff --git a/src/Psalm/IssueBuffer.php b/src/Psalm/IssueBuffer.php index b1750133ce6..515402feeeb 100644 --- a/src/Psalm/IssueBuffer.php +++ b/src/Psalm/IssueBuffer.php @@ -17,6 +17,7 @@ use Psalm\Issue\UnusedPsalmSuppress; use Psalm\Plugin\EventHandler\Event\AfterAnalysisEvent; use Psalm\Plugin\EventHandler\Event\BeforeAddIssueEvent; +use Psalm\Report\ByIssueSeverityReport; use Psalm\Report\CheckstyleReport; use Psalm\Report\CodeClimateReport; use Psalm\Report\CompactReport; @@ -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_SEVERITY: + $output = new ByIssueSeverityReport($normalized_data, self::$fixable_issue_counts, $report_options); + break; + case Report::TYPE_JSON_SUMMARY: $output = new JsonSummaryReport( $normalized_data, diff --git a/src/Psalm/Report.php b/src/Psalm/Report.php index a56469af089..3d2c9a4633b 100644 --- a/src/Psalm/Report.php +++ b/src/Psalm/Report.php @@ -29,6 +29,7 @@ abstract class Report public const TYPE_SARIF = 'sarif'; public const TYPE_CODECLIMATE = 'codeclimate'; public const TYPE_COUNT = 'count'; + const TYPE_BY_ISSUE_SEVERITY = 'by-issue-severity'; /** * @var array diff --git a/src/Psalm/Report/ByIssueSeverityReport.php b/src/Psalm/Report/ByIssueSeverityReport.php new file mode 100644 index 00000000000..02116c32393 --- /dev/null +++ b/src/Psalm/Report/ByIssueSeverityReport.php @@ -0,0 +1,192 @@ +sortIssuesByLevelAndType(); + + $output = ''; + + foreach ($this->issues_data as $issue_data) { + $output .= $this->format($issue_data) . "\n" . "\n"; + } + + return $output; + } + + 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 . ')' : ''; + + $issue_string .= " ($issue_data->error_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; + } + + /** + * @param non-empty-list $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 .= ' ' . "\n\n"; + } + } + + return $snippets; + } + + /** + * @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\\"; + } + + /** + * @param $severity + */ + public function errorLevelMessage(int $severity): string + { + if ($severity < -1) { + return "Issues reported based on feature-specific config:"; + } + + if ($severity < 0) { + return "Issues always reported:"; + } + + return "Issues reported at error level $severity" . + ($severity === 1) ? ":" : " or less:"; + } + public function sortIssuesByLevelAndType(): void + { + usort($this->issues_data, function (IssueData $left, IssueData $right): int { + $leftLevel = $left->error_level; + $rightLevel = $right->error_level; + + if ($leftLevel != $rightLevel) { + if ($rightLevel > 0 && $leftLevel > 0) { + return $rightLevel <=> $leftLevel; + } + + if ($rightLevel > 0) { + return -1; + } + + return $leftLevel <=> $rightLevel; + } + + return $left->type <=> $right->type; + }); + } +} From 6bde0d3a68fee2c4ee23ac9dab6d803ab6d6e44e Mon Sep 17 00:00:00 2001 From: Barney Laurance Date: Sat, 26 Nov 2022 22:33:18 +0000 Subject: [PATCH 02/18] Delete unused code --- src/Psalm/Report/ByIssueSeverityReport.php | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/Psalm/Report/ByIssueSeverityReport.php b/src/Psalm/Report/ByIssueSeverityReport.php index 02116c32393..4042307a638 100644 --- a/src/Psalm/Report/ByIssueSeverityReport.php +++ b/src/Psalm/Report/ByIssueSeverityReport.php @@ -152,22 +152,6 @@ private function getFileReference($data): string return "\033]8;;" . $link . "\033\\" . $reference . "\033]8;;\033\\"; } - /** - * @param $severity - */ - public function errorLevelMessage(int $severity): string - { - if ($severity < -1) { - return "Issues reported based on feature-specific config:"; - } - - if ($severity < 0) { - return "Issues always reported:"; - } - - return "Issues reported at error level $severity" . - ($severity === 1) ? ":" : " or less:"; - } public function sortIssuesByLevelAndType(): void { usort($this->issues_data, function (IssueData $left, IssueData $right): int { From 32881a3c655d0a9a58edefc2d17e2fb50bd5d231 Mon Sep 17 00:00:00 2001 From: Barney Laurance Date: Sat, 26 Nov 2022 22:36:04 +0000 Subject: [PATCH 03/18] Add comments --- src/Psalm/Report/ByIssueSeverityReport.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Psalm/Report/ByIssueSeverityReport.php b/src/Psalm/Report/ByIssueSeverityReport.php index 4042307a638..d1d6e2122ce 100644 --- a/src/Psalm/Report/ByIssueSeverityReport.php +++ b/src/Psalm/Report/ByIssueSeverityReport.php @@ -33,6 +33,9 @@ public function create(): string return $output; } + /** + * Copied from ConsoleReport with only very minor changes. Todo consider reducing code duplication + */ private function format(IssueData $issue_data): string { $issue_string = ''; @@ -82,6 +85,7 @@ private function format(IssueData $issue_data): string } /** + * Copied from ConsoleReport unchanged. Todo consider reducing code duplication * @param non-empty-list $taint_trace */ private function getTaintSnippets(array $taint_trace): string From aea1c5603d5b68543fd8f6410faf1416ba86445b Mon Sep 17 00:00:00 2001 From: Barney Laurance Date: Sun, 27 Nov 2022 12:18:34 +0000 Subject: [PATCH 04/18] Reduce function visibility Co-authored-by: Bruce Weirdan --- src/Psalm/Report/ByIssueSeverityReport.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Psalm/Report/ByIssueSeverityReport.php b/src/Psalm/Report/ByIssueSeverityReport.php index d1d6e2122ce..c9eb684ef3e 100644 --- a/src/Psalm/Report/ByIssueSeverityReport.php +++ b/src/Psalm/Report/ByIssueSeverityReport.php @@ -156,7 +156,7 @@ private function getFileReference($data): string return "\033]8;;" . $link . "\033\\" . $reference . "\033]8;;\033\\"; } - public function sortIssuesByLevelAndType(): void + private function sortIssuesByLevelAndType(): void { usort($this->issues_data, function (IssueData $left, IssueData $right): int { $leftLevel = $left->error_level; From 4fde49313ea29223dbef4e9ddd42b635f6e525df Mon Sep 17 00:00:00 2001 From: Barney Laurance Date: Thu, 1 Dec 2022 22:05:27 +0000 Subject: [PATCH 05/18] Fix implicit constant visibility --- src/Psalm/Report.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Psalm/Report.php b/src/Psalm/Report.php index 3d2c9a4633b..0ca186bdfd4 100644 --- a/src/Psalm/Report.php +++ b/src/Psalm/Report.php @@ -29,7 +29,7 @@ abstract class Report public const TYPE_SARIF = 'sarif'; public const TYPE_CODECLIMATE = 'codeclimate'; public const TYPE_COUNT = 'count'; - const TYPE_BY_ISSUE_SEVERITY = 'by-issue-severity'; + public const TYPE_BY_ISSUE_SEVERITY = 'by-issue-severity'; /** * @var array From 3bc3f1c273fd423e2375068cad8c2185035820ce Mon Sep 17 00:00:00 2001 From: Barney Laurance Date: Thu, 1 Dec 2022 22:07:13 +0000 Subject: [PATCH 06/18] Fix report name: Issue level, not issue severity --- src/Psalm/IssueBuffer.php | 6 +++--- src/Psalm/Report.php | 2 +- ...ssueSeverityReport.php => ByIssueLevelAndTypeReport.php} | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) rename src/Psalm/Report/{ByIssueSeverityReport.php => ByIssueLevelAndTypeReport.php} (99%) diff --git a/src/Psalm/IssueBuffer.php b/src/Psalm/IssueBuffer.php index 515402feeeb..369f6a7629d 100644 --- a/src/Psalm/IssueBuffer.php +++ b/src/Psalm/IssueBuffer.php @@ -17,7 +17,7 @@ use Psalm\Issue\UnusedPsalmSuppress; use Psalm\Plugin\EventHandler\Event\AfterAnalysisEvent; use Psalm\Plugin\EventHandler\Event\BeforeAddIssueEvent; -use Psalm\Report\ByIssueSeverityReport; +use Psalm\Report\ByIssueLevelAndTypeReport; use Psalm\Report\CheckstyleReport; use Psalm\Report\CodeClimateReport; use Psalm\Report\CompactReport; @@ -855,8 +855,8 @@ public static function getOutput( $output = new JsonReport($normalized_data, self::$fixable_issue_counts, $report_options); break; - case Report::TYPE_BY_ISSUE_SEVERITY: - $output = new ByIssueSeverityReport($normalized_data, self::$fixable_issue_counts, $report_options); + case Report::TYPE_BY_ISSUE_LEVEL: + $output = new ByIssueLevelAndTypeReport($normalized_data, self::$fixable_issue_counts, $report_options); break; case Report::TYPE_JSON_SUMMARY: diff --git a/src/Psalm/Report.php b/src/Psalm/Report.php index 0ca186bdfd4..98350d1e1de 100644 --- a/src/Psalm/Report.php +++ b/src/Psalm/Report.php @@ -29,7 +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_SEVERITY = 'by-issue-severity'; + public const TYPE_BY_ISSUE_LEVEL = 'by-issue-level'; /** * @var array diff --git a/src/Psalm/Report/ByIssueSeverityReport.php b/src/Psalm/Report/ByIssueLevelAndTypeReport.php similarity index 99% rename from src/Psalm/Report/ByIssueSeverityReport.php rename to src/Psalm/Report/ByIssueLevelAndTypeReport.php index c9eb684ef3e..8268da501c4 100644 --- a/src/Psalm/Report/ByIssueSeverityReport.php +++ b/src/Psalm/Report/ByIssueLevelAndTypeReport.php @@ -15,7 +15,7 @@ use function substr; use function usort; -final class ByIssueSeverityReport extends Report +final class ByIssueLevelAndTypeReport extends Report { /** @var string|null */ private $link_format; From 386aa27f4c0d54919528145833e7d460ec4dac84 Mon Sep 17 00:00:00 2001 From: Barney Laurance Date: Thu, 1 Dec 2022 23:00:48 +0000 Subject: [PATCH 07/18] Improve issue level and type report --- src/Psalm/Internal/Cli/Psalm.php | 2 +- .../Report/ByIssueLevelAndTypeReport.php | 40 +++++++++++-------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/Psalm/Internal/Cli/Psalm.php b/src/Psalm/Internal/Cli/Psalm.php index b9f754d7ffe..383ea361914 100644 --- a/src/Psalm/Internal/Cli/Psalm.php +++ b/src/Psalm/Internal/Cli/Psalm.php @@ -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 diff --git a/src/Psalm/Report/ByIssueLevelAndTypeReport.php b/src/Psalm/Report/ByIssueLevelAndTypeReport.php index 8268da501c4..fd6ce8d870b 100644 --- a/src/Psalm/Report/ByIssueLevelAndTypeReport.php +++ b/src/Psalm/Report/ByIssueLevelAndTypeReport.php @@ -24,7 +24,25 @@ public function create(): string { $this->sortIssuesByLevelAndType(); - $output = ''; + $output = <<