Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs: no-mixed-operators - allowSamePrecedence option docs improvement #9962

Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation rule Relates to ESLint's core rules

Comments

@munkychop
Copy link
Contributor

Tell us about your environment
N/A (macOS Sierra 10.12.6)

  • ESLint Version:
    v4.17.0

  • Node Version:
    N/A (v8.9.1)

  • npm Version:
    N/A (v5.5.1)

What parser (default, Babel-ESLint, etc.) are you using?
N/A (default)

Please show your full configuration:
N/A

What did you expect to happen?
The allowSamePrecedence option of the no-mixed-operators rule should correctly document "Examples of incorrect code for this rule with {"allowSamePrecedence": false} option".

What actually happened? Please include the actual, raw output from ESLint.
The documentation is incorrect, as it is the same as "Examples of correct code for this rule with {"allowSamePrecedence": true} option":

/*eslint no-mixed-operators: ["error", {"allowSamePrecedence": true}]*/

// + and - have the same precedence.
var foo = a + b - c;
/*eslint no-mixed-operators: ["error", {"allowSamePrecedence": false}]*/

// + and - have the same precedence.
var foo = a + b - c;

This should simply be changed to something like this:

/*eslint no-mixed-operators: ["error", {"allowSamePrecedence": false}]*/

// + and * have different precedence.
var foo = a + b * c;
@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Feb 9, 2018
munkychop added a commit to munkychop/eslint that referenced this issue Feb 9, 2018
no-mixed-operators rule - allowSamePrecedence option is incorrectly documented.
@platinumazure
Copy link
Member

I'm not sure I understand.

The purpose of the rule is to require parentheses around an expression with two or more consecutive binary operators in it. The allowSamePrecedence option relaxes this requirement when the two operators are equal precedence.

So if allowSamePrecedence is false, then var foo = a + b - c; is in fact incorrect code: The rule is flagging all mixed/multiple binary operator expressions, even if they are the same precedence. That same example is correct (i.e., no lint warning) if allowSamePrecedence is true, because addition and subtraction have the same precedence.

The example var foo = a + b * c; should be incorrect whether allowSamePrecedence is true or false, because addition and multiplication are not the same precedence in any case. So this rule would then require the user to specify var foo = a + (b * c); (assuming the user knew what they were doing with the operators in the first place).

Am I missing something?

@platinumazure platinumazure added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules documentation Relates to ESLint's documentation evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon enhancement This change enhances an existing feature of ESLint labels Feb 9, 2018
@munkychop
Copy link
Contributor Author

Ah I see I've slightly misread the docs. I think I was confused due to there being no 'correct' example for allowSamePrecedence:false, although, I suppose code such as var foo = a + (b - c); is undesirable anyway, so this is really a non-issue.

Thanks for clearing things up.

@platinumazure
Copy link
Member

@munkychop I think the natural/correct example in that case would be var foo = (a + b) - c;. Would you like to submit a PR?

@munkychop
Copy link
Contributor Author

@platinumazure sure, I can do that. Maybe it would help clarify things for others :)

@munkychop munkychop reopened this Feb 9, 2018
@platinumazure platinumazure added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Feb 9, 2018
@munkychop munkychop changed the title Docs: no-mixed-operators - allowSamePrecedence option incorrectly documented Docs: no-mixed-operators - allowSamePrecedence option docs improvement Feb 9, 2018
munkychop added a commit to munkychop/eslint that referenced this issue Feb 9, 2018
@munkychop
Copy link
Contributor Author

I've created the PR. Let me know if there's anything more to add to that :)

This was referenced Mar 19, 2018
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Aug 10, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Aug 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.