[no-unnecessary-condition] False positive with nullish coalescing

[mdziekon]

Repro

{
    "rules": {
        "@typescript-eslint/no-unnecessary-condition": ["error"]
    }
}

function test(testVal?: boolean) {
    if (testVal ?? true) {
        console.log('test');
    }
}

Expected Result

No ESLint errors reported (condition inside the function is falsy only when we explicitly pass testVal = false).

Actual Result

2:20 error Unnecessary conditional, value is always truthy @typescript-eslint/no-unnecessary-condition

Additional info

I've noticed this recently when reading about additional options introduced in #1983, namely allowComparingNullableBooleansToFalse. With that option set to false, code like this:

function test(testVal?: boolean) {
    if (testVal !== false) {
        console.log('test');
    }
}

... turns into this with an autofix:

function test(testVal?: boolean) {
    if ((testVal ?? true)) {
        console.log('test');
    }
}

This is a valid transformation, since there should be only one situation when console.log() is run - when the testVal argument was explicitly set to false. Therefore, no-unnecessary-condition incorrectly reports that nullish coalescing is not needed here.

Versions

package	version
@typescript-eslint/eslint-plugin	3.9.0
@typescript-eslint/parser	3.9.0
TypeScript	3.9.7
ESLint	7.6.0
node	14.7.0
npm	6.14.7

package	version
@typescript-eslint/eslint-plugin	4.0.0-rc (2edbca380bd8f317cd96bac0df5030ddcd14a6af)
@typescript-eslint/parser	4.0.0-rc (2edbca380bd8f317cd96bac0df5030ddcd14a6af)
TypeScript	4.0.0-beta
ESLint	7.2.0
node	14.7.0
npm	6.14.7

[bradzacher]

Are you using the strictNullChecks compiler flag?

[mdziekon]

I'm using strict which enables strictNullChecks. The second test run (the one using plugin's RC version) was run directly using the plugin's attached test suite, and as far as I know, it also has this flag enabled.

[bradzacher]

Sorry, it's just my first question before properly reading the issue because 95% of the time the user hasn't got it turned on, so the rule ofc doesn't work as expected.

Can confirm this is a bug.
The issue is that the code inspects the right hand side of the nullish coalesce within the if check. OFC it finds this value to always be true, hence it errors.

Pretty simple to fix - it shouldn't inspect the right hand side of a nullish coalesce.

[mdziekon]

No problem, I understand where did this first question come from.

I was actually going to post my findings (from my little debugging session, quite similar to what you've found, but probably not as accurate), but it looks like you already solved the issue, cool :)

[sindresorhus]

Sorry, it's just my first question before properly reading the issue because 95% of the time the user hasn't got it turned on, so the rule ofc doesn't work as expected.

Maybe that should be part of the issue template.

[bradzacher]

So many users don't read issue templates. So many don't even glance at the comments.
Eg. the first block in the template says "please don't ignore the issue template because we're just going to ask you to fill it out". Yet every week we get a handful of issues where users have ignored and deleted chunks of the (very minimal) template, and I have to ask them to fill it out.

There's also the issue search, which many people don't use.

Instead I'm just making the executive decision to stop these rules working (by default) without strictNullChecks turned on (#2345).