1379256 - Enable ESLint for toolkit/components/osfile

Status: RESOLVED FIXED

(Toolkit Graveyard :: OS.File, enhancement)

Description

[Mark Banner (:standard8)]

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.

Comment 1

[Dan Banner]

Attachment: Bug 1379256 - Enable more ESLint rules for toolkit/components/osfile (automatic fixes).

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

Comment 2

[Dan Banner]

Attachment: Bug 1379256 - fix no-undef issues for toolkit/components/osfile.

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

Comment 3

[Dan Banner]

Attachment: Bug 1379256 - Enable the rest of the recommended ESLint rules for toolkit/components/osfile.

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

Comment 4

[Dave Townsend [:mossop]]

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.

Comment 5

[Dave Townsend [:mossop]]

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

Comment 6

[Dave Townsend [:mossop]]

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.

Comment 7

[Dan Banner]

Comment on attachment 8895382 [details]
Bug 1379256 - Enable more ESLint rules for toolkit/components/osfile (automatic fixes).

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

Comment 8

[Dan Banner]

Comment on attachment 8895383 [details]
Bug 1379256 - fix no-undef issues for toolkit/components/osfile.

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

Comment 9

[Dan Banner]

Comment on attachment 8895384 [details]
Bug 1379256 - Enable the rest of the recommended ESLint rules for toolkit/components/osfile.

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

Comment 10

[Dave Townsend [:mossop]]

Comment on attachment 8895382 [details]
Bug 1379256 - Enable more ESLint rules for toolkit/components/osfile (automatic fixes).

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

Comment 11

[Dave Townsend [:mossop]]

Comment on attachment 8895383 [details]
Bug 1379256 - fix no-undef issues for toolkit/components/osfile.

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

Comment 12

[Dave Townsend [:mossop]]

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

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

Comment 14

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

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

Comment 15

[Dan Banner]

Comment on attachment 8895382 [details]
Bug 1379256 - Enable more ESLint rules for toolkit/components/osfile (automatic fixes).

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

Comment 16

[Dan Banner]

Comment on attachment 8895383 [details]
Bug 1379256 - fix no-undef issues for toolkit/components/osfile.

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

Comment 17

[Dan Banner]

Comment on attachment 8895384 [details]
Bug 1379256 - Enable the rest of the recommended ESLint rules for toolkit/components/osfile.

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

Comment 18

[Mozilla Autoland]

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)

Comment 19

[Dan Banner]

Comment on attachment 8895382 [details]
Bug 1379256 - Enable more ESLint rules for toolkit/components/osfile (automatic fixes).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/166574/diff/3-4/

Comment 20

[Dan Banner]

Comment on attachment 8895383 [details]
Bug 1379256 - fix no-undef issues for toolkit/components/osfile.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/166576/diff/3-4/

Comment 21

[Dan Banner]

Comment on attachment 8895384 [details]
Bug 1379256 - Enable the rest of the recommended ESLint rules for toolkit/components/osfile.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/166578/diff/3-4/

Comment 22

[Pulsebot]

Pushed by [e-mail]:
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

Comment 23

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/305278e3d8ee
https://hg.mozilla.org/mozilla-central/rev/694bf3c600db
https://hg.mozilla.org/mozilla-central/rev/8cc0d745ab26