Thanks for the PR,@zachkirsch!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitorsper day.

Codecov Report

Merging#1983 intomaster willincrease coverage by0.03%.
The diff coverage is100.00%.

@@            Coverage Diff             @@##           master    #1983      +/-   ##==========================================+ Coverage   93.40%   93.43%   +0.03%==========================================  Files         174      174                Lines        7896     7938      +42       Branches     2256     2274      +18     ==========================================+ Hits         7375     7417      +42  Misses        247      247                Partials      274      274

thanks for the PR@zachkirsch - could you please provide some context behind this PR?
Why do you want this change?

To me it seems like this option removes a large portion of the usefulness of the rule as the most common case would be a nullable boolean.

Yeah! Two reasons:

Reason 1: Comparing nullable booleans to literaltrue is unnecessary

Reason 2: Comparing nullable booleans to literalfalse is confusing

I seemyOptionalBoolean !== false a lot of the time, usually as optional props (that default totrue) in react components. For example:

interfaceProps{// whether to display title (defaults to true)displayTitle?:boolean}...publicrender(){return(<div>{this.props.displayTitle!==false&&renderTitle()}        ...

Before?? existed,myOptionalProp !== false was a sensible way to check an optional prop and default totrue if it'sundefined. In my opinion, I think thatmyOptionalProp !== false is a very confusing way to express "use this prop but default totrue if it's missing," and IMO it's harder to see bugs that are written that way.

?? is the perfect replacement for this scenario - it's a built-in operator that provides a default if your variable is undefined at runtime.(this.props.displayTitle ?? true) && renderTitle() very clearly means "render the title based on the value ofthis.props.displayTitle (and default totrue if it's not defined)"

To me it seems like this option removes a large portion of the usefulness of the rule as the most common case would be a nullable boolean.

I think this rule is still very useful with this option, as it

1. catches unnecessary comparisons with boolean variables
   1. e.g.myBoolean !== true --> !myBoolean
2. catches unnecessary comparisons between nullable boolean variables andtrue
   1. e.g.myOptionalBoolean !== true --> !myOptionalBoolean
3. cleans up comparisons between optional booleans andfalse (described above in this comment)

Sweet! I actually am having some second thoughts haha. I feel like auto-fixingvarBooleanOrNull === false to!(varBooleanOrNull ?? true) is kind of weird - the "fixed" version is also a bit hard to parse visually.

FixingvarBooleanOrNull === true tovarBooleanOrNull andvarBooleanOrNull !== true to!varBooleanOrNull is good IMO. I think fixingvarBooleanOrNull !== false tovarBooleanOrNull ?? true is also good but I could see it either way.

What are your thoughts? We could also have additional config options that more granularly control which of these four comparisons are checked/fixed by the rule.

Hmmm. Sorry, I did a quick eyeball and didn't notice the fix output.
That's my bad.

treat the false and nullish cases the same

• foo === true =>foo
• foo !== true =>!foo

treat the true and nullish cases the same

• foo === false =>foo ?? true
• foo !== false =>!(foo ?? true)

I think that is probably fine.
Personally, I think that the latter two cases might be good to ignore, as IMO they're clearer the other way.

The other option for the last two is to fix to something likefoo !== undefined && !foo, but that's pretty verbose, and it's probably better to not fix at all there....

Okay, I brokeallowComparingNullableBooleans into two options:allowComparingNullableBooleansToTrue andallowComparingNullableBooleansToFalse, so users can decide what they want to allow. If you want, I'm happy to just have the rule ignore comparisons between nullable boolean variables and literalfalse, but I think it might be confusing if eslint complains aboutbooleanOrNullVar === true but notbooleanOrNullVar === false.

I also added some more docs to the rule's readme, including a table of what exactly the fixer does in each case.

Lemme know if you need anything else from me!