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)
Developer Infrastructure
Source Code Analysis
Tracking
(firefox54 affected)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox54 | --- | affected |
People
(Reporter: andi, Assigned: andi)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
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
Assignee | ||
Updated•8 years ago
|
Summary: Add support for auto-fix errors detected by static analysis → Add support for auto-fix errors detected by static analysis from mach
Assignee | ||
Comment 1•8 years ago
|
||
Comment 2•8 years ago
|
||
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)
Assignee | ||
Comment 3•8 years ago
|
||
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)
Comment 4•8 years ago
|
||
(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•8 years ago
|
||
(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.)
Assignee | ||
Comment 6•8 years ago
|
||
The patch from this bug has been merged in the parent bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1328454
Assignee | ||
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Product: Core → Firefox Build System
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•