1338484 - Add support for auto-fix errors detected by static analysis from mach

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Andi [:andi]]

Clang-tidy has the possibility of in placement fix for a couple of the checkers that it triggers. Most of these checkers are related with the migration to C++11
Since our static analysis is based on clang-tidy and Ehsan wrote a patch that adds the possibility of running static analysis from mach this patch adds the possibility on having in place auto correction by clang-tidy.

The usage will be:  

./mach static-analysis check [path] -c "-[checkers]" -f

Where -f or --fix is the indicative to in placement fix, provided by clang-tidy

Comment 1

[Andi [:andi]]

Attachment: 341712.patch

Comment 2

[(no longer active)]

I was thinking of having a separate command for it in order to make it more visible, like:

./mach static-analysis rewrite path [-c "checkers"]

This way we can have a different default set of checkers for the rewrite command, and standardize a number of rewrites.  I'm dreaming of creating a periodic Taskcluster job that runs ./mach static-analysis rewrite on the entire tree and commits the result...  :-)

What do you think?

Comment 3

[Andi [:andi]]

This is a great idea but we must find a solution to exclude from analysis the 3rd party libraries, that are just replaced when a new update appears. My guess is that we could use regex when passing the path for the directories that need to be analysed. 
I'll update the current patch in order to comply with the format that you've proposed, also i'll keep it in sync with your work, and i'll push it after yours gets published.

Comment 4

[Sylvestre Ledru [:Sylvestre]]

(In reply to Andi-Bogdan Postelnicu from comment #3)
> This is a great idea but we must find a solution to exclude from analysis the 3rd party libraries, 
We have the list of thirdparty libs, just ignore the content of: https://dxr.mozilla.org/mozilla-central/source/tools/rewriting/ThirdPartyPaths.txt

Comment 5

[(no longer active)]

(In reply to Andi-Bogdan Postelnicu from comment #3)
> This is a great idea but we must find a solution to exclude from analysis
> the 3rd party libraries, that are just replaced when a new update appears.
> My guess is that we could use regex when passing the path for the
> directories that need to be analysed. 

This is why tools/rewriting/ThirdPartyPaths.txt exists.  :-)

I think what we want to do is to read that file and construct a -header-filter argument out of it.  We should also skip them in run-clang-tidy.py.  I already will need to modify that script to deal with unified builds, so having more than 1 exception there should be OK.

> I'll update the current patch in order to comply with the format that you've
> proposed, also i'll keep it in sync with your work, and i'll push it after
> yours gets published.

Sounds good.  You may also want to wait while my patch stabilizes further, I still expect to make a few changes to the check command (and at least one change is waiting on an upstream fix, so the timeline here isn't entirely under my control.)

Comment 6

[Andi [:andi]]

The patch from this bug has been merged in the parent bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1328454