Skip to content

Commit

Permalink
Fix error handling for nested unions and intersections
Browse files Browse the repository at this point in the history
Union and intersection errors are handled using a speculative match
process, where an error on one side of the match raises an exception,
which is caught by the speculative match code, which then tries the
otehr side of the match.

If, however, there are nested speculative matches in a given flow, the
innermost one would raise an error too early. This can happen if a type
alias is a union containing union types.

To fix, instead of using a boolean to track whether to log the error or
raise, we use an integer representing the "depth" of the speculative
match.

Before, there was some code to "flatten" unions of unions during the
convertion of annotations to types, but this doesn't work for the case
where the unions are themselves type aliases.

Unfortunately, this change affects error messages around union types,
and in my opinion, the new messages are worse and less helpful than they
used to be. Any ideas how to make this change without making error
messages worse?

Fixes facebook#582
  • Loading branch information
samwgoldman committed Jun 28, 2015
1 parent 151759d commit ff5cc74
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 25 deletions.
16 changes: 8 additions & 8 deletions src/typing/flow_js.ml
Expand Up @@ -98,14 +98,14 @@ let mk_objecttype ?(flags=default_flags) dict map proto = {

(**************************************************************)

(* for now, we do speculative matching by setting this global.
it's set to true when trying the arms of intersection or
(* For now, we do speculative matching by incrementing this global.
It's incremented when trying the arms of intersection or
union types.
when it's true, the error-logging function throws instead
When it's non-zero, the error-logging function throws instead
of logging, and the speculative harness catches.
TODO
*)
let throw_on_error = ref false
let throw_on_error = ref 0

(* we keep a stack of reasons representing the operations
taking place when flows are performed. the top op reason
Expand Down Expand Up @@ -199,7 +199,7 @@ let silent_warnings = false
exception FlowError of (reason * string) list

let add_output cx level ?(trace_reasons=[]) message_list =
if !throw_on_error
if !throw_on_error > 0
then (
raise (FlowError message_list)
) else (
Expand Down Expand Up @@ -3569,16 +3569,16 @@ and speculative_flow_error cx trace l u =
(* save the ops stack, since throws from within __flow will screw it up *)
let ops = Ops.get () in
let typeapp_stack = TypeAppExpansion.get () in
throw_on_error := true;
throw_on_error := !throw_on_error + 1;
let result =
try rec_flow cx trace (l, u); false
with
| FlowError msgs -> true
| exn ->
throw_on_error := false;
throw_on_error := 0;
raise exn
in
throw_on_error := false;
throw_on_error := !throw_on_error - 1;
TypeAppExpansion.set typeapp_stack;
(* restore ops stack *)
Ops.set ops;
Expand Down
6 changes: 0 additions & 6 deletions src/typing/type_inference_js.ml
Expand Up @@ -576,12 +576,6 @@ let rec convert cx map = Ast.Type.(function

| loc, Union ts ->
let ts = List.map (convert cx map) ts in
(* "Flatten" out any unions in this union, like
* var a: number | (string | bool) *)
let ts = List.map (function
| UnionT (r, ts) -> ts
| t -> [t]) ts in
let ts = List.flatten ts in
UnionT (mk_reason "union type" loc, ts)

| loc, Intersection ts ->
Expand Down
2 changes: 1 addition & 1 deletion tests/namespace/namespace.exp
Expand Up @@ -11,7 +11,7 @@ namespace.js:11:28,29: string
This type is incompatible with
namespace.js:11:7,12: number

namespace.js:15:16,17: string
namespace.js:15:14,19: array literal
This type is incompatible with
namespace.js:13:12,30: union type

Expand Down
20 changes: 10 additions & 10 deletions tests/promises/promises.exp
Expand Up @@ -7,13 +7,13 @@ This type is incompatible with

promise.js:21:11,23:4: constructor call
Error:
promise.js:22:13,13: number
[LIB] core.js:350:1,377:1: Promise
This type is incompatible with
[LIB] core.js:352:25,38: union type

promise.js:32:13,34:6: constructor call
Error:
promise.js:33:15,15: number
[LIB] core.js:350:1,377:1: Promise
This type is incompatible with
[LIB] core.js:352:25,38: union type

Expand All @@ -31,13 +31,13 @@ This type is incompatible with

promise.js:112:17,34: call of method resolve
Error:
promise.js:112:33,33: number
[LIB] core.js:350:1,377:1: Promise
This type is incompatible with
[LIB] core.js:370:32,45: union type

promise.js:118:33,50: call of method resolve
Error:
promise.js:118:49,49: number
[LIB] core.js:350:1,377:1: Promise
This type is incompatible with
[LIB] core.js:370:32,45: union type

Expand All @@ -49,13 +49,13 @@ This type is incompatible with

promise.js:157:32,54: call of method resolve
Error:
promise.js:157:48,53: string
[LIB] core.js:350:1,377:1: Promise
This type is incompatible with
[LIB] core.js:370:32,45: union type
[LIB] core.js:357:33,46: union type

promise.js:165:48,70: call of method resolve
Error:
promise.js:165:64,69: string
[LIB] core.js:350:1,377:1: Promise
This type is incompatible with
[LIB] core.js:370:32,45: union type

Expand All @@ -67,13 +67,13 @@ This type is incompatible with

promise.js:197:33,55: call of method resolve
Error:
promise.js:197:49,54: string
[LIB] core.js:350:1,377:1: Promise
This type is incompatible with
[LIB] core.js:370:32,45: union type
[LIB] core.js:367:34,48: union type

promise.js:205:49,71: call of method resolve
Error:
promise.js:205:65,70: string
[LIB] core.js:350:1,377:1: Promise
This type is incompatible with
[LIB] core.js:370:32,45: union type

Expand Down
8 changes: 8 additions & 0 deletions tests/union/union.js
Expand Up @@ -19,3 +19,11 @@ function corge(b) {
var x = b ? "" : 0;
new F().foo(x);
}

var ok: ('foo' | 'bar') | 'baz' = 'baz';

type FooBar = 'foo' | 'bar';
type Baz = 'baz';
type FooBarBaz = FooBar | Baz;

var bad: FooBarBaz = 'baz';

0 comments on commit ff5cc74

Please sign in to comment.