Skip to content
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

ES6 Generators #534

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions lib/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,18 +295,26 @@ declare class JSON {
}

/* Iterators */
interface IteratorResult<T> {
interface IteratorResult<Y,R> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you decide to parameterize IteratorResult on two things rather than just one? They seem equivalent to me...I'm just curious

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a bit of future proofing. Once #577 is fixed, we can use a better type for this that takes advantage of this parameterization:

type IteratorResult<Y,R> = { done: false, value: Y } | { done: true, value: ?R }

(Annoyingly, the value member is R the first time done switches to true, but if you call next again, it becomes undefined. >:(.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, #577 hasn't changed yet, and interfaces as type unions isn't possible either, so I'll commit this change and you can take it at your discretion.

done: boolean;
value?: T;
value?: Y|R;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So for Iterator<string>, you'll have value?: string | void. It's a little weird that there's a void due to being optional and a void due to there being no return, but I think it's fine.

I actually wonder if maybe we can make value not be optional, since the union'd void captures the behavior perfectly. Like if iterate over a Generator<string, number, void>, will the IteratorResult ever be omitted? Looking at the mozilla docs it seems like it shouldn't be undefined if there is a return

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value is optional—it becomes undefined upon completion of the iterator. i.e., when done=true, value=undefined. (Edit: mixed up true and false.)

}

interface Iterator<T> {
next(): IteratorResult<T>;
@@iterator(): Iterator<T>;
interface $Iterator<Y,R,N> {
@@iterator(): $Iterator<Y,R,N>;
next(value?: N): IteratorResult<Y,R>;
}
type Iterator<T> = $Iterator<T,void,void>;

interface Iterable<T> {
@@iterator(): Iterator<T>;
interface $Iterable<Y,R,N> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we name these type vars more explicitly? They can show up in error messages, and sometimes without context it makes the error more confusing/unclear.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an opinion on the more explicit names? Yield, Return, and Next?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no objection. I can add a commit here, but it seems like an easy enough change to make in your internal merge, too, if you want to make the change yourself. (I'm not sure what the internal merge looks like, of course, so this could be wrong-headed of me.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, Yield/Return/Next seem good. I'll pass it along to @gabelevi who's submitted the internal merge request

@@iterator(): $Iterator<Y,R,N>;
}
type Iterable<T> = $Iterable<T,void,void>;

/* Generators */
interface Generator<Y,R,N> {
@@iterator(): $Iterator<Y,R,N>;
next(value?: N): IteratorResult<Y,R>;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super rough idea, but I wonder if we could express this as something like

interface $Iterator<Y, R, N> {
  next(value?: N): IteratorResult<Y>;
}
type Iterator<T> = $Iterator<T, *, *>

interface $Iterable<Y, R, N> {
  @@iterator(): $Iterator<Y, R, N>;
}
type Iterable<T> = $Iterable<T, *, *>;

// Generator<Y, R, N> should be an $Iterable<Y, R, N>
// Generator<Y, R, N> should be an Iterable<Y>
// Generator<Y, R, N> should be an $Iterator<Y, R, N>
// Generator<Y, R, N> should be an Iterator<Y>
interface Generator<Y, R, N> {
    @@iterator(): Iterator<Y>;
    next(value?: N): IteratorResult<Y|R>;
}

declare function $yieldDelegate<R, N>(g: $Iterable<*, R, N>, n:N): R

If we do this, I think we could deal with something like

yield *({ [Symbol.iterator]: function *() { return yield 4; } })

ignoring the fact that Flow doesn't have support for Symbol.iterator, of course :P

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm a bit unclear about what that buys us. Could you extend your example with a test case showing an expected type error?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So here's what I'm trying to do. If you yield * to a generator, your PR accurately tries to match the delegating generator's R and N to the delegated generator's R and N. This is right and correct and awesome.

If you're yield *'ing to something that is NOT a generator but IS an Iterable, then you're not doing anything with the delegating generator's R and N. This is fine if the Iterable is something like an array, since R is void and N is void. However, I can conceive of a custom Iterable who's Iterator is something fancy, like a generator.

So given the example above

yield *({ [Symbol.iterator]: function *() { return yield 4; } })

If you call next('batman'), this yield * expression should evaluate to batman. (The example in babel: https://goo.gl/1Xq5VV)

I think it's possible to capture this idea, by creating $Iterator and $Iterable which keep track of the N and R types too. I'm not possible it will work, but it would be cool!

/* Maps and Sets */
Expand Down
121 changes: 94 additions & 27 deletions src/typing/type_inference_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,6 @@ let rec convert cx type_params_map = Ast.Type.(function
mk_nominal_type cx reason type_params_map (c, params)
)

(* TODO: unsupported generators *)
| loc, Function { Function.params; returnType; rest; typeParameters } ->
let typeparams, type_params_map =
mk_type_param_declarations cx type_params_map typeParameters in
Expand Down Expand Up @@ -1975,8 +1974,8 @@ and statement cx type_params_map = Ast.Statement.(
let o = Flow_js.get_builtin_typeapp
cx
(mk_reason "iteration expected on Iterable" loc)
"Iterable"
[element_tvar] in
"$Iterable"
[element_tvar; AnyT.at loc; AnyT.at loc] in

Flow_js.flow cx (t, o); (* null/undefined are NOT allowed *)

Expand Down Expand Up @@ -2030,19 +2029,19 @@ and statement cx type_params_map = Ast.Statement.(
| (loc, Debugger) ->
()

(* TODO: unsupported generators *)
| (loc, FunctionDeclaration {
FunctionDeclaration.id;
params; defaults; rest;
body;
generator;
returnType;
typeParameters;
async;
_
}) ->
let reason = mk_reason "function" loc in
let this = Flow_js.mk_tvar cx (replace_reason "this" reason) in
let fn_type = mk_function None cx type_params_map reason ~async
let fn_type = mk_function None cx type_params_map reason ~async ~generator
typeParameters (params, defaults, rest) returnType body this
in
Hashtbl.replace cx.type_table loc fn_type;
Expand Down Expand Up @@ -2608,11 +2607,11 @@ and object_prop cx type_params_map map = Ast.Expression.Object.(function
_ }) ->
Ast.Expression.Function.(
let { params; defaults; rest; body;
returnType; typeParameters; id; async; _ } = func
returnType; typeParameters; id; async; generator; _ } = func
in
let reason = mk_reason "function" vloc in
let this = Flow_js.mk_tvar cx (replace_reason "this" reason) in
let ft = mk_function id cx type_params_map ~async reason typeParameters
let ft = mk_function id cx type_params_map ~async ~generator reason typeParameters
(params, defaults, rest) returnType body this
in
Hashtbl.replace cx.type_table vloc ft;
Expand Down Expand Up @@ -2662,7 +2661,7 @@ and object_prop cx type_params_map map = Ast.Expression.Object.(function
let { body; returnType; _ } = func in
let reason = mk_reason "getter function" vloc in
let this = Flow_js.mk_tvar cx (replace_reason "this" reason) in
let function_type = mk_function None cx type_params_map ~async:false reason None
let function_type = mk_function None cx type_params_map ~async:false ~generator:false reason None
([], [], None) returnType body this
in
let return_t = extract_getter_type function_type in
Expand All @@ -2687,7 +2686,7 @@ and object_prop cx type_params_map map = Ast.Expression.Object.(function
let { params; defaults; body; returnType; _ } = func in
let reason = mk_reason "setter function" vloc in
let this = Flow_js.mk_tvar cx (replace_reason "this" reason) in
let function_type = mk_function None cx type_params_map ~async:false reason None
let function_type = mk_function None cx type_params_map ~async:false ~generator:false reason None
(params, defaults, None) returnType body this
in
let param_t = extract_setter_type function_type in
Expand Down Expand Up @@ -3372,22 +3371,23 @@ and expression_ ~is_cond cx type_params_map loc e = Ast.Expression.(match e with
params; defaults; rest;
body;
async;
generator;
returnType;
typeParameters;
_
} ->
let desc = (if async then "async " else "") ^ "function" in
let reason = mk_reason desc loc in
let this = Flow_js.mk_tvar cx (replace_reason "this" reason) in
mk_function id cx type_params_map reason ~async
mk_function id cx type_params_map reason ~async ~generator
typeParameters (params, defaults, rest) returnType body this

(* TODO: unsupported generators *)
| ArrowFunction {
ArrowFunction.id;
params; defaults; rest;
body;
async;
generator;
returnType;
typeParameters;
_
Expand All @@ -3396,7 +3396,7 @@ and expression_ ~is_cond cx type_params_map loc e = Ast.Expression.(match e with
let reason = mk_reason desc loc in
let this = this_ cx reason in
let super = super_ cx reason in
mk_arrow id cx type_params_map reason ~async
mk_arrow id cx type_params_map reason ~async ~generator
typeParameters (params, defaults, rest) returnType body this super

| TaggedTemplate {
Expand Down Expand Up @@ -3452,8 +3452,37 @@ and expression_ ~is_cond cx type_params_map loc e = Ast.Expression.(match e with
class_t;
| None -> mk_class cx type_params_map loc reason c)

| Yield { Yield.argument; delegate = false } ->
let reason = mk_reason "yield" loc in
let yield = Env_js.get_var cx (internal_name "yield") reason in
let t = expression cx type_params_map argument in
Flow_js.flow cx (t, yield);
let next = Env_js.get_var cx (internal_name "next") reason in
OptionalT next

| Yield { Yield.argument; delegate = true } ->
let reason = mk_reason "yield* delegate" loc in
let next = Env_js.get_var cx
(internal_name "next")
(prefix_reason "next of parent generator in " reason) in
let yield = Env_js.get_var cx
(internal_name "yield")
(prefix_reason "yield of parent generator in " reason) in
let t = expression cx type_params_map argument in

let ret = Flow_js.mk_tvar cx
(prefix_reason "return of child generator in " reason) in

(* widen yield with the element type of the delegated-to iterable *)
let iterable = Flow_js.get_builtin_typeapp cx
(mk_reason "iteration expected on Iterable" loc)
"$Iterable"
[yield; ret; next] in
Flow_js.flow cx (t, iterable);

ret

(* TODO *)
| Yield _
| Comprehension _
| Generator _
| Let _ ->
Expand Down Expand Up @@ -4243,7 +4272,7 @@ and react_create_class cx type_params_map loc class_props = Ast.Expression.(
returnType; typeParameters; _ } = func
in
let reason = mk_reason "defaultProps" vloc in
let t = mk_method cx type_params_map reason ~async:false (params, defaults, rest)
let t = mk_method cx type_params_map reason ~async:false ~generator:false (params, defaults, rest)
returnType body this (MixedT reason)
in
(match t with
Expand All @@ -4264,7 +4293,7 @@ and react_create_class cx type_params_map loc class_props = Ast.Expression.(
returnType; typeParameters; _ } = func
in
let reason = mk_reason "initialState" vloc in
let t = mk_method cx type_params_map reason ~async:false (params, defaults, rest)
let t = mk_method cx type_params_map reason ~async:false ~generator:false (params, defaults, rest)
returnType body this (MixedT reason)
in
let override_state =
Expand All @@ -4284,10 +4313,10 @@ and react_create_class cx type_params_map loc class_props = Ast.Expression.(
_ }) ->
Ast.Expression.Function.(
let { params; defaults; rest; body;
returnType; typeParameters; async; _ } = func
returnType; typeParameters; async; generator; _ } = func
in
let reason = mk_reason "function" vloc in
let t = mk_method cx type_params_map reason ~async (params, defaults, rest)
let t = mk_method cx type_params_map reason ~async ~generator (params, defaults, rest)
returnType body this (MixedT reason)
in
fmap, SMap.add name t mmap
Expand Down Expand Up @@ -5086,6 +5115,8 @@ and mk_class_elements cx instance_info static_info body = Ast.Class.(
let this, super, method_sigs, getter_sigs, setter_sigs =
if static then static_info else instance_info
in
let yield = MixedT (mk_reason "no yield" loc) in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this considering generator methods? It looks like you're not pivoting on the generator param of the Function expression above...

(Also mind adding a test or two for class methods in any case?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I... actually didn't realize generator methods were possible. I'll add a suite of tests and add the probably missing implementation stuff. This PR should be blocked until then. Thanks for catching this.

let next = MixedT (mk_reason "no next" loc) in

let sigs_to_use = match kind with
| Method.Constructor
Expand All @@ -5111,7 +5142,7 @@ and mk_class_elements cx instance_info static_info body = Ast.Class.(
| _ -> name = "constructor"
in
mk_body None cx type_params_map ~async ~derived_ctor
param_types_map param_loc_map ret body this super;
param_types_map param_loc_map ret body this super yield next;
);
ignore (Abnormal.swap Abnormal.Return save_return_exn);
ignore (Abnormal.swap Abnormal.Throw save_throw_exn)
Expand Down Expand Up @@ -5463,7 +5494,7 @@ and mk_interface cx reason_i typeparams type_params_map
signature consisting of type parameters, parameter types, parameter names,
and return type, check the body against that signature by adding `this` and
`super` to the environment, and return the signature. *)
and function_decl id cx type_params_map (reason:reason) ~async
and function_decl id cx type_params_map (reason:reason) ~async ~generator
type_params params ret body this super =

let typeparams, type_params_map =
Expand All @@ -5472,6 +5503,23 @@ and function_decl id cx type_params_map (reason:reason) ~async
let (params, pnames, ret, param_types_map, param_types_loc) =
mk_params_ret cx type_params_map params (body_loc body, ret) in

(* If this is a generator function, the return type annotation can be an
application of the Generator type. We don't want to flow the explicit or
phantom return type into the Generator typeapp, but we still want to be
able to flow the Generator type constructed below into the annotation, so
we store off the converted annotation in _ret until then and proceed with a
tvar in its place. *)
let _ret = ret in
let (yield,ret,next) = if generator then (
Flow_js.mk_tvar cx (prefix_reason "yield of " reason),
Flow_js.mk_tvar cx (prefix_reason "return of " reason),
Flow_js.mk_tvar cx (prefix_reason "next of " reason)
) else (
MixedT (replace_reason "no yield" reason),
ret,
MixedT (replace_reason "no next" reason)
) in

let save_return_exn = Abnormal.swap Abnormal.Return false in
let save_throw_exn = Abnormal.swap Abnormal.Throw false in
Flow_js.generate_tests cx reason typeparams (fun map_ ->
Expand All @@ -5481,7 +5529,7 @@ and function_decl id cx type_params_map (reason:reason) ~async
param_types_map |> SMap.map (Flow_js.subst cx map_) in
let ret = Flow_js.subst cx map_ ret in

mk_body id cx type_params_map ~async param_types_map param_types_loc ret body this super;
mk_body id cx type_params_map ~async param_types_map param_types_loc ret body this super yield next;
);

ignore (Abnormal.swap Abnormal.Return save_return_exn);
Expand All @@ -5493,6 +5541,23 @@ and function_decl id cx type_params_map (reason:reason) ~async
else ret
in

(* If this is a generator function, we don't want to use the type from the
return statement as the return type of the function. Instead, we want to
return a Generator typeapp where the inferred return type flows into the
Generator's R type param. Since generator functions can have explicit type
annotations, flow the inferred type into the annotation as well. *)
let ret =
if generator then
let t = Flow_js.get_builtin_typeapp
cx
reason
"Generator"
[yield; ret; next] in
Flow_js.flow cx (t, _ret);
t
else ret
in

(typeparams,params,pnames,ret)

and is_void cx = function
Expand Down Expand Up @@ -5535,7 +5600,7 @@ and define_internal cx reason x =
Env_js.set_var cx ix (Flow_js.filter_optional cx reason opt) reason

and mk_body id cx type_params_map ~async ?(derived_ctor=false)
param_types_map param_locs_map ret body this super =
param_types_map param_locs_map ret body this super yield next =
let ctx = Env_js.get_scopes () in
let new_ctx = Env_js.clone_scopes ctx in
Env_js.update_env cx new_ctx;
Expand All @@ -5556,6 +5621,8 @@ and mk_body id cx type_params_map ~async ?(derived_ctor=false)
add name entry scope);
(* special bindings for this, super, and return value slot *)
initialize_this_super derived_ctor this super scope;
add (internal_name "yield") (create_entry yield yield None) scope;
add (internal_name "next") (create_entry next next None) scope;
add (internal_name "return") (create_entry ret ret None) scope;
scope
) in
Expand Down Expand Up @@ -5723,18 +5790,18 @@ and extract_type_param_instantiations = function
| Some (_, typeParameters) -> typeParameters.Ast.Type.ParameterInstantiation.params

(* Process a function definition, returning a (polymorphic) function type. *)
and mk_function id cx type_params_map reason ~async type_params params ret body this =
and mk_function id cx type_params_map reason ~async ~generator type_params params ret body this =
(* Normally, functions do not have access to super. *)
let super = MixedT (replace_reason "empty super object" reason) in
let signature =
function_decl id cx type_params_map reason ~async type_params params ret body this super
function_decl id cx type_params_map reason ~async ~generator type_params params ret body this super
in
mk_function_type cx reason this signature

(* Process an arrow function, returning a (polymorphic) function type. *)
and mk_arrow id cx type_params_map reason ~async type_params params ret body this super =
and mk_arrow id cx type_params_map reason ~async ~generator type_params params ret body this super =
let signature =
function_decl id cx type_params_map reason ~async type_params params ret body this super
function_decl id cx type_params_map reason ~async ~generator type_params params ret body this super
in
(* Do not expose the type of `this` in the function's type. The call to
function_decl above has already done the necessary checking of `this` in
Expand Down Expand Up @@ -5767,9 +5834,9 @@ and mk_function_type cx reason this signature =

(* This function is around for the sole purpose of modeling some method-like
behaviors of non-ES6 React classes. It is otherwise deprecated. *)
and mk_method cx type_params_map reason ~async params ret body this super =
and mk_method cx type_params_map reason ~async ~generator params ret body this super =
let (_,params,pnames,ret) =
function_decl None cx type_params_map ~async reason None params ret body this super
function_decl None cx type_params_map ~async ~generator reason None params ret body this super
in
FunT (reason, Flow_js.dummy_static, Flow_js.dummy_prototype,
Flow_js.mk_functiontype2
Expand Down
14 changes: 7 additions & 7 deletions tests/async/async.exp
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,29 @@ async.js:12:3,11: async return
Error:
async.js:12:10,10: number
This type is incompatible with
[LIB] core.js:367:32,45: union type
[LIB] core.js:375:32,45: union type

async.js:30:30,35: number
This type is incompatible with
[LIB] core.js:367:32,45: union type
[LIB] core.js:375:32,45: union type

async.js:45:22,25: undefined
This type is incompatible with
[LIB] core.js:352:1,380:1: Promise
[LIB] core.js:360:1,388:1: Promise

async2.js:6:3,12: async return
Error:
async2.js:6:10,11: number
This type is incompatible with
[LIB] core.js:367:32,45: union type
[LIB] core.js:375:32,45: union type

async2.js:29:21,24: undefined
This type is incompatible with
[LIB] core.js:352:1,380:1: Promise
[LIB] core.js:360:1,388:1: Promise

async2.js:43:28,31: undefined
This type is incompatible with
[LIB] core.js:352:1,380:1: Promise
[LIB] core.js:360:1,388:1: Promise

async2.js:48:3,17: undefined
This type is incompatible with
Expand All @@ -35,7 +35,7 @@ async3.js:23:11,21: await
Error:
async3.js:12:10,11: number
This type is incompatible with
[LIB] core.js:367:32,45: union type
[LIB] core.js:375:32,45: union type

await_not_in_async.js:5:9,9: Unexpected number

Expand Down
Loading