Closed Bug 1217082 Opened 9 years ago Closed 9 years ago

Remove use of for-each from toolkit/.

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 44
Tracking Status
firefox44 --- fixed

People

(Reporter: arai, Assigned: arai)

References

Details

Attachments

(2 files)

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.
Bug 1217082 - Remove for-each from toolkit/. r?Gijs
Attachment #8676981 - Flags: review?(gijskruitbosch+bugs)
Attached file Types for some complicated cases. (deleted) —
Attachment #8676981 - Flags: review?(gijskruitbosch+bugs) → review+
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. :-)
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 ;)
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: