Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Intersection of objects 3+ levels deep. #725

Closed
kkirby opened this issue Aug 17, 2015 · 6 comments
Closed

Intersection of objects 3+ levels deep. #725

kkirby opened this issue Aug 17, 2015 · 6 comments

Comments

@kkirby
Copy link
Contributor

kkirby commented Aug 17, 2015

I'm getting an error when I try to have an object intersection that references an object interaction, that references another object. The error is about the property on the first object in the chain.

Here is the code:

 1  /* @flow */
 2  
 3  type TypeA = {
 4      a: string
 5  };
 6  
 7  type TypeB = {
 8      b: string
 9  } & TypeA;
10  
11  type TypeC = {
12      c: string
13  } & TypeB;
14  
15  class A {
16      action(): TypeA {
17          return {
18              a: 'A'
19          };
20      }
21  }
22  
23  class B extends A {
24      action(): TypeB {
25          return {
26              ...super.action(),
27              b: 'B'
28          };
29      }
30  }
31  
32  class C extends B {
33      action(): TypeC {
34          return {
35              ...super.action(),
36              c: 'C'
37          };
38      }
39  }

And the error:

test.js:3:14,5:1: property a
Property not found in
test.js:34:16,37:9: object literal
Error path:
test.js:34:16,37:9: MixedT [object literal]
test.js:3:14,5:1:   ~> LookupT [property a] comes from
test.js:34:16,37:9: . ObjT [object literal]
test.js:3:14,5:1:   . ~> ObjT [object type] comes from
test.js:34:16,37:9: . . ObjT [object literal]
test.js:9:5,9:      . . ~> OpenT [TypeA] comes from
test.js:34:16,37:9: . . . ObjT [object literal]
test.js:9:5,9:      . . . ~> AnnotT [TypeA] comes from
test.js:34:16,37:9: . . . . ObjT [object literal]
test.js:7:14,9:9:   . . . . ~> IntersectionT [intersection type] comes from
test.js:34:16,37:9: . . . . . ObjT [object literal]
test.js:13:5,9:     . . . . . ~> OpenT [TypeB] comes from
test.js:34:16,37:9: . . . . . . ObjT [object literal]
test.js:13:5,9:     . . . . . . ~> AnnotT [TypeB] comes from
test.js:34:16,37:9: . . . . . . . ObjT [object literal]
test.js:11:14,13:9: . . . . . . . ~> IntersectionT [intersection type] comes from
test.js:34:16,37:9: . . . . . . . . ObjT [object literal]
test.js:33:15,19:   . . . . . . . . ~> OpenT [TypeC] comes from
test.js:34:16,37:9: . . . . . . . . . ObjT [object literal]
test.js:33:15,19:   . . . . . . . . . ~> AnnotT [TypeC] comes from
test.js:7:14,9:1:   . . . . . . . . . . ObjT [object type]
test.js:34:16,37:9: . . . . . . . . . . ~> ObjAssignT [object literal] comes from
test.js:7:14,9:1:   . . . . . . . . . . . ObjT [object type]
test.js:7:14,9:9:   . . . . . . . . . . . ~> SpeculativeMatchFailureT [intersection type] comes from
test.js:7:14,9:9:   . . . . . . . . . . . . IntersectionT [intersection type]
test.js:34:16,37:9: . . . . . . . . . . . . ~> ConcreteT [object literal] comes from
test.js:7:14,9:9:   . . . . . . . . . . . . . IntersectionT [intersection type]
test.js:34:16,37:9: . . . . . . . . . . . . . ~> ObjAssignT [object literal] comes from
test.js:35:16,29:   . . . . . . . . . . . . . . OpenT [TypeB]
test.js:34:16,37:9: . . . . . . . . . . . . . . ~> ObjAssignT [object literal] comes from
test.js:35:16,29:   . . . . . . . . . . . . . . . AnnotT [TypeB]
test.js:34:16,37:9: . . . . . . . . . . . . . . . ~> ObjAssignT [object literal] comes from
test.js:24:5,29:5:  . . . . . . . . . . . . . . . . FunT [method action]
test.js:35:16,29:   . . . . . . . . . . . . . . . . ~> CallT [super.action(...)] comes from
test.js:32:17,17:   . . . . . . . . . . . . . . . . . InstanceT [B]
test.js:35:16,29:   . . . . . . . . . . . . . . . . . ~> MethodT [super.action(...)] comes from
test.js:32:17,17:   . . . . . . . . . . . . . . . . . . OpenT [class type: B]
test.js:35:16,29:   . . . . . . . . . . . . . . . . . . ~> MethodT [super.action(...)]
test.js:34:16,37:9: . . . . . . . . . . . . . . . . ObjT [object literal]
test.js:34:16,37:9: . . . . . . . . . . . . . . . . ~> ObjAssignT [object literal]

Finally, raw code without line numbers:

/* @flow */

type TypeA = {
    a: string
};

type TypeB = {
    b: string
} & TypeA;

type TypeC = {
    c: string
} & TypeB;

class A {
    action(): TypeA {
        return {
            a: 'A'
        };
    }
}

class B extends A {
    action(): TypeB {
        return {
            ...super.action(),
            b: 'B'
        };
    }
}

class C extends B {
    action(): TypeC {
        return {
            ...super.action(),
            c: 'C'
        };
    }
}
@samwgoldman
Copy link
Member

Interesting issue! I am able to reproduce.

I tried the same code without the object spread (duplicating the a, b keys as necessary in the return value of action) and passed the type checker, so I wonder if the intersection types are a red herring here, and the real issue is with object spread?

@kkirby
Copy link
Contributor Author

kkirby commented Aug 17, 2015

Possibly, or at the very least how flow handles Object.assign because this code doesn't type check either:

var a: TypeA = {a:'a'};
var b: TypeB = Object.assign({},{b:'b'},a);
var c: TypeC = Object.assign({},{c:'c'},b);

But this code without using intersections at all type checks just fine, so I'm not really sure ...

/* @flow */

type TypeA = {
    a: string
};

type TypeB = {
    a: string,
    b: string
};

type TypeC = {
    a: string;
    b: string;
    c: string;
};

var a: TypeA = {a:'a'};
var b: TypeB = Object.assign({},{b:'b'},a);
var c: TypeC = Object.assign({},{c:'c'},b);

var _a: TypeA = {a:'a'};
var _b: TypeB = {..._a,b:'b',};
var _c: TypeC = {..._b,c:'c'};

@mroch
Copy link
Contributor

mroch commented Aug 17, 2015

if it's spread related, i'm working on more correctly modeling spreads and that might fix it.

there's also a bug with unions (and possibly intersections?) with nested type variables, whereby the SpeculativeMatchFailureT tries a branch, hits an unresolved tvar and says "yep, this matches because it doesn't fail" instead of "i don't know yet because this is unresolved." later, when the tvar is resolved, it fails because it picked the wrong branch of the union.

@samwgoldman
Copy link
Member

I tested this against my hack to fix the nested speculative match issue and it either a) doesn't solve this case or b) shows that this error is something else.

@samwgoldman
Copy link
Member

So from what I can tell, this is actually a special case of a much simpler bug. Unfortunately, that bug is a bit nontrivial to fix.

I'm going to create a high level issue, close this one w/ a link to the new one. I'm trying to groom the GH issues a bit to increase the signal, so we can react to important stuff like this more easily.

Thanks for the report & simple repro case. Will follow up with a link to the superseding issue.

@samwgoldman
Copy link
Member

Merging with #1329. Thanks for the report!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants