Details

Reviewers: None
Bugzilla Bug ID: 1387052

Diff Detail

Repository

Branch

worse

Build Status

Buildable 418
Build 472: arc lint + arc unit

Event Timeline

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

ReviewBot added inline comments.Jul 4 2018, 12:47 PM

js/ductwork/debugger/JSDebugger.cpp
58 ↗	(On Diff #731)	Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return] else if (!JS_DefineDebuggerObject(cx, obj)) { // readability-else-after-return ^~~~~~~~
js/src/jsapi.cpp
123	Warning: Suspicious string literal, probably missing a comma [clang-tidy: misc-suspicious-missing-comma] "line 6" // misc-suspicious-missing-comma ^
132	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; } // mozilla-dangling-on-temporary ^

Code analysis found 11 defects in this patch:

4 defects found by clang-tidy
7 defects found by mozlint

You can run this analysis locally with:

./mach static-analysis check path/to/file.cpp (C/C++)
./mach lint path/to/file (JS/Python)

If you see a problem in this automated review, please report it here: https://bit.ly/2tb8Qk3

devtools/client/definitions.js
7	Error: Infix operators must be spaced. [eslint: space-infix-ops]
devtools/client/menus.js
42	Error: Unexpected tab character. [eslint: no-tabs]
devtools/client/shared/scroll.js
7	Error: Parsing error: Unexpected character '#' [eslint]
js/ductwork/debugger/JSDebugger.cpp
58 ↗	(On Diff #731)	Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return] else if (!JS_DefineDebuggerObject(cx, obj)) { // readability-else-after-return ^~~~~~~~
js/src/jsapi.cpp
123	Warning: Suspicious string literal, probably missing a comma [clang-tidy: misc-suspicious-missing-comma] "line 6" // misc-suspicious-missing-comma ^
132	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; } // mozilla-dangling-on-temporary ^
133	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; } // mozilla-dangling-on-temporary ^
toolkit/components/telemetry/gen_event_data.py
37	Error: Missing whitespace around operator [flake8: E225]
37	Error: At least two spaces before inline comment [flake8: E261]
37	Error: Line too long (177 > 99 characters) [flake8: E501]
57	Error: Continuation line under-indented for visual indent [flake8: E128]

Rebased, "final" attempt.

Harbormaster completed remote builds in B492: Diff 762.Jul 5 2018, 2:48 PM

Code analysis found 18 defects in this patch:

7 defects found by clang-format
4 defects found by clang-tidy
7 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)

If you see a problem in this automated review, please report it here: https://bit.ly/2tb8Qk3

A full diff for the formatting issues found by clang-format is provided here: https://queue.taskcluster.net/v1/task/S-5A7kahQ_KejF3hOswloA/runs/0/artifacts/public/results/PHID-DIFF-hgz5ezxqh24e6hkef2dg-clang-format.diff

You can use it in your repository with hg import or git apply

devtools/client/definitions.js
7	Error: Infix operators must be spaced. [eslint: space-infix-ops]
devtools/client/menus.js
42	Error: Unexpected tab character. [eslint: no-tabs]
devtools/client/shared/scroll.js
7	Error: Parsing error: Unexpected character '#' [eslint]
js/ductwork/debugger/JSDebugger.cpp
58 ↗	(On Diff #762)	Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return] else if (!JS_DefineDebuggerObject(cx, obj)) { // readability-else-after-return ^~~~~~~~
js/src/jsapi.cpp
117–125	Warning: Incorrect coding style [clang-format] Replace by: const char* Test[] = { "line 1", "line 2", "line 3", "line 4", "line 5", "line 6" // misc-suspicious-missing-comma "line 7", "line 8",
123	Warning: Suspicious string literal, probably missing a comma [clang-tidy: misc-suspicious-missing-comma] "line 6" // misc-suspicious-missing-comma ^
130–136	Warning: Incorrect coding style [clang-format] Replace by: class AnnotateConflict { MOZ_NO_DANGLING_ON_TEMPORARIES int* get() && { return nullptr; } // mozilla-dangling-on-temporary MOZ_NO_DANGLING_ON_TEMPORARIES int test() { return 0; } // mozilla-dangling-on-temporary
132	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; } // mozilla-dangling-on-temporary ^
133	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; } // mozilla-dangling-on-temporary ^
180–185	Warning: Incorrect coding style [clang-format] Replace by: RootedValue val(cx, ObjectValue(*obj)); return ReportValueErrorFlags( cx, flags, code_, JSDVG_IGNORE_STACK, val, nullptr, nullptr, nullptr); } else if (ErrorTakesArguments( code_)) { // readability-misleading-indentation or readability-else-after-return
219–234	Warning: Incorrect coding style [clang-format] Replace by: { for (float x = 0.1f; x <= 1.0f; x += 0.1f) { } // clang-analyzer-security.FloatLoopCounter
225–226	Warning: Incorrect coding style [clang-format] Replace by: int y = 0; if (x < y) ; // misc-suspicious-semicolon
227–228	Warning: Incorrect coding style [clang-format] Replace by: { x++;
354–355	Warning: Incorrect coding style [clang-format] Replace by: std::map<int, std::vector<std::string>> my_map; for (const std::pair<int, std::vector<std::string>>& p : my_map) { } // performance-implicit-conversion-in-loop
toolkit/components/telemetry/gen_event_data.py
37	Error: Missing whitespace around operator [flake8: E225]
37	Error: At least two spaces before inline comment [flake8: E261]
37	Error: Line too long (177 > 99 characters) [flake8: E501]
57	Error: Continuation line under-indented for visual indent [flake8: E128]

"Final final" attempt.

Harbormaster completed remote builds in B493: Diff 763.Jul 5 2018, 4:14 PM

Code analysis found 18 defects in this patch:

7 defects found by clang-format
4 defects found by clang-tidy
7 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)

If you see a problem in this automated review, please report it here: https://bit.ly/2tb8Qk3

A full diff for the formatting issues found by clang-format is provided here: https://queue.taskcluster.net/v1/task/O66uGSEjTDWrfHoBuIvQTA/runs/0/artifacts/public/results/PHID-DIFF-brgzg66z7dceu3st6mqr-clang-format.diff

You can use it in your repository with hg import or git apply

devtools/client/definitions.js
7	Error: Infix operators must be spaced. [eslint: space-infix-ops]
devtools/client/menus.js
42	Error: Unexpected tab character. [eslint: no-tabs]
devtools/client/shared/scroll.js
7	Error: Parsing error: Unexpected character '#' [eslint]
js/ductwork/debugger/JSDebugger.cpp
58 ↗	(On Diff #763)	Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return] else if (!JS_DefineDebuggerObject(cx, obj)) { // readability-else-after-return ^~~~~~~~
js/src/jsapi.cpp
117–125	Warning: Incorrect coding style [clang-format] Replace by: const char* Test[] = { "line 1", "line 2", "line 3", "line 4", "line 5", "line 6" // misc-suspicious-missing-comma "line 7", "line 8",
123	Warning: Suspicious string literal, probably missing a comma [clang-tidy: misc-suspicious-missing-comma] "line 6" // misc-suspicious-missing-comma ^
130–136	Warning: Incorrect coding style [clang-format] Replace by: class AnnotateConflict { MOZ_NO_DANGLING_ON_TEMPORARIES int* get() && { return nullptr; } // mozilla-dangling-on-temporary MOZ_NO_DANGLING_ON_TEMPORARIES int test() { return 0; } // mozilla-dangling-on-temporary
132	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; } // mozilla-dangling-on-temporary ^
133	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; } // mozilla-dangling-on-temporary ^
180–185	Warning: Incorrect coding style [clang-format] Replace by: RootedValue val(cx, ObjectValue(*obj)); return ReportValueErrorFlags( cx, flags, code_, JSDVG_IGNORE_STACK, val, nullptr, nullptr, nullptr); } else if (ErrorTakesArguments( code_)) { // readability-misleading-indentation or readability-else-after-return
219–234	Warning: Incorrect coding style [clang-format] Replace by: { for (float x = 0.1f; x <= 1.0f; x += 0.1f) { } // clang-analyzer-security.FloatLoopCounter
225–226	Warning: Incorrect coding style [clang-format] Replace by: int y = 0; if (x < y) ; // misc-suspicious-semicolon
227–228	Warning: Incorrect coding style [clang-format] Replace by: { x++;
354–355	Warning: Incorrect coding style [clang-format] Replace by: std::map<int, std::vector<std::string>> my_map; for (const std::pair<int, std::vector<std::string>>& p : my_map) { } // performance-implicit-conversion-in-loop
toolkit/components/telemetry/gen_event_data.py
37	Error: Missing whitespace around operator [flake8: E225]
37	Error: At least two spaces before inline comment [flake8: E261]
37	Error: Line too long (177 > 99 characters) [flake8: E501]
57	Error: Continuation line under-indented for visual indent [flake8: E128]

Code analysis found 17 defects in this patch:

10 defects found by clang-tidy
7 defects found by mozlint

You can run this analysis locally with:

./mach static-analysis check path/to/file.cpp (C/C++)
./mach lint path/to/file (JS/Python)

If you see a problem in this automated review, please report it here: https://bit.ly/2tb8Qk3

devtools/client/definitions.js
7	Error: Infix operators must be spaced. [eslint: space-infix-ops]
devtools/client/menus.js
42	Error: Unexpected tab character. [eslint: no-tabs]
devtools/client/shared/scroll.js
7	Error: Parsing error: Unexpected character '#' [eslint]
js/ductwork/debugger/JSDebugger.cpp
58 ↗	(On Diff #763)	Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return] else if (!JS_DefineDebuggerObject(cx, obj)) { // readability-else-after-return ^~~~~~~~
js/src/jsapi.cpp
123	Warning: Suspicious string literal, probably missing a comma [clang-tidy: misc-suspicious-missing-comma] "line 6" // misc-suspicious-missing-comma ^
132	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; } // mozilla-dangling-on-temporary ^
133	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; } // mozilla-dangling-on-temporary ^
185	Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return] else if (ErrorTakesArguments(code_)) { // readability-misleading-indentation or readability-else-after-return ^~~~~~~~
220	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) {} // clang-analyzer-security.FloatLoopCounter ^
222	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); // clang-analyzer-security.insecureAPI.UncheckedReturn ^
226	Warning: Potentially unintended semicolon [clang-tidy: misc-suspicious-semicolon] if (x < y); // misc-suspicious-semicolon ^
247	Warning: Value stored to 'x' is never read [clang-tidy: clang-analyzer-deadcode.DeadStores] x = 1; // clang-analyzer-deadcode.DeadStores ^
346	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"); // performance-faster-string-find ^~~~ 'A'
toolkit/components/telemetry/gen_event_data.py
37	Error: Missing whitespace around operator [flake8: E225]
37	Error: At least two spaces before inline comment [flake8: E261]
37	Error: Line too long (177 > 99 characters) [flake8: E501]
57	Error: Continuation line under-indented for visual indent [flake8: E128]

Testing reviewbot's C++ fix.

Harbormaster completed remote builds in B576: Diff 847.Jul 13 2018, 9:45 AM

Code analysis found 17 defects in this patch:

10 defects found by clang-tidy
7 defects found by mozlint

You can run this analysis locally with:

./mach static-analysis check path/to/file.cpp (C/C++)
./mach lint path/to/file (JS/Python)

If you see a problem in this automated review, please report it here: https://bit.ly/2tb8Qk3

devtools/client/definitions.js
7	Error: Infix operators must be spaced. [eslint: space-infix-ops]
devtools/client/menus.js
42	Error: Unexpected tab character. [eslint: no-tabs]
devtools/client/shared/scroll.js
7	Error: Parsing error: Unexpected character '#' [eslint]
js/ductwork/debugger/JSDebugger.cpp
53 ↗	(On Diff #847)	Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return] else if (!JS_IsGlobalObject(obj)) { // readability-else-after-return ^~~~~~~~
js/src/jsapi.cpp
123	Warning: Suspicious string literal, probably missing a comma [clang-tidy: misc-suspicious-missing-comma] "line 6" // misc-suspicious-missing-comma ^
132	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; } // mozilla-dangling-on-temporary ^
133	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; } // mozilla-dangling-on-temporary ^
185	Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return] else if (ErrorTakesArguments(code_)) { // readability-misleading-indentation or readability-else-after-return ^~~~~~~~
220	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) {} // clang-analyzer-security.FloatLoopCounter ^
222	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); // clang-analyzer-security.insecureAPI.UncheckedReturn ^
226	Warning: Potentially unintended semicolon [clang-tidy: misc-suspicious-semicolon] if (x < y); // misc-suspicious-semicolon ^
247	Warning: Value stored to 'x' is never read [clang-tidy: clang-analyzer-deadcode.DeadStores] x = 1; // clang-analyzer-deadcode.DeadStores ^
346	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"); // performance-faster-string-find ^~~~ 'A'
toolkit/components/telemetry/gen_event_data.py
37	Error: Missing whitespace around operator [flake8: E225]
37	Error: At least two spaces before inline comment [flake8: E261]
37	Error: Line too long (177 > 99 characters) [flake8: E501]
57	Error: Continuation line under-indented for visual indent [flake8: E128]

This review is now long enough, time to move to a new one.

Bug 1387052 - Make Firefox code worse intentionally. r=reviewbot
AbandonedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 684

.arcconfig

devtools/client/definitions.js

devtools/client/menus.js

devtools/client/shared/scroll.js

js/src/ctypes/libffi/src/prep_cif.c

js/src/jsapi.cpp

toolkit/components/telemetry/gen_event_data.py

Bug 1387052 - Make Firefox code worse intentionally. r=reviewbotAbandonedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 684

.arcconfig

devtools/client/definitions.js

devtools/client/menus.js

devtools/client/shared/scroll.js

js/src/ctypes/libffi/src/prep_cif.c

js/src/jsapi.cpp

toolkit/components/telemetry/gen_event_data.py

Bug 1387052 - Make Firefox code worse intentionally. r=reviewbot
AbandonedPublic
Actions

Revision Contents
Changeset List