Remove deprecated uses of array generics from our code

RESOLVED FIXED in Firefox 68

Status

()

task
P2
normal
RESOLVED FIXED
2 months ago
2 months ago

People

(Reporter: robwu, Assigned: robwu)

Tracking

(Blocks 1 bug)

56 Branch
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

2 months ago

Bug 1536860 started to mark several Array generics as deprecated. They are used in our code and cause logspam, causing people to look into it and waste time.

Let's fix it at once and close the bug reports that have been opened by the deprecation warnings (BMO query, also including some unrelated bugs)

Bug 1540444 proposes to add an eslint rule to fix this. The safest transform is to use Array.prototype.XXX.call(...), but that is a bit ugly. So I am going to do it manually, and on a case-by-case basis turn it into idiomatic code (arr.XXX(...)), or if the given parameter is not an array, turn it into something else.

There are currently 44 occurrences in non-test code (26 files), and also some occurrences in test-only code (including unit tests for the generics that should not be changed):
https://searchfox.org/mozilla-central/search?q=%5CbArray%5C.(indexOf%7ClastIndexOf%7Cevery%7Csome%7CforEach%7Cmap%7Cfilter%7Creduce%7CreduceRight%7Cconcat%7Cjoin%7Creverse%7Csort%7Cpush%7Cpop%7Cshift%7Cunshift%7Csplice%7Cslice)%5C(&case=true&regexp=true&path=

Assignee

Comment 1

2 months ago
  • Array.forEach becomes for-of loop or array.forEach.
  • Array.slice(a) or Array.slice(a, 0) becomes Array.from(a).
  • Array.map becomes Array.from
  • Array copy + concatenation becomes Array literal + spread syntax.
  • All other Array.X(a, ...) become Array.prototype.X.call or Array.from(a).X(...)
Assignee

Comment 2

2 months ago
  • Array.map becomes Array.from
  • Array copying via Array.slice and Array.concat becomes Array.from.
  • Array.forEach that did not rely on closures becomes for-of loops.
  • Anything else: Array.X becomes Array.prototype.X.

Complex cases:

dom/bindings/test/TestInterfaceJS.js and
dom/bindings/test/test_exception_options_from_jsimplemented.html
use Array.indexOf to generate an error with a specific error message.
Switched to Array.prototype.forEach to generate the same error.

js/src/jit-test/tests/basic/exception-column-number.js
In this test Array.indexOf() is used to generate an error. Since the
exact message doesn't matter, I switched to Array.from().

Intentionally not changed:

editor/libeditor/tests/browserscope/lib/richtext/richtext/js/range.js
Did not modify because this is 3rd-party code and the code uses
feature detection as a fall back when Array generics are not used.

testing/talos/talos/tests/dromaeo/lib/mootools.js
Did not modify because mootools adds the Array.slice method to the
Array object.

Not changed because they check the implementation of Array generics:
js/src/jit-test/tests/basic/arrayNatives.js
js/src/jit-test/tests/basic/bug563243.js
js/src/jit-test/tests/basic/bug618853.js
js/src/jit-test/tests/basic/bug830967.js
js/src/jit-test/tests/jaeger/recompile/bug656753.js
js/src/jit-test/tests/self-hosting/alternate-static-and-instance-array-extras.js
js/src/tests/non262/Array/generics.js
js/src/tests/non262/Array/regress-415540.js
js/src/tests/non262/extensions/regress-355497.js
js/src/tests/non262/extensions/typedarray-set-neutering.js

Depends on D27802

Assignee

Comment 3

2 months ago

I used the following approach to make those changes:

  1. I listed all files that were returned by the search query from comment 0, and then edited them with vim:
vim [list of files here] +/'\<Array\.\(indexOf\|lastIndexOf\|every\|some\|forEach\|map\|filter\|reduce\|reduceRight\|concat\|join\|reverse\|sort\|push\|pop\|shift\|unshift\|splice\|slice\)('
  1. Replace the occurrence with something sensible, depending on the context. Optionally indent the code to match the surrounding code style if needed.
  2. Press n to find the next result. If there is one, repeat from step 2.
  3. Use :n to switch to the next file. If there is one, repeat from step 2.

I ran the above steps separately for non-test code and test-only code.
Some test-only code was not trivial because they test implementation details of Array generics. I did not modify them and listed them in the commit message (see comment 2) for future reference.

Assignee: nobody → rob
Status: NEW → ASSIGNED
Assignee

Updated

2 months ago
Duplicate of this bug: 1542294
Assignee

Updated

2 months ago
Duplicate of this bug: 1542296
Assignee

Updated

2 months ago
Duplicate of this bug: 1542300
Assignee

Updated

2 months ago
Duplicate of this bug: 1542301
Priority: -- → P2

Comment 9

2 months ago
Pushed by rob@robwu.nl:
https://hg.mozilla.org/integration/autoland/rev/0d59dd598e5e
Replace non-test uses of deprecated Array generics r=evilpie,dao
https://hg.mozilla.org/integration/autoland/rev/edb8832eafa1
Replace deprecated generics in test code r=evilpie
Type: defect → task

Comment 10

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.