Overriden class member annotated with a subtype could be a recipe for disaster (by design.. ;) )

[malibuzios]

TypeScript Version:
1.9.0-dev.20160505 with both strictNullChecks and noImplicitAny enabled.

Code:

class Animal {
    name: string = "Any animal";
}

class Dog extends Animal {
    name: string = "Dog";

    bark() {
        console.log("bark");
    }
}

class AnimalCage {
    member: Animal = new Animal();
}

class DogCage extends AnimalCage {
    member: Dog;
}

let dogCage = new DogCage();

if (dogCage.member != null) { // Yup, it is even checked for null or undefined, 
                              // but it turned out that wasn't enough..
    dogCage.member.bark(); // Bang!
}

Expected behavior:
Design a language where inheritance actually works.
(Edit: I was felling very 'passionate' when I wrote this.. sorry :) nothing personal)

Actual behavior:
By design:

    dogCage.member.bark();
                   ^

TypeError: dogCage.member.bark is not a function
    at Object.<anonymous> (C:\X\Documents\Drafts\TypeScript\Code\a.js:38:20)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:456:32)
    at tryModuleLoad (module.js:415:12)
    at Function.Module._load (module.js:407:3)
    at Function.Module.runMain (module.js:575:10)
    at startup (node.js:159:18)
    at node.js:444:3

Resolution:
tslint volunteers, for the rescue!

[kitsonk]

Personal opinion, emotive statements like "Design a language where inheritance actually works." devalue your contribution and make people not want to listen to you.

It isn't necessarily be design here is potentially something the compiler could identify, though it would be a feature enhancement, which might actually have merit. I suspect what you would like to see:

• If a super class has a an initializer (ergo defined and assigned), then a subseuent attempts to override the type that is not assignable to that initialized type should throw at design time.

[malibuzios]

I wrote this already feeling under 'attack' expecting extreme negativity and hostility from the team, including claims that I'm intentionally trying to 'waste' their time with 'unimportant' matters.

Other things they would say/do:

• "This would be a 'breaking change'"
• "This sounds more suitable for a linter"
• "We don't do one on one"
• Immediately close the issue
• Label with Question
• Label with Canonical
• Label with Duplicate

Perhaps this time I'm not even interested to be personally taken seriously anymore. I'm interested in the subject to be taken seriously, maybe despite the way it is presented..

I don't think that the particular solution you offered would be sufficient, member could be initialized at any point, including in any method of the base class long after construction.

There isn't really any solution I can offer at this point.

[malibuzios]

member could also be reassigned with Animal even after Dog was initialized correctly. This could even happen several times, long after both of them have been initialized.

[malibuzios]

It seems like it wouldn't even help if AnimalCage was an abstract class and member was annotated as abstract:

abstract class AnimalCage {
    abstract member: Animal = new Animal(); // no error

    methodInAbstractClass() {
        this.member = new Animal() // no error
    }
}

(I apologize for sounding a bit 'emotive' in the original post, maybe I need to relax a bit.. :) )

[malibuzios]

The same happens with interfaces, but I don't feel it looks that 'bad' in comparison (emphasize in comparison):

interface Animal {
    name: string;
}

interface Dog extends Animal {
    bark(): void;
}

interface Cat extends Animal {
    meow(): void;
}

interface AnimalCage {
    member: Animal;
}

interface DogCage extends AnimalCage {
    member: Dog;
}

interface CatCage extends AnimalCage {
    member: Cat;
}

declare let animalCage: AnimalCage;
declare let dogCage: DogCage;
declare let someRandomCat: Cat;

animalCage = dogCage; // implicit covariant cast here
animalCage.member = someRandomCat; // another implicit covariant cast here
dogCage.member.bark(); // runtime error

Here it caused by the implications of implicit covariant casts, and I can understand it, to a degree.

However, in the original example with classes there were no implicit 'cast's being done, it was simply a result of allowing subtypes to be declared for overriding members without any 'evidence' they are actually instantiated at all. Since a class is a concrete 'entity', unlike an interface, this is something that the compiler can technically determine in practice, as it has all the implementation code right in front of it.

[RyanCavanaugh]

This certainly isn't desirable behavior.

That said, I'm not sure how we could detect this reliably. For example, if AnimalCage came from a .d.ts file, we have no idea if it was initialized or not (though I suppose with --strictNullChecks on, we have to assume it was). Without having covariance annotations I don't think there's a possible fix that covers all cases, but I'm open to ideas if you have any.

[mhegazy]

(though I suppose with --strictNullChecks on, we have to assume it was)

it should be an error to have an unintialized property whose type does not include undefined. Filed #8476 to track that

[malibuzios]

@RyanCavanaugh

Thanks for taking this seriously. I apologize for the silly/emotional way I for some reason decided to present it. I did not initially believe there was anything that could be done about it aside from a linter rule.

Referring to the original example, it seems like a small step would be to realize that despite the fact the member is technically 'initialized' during runtime in the derived class, in the sense that it has a value, it isn't actually initialized for the type that it was declared to, so it could be considered to be 'uninitialized' in a more abstract sense.

You mentioned that AnimalCage could technically be ambient, so there's no way to know if member was initialized at all, so this could qualify as a nullability check, at least for the case where it is declared non-nullable, which is still a meaningful improvement. So it could be that this unintentionally revealed a somewhat unrelated enhancement that could at least be made for a more general case.

However, even after it is properly initialized, it could still be reassigned to Animal at any time by a method in the base class, this includes even the case where AnimalCage was an abstract class:

abstract class AnimalCage {
    abstract member: Animal;

    doSomething() {
        this.member = new Animal();
    }
}

class DogCage extends AnimalCage {
    member: Dog = new Dog();
}

let dogCage = new DogCage();
dogCage.doSomething();
dogCage.member.bark(); // runtime error

[RyanCavanaugh]

It's fairly unavoidable without massively complicating the type system (the same thing can be seen by e.g. aliasing an HTMLDivElement[] with a HTMLElement[] reference -- you can then push in an HTMLImageElement, which is even worse).

In this specific case, it'd be desirable to rewrite the classes using generics. If you wrote it this way, for example, you'd correctly get an error:

class Cage<T extends Animal> {
    member: T;

    doSomething() {
        // Disallowed
        this.member = new Animal();
    }
}

class DogCage extends Cage<Dog> {
    member: Dog;
}

[malibuzios]

@RyanCavanaugh

Leaving the more general 'design' issues aside for now,

I understand by this that even if the type of the derived member was the same as the base, and it was known to already be initialized in the base class (i.e. this means the base class couldn't be ambient), it would still error with strictNullChecks?

class A {
    prop: number = 1;
}

class B extends A {
    prop: number; // would this error here that 'prop' is not initialized?
}

And here?

class A {
    prop: number = 1;
}

class B extends A {
    // would this error here that 'prop' is not initialized?
}

[RyanCavanaugh]

Those definitely wouldn't be errors.

[malibuzios]

@RyanCavanaugh

OK, so it would also look at base classes, but what about:

class A {
    prop: Animal = new Animal()
}

class B extends A {
    prop: Dog; // would this error here that 'prop' is not initialized to an exact matching type?
}

In order to achieve this, it would need to simply test if Animal and Dog are strictly structurally identical before coming to the conclusion that prop was initialized in the base class.

I feel this could be a significant step here (or perhaps an opportunity since nullability checks have not yet made it to production - no breaking changes involved). I also feel this would be one of those places that programmers may wonder a little what went wrong for getting this error. After all, prop does technically have a value, only it is for a 'less capable' type. However, they may eventually 'thank' the compiler of being 'smart' enough for noticing something that may not be completely trivial for a distracted or tired human to notice.

If the intention was for a contravariant cast, it could perhaps be done like this:

class A {
    prop: Animal = new Animal()
}

class B extends A {
    prop: Dog;

    constructor() {
        super();
        this.prop = <Dog> super.prop; // this isn't valid syntax today, only used for illustration
    }
}

I must say in the years I've used this language, I don't think I ever had a situation where I wanted to perform a contravariant cast from the initialized value of a base class to the more capable type in the derived class. If the intention was to initialize to Dog then it could simply initialize it in the derived class? why would it need to be initialized in the base class through a cast? in that case, the type of prop in the base class should be Dog? or at least Dog | Animal?

[malibuzios]

Just as a side note, more generally, I don't think I've ever actually used a derived type for an inherited member, including cases where the base class was abstract.

[malibuzios]

Simple example:

class A {
    prop: any = 1234;
}


class B extends A {
    prop: string; // is this initialized?
}

[ghost]

@malibuzios it sounds like your issue might be tightly related to #10717? The base class initialized "prop", and "prop: any" is considered bivariantly compatible with "prop: string", so therefore it's initialized.

The bug isn't really in the initialization checker, but a "feature" of how Typescript computes "assignable to" (per section 3.11.2 of the spec, and even has its own FAQ entry).

You can see this bug/feature even more easily here:

var a: A;
a = { prop: 42 };
var b: B;
b = a; // wat?
b.prop.substr(2); // runtime error.

The above code will check as "good" with every single strictness feature of typescript 2.0, all because (for properties and methods) all 'any's are considered instances of string. That sounds backwards right? It is, and it's unsound, but it's convenient for many JavaScript libraries.

I'd suggest tracking issue #10717, which is the only open issue I found discussing it. The issue has been raised many times over the years (e.g #222), but as a colleague noted to me, this time around the type checker is configurable, so perhaps in some future release there will be an option for stricter checking.

I hope that helps.

[nicolo-ribaudo]

Is this fixed now that we have declare for class fields?

[JoshuaKGoldberg]

I think this can be happily closed as resolved now that --strictPropertyInitialization is available. The original issue's snippet gives a Property 'member' has no initializer and is not definitely assigned in the constructor. type error now with the compiler option enabled playground link. Progress!