Skip to content

Regression in type inference? #12733

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
olydis opened this issue Dec 7, 2016 · 4 comments
Closed

Regression in type inference? #12733

olydis opened this issue Dec 7, 2016 · 4 comments
Labels
Breaking Change Would introduce errors in existing code Fix Available A PR has been opened for this issue Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@olydis
Copy link

olydis commented Dec 7, 2016

TypeScript Version: 2.1.1, 2.1.4

Code

declare function func(callback: () => void): any;
declare function func(callback: (arg: number) => void): any;

func(argx => { });

Expected behavior:
Successfully compile, even if --noImplicitAny is set. This is the behavior we saw until 2.0.10 (which appears reasonable, the second overload of func allows inferring that argx: number).

Actual behavior:
error TS7006: Parameter 'argx' implicitly has an 'any' type

@mhegazy
Copy link
Contributor

mhegazy commented Dec 8, 2016

Previously the compiler silently gave argx a type any. The reason is how the compiler resolves function expressions while doing overload resolution. the change in #11905 just exposed the error. So there is no change in what types were inferred, the change is the new error being reported.

you can work around this by specifying an explicit type annotation on argx , or removing the first signature, since it does not add value here.

@mhegazy mhegazy added Working as Intended The behavior described is the intended behavior; this is not a bug Breaking Change Would introduce errors in existing code labels Dec 8, 2016
@olydis
Copy link
Author

olydis commented Dec 8, 2016

Thanks! I'm surprised the compiler silently gave argx type any, after all the tools (e.g. VSCode) show argx: number on hover, so there seems to be a difference between the TypeScript language server and compiler?!

Wait, as I understand #11905, it should skip func(callback: () => void) now during resolution? Doesn't that precisely mean that number instead of any should be inferred?

The workaround is no problem, just thought I'd let you know since this broke the unit tests of https://github.com/Azure/autorest/ today (thousands of errors). Note that we can not remove "the first signature", in our case this is owned by mochajs!

For example, see https://github.com/Azure/autorest/blob/master/src/generator/AutoRest.NodeJS.Tests/AcceptanceTests/validationTests.ts#L34
done is the thing that now supposedly has type any (while VSCode reports MochaDone), it has a number of overloads including the one that "does not add any value" (but that is mochajs code, not ours).
We based all our tests on https://mochajs.org/#asynchronous-code which worked great without type annotations so far, now everything broke.

Cheers from building 18.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 8, 2016

Wait, as I understand #11905, it should skip func(callback: () => void) now during resolution? Doesn't that precisely mean that number instead of any should be inferred?

that was an earlier iteration of the fix. the final change did not do that.

The workaround is no problem, just thought I'd let you know since this broke the unit tests of https://github.com/Azure/autorest/ today (thousands of errors). Note that we can not remove "the first signature", in our case this is owned by mochajs!

I would change the definition in mocha.d.ts and send a PR to DT. so that other ppl do not have to run int this.

the issue is the compiler does not handle overload on arity for callbacks now, which is this case.

@olydis
Copy link
Author

olydis commented Dec 8, 2016

Ahhh I see, thanks!

@mhegazy mhegazy added this to the TypeScript 2.1 milestone Dec 8, 2016
@mhegazy mhegazy closed this as completed Dec 8, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Feb 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Breaking Change Would introduce errors in existing code Fix Available A PR has been opened for this issue Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants