Enhancement: [no-unnecessary-condition] flag when logical equality check has the same variable on either side

[jguddas]

Before You File a Proposal Please Confirm You Have Done The Following...

• I have searched for related issues and found none that match my proposal.
• I have searched the current rule list and found no rules that match my proposal.
• I have read the FAQ and my problem is not listed.

My proposal is suitable for this project

• I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/no-unnecessary-condition/

Description

If the left and right side of a condition are the same, something has gone wrong.

Fail

a == a
a || a
a && a

Pass

true
a
a

Additional Info

No response

[bradzacher]

If the left and right side of a condition are the same, something has gone wrong.

This is actually not always strictly correct!
EG: NaN. NaN === NaN always returns false, so doing x === x can be a way to test if a number variable has gone to NaN.
One could argue that Number.isNaN is better, for sure, but it is technically valid to use such a check to check for NaN.

The NaN case is why rules like no-constant-binary-expression and no-constant-condition do not error on this code.

I would accept a PR adding such a check to the lint rule which works like this:

• for number, error and suggest changing the code to use Number.isNaN instead
• for all other cases, error and suggest removing the check.

---

As for the logical expression side of things - no-unnecessary-condition already handles this to some degree.
Unfortunately due to a bug in our playground (#5079) I can't link it to you, but you can confirm that TS correctly refines the type of a inside the logical:

The only place this behaviour won't work is when TS is unable to refine out the truthy/falsey type (i.e. if the value is exactly string or number).

In the cases of string/number - I think it's getting to be too complicated for us to consider supporting in the lint rule. Logical expressions and understanding their AST is very complicated. For example:

b || a || a

It's obvious to us as humans that this is a duplicated condition, but based on the AST this is more difficult:

b || a || a
^^^^^^ LogicalExpression
^^^^^^^^^^^ LogicalExpression

I.e. the outer logical expression's left side is a logical expression whose right side matches the right side of the outer logical expression.
This can get even more complicated if you get into longer chains, eg:

a || b || c || a
^^^^^^ 
^^^^^^^^^^^
^^^^^^^^^^^^^^^^

The right of the outer logical expression is the same as the left of the logical expression on the left of logical expression the left logical expression.
If you start start mixing in && and ??, then you need to take care of precedence and the rules of logic, like:

a && b || a && c

the a is repeated here - and technically it could be rewritten as a && (b || c), but decyphering that from the AST is hard.

[jguddas]

Hmm, seams quite complicated for not all that much benefit.

And we can't even safely support member expressions, something like x.x == x.x can be false 😞 .

let i = 0
const x = { get x() { return i++ } };
console.log (x.x == x.x); // logs false

[bradzacher]

yeah it's a really dangerous thing!
we can definitely support identifiers, but otherwise it mightn't be worth doing anything here.
Given that code like a === a is super rare, I'm going to close this as no fix