Skip to content

“A type predicate's type must be assignable to its parameter's type” does not always kick in #32541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
karol-majewski opened this issue Jul 24, 2019 · 9 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@karol-majewski
Copy link

karol-majewski commented Jul 24, 2019

TypeScript Version: 3.6.0-dev.20190723

Search Terms: type guard, subtype, instantiated with a different subtype, identity

Code

interface Animal {
  species: string;
}

interface Dog extends Animal {
  bark(): void;
}

interface Cat extends Animal {
  meow(): void;
}

declare const animal: Animal;
declare const dog: Dog;
declare const cat: Cat;

declare const mixed: Animal | Dog | Cat;

/**
 * Good.
 */
declare function isDog(candidate: any): candidate is Dog;

/**
 * Bad.
 */
function INCORRECT_isDog<T extends Animal>(candidate: T): candidate is Dog {
  return ('bark' in candidate);
}

/**
 * Ugly.
 */
function UNSAFE_isDog<T extends Animal>(candidate: T): candidate is T & Dog {
  return ('bark' in candidate);
}

if (isDog(mixed)) {
  mixed.bark(); // 👌
}

if (INCORRECT_isDog(cat)) {
  cat.bark(); // Runtime error!
}

if (UNSAFE_isDog(cat)) {
  cat.bark(); // Runtime error!
}

Expected behavior:

UNSAFE_isDog would fail to compile just like INCORRECT_isDog. Asserting T to be of type T in the return type should not trick the compiler into thinking everything is fine.

Actual behavior:

UNSAFE_isDog compiles fine, which leads to runtime exceptions.

Playground Link: Click

Related issues: #24935, #30240

You can see the full use case below 👇.

Use case

Let's take a hierarchy like this:

interface Animal {
  species: string;
}

interface Dog extends Animal {
  bark(): void;
}

interface Cat extends Animal {
  meow(): void;
}

declare const animal: Animal;
declare const dog: Dog;
declare const cat: Cat;

The good

If we were to check whether a piece of data is a Dog, we could write a type guard.
Its signature could look like this:

declare function isDog(candidate: any): candidate is Dog;

Our isDog would probably check for the existence of the species property to make sure it's
defined and of type string. So far so good.

The bad

Another way this can be done is by mixing runtime validation with compile-time validation. For
example, if we already know if something is an Animal, then it might be tempting to do less work
by checking only for the properties specific to Dogs.

This would require the argument to be an Animal.

function INCORRECT_isDog<T extends Animal>(candidate: T): candidate is Dog { // Compile-time error
  return ("bark" in candidate);
}

This won't work! We get a compile-time error.

Compile-time error: A type predicate's type must be assignable to its parameter's type.
  Type 'Dog' is not assignable to type 'T'.
    'Dog' is assignable to the constraint of type 'T', but 'T' could be instantiated with a different subtype of constraint 'Animal'.

The error message makes perfect sense, because without the error we would be allowed to do:

if (INCORRECT_isDog(cat)) {
  cat.bark(); // Runtime-error!
}

The ugly

However, if we tweak the signature, the error message goes away.

function UNSAFE_isDog<T extends Animal>(candidate: T): candidate is T & Dog {
  return ("bark" in candidate);
}

This fails in runtime.

if (UNSAFE_isDog(cat)) {
  cat.bark(); // Runtime error!
}

Asserting the argument to be itself made the type checker shut up. It didn't make the code any safer though. Is that intended?

I'm aware that

  • type guards are somewhat excluded from type checking
  • it's my responsibility to make my own type guards correct
  • UNSAFE_isDog looks horrible and feels like an abuse of the type system

but I'm having trouble convincing my team that UNSAFE_isDog is not the best way to go.

@jcalz
Copy link
Contributor

jcalz commented Jul 25, 2019

I'm afraid I don't understand what the problem is. You can take your "good" example and do

if (isDog(cat)) {
  // if you're in here, cat is Cat & Dog
  cat.bark();
  cat.meow();
}

which behaves exactly like the UNSAFE_isDog().


Also, nothing stops cat from actually being a Cat & Dog:

const catdog: Cat & Dog = {
  species: "catdog",
  bark() {
    console.log("BARK!");
  },
  meow() {
    console.log("MEOW!");
  }
};
const cat: Cat = catdog;

so it's not even that crazy to be testing isDog(cat) and narrowing to Cat & Dog if so.


I'm not sure what issue you're trying to report. Maybe I'm misunderstanding or something?

@karol-majewski
Copy link
Author

karol-majewski commented Jul 25, 2019

Hey @jcalz, thanks for your answer. Let me put it this way:

If that raises a compile-time error

function INCORRECT_isDog<T extends Animal>(candidate: T): candidate is Dog {

then why this doesn't?

function UNSAFE_isDog<T extends Animal>(candidate: T): candidate is T & Dog {

This modification shouldn't change anything. It takes what we implicitly know to be true — the fact candidate is T — and makes it explicit. It doesn't add anything new. Why does it make the compile-time error go away?


You can take your "good" example and do

You can't, because isDog(cat) will always return false. Assume isDog is a proper validator and it validates the input in its entirety correctly. Cats don't have a bark() method and therefore isDog(cat) will always be false.

If you're worried about overlaps and things like Cat & Dog, let's assume they cannot exist.

interface Dog extends Animal {
  bark(): void;
  meow: never;
}

interface Cat extends Animal {
  meow(): void;
  bark: never;
}

@jack-williams
Copy link
Collaborator

If you look at the type lattice you have something like this:

image

Your first type guard is effectively taking the red line, which is bad because it doesn't respect the lattice structure. You shouldn't be able to convert a Cat to a Dog directly.

Your second type guard respects the lattice structure by taking the meet (intersection) of Cat and Dog.

Using the second type guard forces the user to write a more precise type and therefore manifest any nonsensical type guards that produce never, like:

function silly<T extends number>(candidate: T): candidate is T & boolean {
    
}

It's effectively the same reasoning for type casts: expr as T. You can't just move 'side-ways' between unrelated types; you need to move either up or down the lattice. There is always a way to work around that by doing expr as unknown as T, but it stops people making basic errors.

@karol-majewski
Copy link
Author

@jack-williams Thank you for your explanation. To sum up: which one — isDog or UNSAFE_isDog — would you recommend? Or maybe there is no right and wrong and they are both fine?

@jcalz
Copy link
Contributor

jcalz commented Jul 25, 2019

If there is a meaningful Cat & Dog, then the code block inside if (isDog(cat)) { ... } is relevant and does reasonable things. If there is no meaningful Cat & Dog, then that block is a bunch of unreachable code that the compiler doesn't aggressively warn you about, which might be weird but isn't terrible or unsafe.


Let's use your latter mutually-inconsistent definitions of Cat and Dog. In that case you can still check isDog(cat) (or UNSAFE_isDog(cat) which acts the same).

if (isDog(cat)) { 
  cat.bark(); // error, is never now
  cat.meow(); // error, is never now
  cat.species.charAt(0); // okay, still string
}

So cat will still be seen as Cat & Dog. The compiler doesn't aggressively reduce Cat & Dog to never (that is, a type like {foo: never} does not itself get reduced to never even though you shouldn't be able to have a value of type {foo: never}). It does, however, see that cat.bark and cat.meow are of type never, so you can't call them.

Actually I'd say you want Cat to have property bark?: never and not bark: never, in which case the previous block goes back to having no error, since cat.bark is seen as being both a function and undefined, and I guess you can call those. But it really doesn't matter:

If you make sure that Cat and Dog have no overlap, then you know the control flow will never actually end up inside that block... and therefore in practice it's a bit academic to worry about which lines inside that block the compiler should be happy or unhappy about. From falsehood, anythihng follows. So the compiler is free to say "if the impossible happens, then X is an error" or "if the impossible happens, then X is not an error". Both are valid (although one might be more or less surprising to developers).


If you really want to prevent people from passing in known Cat instances to isDog() you can fake up a lower-bound type parameter constraint like this:

function strictIsDog<T extends Dog extends T ? unknown : never>( // like <T super Dog>
  candidate: Dog | T // if Dog extends T then Dog | T is T 
): candidate is Dog { // compiler recognizes that Dog | T can narrow to T
  return "bark" in candidate;
}

if (strictIsDog(animal)) {} // okay
if (strictIsDog(dog)) {} // okay
if (strictIsDog(mixed)) {} // okay
if (strictIsDog(cat)) {} // error! 
//              ~~~ <-- Cat is not assignable to Dog

Maybe that will behave reasonably for you? Not sure. Good luck!

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Jul 25, 2019
@karol-majewski
Copy link
Author

karol-majewski commented Jul 25, 2019

@jack-williams @jcalz thank you again for taking the time to explain how the compiler thinks in such an elaborate way. This is some blog post material!

I think I get why adding a seemingly neutral T is T bit actually changes a lot.

I wrote down how I understand it using layman's terms for anyone finding this issue in the future.
Please correct me if anything in there is incorrect!

/**
 * Error: "A type predicate's type must be assignable to its parameter's type".
 * 
 * (Right-hand side must be a subtype of the left-hand side.)
 */
declare function isDate(candidate: Date): candidate is string;
/**
 * Now it's correct within the laws of the type system, but makes zero practical sense,
 * because there exists no runtime representation of the type `Date & string`.
 * 
 * The type system doesn't care whether a type can be represented in runtime though.
 * 
 * Adding the seemingly neutral `candidate is typeof candidate` bit makes TypeScript
 * happy because all it cares about is for the right-hand side to be a subtype of the left-hand side.
 * In this case, `Date & string` is more specific than (is a subtype of) `Date`.
 * 
 * As a design decision, TypeScript does not collapse that type to `never` (although it could).
 */
declare function isDate(candidate: Date): candidate is typeof candidate & string;

If there is no bug, and there are no plans to reduce empty cases to never more aggressively to help developers exclude weird/absurd/accidental cases, then I think we are done here.

Thanks for the help!

@fatcerberus
Copy link

The Cat & Dog says meowoof?

@sophistifunk
Copy link

What I don't understand is why you need to make it explicit? Given:

function isBarAlsoFoo(obj: Bar): obj is Foo;

Without resorting to any, how can we get any code to typecheck in which you pass an object that's not a Bar to isBarAlsoFoo()?

@doanestudio
Copy link

🎉 This issue has been resolved in version 10.0.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

7 participants