1767061 - Phabricator source code analysis (clang-tidy) results are hard to find

Status: RESOLVED DUPLICATE of bug 1762034

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I'm working on a changeset: https://phabricator.services.mozilla.com/D143858 At some point, reviewbot told me there were static analysis errors (in clang-tidy) in my code. I found the results in a few places but they were all problematic:

1. The reviewbot comment directly links to treeherder results. I had to dig around to find the clang-tidy task and its .json artifact. That artifact unexpectedly included warnings for code I did not change, making it difficult to interpret (did it want me to change those too?)
2. I ran the ./mach static-analysis command it suggested but it printed warnings for code I didn't change, like #1. It was also difficult to read the verbose output
3. The reviewbot comment didn't mention this but the warning (for only my code, unlike #1 & #2) was linked at the top of the phabricator page in "Diff Details". This is what I had wanted to find. However, this result disappears if you make revisions to your push, as I had. You can get it back by opening the old revision directly (example) or waiting for the next analysis to complete but I didn't even know this section existed. This result is basically invisible if you're actively iterating on a patch (e.g. push every day) due to bug 1767059 where source code analysis takes > 1 day to run.

It would be great if these warnings were easier to find for beginning users.

Miscellaneous notes:

• the reviewbot comment's code-review frontend link took me to a page with no results
• fwiw, I think I've seen other lint tools inline the comments or include the errors in its initial comment. That could be a possible solution

Comment 1

[Marco Castelluccio [:marco]]

(In reply to Michael Comella (:mcomella) [needinfo or I won't see it] from comment #0)

I'm working on a changeset: https://phabricator.services.mozilla.com/D143858 At some point, reviewbot told me there were static analysis errors (in clang-tidy) in my code. I found the results in a few places but they were all problematic:

We have a few plans to improve the presentation of the results, see all bugs linked or duplicated to bug 1762034.
I'm going to mark this as a duplicate of bug 1762034 as we're going to use that bug to track improvements to the presentation.

1. The reviewbot comment directly links to treeherder results. I had to dig around to find the clang-tidy task and its .json artifact. That artifact unexpectedly included warnings for code I did not change, making it difficult to interpret (did it want me to change those too?)

This is covered by bug 1762034.

2. I ran the ./mach static-analysis command it suggested but it printed warnings for code I didn't change, like #1. It was also difficult to read the verbose output

This probably deserves its own bug.

3. The reviewbot comment didn't mention this but the warning (for only my code, unlike #1 & #2) was linked at the top of the phabricator page in "Diff Details". This is what I had wanted to find.

https://github.com/mozilla/code-review/issues/451 is covering this.

However, this result disappears if you make revisions to your push, as I had. You can get it back by opening the old revision directly (example) or waiting for the next analysis to complete but I didn't even know this section existed. This result is basically invisible if you're actively iterating on a patch (e.g. push every day) due to bug 1767059 where source code analysis takes > 1 day to run.

This behavior is expected: when you push an update, you don't want the previous results to show up otherwise it'll be confusing. After fixing https://github.com/mozilla/code-review/issues/451, this will become much better as the reviewbot comment will link directly to the warning in the right diff.

Miscellaneous notes:

• the reviewbot comment's code-review frontend link took me to a page with no results

This is probably bug 1732902.