Rule proposal: no-variable-declaration-type-superset

[Dionysusnu]

Description

Currently, the following snippet produces a false-positive no-unnecessary-condition due to a TS limitation:

declare const arr: Array<boolean>;
const foo: boolean | undefined = arr[0];
const bar = foo !== undefined;

where foo gets narrowed to just boolean, despite the explicit type annotation. When foo is declared without type annotation, with a type assertion of as boolean | undefined instead, this false-positive does not occur. This rule will recommend the replacement style to avoid false positive rules due to TS type narrowing. It will only report on cases where the variable type assigned is a subset of the type annotation.

I'm not really sure this rule is really the best solution, but it felt the most fitting. Alternatively, no-unnecessary-condition could get fine-grain control over which expressions and types it specifically reports on. For example, the case of a !== undefined is very common in generating this false-positive.

Fail

declare const arr: Array<boolean>;
const variable: boolean | undefined = arr[0];
variable; // boolean, does not include `| undefined`, leads to type inconsistency errors later

Pass

declare const arr: Array<boolean>;
const variable = arr[0] as boolean | undefined;
variable; // boolean | undefined

[Dionysusnu]

Hm, now I can't reproduce the false-positive, but it's definitely happening in my local project.
Failed repro attempt

[JoshuaKGoldberg]

where foo gets narrowed to just boolean, despite the explicit type annotation

Indeed, this is an intentional -and good- result of TypeScript's type narrowing. The variable foo is allowed to receive boolean | undefined but TypeScript can tell that its actual value can immediately be narrowed to boolean. It'd be very annoying if TypeScript didn't narrow variables with types wider than their initial values.

let nameMaybe: string | undefined = "dionysus";

// We know nameMaybe is `string` here, not `string | undefined`.
// TypeScript shouldn't yell at us that it might not be defined.
console.log(nameMaybe.toUpperCase());

if (Math.random() > 0.5) {
  nameMaybe = undefined;
}

It will only report on cases where the variable type assigned is a subset of the type annotation.

Because of the above, I don't think we'd want to take this rule into core. It's not uncommon for TypeScript code to intentionally use a wider type in the type annotation than the value.

// Would be flagged by no-variable-declaration-type-superset
let status: "fail" | "pass" = "pass";

if (!riskyOperation() || !riskyOtherOperation()) {
  status = "fail";
}

can't reproduce

Have you tried enabling strictNullChecks and noUncheckedIndexedAccess?

---

I'm going to close this for now as it's bit too niche of a use case for including in typescript-eslint core. If you'd like to create it yourself you can always use the steps in https://typescript-eslint.io/docs/development/custom-rules. Thanks for posting though!

[Dionysusnu]

Have you tried enabling strictNullChecks and noUncheckedIndexedAccess?

For the local project, strictNullChecks is already enabled.
With noUncheckedIndexedAccess, the type would include undefined and therefore not trigger the false-positive, but the compiler option causes widespread TypeScript errors throughout the codebase because it is quite pedantic, so it is not enabled.

With copying in the TSConfig from the local project (which I initially forgot so strictNullChecks was not enabled), I still can't reproduce the issue in playground. Failed link again

[Dionysusnu]

I'm quite confused why the playground repro does not work.
Hovering over the variables shows that foo is boolean and undefined is, well, undefined. Those types have no overlap, so the rule should trigger. strictNullChecks is true, so the rule is not disabling itself because of that. So why is it not reporting the warning?
I've also installed all the following at the version specified by the playground on the left:

• typescript
• eslint
• tseslint/eslint-plugin
• tseslint/parser

And the warning still gets reported in my local project.
Link again

[JoshuaKGoldberg]

Maybe the same root cause as #4493?

[Dionysusnu]

Hm, that seems quite plausible. Trying to "peek definition" on boolean also gives "no definition found for type boolean".
Anyways, what would you recommend as a fix for the initial false-positive? (apart from enabling strictNullChecks, which I'm working on but will take a lot of work)

[JoshuaKGoldberg]

enabling strictNullChecks

Do you mean noUncheckedIndexedAccess? strictNullChecks is a flag I'd generally recommend always have enabled, but noUncheckedIndexedAccess is pretty irksome as you said and I wouldn't recommend it for most projects.

Without a reproduction in front of me I can't speak to any projects in particular. If you can get your project up online that would be great!

[Dionysusnu]

Do you mean noUncheckedIndexedAccess? strictNullChecks is a flag I'd generally recommend always have enabled, but noUncheckedIndexedAccess is pretty irksome as you said and I wouldn't recommend it for most projects.

Whoops, yeah that's a typo on my part. As I said earlier, strictNullChecks is already enabled.

The reproduction from the playground is almost an exact example, it just doesn't warn in the playground due to the issue you mentioned. In local projects, it creates a false-positive on foo !== undefined, because it thinks foo can only be boolean, not being able to consider that foo comes from an array and might be undefined. It's a shame this is limited, because without arrays this rule would work very well to catch coding errors. For example, upon enabling it, it caught one case where I was not calling the second part of a curried function. What about the original proposal of configuring which expression contexts/types the rule activates on?

[JoshuaKGoldberg]

configuring which expression contexts/types the rule activates on

That's a pretty niche use case. We wouldn't take it into typescript-eslint itself right now as very few people have mentioned wanting it so far. I'd recommend trying it out as a custom rule you write with the APIs documented at https://typescript-eslint.io/docs/development/custom-rules, and if you find it useful releasing it as an open source package. If it ends up being useful and popular we can always take it in.

[Dionysusnu]

Interesting that no one has requested that before. I would've thought the undefined case specifically is a common enough occurence to want to make a configurable adjustment to the rule conditions.

I'll have a look at the custom rule thing. Thanks for the help!