1478305 - ESLint should treat ChomeUtils.import as real variable definitions where possible

Status: RESOLVED FIXED

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

Description

[Mark Banner (:standard8)]

Building on Kris' change in bug 1456686 to treat some imports as real variable definitions, I think we can do the same for Cu.import, where the imported module has only one export.

This will help us eliminate more unused imports, mainly in *.jsm files where we have no-unused-vars in the global scope. There's a few other locations where that is also enabled globally so they will benefit as well.

Comment 1

[Mark Banner (:standard8)]

Note: the reason we can't really do it for modules that have more than one export (at the moment), is that if only one export is used it'll flag the others as unused.

We can maybe have a follow-up to start to get people to use the explicit form of imports.

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 1478305 - Update ESLint to treat ChromeUtils.import definitions as real variable definitions for single-export modules.

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

Comment 3

[Mark Banner (:standard8)]

Attachment: Bug 1478305 - For xpcshell-test head files, limit checking no-unused-vars to the local scope only.

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

Comment 4

[Mark Banner (:standard8)]

Attachment: Bug 1478305 - Remove unnecessary imports and fix ESLint warnings about unused variables for browser/.

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

Comment 5

[Mark Banner (:standard8)]

Attachment: Bug 1478305 - Remove unnecessary ChromeUtils.import calls in dom/media.

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

Comment 6

[Mark Banner (:standard8)]

Attachment: Bug 1478305 - Remove unnecessary ChromeUtils.import calls in mobile/.

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

Comment 7

[Mark Banner (:standard8)]

Attachment: Bug 1478305 - Remove unnecessary ChromeUtils.import calls in testing/.

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

Comment 8

[Mark Banner (:standard8)]

Attachment: Bug 1478305 - Remove unnecessary imports and fix ESLint warnings about unused variables for toolkit/.

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

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 8994782 [details]
Bug 1478305 - Update ESLint to treat ChromeUtils.import definitions as real variable definitions for single-export modules.

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

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 8995921 [details]
Bug 1478305 - Remove unnecessary imports and fix ESLint warnings about unused variables for browser/.

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

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 8995922 [details]
Bug 1478305 - Remove unnecessary ChromeUtils.import calls in dom/media.

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

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 8995923 [details]
Bug 1478305 - Remove unnecessary ChromeUtils.import calls in mobile/.

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

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 8995924 [details]
Bug 1478305 - Remove unnecessary ChromeUtils.import calls in testing/.

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

Comment 14

[Mark Banner (:standard8)]

Comment on attachment 8995925 [details]
Bug 1478305 - Remove unnecessary imports and fix ESLint warnings about unused variables for toolkit/.

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

Comment 15

[Mark Banner (:standard8)]

Comment on attachment 8994782 [details]
Bug 1478305 - Update ESLint to treat ChromeUtils.import definitions as real variable definitions for single-export modules.

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

Comment 16

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8995923 [details]
Bug 1478305 - Remove unnecessary ChromeUtils.import calls in mobile/.

https://reviewboard.mozilla.org/r/260222/#review267288

Comment 17

[Byron Campen [:bwc]]

Comment on attachment 8995922 [details]
Bug 1478305 - Remove unnecessary ChromeUtils.import calls in dom/media.

https://reviewboard.mozilla.org/r/260220/#review267296

Comment 18

[Dave Townsend [:mossop]]

Comment on attachment 8995920 [details]
Bug 1478305 - For xpcshell-test head files, limit checking no-unused-vars to the local scope only.

https://reviewboard.mozilla.org/r/260216/#review267344

Comment 19

[Dave Townsend [:mossop]]

Comment on attachment 8995924 [details]
Bug 1478305 - Remove unnecessary ChromeUtils.import calls in testing/.

https://reviewboard.mozilla.org/r/260224/#review267346

::: testing/marionette/test/unit/test_cookie.js:43
(Diff revision 2)
> -    let hostCookies = this.cookies.filter(cookie => cookie.host === host ||
> -       cookie.host === "." + host);
> +    let hostCookies = this.cookies.filter(c => c.host === host ||
> +       c.host === "." + host);

This change looks unrelated.

Comment 20

[Dave Townsend [:mossop]]

Comment on attachment 8995925 [details]
Bug 1478305 - Remove unnecessary imports and fix ESLint warnings about unused variables for toolkit/.

https://reviewboard.mozilla.org/r/260226/#review267348

Comment 21

[Dave Townsend [:mossop]]

Comment on attachment 8994782 [details]
Bug 1478305 - Update ESLint to treat ChromeUtils.import definitions as real variable definitions for single-export modules.

https://reviewboard.mozilla.org/r/259314/#review267350

Comment 22

[Mark Banner (:standard8)]

Comment on attachment 8995924 [details]
Bug 1478305 - Remove unnecessary ChromeUtils.import calls in testing/.

https://reviewboard.mozilla.org/r/260224/#review267346

> This change looks unrelated.

This is needed, because the import at the top of the file is importing a global named `cookie`, which now conflicts with this variable and triggers the no-shadow rule.

Comment 23

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8995921 [details]
Bug 1478305 - Remove unnecessary imports and fix ESLint warnings about unused variables for browser/.

https://reviewboard.mozilla.org/r/260218/#review267810

Comment 24

[Mark Banner (:standard8)]

Comment on attachment 8995920 [details]
Bug 1478305 - For xpcshell-test head files, limit checking no-unused-vars to the local scope only.

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

Comment 25

[Mark Banner (:standard8)]

Comment on attachment 8995921 [details]
Bug 1478305 - Remove unnecessary imports and fix ESLint warnings about unused variables for browser/.

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

Comment 26

[Mark Banner (:standard8)]

Comment on attachment 8995922 [details]
Bug 1478305 - Remove unnecessary ChromeUtils.import calls in dom/media.

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

Comment 27

[Mark Banner (:standard8)]

Comment on attachment 8995923 [details]
Bug 1478305 - Remove unnecessary ChromeUtils.import calls in mobile/.

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

Comment 28

[Mark Banner (:standard8)]

Comment on attachment 8995924 [details]
Bug 1478305 - Remove unnecessary ChromeUtils.import calls in testing/.

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

Comment 29

[Mark Banner (:standard8)]

Comment on attachment 8995925 [details]
Bug 1478305 - Remove unnecessary imports and fix ESLint warnings about unused variables for toolkit/.

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

Comment 30

[Mark Banner (:standard8)]

Comment on attachment 8994782 [details]
Bug 1478305 - Update ESLint to treat ChromeUtils.import definitions as real variable definitions for single-export modules.

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

Comment 31

[Mark Banner (:standard8)]

Comment on attachment 8995921 [details]
Bug 1478305 - Remove unnecessary imports and fix ESLint warnings about unused variables for browser/.

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

Comment 32

[Mark Banner (:standard8)]

Comment on attachment 8995922 [details]
Bug 1478305 - Remove unnecessary ChromeUtils.import calls in dom/media.

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

Comment 33

[Mark Banner (:standard8)]

Comment on attachment 8995923 [details]
Bug 1478305 - Remove unnecessary ChromeUtils.import calls in mobile/.

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

Comment 34

[Mark Banner (:standard8)]

Comment on attachment 8995924 [details]
Bug 1478305 - Remove unnecessary ChromeUtils.import calls in testing/.

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

Comment 35

[Mark Banner (:standard8)]

Comment on attachment 8995925 [details]
Bug 1478305 - Remove unnecessary imports and fix ESLint warnings about unused variables for toolkit/.

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

Comment 36

[Mark Banner (:standard8)]

Comment on attachment 8994782 [details]
Bug 1478305 - Update ESLint to treat ChromeUtils.import definitions as real variable definitions for single-export modules.

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

Comment 37

[Mark Banner (:standard8)]

The updates were for bitrot (and fixing my forget to change reviewer in commit message), and to add a version bump for eslint-plugin-mozilla so that we can update it for those that use it outside.

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/60a10f910397
For xpcshell-test head files, limit checking no-unused-vars to the local scope only. r=mossop
https://hg.mozilla.org/integration/autoland/rev/ed8954c61a36
Remove unnecessary imports and fix ESLint warnings about unused variables for browser/. r=jaws
https://hg.mozilla.org/integration/autoland/rev/a0a4329018c2
Remove unnecessary ChromeUtils.import calls in dom/media. r=bwc
https://hg.mozilla.org/integration/autoland/rev/90115883e292
Remove unnecessary ChromeUtils.import calls in mobile/. r=snorp
https://hg.mozilla.org/integration/autoland/rev/d3cca460f38d
Remove unnecessary ChromeUtils.import calls in testing/. r=mossop
https://hg.mozilla.org/integration/autoland/rev/d28accbc115d
Remove unnecessary imports and fix ESLint warnings about unused variables for toolkit/. r=mossop
https://hg.mozilla.org/integration/autoland/rev/0fd5b04d2dee
Update ESLint to treat ChromeUtils.import definitions as real variable definitions for single-export modules. r=mossop

Comment 39

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/60a10f910397
https://hg.mozilla.org/mozilla-central/rev/ed8954c61a36
https://hg.mozilla.org/mozilla-central/rev/a0a4329018c2
https://hg.mozilla.org/mozilla-central/rev/90115883e292
https://hg.mozilla.org/mozilla-central/rev/d3cca460f38d
https://hg.mozilla.org/mozilla-central/rev/d28accbc115d
https://hg.mozilla.org/mozilla-central/rev/0fd5b04d2dee

Comment 40

[[github robot]]

Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/1150780da479166f959008147bdb5995ac3de093
Port Bug 1478305 - Remove unnecessary imports and fix ESLint warnings about unused variables for browser/. r=jaws