Enable the no-cond-assign rule for eslint

RESOLVED FIXED in Firefox 53

Status

()

Toolkit
General
RESOLVED FIXED
4 months ago
4 months ago

People

(Reporter: jaws, Assigned: jaws)

Tracking

unspecified
mozilla53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

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 hidden (mozreview-request)
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

4 months ago
mozreview-review
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
Attachment #8823954 - Flags: review?(dtownsend) → review+

Comment 4

4 months ago
Pushed by jwein@mozilla.com:
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

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/51adf5284d7c
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.