Report by issues grouped by level and type #8774

bdsl · 2022-11-26T22:45:45Z

This adds a new output format, --output-format=by-issue-level, which is almost exactly the same as the default console output, except that:

Issues are sorted by error level then issue type, rather than by file and position in file
Error level number is included in output, e.g. ERROR (3): PossiblyNullArgument

This is intended to be useful for getting an overview of a project, particularly when onboarding Psalm to a new codebase, and for choosing what error level to set in psalm config. The most severe errors (plus any reported due to feature specific config) appear first in the output, with the issues that would only be reported at error level 1 appearing at the end.

I wasn't sure where to document this, so for now I've made the report include a block of explanation as a heading. Happy to move that text to a more appropriate place.

src/Psalm/Report/ByIssueSeverityReport.php

weirdan · 2022-11-26T23:29:42Z

src/Psalm/Report/ByIssueSeverityReport.php

+            $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;


isn't it equivalent to return [$left->error_level, $left->type] <=> [$right->error_level, $right->type]?

I think you're right, I didn't realise you can use comparison operators with arrays.

Turns out this was almost right, but not quite. complication is that negative levels sort to the top, small positive levels sort to the bottom.

…by level and type

Co-authored-by: Bruce Weirdan <weirdan@gmail.com>

bdsl · 2022-12-01T23:43:08Z

I'm not sure why the composer update inside the Code Style Analysis job is failing - I don't think I've changed anything that should affect that.

weirdan · 2022-12-01T23:39:49Z

src/Psalm/Report/ByIssueLevelAndTypeReport.php

+|    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 4 or below.                                |


Is it level 2 or level 4?

It's level 2, thanks for catching that. Fixed in 5423983.

weirdan · 2022-12-01T23:45:50Z

tests/ByIssueLevelAndTypeReportTest.php

+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


This looks a little sparse. Wouldn't it be more readable if we had 1 or two empty lines between levels and none between the issues of the same level?

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

It's basically the same format as the console report, it just looks sparse in the test because the code snippets are empty. I considered adding a separator between levels but I thought it's probably not worth it. Users should be a be able to do a search for whatever level they're interested in if they want to highlight it in their shell or editor.

weirdan · 2022-12-01T23:47:51Z

tests/ByIssueLevelAndTypeReportTest.php

+
+
+
+EXPECTED, $report->create());


BTW now that we require PHP >7.3 you can indent the heredoc terminator.

weirdan · 2022-12-01T23:54:26Z

I'm not sure why the composer update inside the Code Style Analysis job is failing

I think we need to add an explicit COMPOSER_ROOT_VERSION=dev-master env variable to composer install / update. Otherwise composer won't be able to guess the root package version and fail the install where there circular dependencies are involved.

I think it makes more sense to have the errors that almost always appear (level 7 errors) next to the errors that always appear, instead of the level 1, least likely to appear errors being next to the ones that always appear. This also makes the order more similar to that output by the new --by-issue-level format report. Some time it might be nice to see if there's a way to auto generate most of this docs page from the actual issue class definitions, or have a test that checks the list of issues for each level is accurate and complete.

…vel & type equal PHP sorting only became stable in 8.0. For previous versions we would still like duplicate issues to be sorted into a logical order.

weirdan · 2022-12-12T06:58:32Z

@bdsl the docs/running_psalm/error_levels.md file is generated by bin/generate_levels_doc.php script, so perhaps you need to update that too.

weirdan added the release:feature label Nov 26, 2022

weirdan reviewed Nov 26, 2022

View reviewed changes

src/Psalm/Report/ByIssueSeverityReport.php Outdated Show resolved Hide resolved

weirdan reviewed Nov 26, 2022

View reviewed changes

bdsl and others added 4 commits December 1, 2022 22:02

WIP: Add new option --output-format=by-issue-severity to sort issues …

b70d3e2

…by level and type

Delete unused code

6bde0d3

Add comments

Verified

This commit was signed with the committer’s verified signature. The key has expired.

lieryan Lie Ryan

GPG key ID: 32C69FAD3DAC6BC9
Expired

Verified
Learn about vigilant mode

32881a3

Reduce function visibility

Verified

This commit was signed with the committer’s verified signature. The key has expired.

lieryan Lie Ryan

GPG key ID: 32C69FAD3DAC6BC9
Expired

Verified
Learn about vigilant mode

aea1c56

Co-authored-by: Bruce Weirdan <weirdan@gmail.com>

bdsl force-pushed the report-by-issue-type-severity branch from 2aebebd to aea1c56 Compare December 1, 2022 22:03

Fix implicit constant visibility

Verified

This commit was signed with the committer’s verified signature. The key has expired.

lieryan Lie Ryan

GPG key ID: 32C69FAD3DAC6BC9
Expired

Verified
Learn about vigilant mode

4fde493

bdsl changed the title ~~Report by issue type severity~~ Report by issues grouped by level and type Dec 1, 2022

Fix report name: Issue level, not issue severity

3bc3f1c

bdsl force-pushed the report-by-issue-type-severity branch from b18d502 to 3bc3f1c Compare December 1, 2022 22:11

bdsl added 2 commits December 1, 2022 23:00

Improve issue level and type report

386aa27

Add test for ByIsueLevelAndTypeReport

3f56166

bdsl marked this pull request as ready for review December 1, 2022 23:35

weirdan reviewed Dec 1, 2022

View reviewed changes

weirdan and others added 10 commits December 1, 2022 19:59

Set vimeo/psalm version explicitly

c1d2e56

Minor code edits in ByIssueLevelAndType

9e63bf6

Remove unecassary subheadings in error levels documentation

d6c7c86

Indent heredoc in test

699ee34

Fix error in ByIssueLvelAndTypeReport heading

5423983

Code style fix

6693421

Fix too lax function visibility in test

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

a29f65e

Sort issue by position in codebase in ByIssueLevelAndTypeReport if le…

ad57727

…vel & type equal PHP sorting only became stable in 8.0. For previous versions we would still like duplicate issues to be sorted into a logical order.

Code style fix

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

1dbdf78

weirdan merged commit 4defa17 into vimeo:master Dec 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Report by issues grouped by level and type #8774

Report by issues grouped by level and type #8774

bdsl commented Nov 26, 2022 •

edited

Loading

weirdan Nov 26, 2022

bdsl Nov 27, 2022

bdsl Dec 1, 2022

bdsl commented Dec 1, 2022

weirdan Dec 1, 2022

bdsl Dec 2, 2022

weirdan Dec 1, 2022

bdsl Dec 2, 2022 •

edited

Loading

weirdan Dec 1, 2022

bdsl Dec 2, 2022

weirdan commented Dec 1, 2022

weirdan commented Dec 12, 2022




		EXPECTED, $report->create());

Report by issues grouped by level and type #8774

Report by issues grouped by level and type #8774

Conversation

bdsl commented Nov 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdsl commented Dec 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdsl Dec 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weirdan commented Dec 1, 2022

weirdan commented Dec 12, 2022

bdsl commented Nov 26, 2022 •

edited

Loading

bdsl Dec 2, 2022 •

edited

Loading