Skip to content

Commit

Permalink
don't assert_false on invalid typeof values
Browse files Browse the repository at this point in the history
Summary: `typeof x === "foo"` should show a warning and return false, not assert.

while I was there, I also got rid of `IsP <string>` in favor of a more explicit variant.

See #712

Reviewed By: bhosmer

Differential Revision: D2586915

fb-gh-sync-id: 5761e3202e9849838fef940131f95fb8139cc297
  • Loading branch information
mroch authored and facebook-github-bot-3 committed Oct 30, 2015
1 parent 859be4d commit d45c099
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 65 deletions.
27 changes: 24 additions & 3 deletions src/typing/debug.ml
Expand Up @@ -21,7 +21,17 @@ let string_of_pred_ctor = function
| LeftP _ -> "LeftP"
| RightP _ -> "RightP"
| ExistsP -> "ExistsP"
| IsP _ -> "IsP"
| TrueP -> "TrueP"
| FalseP -> "FalseP"
| VoidP -> "VoidP"
| NullP -> "NullP"
| MaybeP -> "MaybeP"
| BoolP -> "BoolP"
| StrP -> "StrP"
| NumP -> "NumP"
| FunP -> "FunP"
| ObjP -> "ObjP"
| ArrP -> "ArrP"

let string_of_binary_test_ctor = function
| Instanceof -> "Instanceof"
Expand Down Expand Up @@ -554,8 +564,19 @@ and json_of_pred_impl json_cx p = Json.(
"binaryTest", json_of_binary_test json_cx b;
"type", _json_of_t json_cx t
]
| ExistsP -> []
| IsP s -> ["typeName", JString s]
| TrueP
| FalseP
| ExistsP
| VoidP
| NullP
| MaybeP
| BoolP
| StrP
| NumP
| FunP
| ObjP
| ArrP
-> []
))

and json_of_binary_test json_cx = check_depth json_of_binary_test_impl json_cx
Expand Down
76 changes: 44 additions & 32 deletions src/typing/flow_js.ml
Expand Up @@ -4322,131 +4322,131 @@ and predicate cx trace t (l,p) = match (l,p) with
(* typeof _ ~ "boolean" *)
(***********************)

| (MixedT r, IsP "boolean") ->
| (MixedT r, BoolP) ->
rec_flow cx trace (BoolT.why r, t)

| (_, IsP "boolean") ->
| (_, BoolP) ->
filter cx trace t l is_bool

| (_, NotP(IsP "boolean")) ->
| (_, NotP(BoolP)) ->
filter cx trace t l (not_ is_bool)

(***********************)
(* typeof _ ~ "string" *)
(***********************)

| (MixedT r, IsP "string") ->
| (MixedT r, StrP) ->
rec_flow cx trace (StrT.why r, t)

| (_, IsP "string") ->
| (_, StrP) ->
filter cx trace t l is_string

| (_, NotP(IsP "string")) ->
| (_, NotP(StrP)) ->
filter cx trace t l (not_ is_string)

(***********************)
(* typeof _ ~ "number" *)
(***********************)

| (MixedT r, IsP "number") ->
| (MixedT r, NumP) ->
rec_flow cx trace (NumT.why r, t)

| (_, IsP "number") ->
| (_, NumP) ->
filter cx trace t l is_number

| (_, NotP(IsP "number")) ->
| (_, NotP(NumP)) ->
filter cx trace t l (not_ is_number)

(***********************)
(* typeof _ ~ "function" *)
(***********************)

| (MixedT r, IsP "function") ->
| (MixedT r, FunP) ->
rec_flow cx trace (AnyFunT (replace_reason "function" r), t)

| (_, IsP "function") ->
| (_, FunP) ->
filter cx trace t l is_function

| (_, NotP(IsP "function")) ->
| (_, NotP(FunP)) ->
filter cx trace t l (not_ is_function)

(***********************)
(* typeof _ ~ "object" *)
(***********************)

| (MixedT r, IsP "object") ->
| (MixedT r, ObjP) ->
let obj = AnyObjT (replace_reason "object" r) in
let union = create_union [NullT.why r; obj] in
rec_flow cx trace (union, t)

| (_, IsP "object") ->
| (_, ObjP) ->
filter cx trace t l is_object

| (_, NotP(IsP "object")) ->
| (_, NotP(ObjP)) ->
filter cx trace t l (not_ is_object)

(*******************)
(* Array.isArray _ *)
(*******************)

| (MixedT r, IsP "array") ->
| (MixedT r, ArrP) ->
let filtered_l = ArrT (replace_reason "array" r, AnyT.why r, []) in
rec_flow cx trace (filtered_l, t)

| (_, IsP "array") ->
| (_, ArrP) ->
filter cx trace t l is_array

| (_, NotP(IsP "array")) ->
| (_, NotP(ArrP)) ->
filter cx trace t l (not_ is_array)

(***********************)
(* typeof _ ~ "undefined" *)
(***********************)

| (_, IsP "undefined") ->
| (_, VoidP) ->
rec_flow cx trace (filter_undefined l, t)

| (_, NotP(IsP "undefined")) ->
| (_, NotP(VoidP)) ->
rec_flow cx trace (filter_not_undefined l, t)

(********)
(* null *)
(********)

| (_, IsP "null") ->
| (_, NullP) ->
rec_flow cx trace (filter_null l, t)

| (_, NotP(IsP "null")) ->
| (_, NotP(NullP)) ->
rec_flow cx trace (filter_not_null l, t)

(*********)
(* maybe *)
(*********)

| (_, IsP "null or undefined") ->
| (_, MaybeP) ->
rec_flow cx trace (filter_maybe l, t)

| (_, NotP(IsP "null or undefined")) ->
| (_, NotP(MaybeP)) ->
rec_flow cx trace (filter_not_maybe l, t)

(********)
(* true *)
(********)

| (_, IsP "true") ->
| (_, TrueP) ->
rec_flow cx trace (filter_true l, t)

| (_, NotP(IsP "true")) ->
| (_, NotP(TrueP)) ->
rec_flow cx trace (filter_not_true l, t)

(*********)
(* false *)
(*********)

| (_, IsP "false") ->
| (_, FalseP) ->
rec_flow cx trace (filter_false l, t)

| (_, NotP(IsP "false")) ->
| (_, NotP(FalseP)) ->
rec_flow cx trace (filter_not_false l, t)

(************************)
Expand All @@ -4460,7 +4460,9 @@ and predicate cx trace t (l,p) = match (l,p) with
rec_flow cx trace (filter_not_exists l, t)

(* unreachable *)
| (_, (NotP _ | IsP _)) ->
| (_, NotP (NotP _))
| (_, NotP (AndP _))
| (_, NotP (OrP _)) ->
assert_false (spf "Unexpected predicate %s" (string_of_predicate p))

and binary_predicate cx trace sense test left right result =
Expand Down Expand Up @@ -5657,9 +5659,19 @@ and gc_pred cx state = function
| NotP (p) ->
gc_pred cx state p

| ExistsP -> ()

| IsP _ -> ()
| TrueP
| FalseP
| ExistsP
| NullP
| MaybeP
| BoolP
| FunP
| NumP
| ObjP
| StrP
| VoidP
| ArrP
-> ()

(* Keep a reachable type variable around. *)
let live cx state id =
Expand Down
47 changes: 41 additions & 6 deletions src/typing/type.ml
Expand Up @@ -315,11 +315,20 @@ and predicate =
| LeftP of binary_test * t
| RightP of binary_test * t

(* truthy *)
| ExistsP
| ExistsP (* truthy *)
| TrueP (* true *)
| FalseP (* false *)
| NullP (* null *)
| MaybeP (* null or undefined *)

| BoolP (* boolean *)
| FunP (* function *)
| NumP (* number *)
| ObjP (* object *)
| StrP (* string *)
| VoidP (* undefined *)

(* typeof, null check, Array.isArray *)
| IsP of string
| ArrP (* Array.isArray *)

and binary_test =
(* e1 instanceof e2 *)
Expand Down Expand Up @@ -735,7 +744,19 @@ let rec loc_of_predicate = function
-> loc_of_t t

| ExistsP
| IsP _
| TrueP
| FalseP
| NullP
| MaybeP

| BoolP
| FunP
| NumP
| ObjP
| StrP
| VoidP

| ArrP
-> Loc.none (* TODO!!!!!!!!!!!! *)


Expand Down Expand Up @@ -979,4 +1000,18 @@ let rec string_of_predicate = function
spf "right operand of %s with left operand = %s"
(string_of_binary_test b) (desc_of_t t)
| ExistsP -> "truthy"
| IsP s -> s
| TrueP -> "true"
| FalseP -> "false"
| NullP -> "null"
| MaybeP -> "null or undefined"

(* typeof *)
| VoidP -> "undefined"
| BoolP -> "boolean"
| StrP -> "string"
| NumP -> "number"
| FunP -> "function"
| ObjP -> "object"

(* Array.isArray *)
| ArrP -> "array"

0 comments on commit d45c099

Please sign in to comment.