microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.06k stars 12.37k forks source link

Widening after inferring null or undefined for a type parameter should be an implicit any #531

Open mhegazy opened 10 years ago

mhegazy commented 10 years ago

Courtesy of @JsonFreeman

function f<T>(t: T): T { return null }
f(null);

When compiling with -noImplicitAny, we don't get an error here, even though T is implicitly inferred to be any when we call f

sophiajt commented 10 years ago

I think the no implicit any error should be on the 'null' itself.

JsonFreeman commented 10 years ago

Nope, it would be at the widening point, which is at the very end of type argument inference.

DanielRosenwasser commented 10 years ago

I discussed this offline with @vladima and @JsonFreeman and we made some really great findings - that this actually shouldn't be an error!

Consider the return type of the given call: f(null). As stated, its final type is resolved to any, however, it would appear that it doesn't really matter - we're not doing anything with that type, and its type impacts checking in no way.

As a motivating example for why this is true, let's think of a function whose return type doesn't even rely on the generic type:

function compare<T>(a: T, b: T): number {
    return a < b ? -1 : (b < a ? 1 : 0)
}

compare (null, null);

Recall that the safety of functions with generic type parameters are unaffected by the types they are instantiated with, so our declaration is "safe" regardless of our type arguments.

So what about our use of compare? Is it an issue that we can pass null in and that T is instantiated to any? No! Well, not any more than if T was explicitly instantiated to DuckBilledPlatypus and passed in null and null. null and undefined can flow in to anywhere, and it's up to the function implementation to guard against this appropriately.


So we've shown that f(null) isn't actually a problem if nobody tries to use the result. What if someone does?

var x = f(null);

Of course, now you should care that x gets assigned the type any, and we'd like for '--noImplicitAny' to raise a flag for us. By now I hope you're convinced that we should actually be reporting on the assignment itself.

But actually, we don't catch this because we perform a widening when we instantiate type arguments, so we miss the chance to catch the widening process.

It would seem that this warrants a change to contextual typing - not in an effort to accommodate '--noImplicitAny', but rather, because '--noImplicitAny' has given us some indication in how we should appropriately let types flow.

Here is our proposed change:

  1. When contextually typing the parameters of a function expression, or the indexers of an object literal, use the widened type of the contextual type, reporting implicit-any errors if necessary.
  2. Do not widen the result of best common type in type argument inference.
JsonFreeman commented 10 years ago

Well summarized Daniel! To clarify some of the points here, in the initial example, the return type of the call f(null) doesn't matter only if it does not flow anywhere. There are only 2 places where an inferred type argument can be observed: the return position of the call and the contextual types of the arguments. The return position of the call is subject to normal expression widening rules. For contextual typing, here is a salient example:

declare function filter<T>(arr: T[], func: (x: T) => boolean): T[];
var filtered/*implict any here*/ = filter([null], x/*implicit any here*/ => true);

The x inside the lambda is the reason for part 1 of the proposal.

mattmccutchen commented 7 years ago

FWIW, I just missed an implicit-any error in my code due to this issue:

// Initialize a "height x 0" array of arrays.
let grid = _.range(0, height).map((i) => []);
// Append to rows...

The type of the empty array was inferred as undefined[] and then widened to any[] without error. (Hopefully adding some keywords will help the next person who hits this with an empty array but isn't familiar with widening to find this issue.)

JsonFreeman commented 7 years ago

As I understand it, there has been a big shift to better integrate null and undefined into the type system, though I'm not up on the details. In 2014, when this issue was last looked at, these types were sort of the dark corners of the type system. I think it is worth asking whether this widening step still needs to occur given the latest semantics for null and undefined types.

UselessPickles commented 6 years ago

I just encountered a much simpler situation where I expected "noImplicitAny" error:

// Type of 'something' is 'any'! No error :(
let something = null;

Is this the same issue, or should I create a new one?

mattmccutchen commented 6 years ago

@UselessPickles: That any doesn't indicate a type safety problem, since TypeScript's control flow analysis will compute a more specific type for the variable each time it is used. If you like, you can file an issue (after checking for an existing one) to have the language service show something more informative than just any.