1395890 - Change all ESLint rules that are warnings to errors

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement, P3)

Description

[Mark Banner (:standard8)]

There's a number of rules in the tree that are set up for warnings rather than errors.

http://searchfox.org/mozilla-central/search?q=%22warn%22&case=true&regexp=false&path=.eslintrc.js
http://searchfox.org/mozilla-central/search?q=%22warn%22&case=true&regexp=false&path=tools%2Flint%2Feslint

Since warnings are frequently ignored or left unaddressed (and then just fill up consoles, making it hard to find real issues), I think we should go for making all the ESLint rules error all the time.

Comment 1

[Dan Banner]

Attachment: Bug 1395890 - Change all ESLint rules that are warnings to errors.

Review commit: https://reviewboard.mozilla.org/r/180930/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180930/

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 8909429 [details]
Bug 1395890 - Change all ESLint rules that are warnings to errors.

https://reviewboard.mozilla.org/r/180930/#review186396

Looks great. r=Standard8

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4c33a37fe6d
Change all ESLint rules that are warnings to errors. r=standard8

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for failing browser-chrome's browser/components/extensions/test/browser/browser_ext_find.js:

https://hg.mozilla.org/integration/autoland/rev/d86b4cdd17d2790797a5052241b0e3b2be956ef8

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f4c33a37fe6da79e663ee412581ac05f5234ee1f&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=131943035&repo=autoland

[task 2017-09-19T10:51:41.402Z] 10:51:41     INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_find.js | Promise rejected, expecting rejection to match /no search results to highlight/, got "no search results to highlight": rejected Promise should pass the expected error - 
[task 2017-09-19T10:51:41.408Z] 10:51:41     INFO - Console message: [JavaScript Error: "SyntaxError: missing } after function body" {file: "data:,(function frameScript() {
[task 2017-09-19T10:51:41.409Z] 10:51:41     INFO -   function getSelectedText() {
[task 2017-09-19T10:51:41.411Z] 10:51:41     INFO -     // eslint-disable-next-line mozilla/no-cpows-in-tests
[task 2017-09-19T10:51:41.413Z] 10:51:41     INFO -     let frame = this.content.frames[0].frames[1];
[task 2017-09-19T10:51:41.415Z] 10:51:41     INFO -     let Ci = Components.interfaces;
[task 2017-09-19T10:51:41.416Z] 10:51:41     INFO -     let docShell = frame.QueryInterface(Ci.nsIInterfaceRequestor)
[task 2017-09-19T10:51:41.419Z] 10:51:41     INFO -                         .getInterface(Ci.nsIWebNavigation)
[task 2017-09-19T10:51:41.420Z] 10:51:41     INFO -                         .QueryInterface(Ci.nsIDocShell);
[task 2017-09-19T10:51:41.423Z] 10:51:41     INFO -     let controller = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
[task 2017-09-19T10:51:41.427Z] 10:51:41     INFO -                              .getInterface(Ci.nsISel" line: 1 column: 1058 source: "xt: selection.toString(), rect});  }  getSelectedText();})()"}]
[task 2017-09-19T10:51:41.429Z] 10:51:41     INFO - Buffered messages finished
[task 2017-09-19T10:51:41.432Z] 10:51:41     INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_find.js | Test timed out -

Comment 5

[Boris Zbarsky [:bzbarsky]]

Fwiw, the issue is this bit in browser_ext_find.js:

  let frameScriptUrl = `data:,(${frameScript})()`;

It's clever and all, but data: URI parsing (and all URI parsing, really) strips newlines.  So if you put a line comment in that function, you will get the entire function after that part commented out.

Comment 6

[Dan Banner]

Comment on attachment 8909429 [details]
Bug 1395890 - Change all ESLint rules that are warnings to errors.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/180930/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33181ffec32a
Change all ESLint rules that are warnings to errors. r=standard8

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/33181ffec32a