1328565 - [eslint] Figure out how/if to handle importing globals from "var foo = Cu.import("...", this);"

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, defect)

Description

[Mark Banner (:standard8)]

Currently, when the import-globals plugin runs, it doesn't detect globals imported via:

```
var foo = Cu.import("bar.jsm", this);
```

The expected result is that `foo` and `bar` should both be globals here. However, currently only `foo` is a global.

The plugin doesn't detect this case, as it expects the `Cu.import` to be at the beginning of the line.

I'd also argue that this is a non-obvious case: we're importing into a specific variable *and* the global scope. To me, that feel unexpected, and I think we should consider banning this case.

Comment 1

[Mark Banner (:standard8)]

As far as I can tell, there's only four instances of this in the tree:

https://dxr.mozilla.org/mozilla-central/search?q=regexp%3A%5Evar.*%3D.*Components.utils.import%5C(%5B%27%22%5D.*%5C.js.*%2C+this)%3B&redirect=false

-> toolkit/modules/tests/browser/browser_Battery.js

https://dxr.mozilla.org/mozilla-central/search?q=regexp%3A%5Evar.*%3D.*Cu.import%5C(%5B%27%22%5D.*%5C.js.*%2C+this)%3B&redirect=false

-> toolkit/components/crashes/tests/xpcshell/test_crash_manager.js
-> toolkit/components/crashes/tests/xpcshell/test_crash_service.js
-> toolkit/components/crashes/tests/xpcshell/test_crash_store.js

Comment 2

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

Due to the low occurrences, awkwardness of the behavior, as well as where this practice is used (tests only) I'm fine with removing these and banning them.

Comment 3

[Mark Banner (:standard8)]

Attachment: Bug 1328565 - Prevent cases of Cu.import importing into variables and global scope at the same time.

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

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 8823781 [details]
Bug 1328565 - Prevent cases of Cu.import importing into variables and global scope at the same time.

https://reviewboard.mozilla.org/r/102300/#review102686

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b6d798426ca0
Prevent cases of Cu.import importing into variables and global scope at the same time. r=mossop

Comment 6

[Mark Banner (:standard8)]

Attachment: Bump the eslint-plugin-mozilla version

I just realised that I forgot to bump the eslint-plugin-mozilla version, hence developers won't automatically pick up the new files.

Going to get this landed as a bustage fix for developers.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/01d4695721e1
bump the eslint-plugin-mozilla version to ensure developers get the changes installed properly. r=bustage-fix

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b6d798426ca0
https://hg.mozilla.org/mozilla-central/rev/01d4695721e1