Skip to content

Suggest specifying generic as union if candidates are different #20339

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

Open
OliverJAsh opened this issue Nov 29, 2017 · 10 comments
Open

Suggest specifying generic as union if candidates are different #20339

OliverJAsh opened this issue Nov 29, 2017 · 10 comments
Labels
Domain: Error Messages The issue relates to error messaging Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@OliverJAsh
Copy link
Contributor

TypeScript Version: 2.6.1.

Code

If a generic type can't be formed by picking one of the inference candidates, you'll get the error you posted.

https://stackoverflow.com/questions/39905523/why-isnt-the-type-argument-inferred-as-a-union-type/39905723#39905723

This makes sense, however I would like to question whether we can improve the user experience around this, so errors due to this constraint are easier to understand and fix.

For example:

{
    function compare<T>(x: T, y: T): number {
        return 1;
    }
    compare(
        'oops',
        /* Argument of type '42' is not assignable to parameter of type 'string'. */
        42,
    );
}

{
    function match<T>(cases: { foo: T; bar: T }): T {
        return cases.foo;
    }
    /*
    Argument of type '{ foo: number; bar: string; }' is not assignable to parameter of type '{ foo: number; bar: number; }'.
        Types of property 'bar' are incompatible.
            Type 'string' is not assignable to type 'number'.
    */
    match({
        foo: 1,
        bar: 'foo',
    });
}

As a TypeScript user, I have struggled with these errors many times, and I've only recently realised the specific constraint on the type system which is the root cause of these errors: generics are picked from the first candidate and are not widened to include all candidates. I have also seen other people struggle with this when learning TypeScript.

We can fix this error by specifying the generic as a union:

    match<string | number>({
        foo: 1,
        bar: 'foo',
    });

However, this fix is really not obvious from the error message, especially if the user is not aware of this constraint on the type system (that generics will not be inferred as unions).

I'm wondering if there's any way we can better surface this constraint to the user, to make it clearer to users how they can fix these type errors, such as by specifying the generic as a union (if that is what they intend).

@OliverJAsh
Copy link
Contributor Author

Elm provides some inspiration here. Taking the same examples:

compare : a -> a -> a
compare a b = a

result = compare 1 "foo"
-- TYPE MISMATCH ------------------------------------------------------ main.elm

The 2nd argument to function `compare` is causing a mismatch.

4|          compare 1 "foo"
                      ^^^^^
Function `compare` is expecting the 2nd argument to be:

    number

But it is:

    String

Hint: I always figure out the type of arguments from left to right. If an
argument is acceptable when I check it, I assume it is "correct" in subsequent
checks. So the problem may actually be in how previous arguments interact with
the 2nd.

match : { foo: a, bar: a } -> a
match cases = cases.foo

result = match { foo = 1, bar = "foo" }
The argument to function `match` is causing a mismatch.

4|          match { foo = 1, bar = "foo" }
                  ^^^^^^^^^^^^^^^^^^^^^^^^
Function `match` is expecting the argument to be:

    { ..., foo : String }

But it is:

    { ..., foo : number }

Hint: Problem in the `foo` field. I always figure out field types in
alphabetical order. If a field seems fine, I assume it is "correct" in
subsequent checks. So the problem may actually be a weird interaction with
previous fields.

@RyanCavanaugh
Copy link
Member

We've been chatting internally about how to make this experience better from the type system side. There are some functions (e.g. VB's IIf) where a union type inference makes a lot of sense, and others (e.g. find(needle: T, haystack: T[])) where a union type inference would be a disaster. Some notes in #19596

After reviewing all the functions in that linked issue, I'm increasingly convinced that the correctness of a union inference is an intrinsic property of the function itself, rather than a call-site decision. If the provided call to match is indeed valid, then match should really have two type parameters and return a union of them (or in the world where we allow specifying that union inference is legal for this function, do that).

@mhegazy
Copy link
Contributor

mhegazy commented Nov 29, 2017

Regarding the error message.. this happens in an earlier phase before overload resolution, and the compiler does not have the full information about which parameter is used here, all it knows is that it made two inferences that are not compatible for a type parameter. we are open to ideas to make the error message better if you want to take a stab at it.

@mhegazy mhegazy added Domain: Error Messages The issue relates to error messaging Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Nov 29, 2017
@OliverJAsh
Copy link
Contributor Author

all it knows is that it made two inferences that are not compatible for a type parameter

Is this enough information for us to provide an explicit warning to the user that the two type parameter inferences are incompatible? Currently, TypeScript just tells us the argument is not assignable to the expected parameter type, but it doesn't say why (because the second inference is different from the first?).

If I understand the situation correctly, I will attempt a proposal.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 30, 2017

Some historical context, the behavior has changed in TS 2.4, and we have tried both.. so consider:

declare function add<T>(list: T[], newItem: T): void;

add([1, 2], "string");

TS 2.4 and later

a.ts(3,13): error TS2345: Argument of type '"string"' is not assignable to parameter of type 'number'.

Before TS 2.4

a.ts(3,1): error TS2453: The type argument for type parameter 'T' cannot be inferred from the usage. Consider specifying the type arguments explicitly.
Type argument candidate 'number' is not a valid type argument because it is not a supertype of candidate '"string"'.

Users have found the old error less helpful. we switched to pick the first candidate in cases inference ends up with multiple irreducible candidates to make the error closer to the arguments (like you mentioned with Elm), the idea is that would help you fix it better than telling you string and number are both inferences, and they do not match.

My comment earlier was if we stop doing what we are doing today, and instead go back to the old behavior where all we know is that number and string are candidates, and they neither is a subtype of the other.

I think the current error is better, but we are always open to suggestions to make the experience better. as a matter of fact, that change in TS 2.4 was a user suggestion as well (can not seem to find the link though).

@OliverJAsh
Copy link
Contributor Author

Very interesting, thank you for that explanation.

I actually think both of those error messages are useful. The current error message tells me what the exact type issue is, and the former error message gives me some information as to the cause. Perhaps we could combine the two somehow?

Out of interest, what would be the equivalent pre TS 2.4 error for the match example I gave? I'd like to see how it deals with more complicated examples.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 30, 2017

a.ts(5,5): error TS2453: The type argument for type parameter 'T' cannot be inferred from the usage. Consider specifying the type arguments explicitly.
  Type argument candidate '"oops"' is not a valid type argument because it is not a supertype of candidate '42'.
a.ts(21,5): error TS2453: The type argument for type parameter 'T' cannot be inferred from the usage. Consider specifying the type arguments explicitly.
  Type argument candidate 'number' is not a valid type argument because it is not a supertype of candidate 'string'.

@OliverJAsh
Copy link
Contributor Author

Continuing with the examples above, does something like this work? (Probably not, I'm most definitely missing edge cases.)

Argument of type '42' is not assignable to parameter of type 'string'.

Hint: these types must match because they share type parameter 'T'. The second instance of type
parameter 'T' ('42') is different from the first ('string'). If this is intended, the type parameter
must be annotated as a union of these different types: `42 | string`.
Argument of type '{ foo: number; bar: string; }' is not assignable to parameter of type '{ foo: number; bar: number; }'.
  Types of property 'bar' are incompatible.
    Type 'string' is not assignable to type 'number'.

Hint: these types must match because they share type parameter 'T'. The second instance of type
parameter 'T' ('string') is different from the first ('number'). If this is intended, the type
parameter must be annotated as a union of these different types: `number | string`.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 1, 2017

the issue is not the error text, it is how the error would be produced in the compiler. the two processes, inference, and overload resolution are rather detached..

@OliverJAsh
Copy link
Contributor Author

OliverJAsh commented Jun 12, 2019

@RyanCavanaugh

If the provided call to match is indeed valid, then match should really have two type parameters and return a union of them

What if match is defined dynamically, as is the case in Unionize?

https://github.com/pelotom/unionize/blob/6c4a14462f83984cbfa03aa59fa03304214cd9b1/src/index.ts#L44-L47

In that case it's not possible to provide a generic for each parameter, since the number of parameters is dynamic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Error Messages The issue relates to error messaging Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants