Skip to content

Enhancement: [no-unnecessary-condition] use assignability checks in checkTypePredicates #11798

@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

Currently the rule only flags an asserted argument if the type of the variable is equal to the asserted type. I propose to change the logic so that the rule also flags if it's a subtype.

Fail

declare function isStringOrNumber(x: unknown): x is string | number;

declare const s: string;

if (isStringOrNumber(s)) {
}

Pass

declare function isStringOrNumber(x: unknown): x is string | number;

declare const s: string | number | bigint;

if (isStringOrNumber(s)) {
}

Additional Info

This is split out from #11716 (comment).

I've begun working on this and there is a bit of nuance beyond simply using checker.isAssignableTo() and calling it a day... Examples like the following are troublesome:

interface Wider {
  a: string;
}

interface Narrower {
  a: string;
  b?: number;
}

declare function isNarrower(x: unknown): x is Narrower;

declare const n: Narrower;
declare const w: Wider;

// These types are mutually assignable:
n satisfies Wider;
w satisfies Narrower;

// Nominally, `isNarrower(w)` seems to be unnecessary, since `w` is 
// already assignable to `Narrower`.
//
// But, using the `isNarrower()` narrowing has a real impact: it allows
// the optional methods in `Narrower` to be invoked on `w`.
if (isNarrower(w)) {
  w.b?.toFixed(); 
}

"is assignable to" and "is a subtype of" are not quite same thing in TS, so avoiding false positives on these may require some thought.

cc @ronami

Metadata

Metadata

Assignees

No one assigned

    Labels

    accepting prsGo ahead, send a pull request that resolves this issueenhancement: plugin rule optionNew rule option for an existing eslint-plugin rulepackage: 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