Skip to content

Enhancement: [no-unnecessary-condition] improve error message for statically analyzable literal type comparisons again #10455

@kirkwaiblinger

Description

@kirkwaiblinger

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

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

I've thought more about the error message change in #9643 / #10194, and I'm not happy with the current error message (changed in #10194).

'Unnecessary conditional, comparison is always {{trueOrFalse}}. Both sides of the comparison always have a literal type.' is more helpful than the prior message was, yes, but it doesn't fully explain what we're flagging to the user.

I propose we change it to 'Unnecessary conditional, comparison is always {{trueOrFalse}}, since {{left}} {{operator}} {{right}} is {{trueOrFalse}}.', so that something like if (expressionResultingInMinus55 < expressionResultingIn23) {} will report an error as 'Unnecessary conditional, comparison is always true, since -55 < 23 is true.'.

This would IMO make it clearer what types we're seeing and why that indicates the expression has a constant value, helping the user assess whether the types are correct before resolving the complaint.

I have successfully implemented this in #10454 and I find the reports to be a big improvement when experimenting with it in the playground.

Fail

// unnecessary condition; 1 === 2 is false
if (1 === 2) {}


declare const expressionResultingIn23: 23;
declare const expressionResultingInMinus55: -55;

// unnecessary condition; -55 < 23 is true.
if (expressionResultingInMinus55 < expressionResultingIn23) {}

Pass

n/a

Additional Info

cc @StyleShit

Metadata

Metadata

Assignees

No one assigned

    Labels

    accepting prsGo ahead, send a pull request that resolves this issueenhancementNew feature or requestlocked due to agePlease open a new issue if you'd like to say more. See https://typescript-eslint.io/contributing.package: eslint-pluginIssues related to @typescript-eslint/eslint-plugin

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions