Skip to content
Permalink
Browse files

Infer void early for functions with no explicit return

Summary:
This change makes Flow look through a function body early on in the `statement.ml` pass. If no `return` statement, `throw` statement, or `invariant` call is found, it locks the return type down to `void` early on, rather than generating a tvar for the return type.

This fixes the narrow case described in #7322, though the general case remains unsound. It's also theoretically an optimization, though any change in typechecking time appears to have been lost in the noise.

Closes #7322

Reviewed By: samwgoldman

Differential Revision: D14758836

fbshipit-source-id: 49ba9b83413619c41b739389d81935758a5102bb
  • Loading branch information...
nmote authored and facebook-github-bot committed Apr 8, 2019
1 parent 1ae6002 commit 7b1798a5443671010bdf9138985ed25cd6d0a9fe
@@ -86,6 +86,43 @@ let convert_targs cx = function
) targts;
Some targts, Some (loc, targs_ast)

class return_finder = object(this)
inherit [bool, ALoc.t] Flow_ast_visitor.visitor ~init:false as super

method! return _ node =
(* TODO we could pass over `return;` since it's definitely returning `undefined`. It will likely
* reposition existing errors from the `return;` to the location of the type annotation. *)
this#set_acc true;
node

method! call _loc expr =
begin if is_call_to_invariant Ast.Expression.Call.(expr.callee) then
this#set_acc true
end;
expr

method! throw _loc stmt =
this#set_acc true;
stmt

method! function_body_any body =
begin match body with
(* If it's a body expression, some value is implicitly returned *)
| Flow_ast.Function.BodyExpression _ -> this#set_acc true
| _ -> ()
end;
super#function_body_any body

(* Any returns in these constructs would be for nested function definitions, so we short-circuit
*)
method! class_ _ x = x
method! function_declaration _ x = x
end

let might_have_nonvoid_return loc function_ast =
let finder = new return_finder in
finder#eval (finder#function_ loc) function_ast

(************)
(* Visitors *)
(************)
@@ -6608,7 +6645,14 @@ and mk_func_sig =
let fparams, params = mk_params cx tparams_map params in
let body = Some body in
let ret_reason = mk_reason RReturn (return_loc func) in
let return_t, return = Anno.mk_type_annotation cx tparams_map ret_reason return in
let return_t, return =
let has_nonvoid_return = might_have_nonvoid_return loc func in
let definitely_returns_void =
kind = Ordinary &&
not has_nonvoid_return
in
Anno.mk_return_type_annotation cx tparams_map ret_reason ~definitely_returns_void return
in
let return_t, predicate = Ast.Type.Predicate.(match predicate with
| None ->
return_t, None
@@ -1376,6 +1376,16 @@ and mk_type_annotation cx tparams_map reason = function
let t, ast_annot = mk_type_available_annotation cx tparams_map annot in
t, T.Available ast_annot

and mk_return_type_annotation cx tparams_map reason ~definitely_returns_void annot =
match annot with
| T.Missing loc when definitely_returns_void ->
let t = VoidT.why reason |> with_trust literal_trust in
t, T.Missing (loc, t)
(* TODO we could probably take the same shortcut for functions with an explicit `void` annotation
and no explicit returns *)
| _ ->
mk_type_annotation cx tparams_map reason annot

and mk_type_available_annotation cx tparams_map (loc, annot) =
let (_, t), _ as annot_ast = convert cx tparams_map annot in
t, (loc, annot_ast)
@@ -46,6 +46,14 @@ val mk_type_annotation: Context.t ->
(ALoc.t, ALoc.t) Flow_ast.Type.annotation_or_hint ->
Type.t * (ALoc.t, ALoc.t * Type.t) Flow_ast.Type.annotation_or_hint

val mk_return_type_annotation:
Context.t ->
Type.t SMap.t ->
Reason.t ->
definitely_returns_void:bool ->
(ALoc.t, ALoc.t) Flow_ast.Type.annotation_or_hint ->
Type.t * (ALoc.t, ALoc.t * Type.t) Flow_ast.Type.annotation_or_hint

val mk_type_available_annotation: Context.t ->
Type.t SMap.t ->
(ALoc.t, ALoc.t) Flow_ast.Type.annotation ->
@@ -134,7 +134,7 @@ class-0.js:24:5 = {
"end":5
}
class-1.js:4:3 = {
"type":"() => (void | number)",
"type":"() => void",
"reasons":[],
"loc":{
"source":"class-1.js",
@@ -464,7 +464,7 @@ class-poly-1.js:9:11 = {
"end":11
}
class-statics.js:4:10 = {
"type":"() => (void | Class<B>)",
"type":"() => void",
"reasons":[],
"loc":{
"source":"class-statics.js",
@@ -569,7 +569,7 @@ class-statics.js:20:11 = {
"end":11
}
class-statics-poly.js:4:10 = {
"type":"() => (void | Class<B<mixed>> | Class<B<empty>>)",
"type":"() => void",
"reasons":[],
"loc":{
"source":"class-statics-poly.js",
@@ -29,6 +29,23 @@ References:
^^^ [2]


Error ----------------------------------------------------------------------------------------------- issue-7322.js:22:2

Cannot cast `x.noReturn()` to number because undefined [1] is incompatible with number [2].

issue-7322.js:22:2
22| (x.noReturn(): number);
^^^^^^^^^^^^

References:
issue-7322.js:4:13
4| noReturn() { }
^ [1]
issue-7322.js:22:16
22| (x.noReturn(): number);
^^^^^^ [2]


Error ----------------------------------------------------------------------------------------------------- union.js:2:5

Cannot call `bar` with `0` bound to `x` because number [1] is incompatible with string [2].
@@ -75,7 +92,7 @@ References:



Found 5 errors
Found 6 errors

Only showing the most relevant union/intersection branches.
To see all branches, re-run Flow with --show-all-branches

0 comments on commit 7b1798a

Please sign in to comment.
You can’t perform that action at this time.