1328800 - Enable the no-sparse-arrays rule for eslint

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

There are only two errors reported with this rule enabled and they are explicitly using sparse arrays for error testing. Enabling this rule may prevent unintended bugs from being created.

c:\fx\toolkit\components\places\tests\unit\test_tagging.js
  122:26  error  Unexpected comma in middle of array.        no-sparse-arrays (eslint)
  128:28  error  Unexpected comma in middle of array.        no-sparse-arrays (eslint)

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1328800 - Enable the no-sparse-arrays rule for eslint and change the two instances of it to use 'undefined' instead of a sparse array since the test will still be fine either way.

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

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 8823950 [details]
Bug 1328800 - Enable the no-sparse-arrays rule for eslint and change the two instances of it to use 'undefined' instead of a sparse array since the test will still be fine either way.

https://reviewboard.mozilla.org/r/102426/#review102956

::: toolkit/components/places/tests/unit/test_tagging.js:119
(Diff revision 1)
>    do_check_true(uri4Tags.includes(tagTitle));
>    do_check_true(uri4Tags.includes("tag 3"));
>    do_check_true(uri4Tags.includes("456"));
>  
>    // Test sparse arrays.
> +  /* eslint-disable no-sparse-arrays */

Based on the code, you can just put undefined instead of the empty entries, and the test will still be fine, without the need to special-case it.
It's basically the same:
[,][0] === undefined

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8823950 [details]
Bug 1328800 - Enable the no-sparse-arrays rule for eslint and change the two instances of it to use 'undefined' instead of a sparse array since the test will still be fine either way.

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

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8823950 [details]
Bug 1328800 - Enable the no-sparse-arrays rule for eslint and change the two instances of it to use 'undefined' instead of a sparse array since the test will still be fine either way.

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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/579916ba9456
Enable the no-sparse-arrays rule for eslint and change the two instances of it to use 'undefined' instead of a sparse array since the test will still be fine either way. r=mak

Comment 6

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

https://hg.mozilla.org/mozilla-central/rev/579916ba9456