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

Allow calls on unions of dissimilar signatures #29011

Merged
merged 13 commits into from
Dec 20, 2018

Conversation

weswigham
Copy link
Member

Fixes #7294 with caveats.

function test1() {
    type stringType1 = "foo" | "bar";
    type stringType2 = "baz" | "bar";

    interface Temp1 {
        getValue(name: stringType1): number;
    }

    interface Temp2 {
        getValue(name: stringType2): string;
    }

    function test(t: Temp1 | Temp2) {
        const z = t.getValue("bar"); // Should be fine
    }
}

The above now has no errors, and z is of type number | string.

The caveats are that only one type in the list of union members is allowed to have overloads, and only one type within the list of union members is allowed to have type parameters. The first is because creating the power set of overloads results in a (long) list of overloads whose ordering is unclear. This is maybe surmountable in the future, but for now this seems like a sane place to stop. The second is because introducing multiple sets of type parameters requires unifying them, as multiple type parameters in the same position likely do not infer well (eg, (arg: A & B & {x: string}) => A | B | {x: string}). Again, with more infrastructure, this is potentially fixable, but for now, this seems like a good stopping point. In the case of a union where either of those things are true, we simply do not create a union signature and fall back to the old behavior of not making a signature at all (and so you'll get the "type has no call or construct signatures" error).

There's one open question here which is contextual typing: In the new test I added, I included one of the motivating examples: using forEach on a union of array types:

function test3(items: string[] | number[]) {
    items.forEach(item => console.log(item));
}

With this change, that is now callable, but the argument to the callback you pass will be implicitly any if defined inline. This is because the argument types are intersected, and the intersection of two signatures currently creates an overload list of signatures. As-is, we have logic that explicitly removes contextual typing from overload lists, so no contextual type is provided. This seems incorrect - I believe fixing this could go in one of two ways:

  1. Generate a contextual type from an overload list with similar requirements/caveats that union signature creation has.
  2. Generate a combined intersection signature just like we now do for union signatures, rather than an overload set.

Even as-is, this seems like a huge improvement over what we do today; we could also choose to do nothing and require an annotation and still be better off than now. I'd rather we make the usecase work fully without user code changes, but it's still better than what we do in master.

I also have one small TODO I'm still working on (and then writing tests for) which is deriving the names for the amalgamated signature's parameters from the input signatures' parameter names. It's more complex than it was the last time I tried this since we added tuple types - I don't really know how to look up symbol names through the effective argument types which may have been fulfilled by a tuple.

@weswigham
Copy link
Member Author

Post-design meeting update. In the meeting, we said we wanted to look at changing contextual typing based on overloading. Looked at it, and recognized that when applied, things like

declare var c: {
  (x: number): number;
  (x: string): string;
}
declare function f(a: typeof c): void;

f(x => x);

which was not an error outside noImplicitAny mode (as x had no contextual type and was implicitly any) becomes an error (as x is assigned number | string which then flows to the return type and is incompatible with both overload return types).

In lieu of fixing this now, we think we'll wait on being able to properly run a lambda in multiple contexts to fix that and leave this PR as is (so the .forEach case is still an implicit any error).

Also: I've finished the TODO in the OP - parameter names are now copied into the combined signature (and there are tests verifying this).

@ahejlsberg this is done and could use a review now.

src/compiler/checker.ts Outdated Show resolved Hide resolved
src/compiler/checker.ts Outdated Show resolved Hide resolved
src/compiler/checker.ts Outdated Show resolved Hide resolved
src/compiler/checker.ts Outdated Show resolved Hide resolved
src/compiler/checker.ts Outdated Show resolved Hide resolved
src/compiler/checker.ts Outdated Show resolved Hide resolved
@weswigham
Copy link
Member Author

As discussed, here's a note for future reference:

This PR allows unions of signatures to (almost always) be callable, however take the union var c: (x: number) => string | (x: string) => number. I can call c, but there is no value I can use for the first parameter that actually satisfies both signatures (except something of type any). Our error message in this case will be that your argument type is not assignable to string & number, which is pretty easy to understand. However if that vacuous intersection is simplified to never by the presence of a union (eg, one parameter type is boolean | number and the other is string), it can be harder to understand. So a signature always exists and is always callable, however individual parameters may be unsatisfiable, and that may be represented with the never type for that parameter. Keeping these signatures around is important for correctly checking any other satisfiable arguments (this way you can continue in the presence of errors), allowing just the parameter to be cast to allow the unsatisfiable call (rather than casting the variable you're calling and losing checking on the other arguments), and for allowing the type to be satisfied in contravariant positions (eg, the parameter of a parameter) where the parameter can simply be ignored or unused.

@weswigham
Copy link
Member Author

And as I mentioned in person, if we really hate the Argument of type {0} is not assignable to parameter of type 'never'. error message, we can specialize argument errors to never globally to something we find nicer.

@weswigham weswigham merged commit 08022d5 into microsoft:master Dec 20, 2018
@weswigham weswigham deleted the dissimilar-union-signatures branch December 20, 2018 00:35
@lmcarreiro
Copy link

Does this solution resolve this case too?

type A = { x: string };
type B = { x: string, y: number };

const arr1: (A | B)[] = [];
arr1.find(e => e.x === ""); // OK
arr1.map(e => e.x); // OK

const arr2: A[] | B[] = [];
arr2.find(e => e.x === ""); // OK
arr2.map(e => e.x); // Error: Cannot invoke an expression whose type lacks a call signature.

@weswigham
Copy link
Member Author

find yes, map, no - map has overloads if I recall correctly, which are still quite troublesome.

@nomcopter
Copy link

Does an issue exist for the follow-up of this for overloads? The lack of .map() and .filter() on unioned arrays is pretty painful. Thanks!

@weswigham
Copy link
Member Author

I don't think so, but reference #31023 as maybe related if you open one~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Call signatures of union types
4 participants