1217082 - Remove use of for-each from toolkit/.

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Tooru Fujisawa [:arai]]

Need to replace non-standard for-each with one of:
  * for-of
  * for-in
  * array.map/array.filter for array-comprehension
as a part of bug 1083470.

converting rules are following:
  * for-each
    * for each (let x in array) { ... }
      -> for (let x of array) { ... }
    * for each (let x in object) { ... }
      -> for (let key in object) { let x = object[key]; ... }
    * for each (let [key, value] in Iterator(object)) { ... }
      -> for (let key in object) { let value = object[key]; ... }
    * for each (let x in array) { ... }
      where array can be null or undefined
      -> if (array) { for (let x of array) { ... } }

  * legacy array comprehension with for-each
    * [EXPR for each (x in array)]
      -> array.map(x => EXPR)
    * [EXPR for each (x in array) if (COND)]
      -> array.filter(x => COND).map(x => EXPR)
    * [x for each (x in array) if (COND)]
      -> array.filter(x => COND)
    * [EXPR for each ([i, x] in Iterator(array)) if (g(x, i)]
      -> array.filter((x, i) => g(x, i)).map((x => EXPR)
    * [EXPR for each (x in arraylike)]
      -> Array.from(arraylike).map(x => EXPR)
    * [EXPR for each (x in string)]
      -> Array.prototype.slice.call(string).map(x => EXPR)
         // Array.from behaves differently for surrogate-pair

I'll post a patch shortly.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: MozReview Request: Bug 1217082 - Remove for-each from toolkit/. r?Gijs

Bug 1217082 - Remove for-each from toolkit/. r?Gijs

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Types for some complicated cases.

Comment 3

[:Gijs (he/him)]

Comment on attachment 8676981 [details]
MozReview Request: Bug 1217082 - Remove for-each from toolkit/. r?Gijs

https://reviewboard.mozilla.org/r/22847/#review20385

::: toolkit/components/exthelper/extApplication.js:573
(Diff revision 1)
> +    for (let id in this._cache) {

```
return Object.keys(this._cache).map(id => this._cache[id]));
```

::: toolkit/components/search/nsSearchService.js:4613
(Diff revision 1)
> +      let engine = this._engines[name];
>        engine = engine.wrappedJSObject;

Maybe just unify these into:

let engine = this.\_engines\[name\].wrappedJSObject;

::: toolkit/modules/PropertyListUtils.jsm:263
(Diff revision 1)
> -    return [String.fromCharCode(c) for each (c in new Uint8Array(aBuffer, 0, 8))].
> +    return Array.from(new Uint8Array(aBuffer, 0, 8)).map(c => String.fromCharCode(c)).

MDN says a Uint8Array has a map function, so the Array.from call seems redundant?

::: toolkit/modules/tests/xpcshell/test_propertyListsUtils.js:70
(Diff revision 1)
> -  let dataAsString = [String.fromCharCode(b) for each (b in array[6])].join("");
> +  let dataAsString = Array.from(array[6]).map(b => String.fromCharCode(b)).join("");

Likewise, I don't think Array.from() is necessary here.

::: toolkit/mozapps/extensions/test/xpcshell/head_addons.js:1381
(Diff revision 1)
> -  else for each (let installList in gExpectedInstalls) {
> +  else for (let id in gExpectedInstalls) {

Seeing as we're modifying this anyway, please remove the else.

Not only is "else for" peculiar syntax, else after return is unnecessary, so it can just go away. :-)

Comment 4

[Tooru Fujisawa [:arai]]

Thank you!

(In reply to :Gijs Kruitbosch from comment #3)
> ::: toolkit/modules/PropertyListUtils.jsm:263
> (Diff revision 1)
> > -    return [String.fromCharCode(c) for each (c in new Uint8Array(aBuffer, 0, 8))].
> > +    return Array.from(new Uint8Array(aBuffer, 0, 8)).map(c => String.fromCharCode(c)).
> 
> MDN says a Uint8Array has a map function, so the Array.from call seems
> redundant?
> 
> ::: toolkit/modules/tests/xpcshell/test_propertyListsUtils.js:70
> (Diff revision 1)
> > -  let dataAsString = [String.fromCharCode(b) for each (b in array[6])].join("");
> > +  let dataAsString = Array.from(array[6]).map(b => String.fromCharCode(b)).join("");
> 
> Likewise, I don't think Array.from() is necessary here.

Uint8Array.map returns Uint8Array, so we cannot use it, as we're going to return Array of String ;)

Comment 5

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4e188de86d867916ccc661fa1a9ba149974aba35
Bug 1217082 - Remove for-each from toolkit/. r=Gijs

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4e188de86d86