-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Open
Labels
accepting prsGo ahead, send a pull request that resolves this issueGo ahead, send a pull request that resolves this issueenhancement: plugin rule optionNew rule option for an existing eslint-plugin ruleNew rule option for an existing eslint-plugin rulepackage: eslint-pluginIssues related to @typescript-eslint/eslint-pluginIssues related to @typescript-eslint/eslint-plugin
Description
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
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
JoshuaKGoldberg
Metadata
Metadata
Assignees
Labels
accepting prsGo ahead, send a pull request that resolves this issueGo ahead, send a pull request that resolves this issueenhancement: plugin rule optionNew rule option for an existing eslint-plugin ruleNew rule option for an existing eslint-plugin rulepackage: eslint-pluginIssues related to @typescript-eslint/eslint-pluginIssues related to @typescript-eslint/eslint-plugin