Enable ESLint for toolkit/components/osfile

RESOLVED FIXED in Firefox 57

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: standard8, Assigned: dbugs, Mentored)

Tracking

Trunk
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(3 attachments)

As part of rolling out ESLint across the tree, we should enable it for the toolkit/components/osfile directory.

I'm happy to mentor this bug. There's background on our eslint setups here:

https://developer.mozilla.org/docs/ESLint

Here's some approximate steps:

- In .eslintignore, remove the `toolkit/components/osfile/**` line.
- Run eslint:

./mach eslint --fix extensions

This should fix most/all of the issues.

- Inspect the diff to make sure that the indentation of the lines surrounding the changes look ok.

- Create a commit of the work so far.

- For the remaining issues, you'll need to fix them by hand.

- Create a second commit of the manual changes and push it to mozreview:

http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html

If you're not sure of how to fix something, please look at the rules documentation http://eslint.org/docs/rules/ or just ask here.
Depends on: 1386666
Comment on attachment 8895384 [details]
Bug 1379256 - Enable the rest of the recommended ESLint rules for toolkit/components/osfile.

https://reviewboard.mozilla.org/r/166578/#review171796

::: toolkit/components/osfile/tests/mochi/worker_test_osfile_unix.js:189
(Diff revision 1)
> -    let file = OS.Unix.File.open(undefined, OS.Constants.libc.O_RDWR
> +    OS.Unix.File.open(undefined, OS.Constants.libc.O_RDWR
>                                              | OS.Constants.libc.O_CREAT
>                                              | OS.Constants.libc.O_TRUNC,
>                                              OS.Constants.libc.S_IRWXU);

Please align the following lines under OS.Constants as before.
Attachment #8895384 - Flags: review?(dtownsend) → review-
Comment on attachment 8895382 [details]
Bug 1379256 - Enable more ESLint rules for toolkit/components/osfile (automatic fixes).

https://reviewboard.mozilla.org/r/166574/#review171788

::: toolkit/components/osfile/modules/osfile_unix_back.jsm:244
(Diff revision 1)
> -                        /*return */ctypes.int,
> +                        /* return */ctypes.int, ctypes.int);
> -                        /*fd*/     ctypes.int);

Why did this change happen? I think it's important to keep the comments for the arguments here and throughout
Attachment #8895382 - Flags: review?(dtownsend)
Comment on attachment 8895383 [details]
Bug 1379256 - fix no-undef issues for toolkit/components/osfile.

https://reviewboard.mozilla.org/r/166576/#review171792

Throughout this file there are top-level comments starting indented a space or so. Please correct those to start at the start of the line.

::: toolkit/components/osfile/modules/osfile_async_worker.js:5
(Diff revision 1)
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>   * You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> + /* eslint-env worker */

This should start at the start of the line.

::: toolkit/components/osfile/modules/osfile_shared_allthreads.jsm:20
(Diff revision 1)
>   * - primitives for dealing with integers, pointers, typed arrays;
>   * - the base class OSError;
>   * - a few additional utilities.
>   */
>  
> + /* eslint-env worker */

Same here

::: toolkit/components/osfile/modules/osfile_shared_front.jsm:12
(Diff revision 1)
> + /* eslint-env node */
> + /* global OS */

Ditto

::: toolkit/components/osfile/modules/osfile_unix_allthreads.jsm:20
(Diff revision 1)
>   * This module can be:
>   * - opened from the main thread as a jsm module;
>   * - opened from a chrome worker through require().
>   */
>  
> + /* eslint-env node */

Same here.
Attachment #8895383 - Flags: review?(dtownsend) → review-
Comment on attachment 8895382 [details]
Bug 1379256 - Enable more ESLint rules for toolkit/components/osfile (automatic fixes).

https://reviewboard.mozilla.org/r/166574/#review172862
Attachment #8895382 - Flags: review?(dtownsend) → review+
Comment on attachment 8895383 [details]
Bug 1379256 - fix no-undef issues for toolkit/components/osfile.

https://reviewboard.mozilla.org/r/166576/#review172866
Attachment #8895383 - Flags: review?(dtownsend) → review+
Comment on attachment 8895384 [details]
Bug 1379256 - Enable the rest of the recommended ESLint rules for toolkit/components/osfile.

https://reviewboard.mozilla.org/r/166578/#review172868
Attachment #8895384 - Flags: review?(dtownsend) → review+
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8e52d4dcbaeb
Enable more ESLint rules for toolkit/components/osfile (automatic fixes). r=mossop
https://hg.mozilla.org/integration/autoland/rev/35420ca15900
fix no-undef issues for toolkit/components/osfile. r=mossop
https://hg.mozilla.org/integration/autoland/rev/d319322e33cc
Enable the rest of the recommended ESLint rules for toolkit/components/osfile. r=mossop
Backed out for failing xpcshell's toolkit/components/osfile/tests/xpcshell/test_read_write.js:

https://hg.mozilla.org/integration/autoland/rev/e95831a33a2ef6dd321fe896ac86c0e0bcab1e32
https://hg.mozilla.org/integration/autoland/rev/5f15be9e9ee6cd1101c6bc5eeaba1b4d610fa0a3
https://hg.mozilla.org/integration/autoland/rev/cbac131f1bf85344333b26ff875e051891ef7954

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

[task 2017-08-12T11:25:08.453327Z] 11:25:08     INFO -  TEST-PASS | toolkit/components/osfile/tests/xpcshell/test_read_write.js |  - "[DEFAULT]\\nhead = head.js\\n\\nsupport-files =\\n  test_loader/module_test_loader.js\\n\\n[test_available_free_space.js]\\n[test_comp == "[DEFAULT]\\nhead = head.js\\n\\nsupport-files =\\n  test_loader/module_test_loader.js\\n\\n[test_available_free_space.js]\\n[test_comp
[task 2017-08-12T11:25:08.455149Z] 11:25:08     INFO -  TEST-PASS | toolkit/components/osfile/tests/xpcshell/test_read_write.js |  - true == true
[task 2017-08-12T11:25:08.457165Z] 11:25:08     INFO -  PID 13388 | OS Controller Posting message {"fun":"writeAtomic","args":[{"string":"/tmp/xpc-profile-16HDmo/test_osfile_read.tmp0.08781510437550877"},{"data":"[Uint8Array 10 100]"},{"tmpPath":{"string":"/tmp/xpc-profile-16HDmo/test_osfile_read.tmp0.08781510437550877.tmp"},"bytes":100}],"id":10}
[task 2017-08-12T11:25:08.468073Z] 11:25:08     INFO -  PID 13388 | OS Agent Received message {"fun":"writeAtomic","args":[{"string":"/tmp/xpc-profile-16HDmo/test_osfile_read.tmp0.08781510437550877"},{"data":"[Uint8Array 10 100]"},{"tmpPath":{"string":"/tmp/xpc-profile-16HDmo/test_osfile_read.tmp0.08781510437550877.tmp"},"bytes":100}],"id":10}
[task 2017-08-12T11:25:08.469856Z] 11:25:08     INFO -  PID 13388 | OS Agent Calling method writeAtomic
[task 2017-08-12T11:25:08.471559Z] 11:25:08     INFO -  PID 13388 | OS Agent Method writeAtomic succeeded
[task 2017-08-12T11:25:08.473214Z] 11:25:08     INFO -  PID 13388 | OS Agent Sending positive reply 100 id is 10
[task 2017-08-12T11:25:08.475066Z] 11:25:08     INFO -  PID 13388 | OS Controller Message posted
[task 2017-08-12T11:25:08.479491Z] 11:25:08     INFO -  PID 13388 | OS Controller Expecting reply
[task 2017-08-12T11:25:08.481193Z] 11:25:08     INFO -  PID 13388 | OS Controller Received message from worker {"ok":100,"id":10}
[task 2017-08-12T11:25:08.482883Z] 11:25:08     INFO -  TEST-PASS | toolkit/components/osfile/tests/xpcshell/test_read_write.js |  - 100 == 100
[task 2017-08-12T11:25:08.484692Z] 11:25:08     INFO -  TEST-PASS | toolkit/components/osfile/tests/xpcshell/test_read_write.js |  - 100 == 100
[task 2017-08-12T11:25:08.486404Z] 11:25:08  WARNING -  TEST-UNEXPECTED-FAIL | toolkit/components/osfile/tests/xpcshell/test_read_write.js |  - "undefined" == 54
[task 2017-08-12T11:25:08.488349Z] 11:25:08     INFO -  /home/worker/workspace/build/tests/xpcshell/tests/toolkit/components/osfile/tests/xpcshell/test_read_write.js:read_write_all/test_with_options/<:90
[task 2017-08-12T11:25:08.493555Z] 11:25:08     INFO -  exiting test
[task 2017-08-12T11:25:08.495208Z] 11:25:08     INFO -  Unexpected exception 2147500036
[task 2017-08-12T11:25:08.496785Z] 11:25:08     INFO -  undefined
[task 2017-08-12T11:25:08.498373Z] 11:25:08     INFO -  exiting test
Flags: needinfo?(dbugs)
Flags: needinfo?(dbugs)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s d783a1d70a51 -d 3d7de8517c75: rebasing 414702:d783a1d70a51 "Bug 1379256 - Enable more ESLint rules for toolkit/components/osfile (automatic fixes). r=mossop"
merging toolkit/components/osfile/modules/osfile_async_front.jsm
merging toolkit/components/osfile/tests/xpcshell/test_constants.js
merging toolkit/components/osfile/tests/xpcshell/test_path_constants.js
warning: conflicts while merging toolkit/components/osfile/tests/xpcshell/test_constants.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/305278e3d8ee
Enable more ESLint rules for toolkit/components/osfile (automatic fixes). r=mossop
https://hg.mozilla.org/integration/autoland/rev/694bf3c600db
fix no-undef issues for toolkit/components/osfile. r=mossop
https://hg.mozilla.org/integration/autoland/rev/8cc0d745ab26
Enable the rest of the recommended ESLint rules for toolkit/components/osfile. r=mossop
You need to log in before you can comment on or make changes to this bug.