Page MenuHomePhabricator Dev

Bug 1387052 - Make Firefox code worse intentionally 1/2.
Needs ReviewPublic

Authored by janx on Nov 23 2018, 11:17 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 8, 4:24 PM
Unknown Object (File)
Wed, Sep 23, 4:42 AM
Unknown Object (File)
Aug 9 2026, 8:03 AM
Unknown Object (File)
Aug 4 2026, 5:09 AM
Unknown Object (File)
Aug 4 2026, 5:09 AM
Unknown Object (File)
Aug 4 2026, 2:54 AM
Unknown Object (File)
Aug 4 2026, 2:02 AM
Unknown Object (File)
Aug 3 2026, 8:41 PM
Subscribers
None

Details

Reviewers
ReviewBot
Bugzilla Bug ID
1387052

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Code analysis found 35 defects in this patch:

  • 13 defects found by clang-format
  • 13 defects found by clang-tidy
  • 9 defects found by mozlint

You can run this analysis locally with:

  • ./mach clang-format -p path/to/file.cpp (C/C++)
  • ./mach static-analysis check path/to/file.cpp (C/C++)
  • ./mach lint path/to/file (JS/Python/etc)

For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply).

If you see a problem in this automated review, please report it here.

browser/base/content/docs/tabbrowser/index.rst
7

Error: Exisits ==> exists [codespell]

devtools/client/definitions.js
7

Error: Infix operators must be spaced. [eslint: space-infix-ops]

devtools/client/menus.js
31

Error: Use the global form of 'use strict'. [eslint: strict]

41

Error: Unexpected tab character. [eslint: no-tabs]

devtools/client/shared/scroll.js
7

Error: Parsing error: Unexpected character '#' [eslint]

dom/media/CubebUtils.cpp
661

Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return]

} else if (aPreferred == CUBEB_DEVICE_PREF_ALL) { // before: readability-else-after-return
  ^~~~~~~~
js/ductwork/debugger/JSDebugger.cpp
49

Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return]

else if (!JS_IsGlobalObject(obj)) { // after: readability-else-after-return
^~~~~~~~
js/src/jsapi.cpp
132

Warning: Suspicious string literal, probably missing a comma [clang-tidy: bugprone-suspicious-missing-comma]

"line 6"     // after: bugprone-suspicious-missing-comma
^
144

Error: Methods annotated with MOZ_NO_DANGLING_ON_TEMPORARIES cannot be && ref-qualified [clang-tidy: mozilla-dangling-on-temporary]

MOZ_NO_DANGLING_ON_TEMPORARIES int *get() && { return nullptr; } // after: mozilla-dangling-on-temporary
                                    ^
145

Error: Methods annotated with MOZ_NO_DANGLING_ON_TEMPORARIES must return a pointer [clang-tidy: mozilla-dangling-on-temporary]

MOZ_NO_DANGLING_ON_TEMPORARIES int test() { return 0; } // after: mozilla-dangling-on-temporary
                                   ^
191

Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return]

else if (ErrorTakesArguments(code_)) { // after: readability-misleading-indentation or readability-else-after-return
^~~~~~~~
225

Warning: Variable 'x' with floating point type 'float' should not be used as a loop counter [clang-tidy: clang-analyzer-security.FloatLoopCounter]

for (float x = 0.1f; x <= 1.0f; x += 0.1f) {} // after: clang-analyzer-security.FloatLoopCounter
^
227

Warning: The return value from the call to 'setuid' is not checked. If an error occurs in 'setuid', the following code may execute with unexpected privileges [clang-tidy: clang-analyzer-security.insecureAPI.UncheckedReturn]

setuid(1); // after: clang-analyzer-security.insecureAPI.UncheckedReturn
^
231

Warning: Potentially unintended semicolon [clang-tidy: bugprone-suspicious-semicolon]

if (x < y); // after: bugprone-suspicious-semicolon; readability-braces-around-statements
          ^
231

Warning: Statement should be inside braces [clang-tidy: readability-braces-around-statements]

if (x < y); // after: bugprone-suspicious-semicolon; readability-braces-around-statements
          ^
           {
248

Warning: Value stored to 'x' is never read [clang-tidy: clang-analyzer-deadcode.DeadStores]

x = 1; // after: clang-analyzer-deadcode.DeadStores
^
321

Warning: 'find' called with a string literal consisting of a single character; consider using the more effective overload accepting a character [clang-tidy: performance-faster-string-find]

str.find("A"); // after: performance-faster-string-find
         ^~~~
         'A'
328

Warning: The type of the loop variable 'p' is different from the one returned by the iterator and generates an implicit conversion; you can either change the type to the matching one ('const std::pair<const int, std::vector<std::cxx11::basic_string<char>, std::allocator<std::cxx11::basic_string<char> > > > &' but 'const auto&' is always a valid option) or remove the reference to make it explicit that you are creating a new value [clang-tidy: performance-implicit-conversion-in-loop]

for (const std::pair<int, std::vector<std::string>>& p : my_map) {} // after: performance-implicit-conversion-in-loop
     ^
toolkit/components/telemetry/build_scripts/gen_event_data.py
41

Error: At least two spaces before inline comment [flake8: E261]

41

Error: Missing whitespace around operator [flake8: E225]

41

Error: Line too long (177 > 99 characters) [flake8: E501]

65

Error: Continuation line under-indented for visual indent [flake8: E128]

Code analysis found 39 defects in this patch:

  • 13 defects found by clang-format
  • 17 defects found by clang-tidy
  • 9 defects found by mozlint

You can run this analysis locally with:

  • ./mach clang-format -p path/to/file.cpp (C/C++)
  • ./mach static-analysis check path/to/file.cpp (C/C++)
  • ./mach lint path/to/file (JS/Python/etc)

For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply).

If you see a problem in this automated review, please report it here.

browser/base/content/docs/tabbrowser/index.rst
7

Error: Exisits ==> exists [codespell]

devtools/client/definitions.js
7

Error: Infix operators must be spaced. [eslint: space-infix-ops]

devtools/client/menus.js
31

Error: Use the global form of 'use strict'. [eslint: strict]

41

Error: Unexpected tab character. [eslint: no-tabs]

devtools/client/shared/scroll.js
7

Error: Parsing error: Unexpected character '#' [eslint]

dom/media/CubebUtils.cpp
661

Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return]

} else if (aPreferred == CUBEB_DEVICE_PREF_ALL) { // before: readability-else-after-return
  ^~~~~~~~
js/ductwork/debugger/JSDebugger.cpp
49

Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return]

else if (!JS_IsGlobalObject(obj)) { // after: readability-else-after-return
^~~~~~~~
js/src/jsapi.cpp
132

Warning: Suspicious string literal, probably missing a comma [clang-tidy: bugprone-suspicious-missing-comma]

"line 6"     // after: bugprone-suspicious-missing-comma
^
144

Error: Methods annotated with MOZ_NO_DANGLING_ON_TEMPORARIES cannot be && ref-qualified [clang-tidy: mozilla-dangling-on-temporary]

MOZ_NO_DANGLING_ON_TEMPORARIES int *get() && { return nullptr; } // after: mozilla-dangling-on-temporary
                                    ^
145

Error: Methods annotated with MOZ_NO_DANGLING_ON_TEMPORARIES must return a pointer [clang-tidy: mozilla-dangling-on-temporary]

MOZ_NO_DANGLING_ON_TEMPORARIES int test() { return 0; } // after: mozilla-dangling-on-temporary
                                   ^
191

Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return]

else if (ErrorTakesArguments(code_)) { // after: readability-misleading-indentation or readability-else-after-return
^~~~~~~~
225

Warning: Variable 'x' with floating point type 'float' should not be used as a loop counter [clang-tidy: clang-analyzer-security.FloatLoopCounter]

for (float x = 0.1f; x <= 1.0f; x += 0.1f) {} // after: clang-analyzer-security.FloatLoopCounter
^
227

Warning: The return value from the call to 'setuid' is not checked. If an error occurs in 'setuid', the following code may execute with unexpected privileges [clang-tidy: clang-analyzer-security.insecureAPI.UncheckedReturn]

setuid(1); // after: clang-analyzer-security.insecureAPI.UncheckedReturn
^
231

Warning: Statement should be inside braces [clang-tidy: readability-braces-around-statements]

if (x < y); // after: bugprone-suspicious-semicolon; readability-braces-around-statements
          ^
           {
231

Warning: Potentially unintended semicolon [clang-tidy: bugprone-suspicious-semicolon]

if (x < y); // after: bugprone-suspicious-semicolon; readability-braces-around-statements
          ^
248

Warning: Value stored to 'x' is never read [clang-tidy: clang-analyzer-deadcode.DeadStores]

x = 1; // after: clang-analyzer-deadcode.DeadStores
^
321

Warning: 'find' called with a string literal consisting of a single character; consider using the more effective overload accepting a character [clang-tidy: performance-faster-string-find]

str.find("A"); // after: performance-faster-string-find
         ^~~~
         'A'
328

Warning: The type of the loop variable 'p' is different from the one returned by the iterator and generates an implicit conversion; you can either change the type to the matching one ('const std::pair<const int, std::vector<std::cxx11::basic_string<char>, std::allocator<std::cxx11::basic_string<char> > > > &' but 'const auto&' is always a valid option) or remove the reference to make it explicit that you are creating a new value [clang-tidy: performance-implicit-conversion-in-loop]

for (const std::pair<int, std::vector<std::string>>& p : my_map) {} // after: performance-implicit-conversion-in-loop
     ^
1498

Warning: Escaped string literal can be written as a raw string literal [clang-tidy: modernize-raw-string-literal]

"\"string\", \"number\", or \"default\"",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
R"("string", "number", or "default")"
1539

Warning: Escaped string literal can be written as a raw string literal [clang-tidy: modernize-raw-string-literal]

"\"string\", \"number\", or \"default\"", source);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
R"("string", "number", or "default")"
4747

Warning: Statement should be inside braces [clang-tidy: readability-braces-around-statements]

StringifyBehavior::RestrictedSafe))
                                   ^
                                    {
5374

Warning: Use '= default' to define a trivial destructor [clang-tidy: modernize-use-equals-default]

JSErrorNotes::~JSErrorNotes() {}
              ^               ~~
                              = default;
toolkit/components/telemetry/build_scripts/gen_event_data.py
41

Error: At least two spaces before inline comment [flake8: E261]

41

Error: Line too long (177 > 99 characters) [flake8: E501]

41

Error: Missing whitespace around operator [flake8: E225]

65

Error: Continuation line under-indented for visual indent [flake8: E128]

Code analysis found 35 defects in this patch:

  • 13 defects found by clang-format
  • 13 defects found by clang-tidy
  • 9 defects found by mozlint

You can run this analysis locally with:

  • ./mach clang-format -p path/to/file.cpp (C/C++)
  • ./mach static-analysis check path/to/file.cpp (C/C++)
  • ./mach lint path/to/file (JS/Python/etc)

For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply).

If you see a problem in this automated review, please report it here.

browser/base/content/docs/tabbrowser/index.rst
7

Error: Exisits ==> exists [codespell]

devtools/client/definitions.js
7

Error: Infix operators must be spaced. [eslint: space-infix-ops]

devtools/client/menus.js
31

Error: Use the global form of 'use strict'. [eslint: strict]

41

Error: Unexpected tab character. [eslint: no-tabs]

devtools/client/shared/scroll.js
7

Error: Parsing error: Unexpected character '#' [eslint]

dom/media/CubebUtils.cpp
661

Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return]

} else if (aPreferred == CUBEB_DEVICE_PREF_ALL) { // before: readability-else-after-return
  ^~~~~~~~
js/ductwork/debugger/JSDebugger.cpp
49

Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return]

else if (!JS_IsGlobalObject(obj)) { // after: readability-else-after-return
^~~~~~~~
js/src/jsapi.cpp
132

Warning: Suspicious string literal, probably missing a comma [clang-tidy: bugprone-suspicious-missing-comma]

"line 6"     // after: bugprone-suspicious-missing-comma
^
144

Error: Methods annotated with MOZ_NO_DANGLING_ON_TEMPORARIES cannot be && ref-qualified [clang-tidy: mozilla-dangling-on-temporary]

MOZ_NO_DANGLING_ON_TEMPORARIES int *get() && { return nullptr; } // after: mozilla-dangling-on-temporary
                                    ^
145

Error: Methods annotated with MOZ_NO_DANGLING_ON_TEMPORARIES must return a pointer [clang-tidy: mozilla-dangling-on-temporary]

MOZ_NO_DANGLING_ON_TEMPORARIES int test() { return 0; } // after: mozilla-dangling-on-temporary
                                   ^
191

Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return]

else if (ErrorTakesArguments(code_)) { // after: readability-misleading-indentation or readability-else-after-return
^~~~~~~~
225

Warning: Variable 'x' with floating point type 'float' should not be used as a loop counter [clang-tidy: clang-analyzer-security.FloatLoopCounter]

for (float x = 0.1f; x <= 1.0f; x += 0.1f) {} // after: clang-analyzer-security.FloatLoopCounter
^
227

Warning: The return value from the call to 'setuid' is not checked. If an error occurs in 'setuid', the following code may execute with unexpected privileges [clang-tidy: clang-analyzer-security.insecureAPI.UncheckedReturn]

setuid(1); // after: clang-analyzer-security.insecureAPI.UncheckedReturn
^
231

Warning: Statement should be inside braces [clang-tidy: readability-braces-around-statements]

if (x < y); // after: bugprone-suspicious-semicolon; readability-braces-around-statements
          ^
           {
231

Warning: Potentially unintended semicolon [clang-tidy: bugprone-suspicious-semicolon]

if (x < y); // after: bugprone-suspicious-semicolon; readability-braces-around-statements
          ^
248

Warning: Value stored to 'x' is never read [clang-tidy: clang-analyzer-deadcode.DeadStores]

x = 1; // after: clang-analyzer-deadcode.DeadStores
^
321

Warning: 'find' called with a string literal consisting of a single character; consider using the more effective overload accepting a character [clang-tidy: performance-faster-string-find]

str.find("A"); // after: performance-faster-string-find
         ^~~~
         'A'
328

Warning: The type of the loop variable 'p' is different from the one returned by the iterator and generates an implicit conversion; you can either change the type to the matching one ('const std::pair<const int, std::vector<std::cxx11::basic_string<char>, std::allocator<std::cxx11::basic_string<char> > > > &' but 'const auto&' is always a valid option) or remove the reference to make it explicit that you are creating a new value [clang-tidy: performance-implicit-conversion-in-loop]

for (const std::pair<int, std::vector<std::string>>& p : my_map) {} // after: performance-implicit-conversion-in-loop
     ^
toolkit/components/telemetry/build_scripts/gen_event_data.py
41

Error: At least two spaces before inline comment [flake8: E261]

41

Error: Missing whitespace around operator [flake8: E225]

41

Error: Line too long (177 > 99 characters) [flake8: E501]

65

Error: Continuation line under-indented for visual indent [flake8: E128]

Code analysis found 35 defects in this patch:

  • 13 defects found by clang-format
  • 13 defects found by clang-tidy
  • 9 defects found by mozlint

You can run this analysis locally with:

  • ./mach clang-format -p path/to/file.cpp (C/C++)
  • ./mach static-analysis check path/to/file.cpp (C/C++)
  • ./mach lint path/to/file (JS/Python/etc)

For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply).

If you see a problem in this automated review, please report it here.

browser/base/content/docs/tabbrowser/index.rst
7

Error: Exisits ==> exists [codespell]

devtools/client/definitions.js
7

Error: Infix operators must be spaced. [eslint: space-infix-ops]

devtools/client/menus.js
31

Error: Use the global form of 'use strict'. [eslint: strict]

41

Error: Unexpected tab character. [eslint: no-tabs]

devtools/client/shared/scroll.js
7

Error: Parsing error: Unexpected character '#' [eslint]

dom/media/CubebUtils.cpp
661

Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return]

} else if (aPreferred == CUBEB_DEVICE_PREF_ALL) { // before: readability-else-after-return
  ^~~~~~~~
js/ductwork/debugger/JSDebugger.cpp
49

Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return]

else if (!JS_IsGlobalObject(obj)) { // after: readability-else-after-return
^~~~~~~~
js/src/jsapi.cpp
132

Warning: Suspicious string literal, probably missing a comma [clang-tidy: bugprone-suspicious-missing-comma]

"line 6"     // after: bugprone-suspicious-missing-comma
^
144

Error: Methods annotated with MOZ_NO_DANGLING_ON_TEMPORARIES cannot be && ref-qualified [clang-tidy: mozilla-dangling-on-temporary]

MOZ_NO_DANGLING_ON_TEMPORARIES int *get() && { return nullptr; } // after: mozilla-dangling-on-temporary
                                    ^
145

Error: Methods annotated with MOZ_NO_DANGLING_ON_TEMPORARIES must return a pointer [clang-tidy: mozilla-dangling-on-temporary]

MOZ_NO_DANGLING_ON_TEMPORARIES int test() { return 0; } // after: mozilla-dangling-on-temporary
                                   ^
191

Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return]

else if (ErrorTakesArguments(code_)) { // after: readability-misleading-indentation or readability-else-after-return
^~~~~~~~
225

Warning: Variable 'x' with floating point type 'float' should not be used as a loop counter [clang-tidy: clang-analyzer-security.FloatLoopCounter]

for (float x = 0.1f; x <= 1.0f; x += 0.1f) {} // after: clang-analyzer-security.FloatLoopCounter
^
227

Warning: The return value from the call to 'setuid' is not checked. If an error occurs in 'setuid', the following code may execute with unexpected privileges [clang-tidy: clang-analyzer-security.insecureAPI.UncheckedReturn]

setuid(1); // after: clang-analyzer-security.insecureAPI.UncheckedReturn
^
231

Warning: Potentially unintended semicolon [clang-tidy: bugprone-suspicious-semicolon]

if (x < y); // after: bugprone-suspicious-semicolon; readability-braces-around-statements
          ^
231

Warning: Statement should be inside braces [clang-tidy: readability-braces-around-statements]

if (x < y); // after: bugprone-suspicious-semicolon; readability-braces-around-statements
          ^
           {
248

Warning: Value stored to 'x' is never read [clang-tidy: clang-analyzer-deadcode.DeadStores]

x = 1; // after: clang-analyzer-deadcode.DeadStores
^
321

Warning: 'find' called with a string literal consisting of a single character; consider using the more effective overload accepting a character [clang-tidy: performance-faster-string-find]

str.find("A"); // after: performance-faster-string-find
         ^~~~
         'A'
328

Warning: The type of the loop variable 'p' is different from the one returned by the iterator and generates an implicit conversion; you can either change the type to the matching one ('const std::pair<const int, std::vector<std::cxx11::basic_string<char>, std::allocator<std::cxx11::basic_string<char> > > > &' but 'const auto&' is always a valid option) or remove the reference to make it explicit that you are creating a new value [clang-tidy: performance-implicit-conversion-in-loop]

for (const std::pair<int, std::vector<std::string>>& p : my_map) {} // after: performance-implicit-conversion-in-loop
     ^
toolkit/components/telemetry/build_scripts/gen_event_data.py
41

Error: Line too long (177 > 99 characters) [flake8: E501]

41

Error: At least two spaces before inline comment [flake8: E261]

41

Error: Missing whitespace around operator [flake8: E225]

65

Error: Continuation line under-indented for visual indent [flake8: E128]