hmil / tslint-override

TSLint plugin bringing support for the override keyword
MIT License
27 stars 6 forks source link

Allow opting out variables #23

Open koreem opened 5 years ago

koreem commented 5 years ago

Can we opt out variables? I don't want to see a warning/error when I use an overridden variable, only methods.

hmil commented 5 years ago

I don't want to see a warning/error when I use an overridden variable

It should only warn when you redefine a variable from the parent class, which is a very dangerous behavior. It shouldn't warn when you use it.

There is no mechanism to opt out variables yet. I'm open to discussing the feature if you can provide a compelling use case for it. Please read the note below on why variable overrides are dangerous and should be avoided. Also, it'd be great to see how the TS compiler deals with these cases, and find out if there is some overlap between the compiler and this plugin. If the dangerous cases are already handled by the TS compiler, then it could make sense to add an opt-in setting to exclude variables from the inspection.


Side note: Why is it so dangerous to override member variables?

If the base class defines pet: Animal, and you override it as pet: Cat (type narrowing), then what if the base class, needs to set pet = Dog()? Or if someone casts your CatOwner into a PetOwner and gives him a Dog?
Similarly, say the base class defines pet: Cat and you override it as pet: Animal (type widening). Then you may assign pet to a Dog instance, and then something might blow up in the base class because it assumed pet to be Cats, not Dogs.
As you can see, neither narrowing nor broadening the type is safe.

Method overrides are safe if the method parameters is a supertype of the base class method parameters, and the return value is a subtype of the base class' method return value. How come this works for functions but not variables?
Functions are generally considered immutable, so you'll expect both the base class' and the class consumers to just "call" the function, not to modify it. It doesn't work for variables because you expect both the base class and the consumers to modify and read the variable, which creates an upper and lower type bound whose only solution is to preserve the type defined by the base class.
However, if the variable is public readonly (as it should in proper OO/FP design), then overriding it with a more specific type is safe. Because the only statement you'll expect to encounter is val somePet: Pet = yourClass.pet. You can trivially see that if pet is more specific, this won't be a problem.

Anyway, as you can see, variable overrides aren't trivial and it is a good thing that this module warns you when they happen. I wonder if the TS compiler already deals with these cases though 🤔