1328805 - Enable the no-cond-assign rule for eslint

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

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

There are six errors found with this rule enabled. This rule disallows assignment in conditionals. One of the errors found is a bug (advanced.js).

c:\fx\browser\components\preferences\in-content\advanced.js
  231:9   error  Expected a conditional expression and instead saw an assignment.  no-cond-assign (eslint)

c:\fx\toolkit\components\aboutperformance\content\aboutPerformance.js
1019:30  error  Expected a conditional expression and instead saw an assignment.                                             no-cond-assign (eslint)

c:\fx\toolkit\components\contentprefs\tests\unit_cps2\head.js
  344:14  error  Expected a conditional expression and instead saw an assignment.  no-cond-assign (eslint)

c:\fx\toolkit\components\places\ClusterLib.js
   88:12  error  Expected a conditional expression and instead saw an assignment.  no-cond-assign (eslint)

c:\fx\toolkit\content\aboutSupport.js
461:17  error  Expected a conditional expression and instead saw an assignment.                                             no-cond-assign (eslint)

c:\fx\toolkit\mozapps\extensions\test\xpcshell\test_plugins.js
  75:10  error  Expected a conditional expression and instead saw an assignment.  no-cond-assign (eslint)

Comment 1

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

Attachment: Bug 1328805 - Enable the no-cond-assign rule for eslint and fix the six resulting errors mostly by wrapping the assignment with parentheses to explicitly state that the assignment is intentional with exception for advanced.js where assignment was not inte

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

Comment 2

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

I'm not a fan of surrounding the assignments with parentheses in these cases, but it is a somewhat established convention to explicitly state that these are intentional and I didn't want to spend a bunch of time refactoring this away from these assignments-in-conditionals.

Comment 3

[Dave Townsend [:mossop]]

Comment on attachment 8823954 [details]
Bug 1328805 - Enable the no-cond-assign rule for eslint and fix the six resulting errors mostly by wrapping the assignment with parentheses to explicitly state that the assignment is intentional with exception for advanced.js where assignment was not inte

https://reviewboard.mozilla.org/r/102438/#review102982

::: browser/components/preferences/in-content/advanced.js:231
(Diff revision 1)
>    /**
>     * When the user toggles the layers.acceleration.disabled pref,
>     * sync its new value to the gfx.direct2d.disabled pref too.
>     */
>    updateHardwareAcceleration() {
> -    if (AppConstants.platform = "win") {
> +    if (AppConstants.platform == "win") {

My heart warms everytime eslint catches an error

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/51adf5284d7c
Enable the no-cond-assign rule for eslint and fix the six resulting errors mostly by wrapping the assignment with parentheses to explicitly state that the assignment is intentional with exception for advanced.js where assignment was not intended. r=mossop

Comment 5

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

https://hg.mozilla.org/mozilla-central/rev/51adf5284d7c