Skip to content

Commit

Permalink
Recursively concretize union-like types in AnnotT
Browse files Browse the repository at this point in the history
Summary:
The following case does not produce an error as expected:

```
type Fn2 = <A, B>(A, B) => A | B;
declare var fn2: Fn2;
const x = fn2((42: number), ('foo': string));
(true: typeof x);
```

https://flow.org/try/#0C4TwDgpgBAYgdgJigXigHgIIBooCEB8AFNngJQr5QZQA+eA3AFAAmEAxgDYCGATtAG68oAM0QAuWIiZsA9nADOwKAA8UIxIUIAWBBLgBXALYAjCD1I5CAcmEyZViYp4BLOAHNSpJoWA99ECVBIGWEVL0YgA

This is because even though `A | B` is wrapped in an `AnnotT` and concretized, its individual members `A` and `B` are not. So when we flow `boolean ~> A | B` we add a lower bound of `boolean` to `A` instead of flowing `boolean ~> number` and `boolean ~> string`.

This diff fixes this by wrapping union-like members in `AnnotT` so that they may be concretized before they are checked.

I need this fix for the implementation of `$Call` which is then needed for the `$ObjMap` fix.

**Note:** This diff includes the tests samwgoldman wrote in D5717999.

Reviewed By: samwgoldman

Differential Revision: D5722202

fbshipit-source-id: 8c8aaadfe69184281dfd3aa8cdbe619486a95eea
  • Loading branch information
Caleb Meredith authored and facebook-github-bot committed Sep 6, 2017
1 parent 98c7cc0 commit bd938ab
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 16 deletions.
37 changes: 22 additions & 15 deletions src/typing/flow_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1573,27 +1573,34 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace =
| ReposUpperT (_, l), _ ->
rec_flow cx trace (l, u)

(***************)
(* annotations *)
(***************)

(* Special cases where we want to recursively concretize types within the
lower bound. *)

| DefT (r, UnionT rep), ReposUseT (reason, use_op, l) ->
let rep = UnionRep.map annot rep in
let u = DefT (repos_reason (loc_of_reason reason) r, UnionT rep) in
rec_flow cx trace (l, UseT (use_op, u))

| DefT (r, MaybeT u), ReposUseT (reason, use_op, l) ->
let u = DefT (repos_reason (loc_of_reason reason) r, MaybeT (annot u)) in
rec_flow cx trace (l, UseT (use_op, u))

| DefT (r, OptionalT u), ReposUseT (reason, use_op, l) ->
let u = DefT (repos_reason (loc_of_reason reason) r, OptionalT (annot u)) in
rec_flow cx trace (l, UseT (use_op, u))

(* Waits for a def type to become concrete, repositions it as an upper UseT
using the stored reason. This can be used to store a reason as it flows
through a tvar. *)

| (DefT (r, UnionT rep), ReposUseT (reason, use_op, l)) ->
(* Don't reposition union members when the union appears as an upper
bound. This improves error messages when an incompatible lower bound
does not satisfy any member of the union. The "overall" error points to
the reposition target (an annotation) but the detailed member
information points to the type definition. *)
let u_def = DefT (repos_reason (loc_of_reason reason) r, UnionT rep) in
rec_flow cx trace (l, UseT (use_op, u_def))

| (u_def, ReposUseT (reason, use_op, l)) ->
let u = reposition cx ~trace (loc_of_reason reason) u_def in
| u, ReposUseT (reason, use_op, l) ->
let u = reposition cx ~trace (loc_of_reason reason) u in
rec_flow cx trace (l, UseT (use_op, u))

(***************)
(* annotations *)
(***************)

(* The sink component of an annotation constrains values flowing
into the annotated site. *)

Expand Down
4 changes: 4 additions & 0 deletions src/typing/type.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2494,3 +2494,7 @@ let typeapp t tparams =
let this_typeapp t this tparams =
let reason = replace_reason (fun desc -> RTypeApp desc) (reason_of_t t) in
ThisTypeAppT (reason, t, this, tparams)

let annot = function
| OpenT _ as t -> AnnotT t
| t -> t
9 changes: 9 additions & 0 deletions tests/typeof/maybe.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
function f<A>(a: A): ?A {
if (true) {
return null;
} else {
return a;
}
}
var x = f(42);
('foo': typeof x); // error: string ~> number
9 changes: 9 additions & 0 deletions tests/typeof/optional.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
function f<A>(a: A): {p?: A} {
if (true) {
return {};
} else {
return {p: a};
}
}
var x = f(42).p;
(null: typeof x); // error: null ~> $Optional<number> (i.e., void|number)
36 changes: 35 additions & 1 deletion tests/typeof/typeof.exp
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
Error: maybe.js:9
9: ('foo': typeof x); // error: string ~> number
^^^^^ string. This type is incompatible with
9: ('foo': typeof x); // error: string ~> number
^^^^^^^^ number

Error: optional.js:9
9: (null: typeof x); // error: null ~> $Optional<number> (i.e., void|number)
^^^^ null. This type is incompatible with
9: (null: typeof x); // error: null ~> $Optional<number> (i.e., void|number)
^^^^^^^^ number

Error: typeof.js:43
43: var b: MyClass1 = MyClass1;
^^^^^^^^ class type: MyClass1. This type is incompatible with
Expand Down Expand Up @@ -28,5 +40,27 @@ Error: typeof.js:82
82: var keys: $Keys<Map> = "A"; // Error: ineligible value used in type anno
^^^ Map

Error: union.js:9
9: (null: typeof x); // error: null ~> number|string
^^^^ null. This type is incompatible with
9: (null: typeof x); // error: null ~> number|string
^^^^^^^^ union: type parameter `A` of function call | type parameter `B` of function call
Member 1:
9: (null: typeof x); // error: null ~> number|string
^^^^^^^^ type parameter `A` of function call
Error:
9: (null: typeof x); // error: null ~> number|string
^^^^ null. This type is incompatible with
9: (null: typeof x); // error: null ~> number|string
^^^^^^^^ number
Member 2:
9: (null: typeof x); // error: null ~> number|string
^^^^^^^^ type parameter `B` of function call
Error:
9: (null: typeof x); // error: null ~> number|string
^^^^ null. This type is incompatible with
9: (null: typeof x); // error: null ~> number|string
^^^^^^^^ string


Found 5 errors
Found 8 errors
9 changes: 9 additions & 0 deletions tests/typeof/union.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
function f<A, B>(a: A, b: B): A | B {
if (true) {
return a;
} else {
return b;
}
}
const x = f(42, 'foo');
(null: typeof x); // error: null ~> number|string

0 comments on commit bd938ab

Please sign in to comment.