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

Improper Assignment Allowed on Properties of Unioned Interface type #14369

Closed
mathias999us opened this issue Feb 28, 2017 · 6 comments
Closed
Labels
Duplicate An existing issue was already created

Comments

@mathias999us
Copy link

mathias999us commented Feb 28, 2017

TypeScript Version: 2.1 and current PlayGround

Code

// A *self-contained* demonstration of the problem follows...
interface IDogArgs {
    paws?: number;
}
class Dog {
    constructor(args: IDogArgs) {

    }
}

interface ICatArgs {
    claws: string; 
}
class Cat {
    constructor(args: ICatArgs) {

    }
}

interface IHowToMakeDog {
    ctor: new (args: IDogArgs) => Dog;
    args: IDogArgs;
}

interface IHowToMakeCat {
    ctor: new (args: ICatArgs) => Cat;
    args: ICatArgs;
}

type AnimalMakers = IHowToMakeDog | IHowToMakeCat;

// OK
var testDog: AnimalMakers = {
    ctor: Dog,
    args: {
        paws: 4
    }
};

var testCat: AnimalMakers = {
    ctor: Cat,
    args: {
        claws: 24, // Why is this okay?
        bogus: false // What is this okay?
    }
};

Expected behavior:
Should be restricted from assigning a number to the claws property.
Should be restricted from assigning "bogus" property.
Actual behavior:
The above code is valid and passed typescript compiler checks, and I am able to make an assignment that does not meet the requirements of any of the types specified in the AnimalMakers union type. The key seems to be that the paws property on IDogArgs is optional. If this property is required, the compiler provides errors on the testCat assignment as expected. Also, the compiler correctly provided errors in this scenario as of TypeScript 1.8.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 28, 2017

IDogArgs is an interface with all optional arguments, that makes is look like {} from an assignablity perspective.

A type is assignable to a union type iff it is assignable to at least one constituent. since { claws: number } is assignable to {} and {paws? : number}, the check is allowed.

This is another manifestation of #7485

@mhegazy mhegazy added the Duplicate An existing issue was already created label Feb 28, 2017
@mathias999us
Copy link
Author

Thanks. The example I provided seems a little different though, because of the ctor property. Since I have supplied the Cat constructor for the ctor arg in the testCat assignment, this cannot be an assignment to IHowToMakeDog type (which has the {} args type), and it should require ICatArgs for for the args property. In other words, the assignment is clearly discriminated by the ctor property. #7845 does not include such a discriminator, and my example was working as of TypeScript 1.8 (which 7845 cites as the version that they experienced their problem with).

@mhegazy
Copy link
Contributor

mhegazy commented Feb 28, 2017

The two types Cat and Dog have no properties. that makes both look like {} from a structural type system perspective.

Adding any properties on both types should also trigger the error on the assignment. but i am assuming this is not what you want.

@mathias999us
Copy link
Author

Yes, that is not what I want, but your feedback brought me a lot of clarity on my issue. Something changed in the latest TypeScript because even though the ctor argument will produce a class with an identical structure, it seems TS1.8 used to discriminate on the actual type of class produced. It seems my issue can be distilled to the following code:

class Cat { }
class Dog { }

interface CatMaker { 
    ctor: new () => Cat
}

var test: CatMaker = {
    ctor: Dog // Why is this allowed?
}

This still seems like undesirable behavior to me.

@mhegazy
Copy link
Contributor

mhegazy commented Mar 1, 2017

Cat and Dog are structurally compatible. so a thing that makes a Dog is assignable to a thing that makes a Cat. This specific behavior has not changed since 1.0 i would say.

@mathias999us
Copy link
Author

You are spot on - I did some more testing against TS1.8, and the real reason my case in the original post fails is because the args property wasn't assignable to ANY of the args property types in the union type. Changing the ctor as a "discriminator" in this example has no impact in TS1.8. However, in TS1.8 { claws: 24, bogus: true } was not assignable to { paws?: number }, but now it is in TS2.1, and that's why this surfaced for me. I was apparently under a misunderstanding about how the type checking was working all along. #7485 thus exactly describes my issue. Thanks again for your help.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

2 participants