1314918 - Fix most of the remaining no-unused-vars issues for local scope in browser/

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Mark Banner (:standard8)]

This is the next step for bug 1312407. This fixes most of the no-unused-vars for local scope in browser/.

There's only a handful of issues left, but they either need other patches to land, or some investigation for fixing them.

So in the meantime, lets get this patch landed to clean up (there's a few fixes in here as well).

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1314918 - Fix most of the remaining no-unused-vars issues for local scope in browser/.

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

Comment 2

[Dave Townsend [:mossop] (PTO until Aug 21st)]

Comment on attachment 8807116 [details]
Bug 1314918 - Fix most of the remaining no-unused-vars issues for local scope in browser/.

https://reviewboard.mozilla.org/r/90394/#review90126

::: browser/components/contextualidentity/test/browser/browser_forgetaboutsite.js:184
(Diff revision 1)
>  
>  
>    // Check that caches have been set correctly.
>    for (let userContextId of Object.keys(USER_CONTEXTS)) {
> -    let mem = getCacheStorage("memory");
> -    let disk = getCacheStorage("disk");
> +    let mem = getCacheStorage("memory", LoadContextInfo.custom(false, {userContextId}));
> +    let disk = getCacheStorage("disk", LoadContextInfo.custom(false, {userContextId}));

Why is this necessary?

::: browser/components/feeds/test/bug494328-data.xml:9
(Diff revision 1)
>      <title>Channel title</title>
>      <description>Channel description</description>
>      <link>Channel link</link>
>      <item>
>        <title>Episode 1</title>
> -      <enclosure url="http://www.example.com/podcasts/Episode%201" length="0" type="audio/x-m4a" />
> +      <enclosure url="http://www.example.com/podcasts/Episode%201" length="12340" type="audio/x-m4a" />

Did eslint catch this?

::: browser/components/places/content/editBookmarkOverlay.js:199
(Diff revision 1)
>      // trying to init it again, or we could end up leaking due to observers.
>      if (this.initialized)
>        this.uninitPanel(false);
>  
> -    let { itemId, itemGuid, isItem,
> -          isURI, uri, title,
> +    let { itemId, itemGuid, isItem, // eslint-disable-line no-unused-vars
> +          isURI, uri, title, // eslint-disable-line no-unused-vars

Why disable rather than omit?

::: browser/components/places/content/places.js:1372
(Diff revision 1)
>     */
>    get currentViewOptions() {
>      // Use ContentTree options as default.
>      let viewOptions = ContentTree.viewOptions;
>      if (this._specialViews.has(this.currentPlace)) {
> -      let { view, options } = this._specialViews.get(this.currentPlace);
> +      let { view, options } = this._specialViews.get(this.currentPlace); // eslint-disable-line no-unused-vars

Same question here

::: browser/components/translation/test/browser_translation_telemetry.js:222
(Diff revision 1)
>    gBrowser.removeTab(tab);
>  });
>  
>  add_task(function* test_language_change() {
> -  for (let i of Array(4)) {
> +  // This is run 4 times, the total additions are checked afterwards.
> +  for (let i of Array(4)) { // eslint-disable-line no-unused-vars

Huh. Clever I guess

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 8807116 [details]
Bug 1314918 - Fix most of the remaining no-unused-vars issues for local scope in browser/.

https://reviewboard.mozilla.org/r/90394/#review90126

> Why is this necessary?

The test looked like it was designed to iterate over `USER_CONTEXTS`, however `userContextId` was never used. The previous for loop did use the userContextId to open/add the cache entries. This loop seemed to be testing the additions, but had missed the actually checking the individual `userContextID`.

> Did eslint catch this?

Nope, I was testing something and forgot to undo it.

> Why disable rather than omit?

I was forgetting how object assignments work in an es6 world.

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 8807116 [details]
Bug 1314918 - Fix most of the remaining no-unused-vars issues for local scope in browser/.

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

Comment 5

[Dave Townsend [:mossop] (PTO until Aug 21st)]

Comment on attachment 8807116 [details]
Bug 1314918 - Fix most of the remaining no-unused-vars issues for local scope in browser/.

https://reviewboard.mozilla.org/r/90394/#review90252

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9601d282b06b
Fix most of the remaining no-unused-vars issues for local scope in browser/. r=mossop

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/581e573c82e6
Revert browser eslint rule changes to unbust eslint task; r=Standard8

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/9601d282b06b
https://hg.mozilla.org/mozilla-central/rev/581e573c82e6