Skip to content

Commit

Permalink
Fix idx((obj: {a?: ?b}))
Browse files Browse the repository at this point in the history 
Summary:
Previously we were only unwrapping a single level of maybe-ness. This doesn't account for the fact that you could have `OptionalT(MaybeT(...))`, though.

This diff just switches things up to unwrap until you get to something that isn't a `MaybeT` or an `OptionalT` (see new test).

I'm also re-recording the tests I should've rerecorded yesterday over in D3670635. This should fix test runs in master

Reviewed By: bhosmer

Differential Revision: D3676855

fbshipit-source-id: 7357ce8013467db11a763059e6aa05156638447b
  • Loading branch information
@jeffmo
jeffmo authored and Facebook Github Bot 5 committed Aug 5, 2016
1 parent c0c72b7 commit 031c125
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 50 deletions.
76 changes: 40 additions & 36 deletions newtests/FacebookismIdx/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,101 +5,101 @@ import {suite, test} from '../../tsrc/test/Tester';
export default suite(({addFile, addFiles, addCode}) => [
test('idx(object)', [
addCode('declare var idx: $Facebookism$Idx;\n').noNewErrors(),
addCode('declare var obj1: {a: ?{b: {c: number}}};').noNewErrors(),
addCode('declare var obj2: {a?: {b: {c: number}}};').noNewErrors(),
addCode('declare var obj3: {a: null | {b: {c: number}}};').noNewErrors(),

addCode('declare var obj1: {a: ?{b: {c: number}}};').noNewErrors(),
addCode('obj1.a.b.c;\n').newErrors(
`
test.js:12
12: obj1.a.b.c;
test.js:8
8: obj1.a.b.c;
^ property \`b\`. Property cannot be accessed on possibly null value
12: obj1.a.b.c;
8: obj1.a.b.c;
^^^^^^ null
test.js:12
12: obj1.a.b.c;
test.js:8
8: obj1.a.b.c;
^ property \`b\`. Property cannot be accessed on possibly undefined value
12: obj1.a.b.c;
8: obj1.a.b.c;
^^^^^^ undefined
`,
),

addCode('(idx(obj1, obj => obj.a.b.c): ?number);\n').noNewErrors(),
addCode('(idx(obj1, obj => obj["a"].b.c): ?number);\n').noNewErrors(),
addCode('(idx(obj1, obj => obj.a.b.c): number);\n').newErrors(
`
test.js:21
21: (idx(obj1, obj => obj.a.b.c): number);
test.js:17
17: (idx(obj1, obj => obj.a.b.c): number);
^^^^^^^^^^^^^^^^^^^^^^^^^^^ null. This type is incompatible with
21: (idx(obj1, obj => obj.a.b.c): number);
17: (idx(obj1, obj => obj.a.b.c): number);
^^^^^^ number
test.js:21
21: (idx(obj1, obj => obj.a.b.c): number);
test.js:17
17: (idx(obj1, obj => obj.a.b.c): number);
^^^^^^^^^^^^^^^^^^^^^^^^^^^ undefined. This type is incompatible with
21: (idx(obj1, obj => obj.a.b.c): number);
17: (idx(obj1, obj => obj.a.b.c): number);
^^^^^^ number
`,
),
addCode('(idx(obj1, obj => obj.a.b.c): ?string);\n').newErrors(
`
test.js:24
24: (idx(obj1, obj => obj.a.b.c): ?string);
test.js:20
20: (idx(obj1, obj => obj.a.b.c): ?string);
^^^^^^^^^^^^^^^^^^^^^^^^^^^ number. This type is incompatible with
24: (idx(obj1, obj => obj.a.b.c): ?string);
20: (idx(obj1, obj => obj.a.b.c): ?string);
^^^^^^ string
`,
),

addCode('(idx(obj1, obj => obj["a"].b.c): number);\n').newErrors(
`
test.js:27
27: (idx(obj1, obj => obj["a"].b.c): number);
test.js:23
23: (idx(obj1, obj => obj["a"].b.c): number);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ null. This type is incompatible with
27: (idx(obj1, obj => obj["a"].b.c): number);
23: (idx(obj1, obj => obj["a"].b.c): number);
^^^^^^ number
test.js:27
27: (idx(obj1, obj => obj["a"].b.c): number);
test.js:23
23: (idx(obj1, obj => obj["a"].b.c): number);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ undefined. This type is incompatible with
27: (idx(obj1, obj => obj["a"].b.c): number);
23: (idx(obj1, obj => obj["a"].b.c): number);
^^^^^^ number
`,
),
addCode('idx(obj1, obj => obj.notAProp);\n').newErrors(
`
test.js:30
30: idx(obj1, obj => obj.notAProp);
test.js:26
26: idx(obj1, obj => obj.notAProp);
^^^^^^^^ property \`notAProp\`. Property not found in
30: idx(obj1, obj => obj.notAProp);
26: idx(obj1, obj => obj.notAProp);
^^^ object type
`,
),
addCode('idx(obj1, obj => obj.a = null);\n').newErrors(
`
test.js:33
33: idx(obj1, obj => obj.a = null);
test.js:29
29: idx(obj1, obj => obj.a = null);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function call. idx() callbacks may only access properties on the callback parameter!
`,
),

addCode('declare var obj2: {a?: {b: {c: number}}};').noNewErrors(),
addCode('(idx(obj2, obj => obj.a.b.c): ?number);\n').noNewErrors(),
addCode('(idx(obj2, obj => obj.a.b.c): number);\n').newErrors(
`
test.js:39
39: (idx(obj2, obj => obj.a.b.c): number);
test.js:37
37: (idx(obj2, obj => obj.a.b.c): number);
^^^^^^^^^^^^^^^^^^^^^^^^^^^ null. This type is incompatible with
39: (idx(obj2, obj => obj.a.b.c): number);
37: (idx(obj2, obj => obj.a.b.c): number);
^^^^^^ number
test.js:39
39: (idx(obj2, obj => obj.a.b.c): number);
test.js:37
37: (idx(obj2, obj => obj.a.b.c): number);
^^^^^^^^^^^^^^^^^^^^^^^^^^^ undefined. This type is incompatible with
39: (idx(obj2, obj => obj.a.b.c): number);
37: (idx(obj2, obj => obj.a.b.c): number);
^^^^^^ number
`,
),

addCode('declare var obj3: {a: null | {b: {c: number}}};').noNewErrors(),
addCode('(idx(obj3, obj => obj.a.b.c): ?number);\n').noNewErrors(),
addCode('(idx(obj3, obj => obj.a.b.c): number);\n').newErrors(
`
Expand All @@ -116,6 +116,10 @@ export default suite(({addFile, addFiles, addCode}) => [
^^^^^^ number
`,
),

// Nested maybes/optionals should get unwrapped
addCode('declare var obj4: {a?: ?(?{b: number})};').noNewErrors(),
addCode('(idx(obj4, obj => obj.a.b): ?number)').noNewErrors(),
]),

test('idx(classInst)', [
Expand Down
6 changes: 3 additions & 3 deletions src/typing/flow_js.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1843,9 +1843,9 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace =
| (_, IdxUnwrap (_, t)) -> rec_flow_t cx trace (l, t)

(* De-maybe-ify an idx() property access *)
| (MaybeT inner_t, IdxUnMaybeifyT (_, t))
| (OptionalT inner_t, IdxUnMaybeifyT (_, t))
-> rec_flow_t cx trace (inner_t, t)
| (MaybeT inner_t, IdxUnMaybeifyT _)
| (OptionalT inner_t, IdxUnMaybeifyT _)
-> rec_flow cx trace (inner_t, u)
| (NullT _, IdxUnMaybeifyT _) -> ()
| (VoidT _, IdxUnMaybeifyT _) -> ()
| (_, IdxUnMaybeifyT (_, t)) when (
Expand Down
10 changes: 5 additions & 5 deletions tests/declare_export/declare_export.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,19 @@ es6modules.js:47

es6modules.js:53
53: import {doesntExist} from "CommonJS_Clobbering_Class"; // Error: Not an exported binding
^^^^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module has no named export called `doesntExist`.
^^^^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module only has a default export. Did you mean `import doesntExist from ...`?

es6modules.js:59
59: import {staticNumber1, baseProp, childProp} from "CommonJS_Clobbering_Class"; // Error
^^^^^^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module has no named export called `staticNumber1`.
^^^^^^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module only has a default export. Did you mean `import staticNumber1 from ...`?

es6modules.js:59
59: import {staticNumber1, baseProp, childProp} from "CommonJS_Clobbering_Class"; // Error
^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module has no named export called `baseProp`.
^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module only has a default export. Did you mean `import baseProp from ...`?

es6modules.js:59
59: import {staticNumber1, baseProp, childProp} from "CommonJS_Clobbering_Class"; // Error
^^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module has no named export called `childProp`.
^^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module only has a default export. Did you mean `import childProp from ...`?

es6modules.js:63
63: new CJS_Clobb_Class().doesntExist; // Error: Class has no `doesntExist` property
Expand Down Expand Up @@ -164,7 +164,7 @@ es6modules.js:97

es6modules.js:103
103: import {doesntExist} from "ES6_Default_AnonFunction1"; // Error: Not an exported binding
^^^^^^^^^^^ Named import from module `ES6_Default_AnonFunction1`. This module has no named export called `doesntExist`.
^^^^^^^^^^^ Named import from module `ES6_Default_AnonFunction1`. This module only has a default export. Did you mean `import doesntExist from ...`?

es6modules.js:107
107: var n2: string = ES6_Def_AnonFunc1(); // Error: number ~> string
Expand Down
2 changes: 1 addition & 1 deletion tests/declare_module_exports/declare_module_exports.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ main.js:5

main.js:8
8: import {str} from "declare_m_e_with_other_value_declares";
^^^ Named import from module `declare_m_e_with_other_value_declares`. This module has no named export called `str`.
^^^ Named import from module `declare_m_e_with_other_value_declares`. This module only has a default export. Did you mean `import str from ...`?

main.js:12
12: (42: str2); // Error: number ~> string
Expand Down
10 changes: 5 additions & 5 deletions tests/es6modules/es6modules.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,19 @@ es6modules.js:47

es6modules.js:53
53: import {doesntExist} from "CommonJS_Clobbering_Class"; // Error: Not an exported binding
^^^^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module has no named export called `doesntExist`.
^^^^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module only has a default export. Did you mean `import doesntExist from ...`?

es6modules.js:59
59: import {staticNumber1, baseProp, childProp} from "CommonJS_Clobbering_Class"; // Error
^^^^^^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module has no named export called `staticNumber1`.
^^^^^^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module only has a default export. Did you mean `import staticNumber1 from ...`?

es6modules.js:59
59: import {staticNumber1, baseProp, childProp} from "CommonJS_Clobbering_Class"; // Error
^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module has no named export called `baseProp`.
^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module only has a default export. Did you mean `import baseProp from ...`?

es6modules.js:59
59: import {staticNumber1, baseProp, childProp} from "CommonJS_Clobbering_Class"; // Error
^^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module has no named export called `childProp`.
^^^^^^^^^ Named import from module `CommonJS_Clobbering_Class`. This module only has a default export. Did you mean `import childProp from ...`?

es6modules.js:63
63: new CJS_Clobb_Class().doesntExist; // Error: Class has no `doesntExist` property
Expand Down Expand Up @@ -164,7 +164,7 @@ es6modules.js:97

es6modules.js:103
103: import {doesntExist} from "ES6_Default_AnonFunction1"; // Error: Not an exported binding
^^^^^^^^^^^ Named import from module `ES6_Default_AnonFunction1`. This module has no named export called `doesntExist`.
^^^^^^^^^^^ Named import from module `ES6_Default_AnonFunction1`. This module only has a default export. Did you mean `import doesntExist from ...`?

es6modules.js:107
107: var n2: string = ES6_Def_AnonFunc1(); // Error: number ~> string
Expand Down

0 comments on commit 031c125

Please sign in to comment.