Skip to content

Weak type detection only works with object literals being passed #17656

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

Closed
ownerer opened this issue Aug 7, 2017 · 8 comments
Closed

Weak type detection only works with object literals being passed #17656

ownerer opened this issue Aug 7, 2017 · 8 comments
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@ownerer
Copy link

ownerer commented Aug 7, 2017

TypeScript Version: 2.4.0

Code

interface ITest {
    optional?: string;
}

function test(param: ITest): void {

}

class Test {

}

test({ whatever: 0, optional: "" }); //<-- compiler doesn't allow this
test(test); //<-- compiler allows this
test(Test); //<-- compiler allows this
test(new Test()); //<-- compiler allows this

Expected behavior:
Passing any object of any type that doesn't match with at least one of the properties defined in the weak type, should fail.

Actual behavior:
Only object literals that don't at least partially match are not allowed to be passed.

@RyanCavanaugh
Copy link
Member

These are the rules described in #16047. I don't remember why we allow call signatures to satisfy the weak types, but there was a reason

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Aug 7, 2017
@sylvanaar
Copy link

sylvanaar commented Aug 7, 2017

Before you close it, let me add the following.

    const v1 : ITest = new Test(); //<-- compiler allows this
    const v2 : ITest = test; //<-- compiler allows this

So, it isn't just for function parameters.

@RyanCavanaugh
Copy link
Member

@sandersn is trying to remember why we allow types with call/construct signatures to be assigned in. We had to add it for a reason.

The Test class instance is empty which is always allowed as a weak type source. As usual don't use empty classes in repros because empty types have special behavior in a lot of cases

@sylvanaar
Copy link

Yeah, agree about the Test() class, but the function is strange.

@sandersn
Copy link
Member

sandersn commented Aug 7, 2017

Here's the test case from 548f92a

// Ctor isn't a weak type because it has a construct signature
interface Ctor {
    new (s: string): K
    n?: number
}
let ctor: Ctor = K

// Ctor is similar to the more common type:
class C {
    constructor() { }
    static n?: number;
}

Ctor doesn't look weak to me, and by extension a similar function+optional properties type should also not be weak. Let me look at your example more closely to see if it changes my mind.

@sandersn
Copy link
Member

sandersn commented Aug 7, 2017

@RyanCavanaugh the question that @ownerer raises isn't whether ITest is weak (it clearly is), but whether function test or class Test should raise weak errors when compared to it. I think they should, but it doesn't depend on whether or not they are weak types. I'll look at the code again to see why they don't.

@sandersn
Copy link
Member

sandersn commented Aug 7, 2017

There's just an ad-hoc filter getPropertiesOfType(source).length > 0 that prevents errors for object types with no properties. I'll change that to prevent errors only for object types with index signatures and see if anything breaks.

@sandersn sandersn added the Bug A bug in TypeScript label Aug 7, 2017
@sandersn
Copy link
Member

sandersn commented Aug 7, 2017

It turns out the check needs to be positive, so I changed it to getPropertiesOfType(source).length > 0 || getSignaturesOfType(source, SignatureKind.Call).length > 0 || getSignaturesOfType(source, SignatureKind.Construct).length > 0

@sandersn sandersn added Fixed A PR has been merged for this issue and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Aug 7, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

4 participants