1318089 - Turn on no-unused-vars rule for AOM code

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Kris Maglione [:kmag]]

This rule tends to catch a lot of errors. We should turn it on for all add-on manager code.

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1318089: Turn on no-unused-vars ESLint rule for toolkit/mozapps/extensions.

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

Comment 2

[Robert Helmer [:rhelmer]]

Comment on attachment 8811461 [details]
Bug 1318089: Turn on no-unused-vars ESLint rule for toolkit/mozapps/extensions.

https://reviewboard.mozilla.org/r/93540/#review93902

::: toolkit/mozapps/extensions/test/xpcshell/test_blocklistchange.js:1008
(Diff revision 1)
>    writeInstallRDFForExtension(regexpblock_3, profileDir);
>    setExtensionModifiedTime(getFileForAddon(profileDir, regexpblock_3.id), Date.now() + 10000);
>  
>    startupManager(false);
>  
> -  let [s1, s2, s3, s4, s5, h, r] = yield promiseAddonsByIDs(ADDON_IDS);
> +  let [s1, s2, s3, /* s4 */, /* s5 */, h, r] = yield promiseAddonsByIDs(ADDON_IDS);

I think you can leave the comments out (which is pretty idiomatic in ES6 when dealing with array holes) e.g.:

`let [s1, s2, s3, , , h, r]`

But if you like the commented version better that's fine w/ me.

Comment 3

[Kris Maglione [:kmag]]

Comment on attachment 8811461 [details]
Bug 1318089: Turn on no-unused-vars ESLint rule for toolkit/mozapps/extensions.

https://reviewboard.mozilla.org/r/93540/#review93902

> I think you can leave the comments out (which is pretty idiomatic in ES6 when dealing with array holes) e.g.:
> 
> `let [s1, s2, s3, , , h, r]`
> 
> But if you like the commented version better that's fine w/ me.

I thought about it, but this code is cryptic enough without not knowing what the commented out variables are supposed to be...

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ffe6c6137d8e
Turn on no-unused-vars ESLint rule for toolkit/mozapps/extensions. r=rhelmer

Comment 5

[Kris Maglione [:kmag]]

Apparently I pushed before the last XPCShell test group finished, and of course it turned up a failure.

Apparently this:

  let realAddonRepo = AMscope.AddonRepository;

was meant to trigger a lazy getter to prevent the next line from throwing, but did not have a comment to indicate it. I guess it needs to be changed to:

  // Trigger the lazy getter so we can assign a new value:
  void AMscope.AddonRepository;

Comment 6

[Kris Maglione [:kmag]]

Attachment: Bug 1318089: Follow-up: Re-add a lazy getter trigger that looked like a no-op.

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

Comment 7

[Robert Helmer [:rhelmer]]

Comment on attachment 8811927 [details]
Bug 1318089: Follow-up: Re-add a lazy getter trigger that looked like a no-op.

https://reviewboard.mozilla.org/r/93820/#review93966

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/808a44ee14bd
Follow-up: Re-add a lazy getter trigger that looked like a no-op. r=rhelmer

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ffe6c6137d8e
https://hg.mozilla.org/mozilla-central/rev/808a44ee14bd

Comment 10

[areya]

https://hg.mozilla.org/mozilla-central/rev/ffe6c6137d8e
https://hg.mozilla.org/mozilla-central/rev/808a44ee14bd