Suggestion: warn on always-false typeguards

shicks commented 5 years ago

Search Terms

type guard always-false warning

Suggestion

When a typeguard is always false, the guarded variable is type never within the body. While you can't actually look up any properties on never, you can pass it anywhere or assign it to anything and there's absolutely no warnings. Calling a typeguard function with an argument that has no overlap with the guard should produce a warning.

Use Cases

My immediate motivation is https://github.com/microsoft/TypeScript/pull/24436#issuecomment-392997204, where if Number.isFinite could be written as a typeguard (this would also need a narrower number subtype, see e.g. #32188) then it could easily be allowed to be called with a number|string, but no additional gymnastics would be required to warn if calling with a guaranteed non-number.

In general there's not a good reason to call an always-false typeguard. This is quite different from the always-true case where defensive coding comes into play.

Examples

Example:

declare const x: string;
if (isNumber(x)) { // Should warn: "condition is always false"
    usePrivate(x);
}

If usePrivate requires a private type that this scope doesn't have access to construct, then it would be nice if this code warned somewhere. Instead, it currently just narrows x to never and allows doing whatever you want with it.

Checklist

My suggestion meets these guidelines:

[ ] This wouldn't be a breaking change in existing TypeScript/JavaScript code
- I don't have a good sense of how much this would break, but I expect any breakages would be exposing legitimate errors.
[X] This wouldn't change the runtime behavior of existing JavaScript code
[X] This could be implemented without emitting different JS based on the types of the expressions
[X] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
[X] This feature would agree with the rest of TypeScript's Design Goals.

AnyhowStep commented 5 years ago

declare const x: string;
declare function isString(x: unknown): x is string;

if (!isString(x)) {
    //`x` is now of type `never`
    //but I want this to compile
    //because defensive programming against JS code
    throw new Error(`Expected string; received ${typeof x}`);
}

Playground

fatcerberus commented 5 years ago

@AnyhowStep He mentioned that, actually.

This is quite different from the always-true case where defensive coding comes into play.

isString() is always true (per the type system), you're just negating it afterwards.

RyanCavanaugh commented 4 years ago

I don't think it's justifiable that defensive checks are always written in always-true format. You could easily imagine writing:

function fn(x: number) {
  if (isNullOrUndefined(x)) Debug.fail("x must be provided");
}

It's not clear what distinguishes this from the case in the OP.

shicks commented 4 years ago

That's a good point, though I suspect it's a much less common case. There's always the option to cast to unknown, though it's not particularly satisfying. It's also less relevant for this to be a type guard, since it's not actually narrowing anything.

@RyanCavanaugh - my underlying motivation was a comment you made about wanting to warn on Number.isFinite(shouldNeverBeANumber), which is somewhat similar - I think your example here is more compelling because it's about null/undefined, which we naturally understand to generally be a possible exception to the type system.

Maybe a cleverer approach would be to warn if typeguard(x | undefined | null) would be always false. It lacks a certain symmetry and in general cleverness is bad, but it seems to pass the gut test a little better...

fatcerberus commented 4 years ago

I suppose one could declare the check as

declare function isNullOrUndefined(x: any): x is null | undefined;

But that doesn't seem useful; when do you ever need a value that's guaranteed to be nullish? And if you're not using it as a type guard... well, then that's what distinguishes it from the OP. The issue is about warning on always-false type predicate calls.

I can't think of a single time I've ever seen a type guard which is expected to always be false in normal operation. Generic defensive boolean checks, sure, but no always-false type guards.

microsoft / TypeScript