microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.83k stars 12.46k forks source link

Better propagation of type predicate signatures #12798

Closed zpdDG4gta8XKpMCd closed 7 months ago

zpdDG4gta8XKpMCd commented 7 years ago
declare function isNumber(value: {}): value is number;
declare const values: {}[];
declare function filterAs<a, b extends a>(values: a[], isIt: (value: a) => value is b): b[];
const yayNumbers = filterAs(values, isNumber); // number[]
const ohNoNotAgain = filterAs(values, x => isNumber(x)); /* <-- expected to work,
actual: Argument of type '(x: {}) => boolean' is not assignable to parameter of type '(value: {}) => value is {}'.
        Signature '(x: {}): boolean' must have a type predicate. */
DanielRosenwasser commented 7 years ago

There actually is a way, but it's cumbersome.

const yayayayayayay = filterAs(values, (x): x is number => isNumber(x));
aluanhaddad commented 7 years ago

Is there a way that this can be improved? It seems very counterintuitive for the type of the predicate to be lost when passed as a callback.

DanielRosenwasser commented 7 years ago

I'd like to think so, but there needs to be a proposal. I'd like to think that for the following:

declare function isNumber(x: any): x is number;
declare function isString(x: any): x is string;

function isSomething(x: any) {
    return isNumber(x) || isString(x);
}

isSomething becomes a predicate on x for string | number.

But for the following

declare function isNumber(x: any): x is number;
declare function isString(x: any): x is string;

function uhhhh(x: any, y: any) {
    return isNumber(x) || isString(y);
}

we'd like to potentially maintain the checks from each predicate, but we don't have a way to encode those guarantees on the signature of uhhhh. Maybe that's okay though.

zpdDG4gta8XKpMCd commented 7 years ago

What I dont understand is that why 10% of corner cases prevent 80% of straightforward cases from being implemented. Can we error in these cases you mentioned? Or can we default to the current fall back to conventional boolean as it currently is?

aluanhaddad commented 7 years ago

It seems like the return type of uhhhh would be (hypothetically)

x is number & y is any | x is any & y is string

But while this could be useful maybe it doesn't need to block inference that doesn't require new syntax to express explicitly. It seems like that would be an orthogonal feature, something like "Correlated User-Defined type Gards". I may be misunderstanding the issue however.

gcnew commented 7 years ago

@aleksey-bykov Implementing features that are useful but not entirely thorough attracts a lot of criticism and is oftentimes even questionable. e.g. https://github.com/Microsoft/TypeScript/issues/13002

zpdDG4gta8XKpMCd commented 7 years ago

Exactly, since we dont get to choose, the type guards don't work at certain positions, let's make them more thorough is my message here, and lets make readonly right by breaking some crappy code (https://github.com/Microsoft/TypeScript/pull/6532#issuecomment-174356151) that is standing on the way, that sarcasm me gets too.

aluanhaddad commented 7 years ago

I think it is just unexpected. I think it is surprising because type predicates are as rich and compositional as they already

if (true || isNumber(x) && isString(x) && isFunction(x)) {
  x // {}
}
if (false || isNumber(x) && isString(x) && isFunction(x)) {
  x // number & string & Function
}

so one may be surprised when they do not work

if (!true || isNumber(x) && isString(x) && isFunction(x)) {
  x // {}
}
if (!false || isNumber(x) && isString(x) && isFunction(x)) {
  x // {}
}

these are rather academic however.

zpdDG4gta8XKpMCd commented 7 years ago

I would like to add that && combinator can be replaced by successive application of different typeguards

As for || it doesn't make much sense if we keep in mind that typeguards are for narrowing (not widening), I might be wrong about it, so a real life example proving me wrong would be helpful. Narrowing from any to string | number looks made up, why not to narrow straight to number or string?

With this said there seem not to be a real situation that would require && and || combinators, ans if so it doesn't seem rational to invest in solving a nonexisting problem.

On the other hand the orifinal situation with type guards used for filtering is very real and needs some attention

zpdDG4gta8XKpMCd commented 7 years ago

Situations that imply string | number are usually (in our code) explicitly stated: string | number | boolean | null | undefined | Function and then all it takes is to chip the cases off one type at a time:

if isNumber ... else if isString ... else if isBoolean
aluanhaddad commented 7 years ago

@aleksey-bykov I agree that these are edge cases. I also like the chip them off one type at a time approach and use it myself.

rotemdan commented 7 years ago

Edit: I got confused here between assertions on values and predicates between types, for some reason, please ignore this comment.

Just wanted to mention, on a somewhat unrelated note, that the way the T is U syntax uses the word is is quite ambiguous. Does it mean that type T is exactly U (in the sense of the types being an exact structural or nominal match), or it means that T is a subtype of U?

Common usage suggest the latter (in the sense that subclasses would pass instanceof checks for their super class) so it seems more appropriate that it would have been notated T extends U (and possibly the predicate is would be reserved for exact matches).

I feel it would be even more unfortunate that this misleading syntax is "abused" with composite formulas like T is U || V is W. Somewhat incidentally (almost entirely unrelated) I formulated a boolean syntax that used the more semantically accurate extends in #12942.

aluanhaddad commented 7 years ago

@rotemdan

Just wanted to mention, on a somewhat unrelated note, that the way the T is U syntax uses the word is is quite ambiguous. Does it mean that type T is exactly U (in the sense of the types being an exact structural or nominal match), or it means that T is a subtype of U?

the type predicate type annotation doesn't operate on two types it operates on a value and a type. A basic type predicate has the type

(x: any) => x is T

which means that if it returns true, the argument x is a subtype of T. It does not in any way mean that x is is exactly a T.

What do you mean by nominal match? private members create pseudo nominal-like assignability.

Common usage suggest the latter (in the sense that subclasses would pass instanceof checks for their super class) so it seems more appropriate that it would have been notated T extends U (and possibly the predicate is would be reserved for exact matches).

Type predicates take a value and a TypeScript (i.e. erased) type. The instanceof operator in JavaScript, surfaced in TypeScript, takes two values it doesn't take types at all...

rotemdan commented 7 years ago

@aluanhaddad

I 100% agree that the extends syntax would not be appropriate for type assertions on values, thanks for correcting me, I had types in mind (as that was something I was thinking about for other, unrelated purposes) and got a bit confused.

Now that I sort out the confusion is seems better. The funny thing is that I suggested a syntax for exact types that would read like just T, and I specifically had in mind that val is just T would read well. Silly me, got completely confused there..

Edit: Maybe it was the title that used "type predicates", which probably intended to mean "value type guards", that got me confused.

RyanCavanaugh commented 4 years ago

Bumping off my "stale" list because this is still a good idea

ascott18 commented 3 years ago

Another use case here that seems like it should work but doesn't - direct type assertions in a predicate (as opposed to a type assertion being done by a subroutine of a predicate):

Playground

class Extension { }
class ImageExtension extends Extension {
  imageUrl: string | null = null;
}

const extensions: Extension[] = [];
extensions.push(new ImageExtension());

// Doesn't work
const imageExtension: ImageExtension | undefined = extensions.find(e => e instanceof ImageExtension);

// Does work, but is unnecessarily verbose
const imageExtension2: ImageExtension | undefined = extensions.find((e): e is ImageExtension => e instanceof ImageExtension);
0xdevalias commented 3 years ago

This may not be the right place to raise this, and I can't guarantee that this is a limitation in the language and not just me coding something poorly.. but given the following type predicate:

export const isDefined = <T>(
  value: T | null | undefined
): value is NonNullable<T> => value !== null && value !== undefined;

It seems that I can't do something like the following, as it seems not to narrow the type (minimal contrived example based on an issue I just hit in my code):

const doSomethingWithFoo = (foo?: string) {
  const hasFoo = isDefined(foo);

  const someObject = { bar: "bar" };

  if (hasFoo && someObject.hasOwnProperty(foo) {
    // TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string | number | symbol'.

    doSomething();
  }
}

Screenshot of the actual error case in my IDE:

image

With the variables defined as:

const hasSprites = isDefined(sprites);
const wantsSprites = isDefined(spriteName);

Yet when I use the type predicate functions directly within the if, it seems to work fine and has no type errors:

image

Is this another case of TypeScript not 'passing on' it's narrowed types? Or am I just making some kind of 'rookie error' here?

neongreen commented 2 years ago

This would be really great because currently it forces me to sacrifice the type safety.

E.g. I have the following predicate:

type Checked = ...

function check(smth, x: Blah): x is Blah & Checked { ... }

I'd like to filter by this predicate:

const filtered: (Blah & Checked)[] =
  xs.filter(x => check(smth, x))

For this to work, I have to add a type signature:

const filtered: (Blah & Checked)[] = 
  xs.filter((x): x is typeof x & Checked => check(smth, x))

However, TypeScript doesn't seem to check that check(smth, x) actually is a type-guarding predicate. The following compiles as well:

const filtered: (Blah & Checked)[] = 
  xs.filter((x): x is typeof x & Checked => true /* or anything equally wrong */)

So each of these lambda type guards is an extra line to potentially worry about.

Jamesernator commented 1 year ago

This issue makes (TS 5.0) decorators more difficult to write if they want to be able to be applied to potentially type predicates.

For example suppose we have:

function deprecatedMethod(reason: string) {
    function decorator<This, Args extends any[], Return>(
        method: (this: This, ...args: Args) => Return,
        context: ClassMethodDecoratorContext,
    ): (this: This, ...args: Args) => Return {
        return function decoratedMethod(this: This, ...args: Args): Return {
            // A proper implementation would only print once ever, rather than per call
            console.warn(reason);
            return method.call(this, ...args);
        };
    }

    return decorator;
}

class SomeClass {
    @deprecatedMethod(`SomeClass.isSomeClass is deprecated and will be replaced in future`)
    static isSomeClass(value: any): value is SomeClass {
        return "field" in value;
    }

    field = "blah";
}

Then we get the same problem as in the OP:

Decorator function return type '(this: unknown, value: any) => boolean' is not assignable to type 'void | ((value: any) => value is SomeClass)'.
  Type '(this: unknown, value: any) => boolean' is not assignable to type '(value: any) => value is SomeClass'.
    Signature '(this: unknown, value: any): boolean' must be a type predicate.(1270)

While this can be resolved by decorator authors by adding an overload:

function deprecatedMethod(reason: string) {
    function decorator<This, Kind>(
        method: (this: This, value: any) => value is Kind,
        context: ClassMethodDecoratorContext,
    ): (this: This, value: any) => value is Kind;
    function decorator<This, Args extends any[], Return>(
        method: (this: This, ...args: Args) => Return,
        context: ClassMethodDecoratorContext,
    ): (this: This, ...args: Args) => Return {
        return function decoratedMethod(this: This, ...args: Args): Return {
            // A proper implementation would only print once ever, rather than per call
            console.warn(reason);
            return method.call(this, ...args);
        };
    }

    return decorator;
}

It feels like it will be a large footgun for authors of fairly generic decorators (like deprecated, experimental, metadata, etc) who might not even be aware of the fact that type predicates require separate overloads.

I'm not sure if this could be more narrowly fixed for decorators, or if it would require a general solution like is proposed in the OP.