1370232 - Enable the ESLint no-unneeded-ternary rule across mozilla-central

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement)

Description

[Mark Banner (:standard8)]

no-unneeded-ternary stops code unnecessarily use a conditional expression to select between two Boolean values instead of using ! to convert the test to a boolean.

We should enable it and have simpler to read code (there's about 24 instances that'll need fixing).

I'm happy to mentor this bug. There's background on our eslint setups here:

https://developer.mozilla.org/docs/ESLint

Here's some approximate steps:

- Add a no-unneeded-ternary line in recommended.js:

"no-unneeded-ternary": "error",

- Remove the no-unneeded-ternary related lines in the existing .eslintrc.js configs (apart from devtools/):

https://dxr.mozilla.org/mozilla-central/search?q=no-unneeded-ternary&redirect=false

- Run eslint for fixing:

./mach eslint --fix

This should fix most/all of the instances.

- Fix any remaining instances
- Inspect the diff to make sure that the indentation still looks alright
- Create a commit and push it to mozreview:

http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html

Comment 1

[Rajesh Kathiriya(:rajk)]

you can assign it to me i will work on this.

Comment 2

[Rajesh Kathiriya(:rajk)]

Attachment: Bug 1370232 - Enabled the ESLint no-unneeded-ternary rule across mozilla-central.

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

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 8876829 [details]
Bug 1370232 - Enabled the ESLint no-unneeded-ternary rule across mozilla-central.

https://reviewboard.mozilla.org/r/148160/#review152922

Thank you for taking this on. There's a couple of comments below to address, but after that I think we'll be ready to go.

::: commit-message-c511a:1
(Diff revision 1)
> +Bug 1370232 - Enabled the ESLint no-unneeded-ternary rule across mozilla-central

Just to note, if you end the commit message with something like what I indicate below, it will automatically ask for review from the relevant person:


Bug 1370232 - Enabled the ESLint no-unneeded-ternary rule across mozilla-central. r?Standard8

::: accessible/tests/mochitest/common.js:317
(Diff revision 1)
>   * interfaces.
>   */
>  function isAccessible(aAccOrElmOrID, aInterfaces)
>  {
> -  return getAccessible(aAccOrElmOrID, aInterfaces, null,
> -                       DONOTFAIL_IF_NO_ACC | DONOTFAIL_IF_NO_INTERFACE) ?
> +  return !!getAccessible(aAccOrElmOrID, aInterfaces, null,
> +                       DONOTFAIL_IF_NO_ACC | DONOTFAIL_IF_NO_INTERFACE);

This line needs indenting by 2 more spaces.

::: toolkit/mozapps/update/tests/unit_base_updater/marAppApplyUpdateStageOldVersionFailure.js:69
(Diff revision 1)
>                 "the update manager updateCount attribute" + MSG_SHOULD_EQUAL);
>    Assert.equal(gUpdateManager.getUpdateAt(0).state, STATE_AFTER_STAGE,
>                 "the update state" + MSG_SHOULD_EQUAL);
>    checkPostUpdateRunningFile(false);
>    setTestFilesAndDirsForFailure();
> -  checkFilesAfterUpdateFailure(getApplyDirFile, IS_MACOSX ? false : true, false);
> +  checkFilesAfterUpdateFailure(!getApplyDirFile, IS_MACOSX, false);

This change looks wrong - shouldn't the ! be before the IS_MACOSX?

Comment 4

[Rajesh Kathiriya(:rajk)]

Comment on attachment 8876829 [details]
Bug 1370232 - Enabled the ESLint no-unneeded-ternary rule across mozilla-central.

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

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 8876829 [details]
Bug 1370232 - Enabled the ESLint no-unneeded-ternary rule across mozilla-central.

https://reviewboard.mozilla.org/r/148160/#review152970

Thanks, this looks good now. I've just pushed it to our try servers to double check the test results (link will appear in the review request on reviewboard), if it is green then I'll push it to the main repository.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/42ad46625b89
Enabled the ESLint no-unneeded-ternary rule across mozilla-central. r=standard8

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/42ad46625b89