Closed Bug 1338484 Opened 8 years ago Closed 7 years ago

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

Categories

(Developer Infrastructure :: Source Code Analysis, defect)

defect
Not set
normal

Tracking

(firefox54 affected)

RESOLVED FIXED
Tracking Status
firefox54 --- affected

People

(Reporter: andi, Assigned: andi)

References

Details

Attachments

(1 file)

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
Summary: Add support for auto-fix errors detected by static analysis → Add support for auto-fix errors detected by static analysis from mach
Attached patch 341712.patch (deleted) — Splinter Review
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?
Flags: needinfo?(bpostelnicu)
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.
Flags: needinfo?(bpostelnicu)
(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
(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.)
The patch from this bug has been merged in the parent bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1328454
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: