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

Should not widen in type argument inference #1436

Closed
JsonFreeman opened this issue Dec 10, 2014 · 9 comments
Closed

Should not widen in type argument inference #1436

JsonFreeman opened this issue Dec 10, 2014 · 9 comments
Assignees
Labels
Breaking Change Would introduce errors in existing code Spec Issues related to the TypeScript language specification

Comments

@JsonFreeman
Copy link
Contributor

Compile the following using noImplicitAny:

function foo<T>(x: T): T { return x }

interface I {
    x: string;
}

var i: I = foo({ x: null }); // Error
var s: string = f(null); // No error

Neither of these should be errors. According to the spec, both of these should widen, but in #531 @DanielRosenwasser and I explain why these should not widen.

The noImplicitAny error resulting from widening is a breaking change from 1.0, and it broke a customer.

@JsonFreeman JsonFreeman added Bug A bug in TypeScript Breaking Change Would introduce errors in existing code Spec Issues related to the TypeScript language specification labels Dec 10, 2014
@ahejlsberg
Copy link
Member

We have to widen because foo can return an arbitrary (circular) type in which the null or undefined gets buried. We would require a new algorithm to traverse such a type to find and widen the null and undefined types. Right now we know that the types we widen are never circular.

@DanielRosenwasser
Copy link
Member

Could you give an example of such a type just to document on this issue?

@sophiajt sophiajt added this to the TypeScript 1.5 milestone Jan 12, 2015
@JsonFreeman
Copy link
Contributor Author

I think the following code would break if we do this:

declare function makeArray<T>(x: T): T[];
makeArray(null)[0] = 0; // Error that number is not assignable to null

I am not sure if there are useful examples where the user would blocked by this. But the point is, it leads to null and undefined leaking out in a lot more places. The cost of doing this would involve taking inventory of all the places where this could happen and widening in all those places.

@JsonFreeman
Copy link
Contributor Author

Then again, the following code is already an error because of a null leak:

[null][0] = 0;

So this may not really have to do with type argument inference at the core.

@osdm
Copy link

osdm commented Apr 9, 2015

Hi @JsonFreeman ! You say "According to the spec...". Could you point me, where does the spec says about widening in the type inference? I searched through 1.3, 1.4 and 1.5 versions of the spec and found nothing about widening in the type inference. Maybe it's just me being dumb...

@JsonFreeman
Copy link
Contributor Author

Hmm, now I can't find it anymore. I remember it being in the second bullet point in section 4.12.2 (Type argument inference).

@ahejlsberg, was it intentional that widening at the end of type argument inference was removed from the spec?

@osdm
Copy link

osdm commented Apr 9, 2015

@JsonFreeman Ok, I was wrong about 1.3, sorry. Widening is really there. But it disappeared in 1.4.

@JsonFreeman
Copy link
Contributor Author

Ok, I see it was removed in 7a2701b. I think this was just an oversight, as the definition of C moved inside the bullet. The change we were trying to make was just to add the capability for type argument inference to error. This was at the time we introduced union types.

@mhegazy mhegazy modified the milestones: TypeScript 1.5, TypeScript 1.5.1 Apr 23, 2015
@mhegazy mhegazy removed the Bug A bug in TypeScript label May 27, 2015
@mhegazy mhegazy modified the milestones: TypeScript 1.5.2, TypeScript 1.6 Jun 17, 2015
@JsonFreeman
Copy link
Contributor Author

See my comment on #3733. Also, there is a nice sample their by @bergmark that exercises contrasting cases.

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 Spec Issues related to the TypeScript language specification
Projects
None yet
Development

No branches or pull requests

6 participants