1329614 - [eslint] Catch more cases of importing globals from "var foo = Cu.import("...");"

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, defect)

Description

[Mark Banner (:standard8)]

In working on more no-undef failures, I realised that although bug 1328565 covered the 

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

case, it didn't cover:

`var foo = Cu.import("...");`

i.e. without the scope supplied.

There's about 96 of these cases, mainly in tests, and there's definitely one or two where we're relying on the side effects, so I think its worth catching these as well.

A lot of these are also importing into variables within functions, but then end up affecting the global scope.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1329614 - [eslint] Catch more cases of importing globals from 'var foo = Cu.import('...');'.

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

Comment 2

[Mark Banner (:standard8)]

This has been run through try already:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a30fce088198287bc5f2b374084ec63e6cb46202

(ignore the eslint failures there, I'd left the no-undef rule uncommented).

Comment 3

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

Comment on attachment 8825431 [details]
Bug 1329614 - [eslint] Catch more cases of importing globals from 'var foo = Cu.import('...');'.

https://reviewboard.mozilla.org/r/103596/#review104246

::: toolkit/components/telemetry/tests/unit/head.js:236
(Diff revision 1)
> -  let module = Cu.import("resource://gre/modules/TelemetrySend.jsm");
> +  let module = Cu.import("resource://gre/modules/TelemetrySend.jsm", {});
>    let obj = Cu.cloneInto({set, clear}, module, {cloneFunctions:true});
>    module.Policy.setSchedulerTickTimeout = obj.set;
>    module.Policy.clearSchedulerTickTimeout = obj.clear;
>  }
>  
>  function fakeMidnightPingFuzzingDelay(delayMs) {
> -  let module = Cu.import("resource://gre/modules/TelemetrySend.jsm");
> +  let module = Cu.import("resource://gre/modules/TelemetrySend.jsm", {});
>    module.Policy.midnightPingFuzzingDelay = () => delayMs;

Since TelemetrySend is now defined explicitly in the global scope, we should change these to use TelemetrySend instead of 'module'.

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 8825431 [details]
Bug 1329614 - [eslint] Catch more cases of importing globals from 'var foo = Cu.import('...');'.

https://reviewboard.mozilla.org/r/103596/#review104246

> Since TelemetrySend is now defined explicitly in the global scope, we should change these to use TelemetrySend instead of 'module'.

TelemetrySend.jsm doesn't mark `Policy` as exported, so this is using an alternative to get access to it for the purposes of that bit of the test. The rest of the test needs `TelemetrySend` and that was being side-imported via these imports.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd9a337129ad
[eslint] Catch more cases of importing globals from 'var foo = Cu.import('...');'. r=jaws

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bd9a337129ad