1488687 - Enable braces around statements clang-tidy checker

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, enhancement)

Description

[Sylvestre Ledru [:Sylvestre]]

As it is clear in the coding style that we should have braces: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Control_Structures

I propose to enable the checker:
https://clang.llvm.org/extra/clang-tidy/checks/readability-braces-around-statements.html

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Of course, this will be enabled only at review phase.
If the warning is ignored in phabricator, nothing will happen.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1488687 - Enable braces around statements clang-tidy checker r?ehsan

This will be enabled only at review phase.
If the warning is ignored in phabricator, nothing will happen.

Comment 3

[Jan Keromnes [:janx]]

Seems fair, since it's mandated by our coding style.

> Of course, this will be enabled only at review phase.

Note: This checker will also be active when you run `./mach static-analysis check` locally.

Also, FYI, there are about 32K pre-existing violations of this rule in our tree (including ignored paths like third-party dependencies):

https://github.com/jankeromnes/gecko-dev/commit/5dcb2201de8dd075f3c65c120c5157ec8133f883 (autofix run from 4 months ago)

Comment 4

[Sylvestre Ledru [:Sylvestre]]

(In reply to Jan Keromnes [:janx] from comment #3)
> 
> Also, FYI, there are about 32K pre-existing violations of this rule in our
> tree (including ignored paths like third-party dependencies):
Interesting, codechecker is finding 16270 occurrences including thirdparty...

Comment 5

[(no longer active)]

Comment on attachment 9006485 [details]
Bug 1488687 - Enable braces around statements clang-tidy checker r?ehsan

:Ehsan Akhgari has approved the revision.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19bac49e4951
Enable braces around statements clang-tidy checker r=Ehsan

Comment 7

[Eric Rahm [:erahm]]

Is this going to complain for stuff in the js/ dir or is that ignored? IIRC they don't like braces.

Comment 8

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/19bac49e4951

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Eric, they are discussing about using the braces too:
https://groups.google.com/forum/#!topic/mozilla.dev.tech.js-engine.internals/koX9Yu0uFRw
(this is what triggered this commits ;)

Comment 10

[Sylvestre Ledru [:Sylvestre]]

See bug 1488698

Comment 11

[Eric Rahm [:erahm]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #9)
> Eric, they are discussing about using the braces too:
> https://groups.google.com/forum/#!topic/mozilla.dev.tech.js-engine.internals/
> koX9Yu0uFRw
> (this is what triggered this commits ;)

Oh that's great!