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

Function return type is not inferred when used in generic position #10698

Closed
HerringtonDarkholme opened this issue Sep 4, 2016 · 9 comments
Closed
Labels
Duplicate An existing issue was already created

Comments

@HerringtonDarkholme
Copy link
Contributor

TypeScript Version: typescript@rc 2.0.2

Code

with noImplicityAny being true

function id<T>(p: T) {}
id(() => 123)

Expected behavior:

It compiles. id is inferred as function id<() => number>(p: () => number): void

Actual behavior:

It does not compile.
Compiler error:

Function implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.

p is inferred as () => any

This code works in TS1.8.

@mhegazy mhegazy added the Bug A bug in TypeScript label Sep 6, 2016
@mhegazy mhegazy added this to the TypeScript 2.0.3 milestone Sep 6, 2016
@yuit yuit self-assigned this Sep 8, 2016
@yuit
Copy link
Contributor

yuit commented Sep 12, 2016

After spending time debugging through our code in type-inference and contextual typing and talking with @vladima, the fix for this one will not be trivia at all and would require re-architecture on how we are doing type-inference/contextual typing. What happen here is that when compiler is checking arrow function, in trying to figure out return type of the function, it will then request contextual signature and trying to figure out return type of the contextual signature. This is the part that make this code broke in 2.0 but not 1.8, in 2.0 we introduce concept of literal type in 2.0 so in knowing whether the return type of the arrow function should be literal type of base primitive type (e.g number string) we end up having to figure out whether such position is sensitive to literal contextual type which in turn causes as to have to figure out return type of the contextual signature and the circulation begins.

@mhegazy mhegazy modified the milestones: TypeScript 2.1, Future Sep 29, 2016
@Psvensso
Copy link

Psvensso commented Sep 29, 2016

Found another observation that probably relates to this.

function myFunc():void {
    return ""; //Gives compiler warning 'string is not assignable to void' as expected
}

function badMyFunc(): () => void {
    return () => {
        return ""; // Gives no compiler warning, expected return type error
    }
}

@HerringtonDarkholme
Copy link
Contributor Author

HerringtonDarkholme commented Oct 1, 2016

I don't think this is that hard to fix, if I had read the code correctly.
As @yuit pointed out, we will getReturnTypeOfSignature twice: the first time is for resolving return type directly, and the second time is for figuring out contextual typing bound. For both times type checker will pushTypeResolution to mark whether a signature has been asked for resolution.

When asking for return type for the first time, compiler will not see signature marked so it proceeds to getReturnTypeFromBody, where contextual typing get resolved. To see whether a literal return value needs to be widen, compiler call getReturnTypeOfSignature again. This time compiler will see the signature marked so the return type of the signature is unknownType. Fortunately in getReturnTypeFromBody, unknownType is just used to determine whether return position isLiteralContextualType, so compiler just goes on and widens literal type.

Then the control flow comes back to the first time getReturnTypeOfSignature, where type is correctly resolved, e.g. number in the above example.

But popTypeResolution still gives false and compiler just blindly thinks the return type is unresolvable. If we first check if type is unknownType , then the correct answer can be found.

@HerringtonDarkholme
Copy link
Contributor Author

Any update here?

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Oct 10, 2016

@HerringtonDarkholme refer to this comment

the fix for this one will not be trivial at all and would require re-architecture on how we are doing type-inference/contextual typing

In other words, it's not going to be something we can fix anytime soon, assuming the investigation was correct. We'll review the PR to see if it is the right fix, though.

@masaeedu
Copy link
Contributor

I was experimenting with a fix for #15016 in this branch, I believe it solves this scenario as well:

type inference

@gcnew
Copy link
Contributor

gcnew commented Apr 16, 2017

I don't want to put your work down, but this example seems to be working with the vanilla compiler as well - link. I'm not sure whether the issue is still present in more complex use-cases though.

@masaeedu
Copy link
Contributor

masaeedu commented Apr 16, 2017

@gcnew You're right, I just blindly assumed the vanilla compiler was producing a compiler error for this scenario as described in the issue. The change I made is irrelevant here because no contextual typing is occurring.

Perhaps the issue was already fixed somewhere else? I'm not seeing any compiler errors in 2.2.2 with noImplicitAny enabled.

@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2017

Duplicate of #9659

@mhegazy mhegazy added Duplicate An existing issue was already created and removed Bug A bug in TypeScript labels May 18, 2017
@mhegazy mhegazy removed this from the Future milestone Apr 26, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants