Closed Bug 1329614 Opened 7 years ago Closed 7 years ago

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

Categories

(Developer Infrastructure :: Lint and Formatting, defect)

3 Branch
defect
Not set
normal

Tracking

(firefox53 fixed)

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(1 file)

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.
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 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'.
Attachment #8825431 - Flags: review?(jaws) → review+
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.
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bd9a337129ad
[eslint] Catch more cases of importing globals from 'var foo = Cu.import('...');'. r=jaws
https://hg.mozilla.org/mozilla-central/rev/bd9a337129ad
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Product: Testing → Firefox Build System
Version: Version 3 → 3 Branch
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: