Mixed operators with same precedence

[aboyton]

The ESLint rule no-mixed-operators is great for making things like

var foo = a && b < 0 || c > 0 || d + 1 === 0;
var bar = a && b || c;

clearer by adding parentheses (rather than having people remember the precedence of && and ||. That said, I'm unconvinced that

var foo = a + b - c

is clearer with parentheses, that is according to this style guide you'd need to write this as

var foo = (a + b) - c

The no-mixed-operators rule has an option for this, allowSamePrecedence and the default is true meaning that var foo = a + b - c is valid code (which I think makes a lot more sense). Is there any reason for having this set to false?

[ljharb]

Yes - || and && also have the same precedence, and the parens are very necessary there to avoid ambiguity.

The change I've been considering here is removing some of the arithmetic operators entirely - ie, + - * / but leaving the settings otherwise unchanged. Clearly with + and - though, there's some tweaking that would need to be done.

[aboyton]

Thanks for the quick reply.

Actually || and && do have different precedence (see MDN operator precedence or Microsoft operator precedence). As an example true || true && false is true as && has a higher precedence than ||.

Because of this (with "allowSamePrecedence": true) the following are lint errors:

var foo = a || b && c;
var foo = a + b * c;

but these are fine

var foo = a || (b && c);
var foo = a + (b * c);
var foo = a + b - c;

I personally don't mind which way you go for a + b * c or a + (b * c) (I never get confused here but I get how some people do) but it would be nice to make a + b - c valid code.

[blord-fullscreen]

Any reply on this? Many people work in an environment where this package is treated as holy writ, and changing the eslintrc is not politically possible. Somewhere out there an intern is rewriting addition and subtraction operations to add parens, because a + b - c is invalid.

[ljharb]

@blord-fullscreen then the holy writ you can follow includes https://github.com/airbnb/javascript#amendments which instructs you to override at will :-) In the meantime, having some potentially unnecessary parens isn't that big a deal; your code will just be slightly clearer.

If someone wants to make a PR that includes many test cases to accomplish #1071 (comment) (ie, removing the requirement for parens mixed plus/minus, and mixed multiplication/division, but still preserving the current requirements for &&/||) that'd be appreciated.

[jabacchetta]

@ljharb Could you please clarify what the current issue is with "allowSamePrecedence": true?

As @aboyton pointed out above, || and && do not have the same precedence.

Unless I'm misunderstanding, removing the allowSamePrecedence: false override should not present any issues with "preserving the current requirements for &&/||".

[ljharb]

@jabacchetta the current issue is that nobody's yet done the research (myself or otherwise) to convince me that all the cases I care about will continue to require parens, when allowSamePrecedence is set to true. It's very possible that this is the case, I just don't know that for sure yet :-)

[raduserbanescu]

I think the problem is mainly with arithmetic operators as it makes basic math calculations noisy. I don't want to write this:

const a = 12 - 24 / 3 + 2 ** 3 * 5 - 2;

like this:

const a = ((12 - (24 / 3)) + ((2 ** 3) * 5)) - 2;

Also, setting allowSamePrecedence to true would make code like below valid, which can be confusing:

if (a === b !== c) {
  // do something here
}

if (a < b <= c) {
  // do something here
}

---

What me and my colleagues use on our projects is this modification (removing the Arithmetic Operators group):

'no-mixed-operators': ['error', {
  groups: [
    // ['+', '-', '*', '/', '%', '**'],
    ['&', '|', '^', '~', '<<', '>>', '>>>'],
    ['==', '!=', '===', '!==', '>', '>=', '<', '<='],
    ['&&', '||'],
    ['in', 'instanceof']
  ],
  allowSamePrecedence: false
}],

This way the rule just ignores arithmetic operations and the rest stays the same.

[ljharb]

@raduserbanescu what does it look like if you kept % and **?

[sharmilajesupaul]

@ljharb if you just leave % and ** in the first nested group, then it doesn't warn on this line:

const a = 12 - 24 / 3 + 2 ** 3 * 5 - 2;

But I can understand how that can be a readability issue.

If we want a fix that looks like:

const a = 12 - 24 / 3 + (2 ** 3) * 5 - 2;

^ only warns when ** or % is mixed with other arithmetic operators.

We would need to change our config to look something like this:

"no-mixed-operators": ["error", {
  groups: [
    ["%", "**"],
    ["%", "+"],
    ["%", "-"],
    ["%", "*"],
    ["%", "/"],
    ["**", "+"],
    ["**", "-"],
    ["**", "*"],
    ["**", "/"],
    ["&", "|", "^", "~", "<<", ">>", ">>>"],
    ["==", "!=", "===", "!==", ">", ">=", "<", "<="],
    ["&&", "||"],
    ["in", "instanceof"],
  ],
  allowSamePrecedence: false
}],

I know that looks gross 🙈 but it works.

[ljharb]

That sounds like a great solution then, ugly tho it may be!

Given that our config is JS, we could even do another quasi-gross thing and do something like:

"no-mixed-operators": ["error", {
  groups: [
    ['+', '-', '*', '/', '**'].map(x => [x, '%']),
    ['+', '-', '*', '/', '%'].map(x => [x, '**']),
    ["&", "|", "^", "~", "<<", ">>", ">>>"],
    ["==", "!=", "===", "!==", ">", ">=", "<", "<="],
    ["&&", "||"],
    ["in", "instanceof"],
  ],
  allowSamePrecedence: false
}],

[raduserbanescu]

If you would still like to enforce grouping only for % and ** then @sharmilajesupaul's solution is good. I've tried something like that while initially testing.

@ljharb: The variant with map() would generate nested arrays... and ESLint throws an error because it cannot read the configuration file.

[ljharb]

Forget about my fancy suggestion then :-) #1071 (comment) seems good