Last Comment Bug 1328800 - Enable the no-sparse-arrays rule for eslint
: Enable the no-sparse-arrays rule for eslint
Status: RESOLVED FIXED
:
Product: Toolkit
Classification: Components
Component: General (show other bugs)
: unspecified
: Unspecified Unspecified
-- normal (vote)
: mozilla53
Assigned To: Jared Wein [:jaws] (please needinfo? me)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-01-04 21:25 PST by Jared Wein [:jaws] (please needinfo? me)
Modified: 2017-01-05 17:29 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed

MozReview Requests
Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:
Show discarded requests

Attachments
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. (59 bytes, text/x-review-board-request)
2017-01-04 21:34 PST, Jared Wein [:jaws] (please needinfo? me)
mak77: review+
Details | Review

Description User image Jared Wein [:jaws] (please needinfo? me) 2017-01-04 21:25:18 PST
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 User image Jared Wein [:jaws] (please needinfo? me) 2017-01-04 21:34:06 PST Comment hidden (mozreview-request)
Comment 2 User image Marco Bonardo [::mak] 2017-01-05 07:50:53 PST
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 User image Jared Wein [:jaws] (please needinfo? me) 2017-01-05 09:06:37 PST Comment hidden (mozreview-request)
Comment 4 User image Jared Wein [:jaws] (please needinfo? me) 2017-01-05 09:09:57 PST Comment hidden (mozreview-request)
Comment 5 User image Pulsebot 2017-01-05 09:11:05 PST
Pushed by jwein@mozilla.com:
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 User image Wes Kocher (:KWierso) 2017-01-05 17:29:51 PST
https://hg.mozilla.org/mozilla-central/rev/579916ba9456

Note You need to log in before you can comment on or make changes to this bug.