Contextually type implemented properties

[sandersn]

Fixes #3667. Fixes #1373.

interface I {
  f(n: number): string;
  p: string;
}
class C implements I {
  f = n => n.toString(); // n is correctly typed as `number`
  p = undefined; // correctly retains type `string`
}

Interesting baseline changes in the latest commit:

let x: number;
let y: string;
x = y = undefined;

is now illegal. This is a breaking change.

let [...r] = null

This is now legal because null gets contextually typed as an array, although node says "Cannot read property 'Symbol(Symbol.iterator)' of null" when running the code.

There are also a couple of implicit any errors that went missing from wideningTuples3 and wideningTuples5. I still need to find out why the error no longer fires:

var a: [any];
var b = a = [undefined, null];

Should have an an error when b implicitly gets the type [any, any].

[ahejlsberg]

change to declaration: PropertyDeclaration.

[DanielRosenwasser]

Since the first argument never changes, you could factor it out to a constant.

[DanielRosenwasser]

I'm not so sure this works - doesn't this map all undefined and null types in the expression? So if I have

class Base {
    a: (x: string) => void;
}

class Derived extends Base {
    a = (x = null) => {}
}

won't x in Derived get the type (x: string) => void meaning a will have the type (x: (x: string) => void) => void.

Maybe I misunderstood.

[JsonFreeman]

+1

Also, I think the TypeMappers are supposed to be for generics.

[sandersn]

Yes, Anders explained contextual typing in person and I'll push new changes with getContextualType as soon as I make sure that all the baseline changes are correct.

[sandersn]

Here's why wideningTuples3 fails to report a noImplicitAny error:

var a: any[];
var b = a = [undefined, null];

a contextually types [undefined, null]: any[], which in turn contextually types undefined and null as any. Then b infers the type [any, any] since it has an array of two elements, each of type any.

But this doesn't trigger the implicit any check in reportErrorsFromWidening because it looks for types that will be widened to any -- undefined or null. But these particular undefined/null values have already been assigned the type any. It's arguably no longer implicit since their types were contextually propagated from the explicit var a: any[].

I'm not sure whether this needs to be fixed, or whether it's an obscure enough corner case to let the technicality slide.

[sandersn]

After discussing with Mohamed, I think dropping the implicit any error is the right thing in this case.

[RyanCavanaugh]

Observable types from contextually-typed expressions have always been very weird, so I don't think it's much to worry about. I think that code has changed meaning more than once since 1.0.

[JsonFreeman]

I agree with @mhegazy and @sandersn about dropping the no implicit any error.

[DanielRosenwasser]

Remind me, does this also fix #1373?

[sandersn]

Running on real world code

We discussed this PR in the design meeting last week and decided that it was too inconsistent and didn't capture intent well enough. Instead, I tried a simpler approach that gives inherited properties exactly their inherited type (unless there is a type annotation). The change is found in the branch contextually-type-inherited-properties-WIP. I ran this branch on our real world code base consisting of Microsoft internal code as well as projects like Angular and Grunt. This showed that the new change broke more than it helped. Since both approaches failed, I am going to close this PR -- we may revisit this problem in the future.

The good

The improved typing exposed one bug that happens twice in the same project:

interface I {
  method(o: SomeType): string;
}
class C extends I {
  method(o) {
    return o.doesNotExist;
  }
}

Before this change, o was implicitly any in the parameter list of C#method.
Now an error is reported because o: SomeType, which does not have a
property named doesNotExist.

Both methods would have caught this bug. So would --noImplicitAny.

The bad

Consider the following example:

namespace Base {
    export interface Item {
        a;
    }
    export abstract class Base {
        abstract item: Item;
    }
}
namespace User {
    export interface Subitem extends Base.Item {
        sub;
    }
    export interface Contract {
        item: Subitem;
    }
    export function createC(): Contract {
    }
    function createSubitem(): Subitem {
        // factory code here ...
    }
    class C extends Base.Base implements Contract {
        item = createSubitem();
    }
}

The intended usage is something like

let c: User.Contract = User.createC();
console.log(c.item.a);
console.log(c.item.sub);

Unfortunately, if C.item takes its type from Base.item, then
C.item: Item. Then C no longer implements Contract because
Contract.item: Subitem.

The ugly

In several projects, properties of type KnockoutObservable changed to
KnockoutObservableBase, which is a superclass that has fewer
properties. This can be fixed by upgrading to a non-ancient version of
knockout.d.ts, though. Current versions of knockout.d.ts don't have
KnockoutObservableBase.

[DanielRosenwasser]

👍 Thanks for reporting on what sorts of problems/improvements this brought.

[tinganho]

if A extends B and implements C. Why doesn't it just contextually type based on members of B and C and when there exists common members contextually type based on the most sub subtype member of B and C?

Wouldn't that remove the bad and ugly part?

[JsonFreeman]

@tinganho I believe that in general there is not a lowest subtype for a set of types. The type system does not provide a total ordering.

[DanielRosenwasser]

Keywords: base derived methods properties property contextual contextually type subclass superclass