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

Generic parameters constrained by polymorphic types allow unsafe operations but are not widened on return positions #7933

Closed
malibuzios opened this issue Apr 7, 2016 · 18 comments
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead

Comments

@malibuzios
Copy link

TypeScript Version:

1.9.0-dev.20160405

Code

class Animal {
    name: string;
}

class Dog extends Animal {
    bark() {};
}

class Cat extends Animal {
    meow() {};
}

class Box<V> {
    item: V;
}

function f<T extends Box<Animal>>(box: T) {
    box.item = new Cat(); // <-- This is perfectly valid and allowed by the constraint
    return box; // <-- The returned type is T
}

let result = f(new Box<Dog>()); // Type of 'result' is inferred as Box<Dog>
result.item.bark(); // <-- No compilation error. Run-time error..

Expected behavior:
Type of result should be inferred as Box<Animal>.
result.item.bark(); should give a compilation error.

Actual behavior:
Type of result is inferred as Box<Dog>.
result.item.bark(); does not error at compile-time but does error at run-time.

Comments:
Type of box should be widened from T to Box<Animal> within the body of the function, as there is no (current) way to ensure the members of box are not reassigned with types incompatible to the ones specified in T. The return type should therefore be Box<Animal> and not T.

[Edits: rewritten main example, heavily shortened and removed unneeded sections]

@malibuzios
Copy link
Author

Another way to state this is that generic constraints, as implemented today, ensure type-safety [for polymorphic types] in the 'read' direction, but not in the 'write' direction. Implicitly making the affected type immutable is a step forward, although I do understand it may be difficult to recursively apply this to all its sub properties etc.. [Edit: thinking about this more I'm not sure it would even be possible in some cases..]

Alternatively, if the type would be widened to the constraint in return positions, it would be similar to the effect of having the parameter type simply set to the constraint:

function f(box: Box<Animal>) {
    box.item = new Cat(); // <-- This is perfectly valid and allowed by the type
    return box; // <-- The returned type is Box<Animal>
}
let result = f(new Box<Dog>()); // Type of 'result' is Box<Animal>

So the benefit of having it in a constraint may be quite limited..

@malibuzios malibuzios changed the title Generic parameters constrained by polymorphic types allow unsafe assignments but are not widened on return positions Generic parameters constrained by polymorphic types allow unsafe operations but are not widened on return positions Apr 8, 2016
@malibuzios
Copy link
Author

In order to "convert" this type to a "safe" one for this context:

interface Example<T> {
    a: T;
    possiblyMutatingMethod(arg: T);
}

one would need to completely remove possiblyMutatingMethod and in general any method that contains the T type in any of its parameters (return types should be fine, I believe, but I'm not 100% sure). However since a may represent a setter and may mutate the object, it needs to be removed as well.. Leaving an empty interface that is quite useless. There is no way to just "mark" T as "immutable"..


Essentially what happens here is that it takes the the type Box<Dog> and substitutes it with the "more capable" type Box<Animal> and then substitutes it back to the "less capable" type Box<Dog> on return positions.

In C# this is not permitted. The answer by Eric Lippert cites the Liskov Substitution Principle:

The type system does not permit that conversion because doing so would violate the rule that the capabilities of the source type must not be less than the capabilities of the target type. (This is a form of the famous "Liskov substitution principle".)

@Arnavion
Copy link
Contributor

Arnavion commented Apr 8, 2016

The keywords you're looking for are covariance / contravariance, i.e. #1394

@malibuzios
Copy link
Author

@Arnavion

I don't think this is strictly related to covariance and contravariance. After further research I started to see it is particular to the behavior of the any type. It has to do with the fact that any can be assigned to anything (including number), this causes a generic type that has any as it its type argument to allow unsafe operations:

class Box<T> {
    item: T;
}

let boxContainingNumber = new Box<number>();
let boxContainingAnything = new Box<any>();

boxContainingNumber.item = 42;
boxContainingAnything = boxContainingNumber; 
boxContainingAnything.item = new Date();
boxContainingNumber = boxContainingAnything; // No error :`(

But this is not really the issue here (it is a different issue by itself). The issue is that the semantics of the constraint (e.g T extends Cage<Animal>) should provide the 'guarantee' that type is at least what is specified. However, Example<number> can only hold a number, it cannot hold 'anything' (just like Cage<Tiger> can only hold a tiger, not any animal).

Despite this fact it is still allowed to use it in this position. The fact that no one for more than 4 years has noticed that means that very few people actually understand it and I believe it should be reconsidered.

@Arnavion
Copy link
Contributor

Arnavion commented Apr 8, 2016

It is absolutely related. To be specific, TS treats types as covariant. Type T is assignable to type U if all members of T are assignable to U, which is the definition of covariance.

It also is not specific to any, as you can see from that issue thread. Eg arrays:

class Animal { foo: string; }
class Cat extends Animal { meow() { } }
class Dog extends Animal { bark() { } }

var cats: Cat[] = [new Cat()];
var animals: Animal[] = cats;
animals[0] = new Dog();
cats[0].meow(); // Blows up at runtime because cats[0] is a Dog

or union types:

var x: { y: number } = { y: 5 };
function f(a: { y: number | string }) {
    a.y = "foo";
}
f(x); // x.y is now "foo"

@T18970237136
Copy link
Contributor

Hi @malibuzios,
if I understand you correctly then you are describing two things.

The first one is that any is assignable to everything, and everything is assignable to any (AFAIK):

let f: any;
let c: number;
f = c = f; // works

This is by design. See the Any type in the Handbook/Spec.

The second is that generic types in Typescript appear to be covariant, just as @Arnavion described.
You are right that such behavior is not allowed in C# by default, but you can explicitely mark type parameters in generic interfaces as covariant or contravariant, if the type parameter is used in methods/properties and follows some rules (e.g. a contravariant type doesn't work as a return type, and a covariant type doesn't work as function argument type), see Variance in Generic Interfaces.

However, in Typescript, Function argument types are considered bivariant as described in the thread that @Arnavion linked, whereas in C# they are considered contravariant (so it makes sense to treat generic type parameters as covariant in TypeScript).

@malibuzios
Copy link
Author

@Arnavion

I understand what you are describing, but the particular aspect I'm concentrating on has one 'subtle' difference from the examples you gave:

The programmer is given the 'illusion' that a type is preserved and type-safety is guaranteed when passed and returned from the function.

In this example you gave: The programmer is at least 'informed' in some way that the type of y is widened from number to number | string in the body of the function:

var x: { y: number } = { y: 5 };
function f(a: { y: number | string }) {
    a.y = "foo";
}

However here there is no mark of that, most users would expect full type safety as the type T should be a 'black box' to the compiler (moreover, it is returned with the unsafe assumption that it still is T, but it isn't):

class Box<T> {
    item: T;
}

function f<T extends Box<any>> (box: T) {
    box.item = "ABCD";
    return box;
}

let x = f(new Box<number>());

[Edit: removed incorrect statements, see next comment]

@malibuzios
Copy link
Author

@Arnavion

You were right, this is not limited just to any, this was my mistake. Your first example worked better here:

class Animal {
    name: string;
}

class Dog extends Animal {
    bark() {};
}

class Cat extends Animal {
    meow() {};
}

class Box<V> {
    item: V;
}

function f<T extends Box<Animal>>(box: T) {
    box.item = new Cat(); // <-- This is perfectly valid and allowed by the constraint
    return box; // <-- The returned type is T
}

let boxWithDog = f(new Box<Dog>()); // Type of 'boxWithDog' is inferred as Box<Dog>
boxWithDog.item.bark(); // <-- Runtime error..

So the particular issue I'm talking about is the fact that f returns type T and not Box<Animal>, and that gives the 'illusion' of type safety.

(I've re-written the original comment to use this example instead)

@T18970237136
Copy link
Contributor

Hi,

So the particular issue I'm talking about is the fact that f returns type T and not Box, and that gives the 'illusion' of type safety.

f returns T because it returns a value of type T and you did not specify a explicit return type. If you want it to return Box<Animal>, you can declare it as such:

function f<T extends Box<Animal>>(box: T): Box<Animal> {
    box.item = new Cat();
    return box;
}

However, the fact that you can assign a Box<Cat> to a Box<Animal> is due to TS handling generic types as covariant, as @Arnavion already said, which (I think) has to do with function arguments being considered bivariant in Typescript, whereas they are considered contravariant in C#:

let fCat: (d: Cat) => void = d => d.meow();
let fAnimal: (d: Animal) => void;

fAnimal = fCat;
fAnimal(new Dog()); // TypeError: d.meow is not a function

This is explained in the FAQ: Why are function parameters bivariant?

Edit:
For example, with the Box<V> you could think of it having both a getter and setter for item:

class Box<V> {
    private item: V;

    getItem(): V {
        return this.item;
    }
    setItem(item: V): void {
        this.item = item;
    }
}

Now, If you assign a Box<Cat> to a Box<Animal>, you assign both getItem(): Cat to getItem(): Animal, which is OK because return types are covariant (this would also work in C# if you declare the V parameter to be covariant). This means: A function that returns a Cat is also returning an Animal.

You are also assigning a setItem(item: Cat): void to setItem(item: Animal): void, which is OK in Typescript due to the function parameters being bivariant. That case however would not work in C#, as there function parameters are contravariant. In C# this means: A function which "accepts" a Cat may not accepting an Animal, e.g. a dog.

@malibuzios
Copy link
Author

@T18970237136

What I basically say is that the fact that TypeScript allows implicit covariant casts still does not necessarily justify accepting these when deciding on compatibility with generic constraints.

You suggested the programmer should 'manually' declare the return type to improve type-safety, but isn't the whole idea of the compiler is that it should enforce type safety, well, automatically?..

@T18970237136
Copy link
Contributor

Hi @malibuzios,

What I basically say is that the fact that TypeScript allows implicit covariant casts still does not necessarily justify accepting these when deciding on compatibility with generic constraints.

I'm not sure I understand you here, but I have edited my last post adding an example with getters/setters. I think you are looking for the C# behavior here, which wouldn't allow a setItem(T item) method to be declared in a Interface Box<out T> which marks T as covariant, but would allow a T getItem() method to be declared:

    interface IBox<out T>
    {
        T GetItem(); // OK
        void SetItem(T item); // Error: T is covariant but needs to be contravariant
    }

Declaring T as covariant, C# would allow you to assign a IBox<Cat> to a IBox<Animal> just like your example with a method like

    T f<T>(T box) where T : IBox<Animal>
    {
        box.SetItem(new Cat());
        return box;
    }

which you could call as f<IBox<Dog>> and get back a IBox<Dog>, but you couldn't declare a setter in IBox that accepts an item of type T.

To be honest, I expected the same behavior in Typescript and was surprised that it treats function parameter types as bivariant instead of contravariant, but I think others can explain the reasons for this much better than me.

@malibuzios
Copy link
Author

@T18970237136

I will need some more time to fully understand what you are trying to say. But I will try to clarify because I'm not sure you have understood exactly what I meant:

  1. When the T extends Box<Animal> constraint is evaluated, Box<Dog> is implicitly cast to Box<Animal> which satisfies the constraint. That would be the covariant cast.
  2. When a value having type T is returned from the function, it is implicitly cast back from Box<Animal> to Box<Dog>. That would be an implicit contravariant cast!

Do you think this provides a reasonable level of type-safety, or perhaps the compiler could have done better?

@T18970237136
Copy link
Contributor

Hi @malibuzios,

When a value having type T is returned from the function, it is implicitly cast back from Box<Animal> to Box<Dog>. That would be an implicit contravariant cast!

I'm not sure what you mean with this. A Box<Animal> cannot be implicitly cast to Box<Dog>:

let o1: Box<Animal> = new Box<Animal>();
let o2: Box<Dog> = o1;  // Error:
// Type 'Box<Animal>' is not assignable to type 'Box<Dog>'.
//    Type 'Animal' is not assignable to type 'Dog'.
//        Property 'bark' is missing in type 'Animal'.

Instead, you have declared f as generic function with a type parameter T that has to satisfy Box<Animal>, which Box<Dog> does, due to the generic parameter T of Box<T> being covariant. And because f returns its parameter which is of type T, f<Box<Dog>> is of type (box: Box<Dog>) => Box<Dog>.

You could also simply declare f to be non-generic and you would get the expected compile-time error:

function f(box: Box<Animal>) {
    box.item = new Cat();
    return box;
}
let boxWithDog = f(new Box<Dog>());
boxWithDog.item.bark(); // Compiler Error: Property 'bark' does not exist on type 'Animal'.

Comparing with C# (I'm using C# examples because you mentioned it in your first post), you can declare a generic function f just like in your typescript example and a Interface IBox<out T> with a covariant type parameter T:

interface IBox<out T>
{
    T Item { get; }
    // Cannot declare a Item setter with type T
}
class Box<T> : IBox<T>
{
    public T Item { get; }
    public Box(T item)
    {
        this.Item = item;
    }
}

class Animal
{
    public string name;
}
class Dog : Animal
{
    public void bark() { }
}
class Cat : Animal
{
    public void meow() { }
}

class Program
{
    static T f<T>(T box) where T : IBox<Animal>
    {
        // Do something with the animal
        Console.WriteLine("Hi, " + box.Item.name);
        // Cannot call a Setter of box.Item with type T here
        return box;
    }

    static void Main()
    {
        IBox<Dog> dog = f(new Box<Dog>(new Dog()));
        dog.Item.bark(); // OK
    }
}

The difference between C# and Typescript here is that in C# you cannot declare a setter for IBox.Item, or a method like void SetItem(T item); because T has been declared as covariant whereas for that method it would need to be contravariant (and it cannot be both); or otherwise T would need to be invariant (which would prohibit calling f<IBox<Dog>> because IBox<Dog> cannot be cast implicitly to IBox<Animal> in that case).

In TypeScript however, such a method is possible due to the function parameters being bivariant (and if you have a property item: T; you can imagine it as a replacement for a getter() and setter() method). And due to this function parameter bivariance, Generic parameters of classes and interfaces are implicitely covariant instead of invariant (I think).

But as said, others can probably explain the reasons for function parameters being bivariant better than me (as I'm relatively new to TypeScript).

@malibuzios
Copy link
Author

@T18970237136

I will try to explain through the original example, this is really simple:

function f<T extends Box<Animal>>(box: T) {
    box.item = new Cat();
    return box;
}

let result = f(new Box<Dog>());
  1. In the body of the function f, the type T is functionally equivalent to Box<Animal>. For all purposes, everything that can be done with Box<Animal>, can be done with T.
  2. Now notice that Box<Animal> is a "more capable" type than Box<Dog>.
  3. Now that same T type is returned back from the function (return box).
  4. Now result is inferred a type. That type is Box<Dog>, not Box<Animal>.

The compiler took a type that was functionally equivalent to Box<Animal>, which is "more capable" than Box<Dog>, and cast it to Box<Dog>! This is an implicit contravariant cast.

Edit: I had a mistake in the code: I wrote new Dog() instead of new Box<Dog>(). Fixed it.

@RyanCavanaugh RyanCavanaugh added the By Design Deprecated - use "Working as Intended" or "Design Limitation" instead label Apr 8, 2016
@RyanCavanaugh
Copy link
Member

Any language without explicit co/contra-variance annotations is going to have this problem (or be hyperprescriptive: imagine not being able to add an HTMLDivElement to an HTMLElement[]).

The fact that no one for more than 4 years has noticed that means that very few people actually understand it and I believe it should be reconsidered.

Please don't confuse an intentional design trade-off with ignorance or uncriticalness (as you did in the nominal class threads as well). Assuming that anyone who has a different opinion from you simply hasn't even considered the alternative is disrespectful and reductive. From the beginning this was known to be a soundness / complexity trade-off and it was one taken with full knowledge of the consequences.

At this point it's not feasible to break that behavior in favor of a stricter default. As for adding variance annotations, see #1394.

@malibuzios
Copy link
Author

@RyanCavanaugh

When I wrote "no one" I meant "no user of the language" and did not necessarily include the design and development team. I think you're taking it too personally here.

Anyway, I think you should be flattered that someone is willing to spend 20 hours of their unpayed time to investigate just one particular aspect of your language. I do it because I care about the TypeScript and definitely like it. I've been using it for more than three years now and it has proven extremely useful.

There is no benefit for me to just go to a forum and spend so much time just to "prove a point" or just "argue". I had no evidence this was ever discussed before. I have not found any mention of this in GitHub so I had no way to know. I genuinely thought that addressing this would improve the type-safety of the language, and it seemed to be worth it.

@malibuzios
Copy link
Author

@RyanCavanaugh

This whole thing started originally when I was trying to provide better type safety for tuple types that are passed into a function, which would be essential for a new feature suggestion I was developing:

function f<T extends Array<any>>(tuple: T) {
    return tuple;
}

let result = f([1234, "abcd", true]);

And realized that despite the fact that I could modify the members of the tuple to incompatible types in the body of the function, the returned type was still erroneously inferred as (number | string | boolean)[].

I started generalizing the problem and eventually reached the more abstract notion described here!

Anyway, without this basic level of type safety this feature cannot be achieved.

@RyanCavanaugh
Copy link
Member

@malibuzios thanks for the feedback and the consideration. We're always trying to find the balance between "Not a bug, closed as by design" (which can be considered rude) and "[6-paragraph explanation of the design process that led us to this point]" (which doesn't leave us any time to do any actual work). Hopefully I can find a more optimal point in the future on that front.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead
Projects
None yet
Development

No branches or pull requests

4 participants