1217072 - Remove use of for-each from addon-sdk/.

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: 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: Remove for-each from addon-sdk/.

try runs here
linux: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f521b1264cb4
mac (after linux with some fix): https://treeherder.mozilla.org/#/jobs?repo=try&revision=d5ecb19c84f6
win (after mac with some fix): https://treeherder.mozilla.org/#/jobs?repo=try&revision=adf800f88efd
finally xpcshell passed on linux (after win with some fix): https://treeherder.mozilla.org/#/jobs?repo=try&revision=526c234c897d

Comment 2

[Tooru Fujisawa [:arai]]

I didn't replace this line, as it looks like it's testing behavior for for-each.

https://dxr.mozilla.org/mozilla-central/rev/605de27d4e7f530159842649c02075c578d7a4a5/addon-sdk/source/test/test-api-utils.js#307
>  for each (let val in obj)
>    valsItr.push(val);

I think this should be removed at the same time as removing for-each support from SpiderMonkey.

Comment 3

[Dave Townsend [:mossop]]

(In reply to Tooru Fujisawa [:arai] from comment #0)
>   * 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

Why aren't we switching to the new form of array comprehensions for these?

Comment 4

[Tooru Fujisawa [:arai]]

because it's removed from the spec (iiuc, even from ES7).

Comment 5

[Tooru Fujisawa [:arai]]

"removed from ES7" seems to be somewhat wrong, but it looks like there will be some change in syntax and semataics, and we shouldn't use it for now.

https://esdiscuss.org/topic/array-comprehensions-with-spread-operator
https://github.com/rwaldron/tc39-notes/blob/c61f48cea5f2339a1ec65ca89827c8cff170779b/es6/2014-06/jun-5.md#generator-comprehensions-slides-plz
https://speakerdeck.com/dherman/a-better-future-for-comprehensions
https://github.com/tc39/ecma262
https://tc39.github.io/ecma262/

I'll post when I get more information.

Comment 6

[Tooru Fujisawa [:arai]]

array comprehension and generator comprehension are not likely to be added to the spec, and we'll eventually remove it from SpiderMonkey if we can.
I've just updated documentation on MDN from experimental to non-standard.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Array_comprehensions
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Generator_comprehensions

let's go with map/filter :)

Comment 7

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a2965479d63d09610308518d03aa63a2957c29f4
Bug 1217072 - Remove for-each from addon-sdk/. r=mossop

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/a2965479d63d