Skip to content
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

Type predicate calling another predicate with different argument position #6311

Closed
andy-hanson opened this issue Jan 1, 2016 · 8 comments
Closed
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@andy-hanson
Copy link

This code will not compile:

function isString1(a: number, b: Object): b is string {
    return typeof b === 'string'
}

function isString2(a: Object): a is string {
    return isString1(0, a)
}

Output:

foo.ts(6,9): error TS1226: Type predicate 'b is string' is not assignable to 'a is string'.
  Parameter 'b' is not in the same position as parameter 'a'.

The code will work if I swap the parameters of isString1 so that the predicated parameter for both functions is the first argument.
Is there a good reason that this is required?

@weswigham
Copy link
Member

@DanielRosenwasser - This is a bug. And by "bug" I mean "unaccounted for case in our widening logic". We need to make predicate return types get widened so the predicates don't get compared. Should probably also add a test case for this.

@DanielRosenwasser DanielRosenwasser added the Bug A bug in TypeScript label Jan 1, 2016
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 1.8 milestone Jan 1, 2016
@DanielRosenwasser DanielRosenwasser self-assigned this Jan 1, 2016
@DanielRosenwasser
Copy link
Member

So by "a bug", you mean "a bug". 😄

@DanielRosenwasser DanielRosenwasser removed their assignment Jan 2, 2016
@DanielRosenwasser
Copy link
Member

@weswigham There's a couple of places that we need to widen type predicates that we don't:

declare function isString1(a: number, b: Object): b is string;

declare function isString2(a: Object): a is string;

switch (isString1(0, "")) {
    case isString2(""): // error
}

var x = isString1(0, "") === isString2(""); // error

I'd like to hear opinions from others on how we can mitigate this. Is it feasible to widen at all locations where we need to relate types? @mhegazy @JsonFreeman

@weswigham
Copy link
Member

@DanielRosenwasser all locations where we need to relate types - wouldn't that just mean the relationship itself?

@JsonFreeman
Copy link
Contributor

Oh no... This is an example of why I was so against having type predicate types.

It is not good to widen all places where we compare types. If that happens, then any place where we do a subtype comparison (type argument inference, subtype reduction, etc), you will have null and undefined widening to any, and then dominating other types in the comparison.

I can think of 3 options, so as usual, here comes a list:

  1. Change the type system so that type predicates are not types. This would essentially revert the internal representation of type predicates to how it was after @tinganho's implementation. @weswigham can comment on the potential drawbacks of this solution.
  2. When comparing type predicates, don't compare the argument index, just the type portion of the type predicate. This is less of a relaxation than option 1, since it still requires the type portion of the type predicate to be compatible.
  3. If the result of a call expression (for example the call to isString1 in the example) is about to be compared to a type predicate, then the parameter a in isString2 can be unified with the b in isString1. This is not a fully formed idea, but the gist is that two type predicates are compatible if they are talking about the same value. This is sort of similar to how we narrow types, an identifier referenced as an argument to a call is unified with the parameter of the relevant signature, and the type predicate is applied.

If I understand correctly, widening is more about preventing the type predicate from "landing" on a variable. That is not the issue in this case, as there are no variables involved.

@weswigham
Copy link
Member

TBH, I think the part of the type relationship which checks predicate index compatibility just needs to be moved form the predicate relationship code into the signature relationship code, since its only relevant during signature comparison.

weswigham added a commit to weswigham/TypeScript that referenced this issue Jan 3, 2016
@weswigham weswigham mentioned this issue Jan 3, 2016
@JsonFreeman
Copy link
Contributor

This approach is similar to option 2 I've mentioned above.

@DanielRosenwasser
Copy link
Member

This should be fixed. Give it a shot in our nightlies. Thanks @andy-hanson!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

5 participants