Change all ESLint rules that are warnings to errors

RESOLVED FIXED in Firefox 58

Status

Testing
Lint
P3
normal
RESOLVED FIXED
5 months ago
3 months ago

People

(Reporter: standard8, Assigned: Dan Banner)

Tracking

Version 3
mozilla58
Points:
---

Firefox Tracking Flags

(firefox58 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

5 months ago
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.
(Reporter)

Updated

5 months ago
Priority: -- → P3
Comment hidden (mozreview-request)
(Reporter)

Comment 2

4 months ago
mozreview-review
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
Attachment #8909429 - Flags: review?(standard8) → review+

Comment 3

4 months ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f4c33a37fe6d
Change all ESLint rules that are warnings to errors. r=standard8
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 -
Flags: needinfo?(dbugs)
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.
(Assignee)

Updated

3 months ago
Flags: needinfo?(dbugs)
Comment hidden (mozreview-request)

Comment 7

3 months ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/33181ffec32a
Change all ESLint rules that are warnings to errors. r=standard8
https://hg.mozilla.org/mozilla-central/rev/33181ffec32a
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.