1342459 - Enable eslint no-undef for browser/

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Mark Banner (:standard8)]

I'm working on a couple of patches to get no-undef enabled for all of browser/. It is already enabled for some of browser, but it is time to get it everywhere.

Comment 1

[Mark Banner (:standard8)]

For those watching, this is still in my queue - the remaining bit is browser/base. Currently I'm waiting on the bugs that block this, and also doing some more general refactoring of eslint-plugin-mozilla to make it easier to use elsewhere (bug 1347645).

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 1342459 - Fix various no-undef issues in browser/base.

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

Comment 3

[Mark Banner (:standard8)]

Attachment: Bug 1342459 - Add a rule to automatically detect ContentTask.spawn and inject the relevant globals.

This reduces the amount of places where we need to specify the mozilla/frame-script environment. It does have
the side effect of allowing those globals in the whole file, but that is what specifying the environment would
do, and this is also for mochitest test files only.

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

Comment 4

[Mark Banner (:standard8)]

Attachment: Bug 1342459 - Enable no-undef on browser/base, and clean up unnecessary no-undef references.

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

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 8855238 [details]
Bug 1342459 - Fix various no-undef issues in browser/base.

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

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 8855239 [details]
Bug 1342459 - Add a rule to automatically detect ContentTask.spawn and inject the relevant globals.

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

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 8855240 [details]
Bug 1342459 - Enable no-undef on browser/base, and clean up unnecessary no-undef references.

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

Comment 8

[Mark Banner (:standard8)]

I was going to wait a bit for these last patches, but it happened the other night in-between waiting for things.

The patches I've just attached fix the last 100 or so issues in browser/base.

I think there's more potential improvements to do for no-undef, and cleaning up some globals definitions, but they can wait whilst I get some other eslint issues sorted out.

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 8855239 [details]
Bug 1342459 - Add a rule to automatically detect ContentTask.spawn and inject the relevant globals.

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

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 8855240 [details]
Bug 1342459 - Enable no-undef on browser/base, and clean up unnecessary no-undef references.

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

Comment 11

[Dave Townsend [:mossop]]

Comment on attachment 8855239 [details]
Bug 1342459 - Add a rule to automatically detect ContentTask.spawn and inject the relevant globals.

https://reviewboard.mozilla.org/r/127100/#review129988

::: tools/lint/eslint/eslint-plugin-mozilla/lib/rules/import-content-task-globals.js:32
(Diff revision 3)
> +  // ---------------------------------------------------------------------------
> +  // Public
> +  // ---------------------------------------------------------------------------
> +
> +  return {
> +    "CallExpression[callee.object.name='ContentTask'][callee.property.name='spawn']": function(node) {

Oh this is nice syntax, is that a new eslint feature?

Comment 12

[Mark Banner (:standard8)]

(In reply to Dave Townsend [:mossop] from comment #11)
> > +    "CallExpression[callee.object.name='ContentTask'][callee.property.name='spawn']": function(node) {
> 
> Oh this is nice syntax, is that a new eslint feature?

Yes, added in 3.18.0. I'm planning on rolling it out to wherever we can soon.

http://eslint.org/blog/2017/03/eslint-v3.18.0-released

Comment 13

[Dave Townsend [:mossop]]

Comment on attachment 8855240 [details]
Bug 1342459 - Enable no-undef on browser/base, and clean up unnecessary no-undef references.

https://reviewboard.mozilla.org/r/127102/#review130066

Comment 14

[Dave Townsend [:mossop]]

Comment on attachment 8855238 [details]
Bug 1342459 - Fix various no-undef issues in browser/base.

https://reviewboard.mozilla.org/r/127098/#review130048

I wonder if it would be easier to just define content as a global, but then we open the door for all the other window properties I guess.

Comment 15

[Mark Banner (:standard8)]

Comment on attachment 8855238 [details]
Bug 1342459 - Fix various no-undef issues in browser/base.

https://reviewboard.mozilla.org/r/127098/#review130048

Yeah, I did wonder about doing it differently, but the entire list of properties would be quite big (and also not appropriate in some files like .jsm).

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/28d99fb23327
Fix various no-undef issues in browser/base. r=mossop
https://hg.mozilla.org/integration/autoland/rev/88f6f749466f
Add a rule to automatically detect ContentTask.spawn and inject the relevant globals. r=mossop
https://hg.mozilla.org/integration/autoland/rev/ca3083864db1
Enable no-undef on browser/base, and clean up unnecessary no-undef references. r=mossop

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/28d99fb23327
https://hg.mozilla.org/mozilla-central/rev/88f6f749466f
https://hg.mozilla.org/mozilla-central/rev/ca3083864db1

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/28d99fb23327
https://hg.mozilla.org/mozilla-central/rev/88f6f749466f
https://hg.mozilla.org/mozilla-central/rev/ca3083864db1