no-unneeded-ternary incorrect documentation for defaultAssignment option

[mdjermanovic]

This is a "documentation bug".

The version of ESLint you are using.

6.1.0

The problem you want to solve.

Documentation for defaultAssignment option of the no-unneeded-ternary rule is incorrect.

defaultAssignment = true means don't report expressions such as x ? x : y.

defaultAssignment = false means report expressions such as x ? x : y.

Default is true.

The rule doesn't actually check if the expression is in an assignment context. That's by design from the start (#3232 and #3260). It simply searches for all ternary expressions where test and consequent are same identifier.

Incorrect parts of the documentation are:

1. The incorrect example:

/*eslint no-unneeded-ternary: "error"*/

var a = x === 2 ? true : false;

var a = x ? true : false;

var a = f(x ? x : 1);

The example is wrong, var a = f(x ? x : 1); is not a warning because of the default option value.

2. The correct example:

/*eslint no-unneeded-ternary: "error"*/

var a = x === 2 ? "Yes" : "No";

var a = x !== false;

var a = x ? "Yes" : "No";

var a = x ? y : x;

var a = x ? x : 1;  // Note that this is only allowed as it on the right hand side of an assignment; this type of ternary is disallowed everywhere else. See defaultAssignment option below for more details.

The comment is wrong.

3. defaultAssignment section

The defaultAssignment option allows expressions of the form x ? x : expr (where x is any identifier and expr is any expression) as the right hand side of assignments (but nowhere else).

The option (when true) allows such expressions everywhere.

Your take on the correct solution to problem.

Fix the documentation.

Perhaps also consider changing the name of the option for two reasons:

• The Assignment part is confusing.
• When the option name doesn't have an explicit prefix, true ususally means "enforce on", rather than "allow".

Maybe allowSameConsequent or allowSameIfTrue.

Are you willing to submit a pull request to implement this change?

Yes, I would be glad to do it.

[kaicataldo]

I'm not against renaming the option if we do so in a backwards compatible way.

[mdjermanovic]

Perhaps to make this issue be just a task to fix the documentation (if confirmed that it's incorrect) ?

I could open another issue to evaluate renaming.

[kaicataldo]

@mdjermanovic Now that you're a team member, if you can verify this is a bug you can feel free to mark it as accepted!

[samrae7]

I would like to make the doc change. I will verify at the same time

[mdjermanovic]

@samrae7 Thanks, a PR to fix the documentation is welcome!

[mdjermanovic]

Marking as accepted based on this Online Demo

The code is copy & paste from the 'incorrect' example in the docs. In this example, the rule doesn't report error for the third statement.

To avoid confusion, I removed the part about changing the option's name from the original post.

This is now just an issue to fix the docs to match the current behavior.