[enums] Move exhaustive checking logic to flow_js.ml

Summary: Move exhaustive checking logic to flow_js.ml. This does several things: - Cleans up the code a bit as we no longer need to store information in the context - We no longer need to have logic for getting the resolved type of case tests (e.g. the former `search_for_enum_object_type`) - This allows exhaustive checking to potentially introduce flows in the future, if desired General idea is that we initially use `EnumResolveDiscriminant` and resolve and store the information about the discriminant, and then use `EnumResolveCaseTest` to resolve each case test one by one, adding them as `checks` if they are valid. Finally, when there are no `possible_checks` left, we trigger the exhaustive checking logic. Based off of an idea by samwgoldman Reviewed By: samwgoldman Differential Revision: D21758989 fbshipit-source-id: 0d70ecb3e478724e09738cc2536cb58a9c0dd6fe
facebook · May 29, 2020 · 71939e7 · 71939e7
1 parent a193b68
commit 71939e7
Show file tree

Hide file tree

Showing 12 changed files with 287 additions and 148 deletions.
diff --git a/src/typing/context.ml b/src/typing/context.ml
@@ -82,13 +82,6 @@ type computed_property_state =
   | ResolvedOnce of Reason.t
   | ResolvedMultipleTimes
 
-type enum_exhaustive_check_with_context = {
-  check_reason: Reason.t;
-  enum_reason: Reason.t;
-  enum: Type.enum_t;
-  exhaustive_check: Type.enum_exhaustive_check_t;
-}
-
 type sig_t = {
   (* map from tvar ids to nodes (type info structures) *)
   mutable graph: Constraint.node IMap.t;
@@ -158,7 +151,6 @@ type component_t = {
   mutable spread_widened_types: Type.Object.slice IMap.t;
   mutable optional_chains_useful: (Reason.t * bool) ALocMap.t;
   mutable invariants_useful: (Reason.t * bool) ALocMap.t;
-  mutable enum_exhaustive_checks: enum_exhaustive_check_with_context list;
   mutable openness_graph: Openness.graph;
 }
 
@@ -298,7 +290,6 @@ let make_ccx sig_cx =
     spread_widened_types = IMap.empty;
     optional_chains_useful = ALocMap.empty;
     invariants_useful = ALocMap.empty;
-    enum_exhaustive_checks = [];
     openness_graph = Openness.empty_graph;
   }
 
@@ -726,11 +717,6 @@ let unnecessary_invariants cx =
     cx.ccx.invariants_useful
     []
 
-let add_enum_exhaustive_check cx check =
-  cx.ccx.enum_exhaustive_checks <- check :: cx.ccx.enum_exhaustive_checks
-
-let enum_exhaustive_checks cx = cx.ccx.enum_exhaustive_checks
-
 (* utils *)
 let find_real_props cx id =
   find_props cx id |> SMap.filter (fun x _ -> not (Reason.is_internal_name x))

diff --git a/src/typing/context.mli b/src/typing/context.mli
@@ -38,13 +38,6 @@ type component_t
  * resolved tvars. *)
 type sig_t
 
-type enum_exhaustive_check_with_context = {
-  check_reason: Reason.t;
-  enum_reason: Reason.t;
-  enum: Type.enum_t;
-  exhaustive_check: Type.enum_exhaustive_check_t;
-}
-
 type metadata = {
   (* local *)
   checked: bool;
@@ -407,10 +400,6 @@ val mark_invariant : t -> ALoc.t -> Reason.t -> useful:bool -> unit
 
 val unnecessary_invariants : t -> (ALoc.t * Reason.t) list
 
-val add_enum_exhaustive_check : t -> enum_exhaustive_check_with_context -> unit
-
-val enum_exhaustive_checks : t -> enum_exhaustive_check_with_context list
-
 (* utils *)
 val iter_props : t -> Type.Properties.id -> (string -> Type.Property.t -> unit) -> unit
 

diff --git a/src/typing/enum_exhaustive_check.ml b/src/typing/enum_exhaustive_check.ml
diff --git a/src/typing/enum_exhaustive_check.mli b/src/typing/enum_exhaustive_check.mli
diff --git a/src/typing/flow_js.ml b/src/typing/flow_js.ml
@@ -6512,20 +6512,67 @@ struct
                  use_op;
                  representation_type;
                })
+        (* Entry point to exhaustive checking logic - when resolving the discriminant as an enum. *)
         | ( DefT (enum_reason, _, EnumT enum),
-            EnumExhaustiveCheckT (check_reason, EnumExhaustiveCheckPossiblyValid exhaustive_check)
-          ) ->
-          Context.add_enum_exhaustive_check
+            EnumExhaustiveCheckT
+              ( check_reason,
+                EnumExhaustiveCheckPossiblyValid
+                  { tool = EnumResolveDiscriminant; possible_checks; checks; default_case } ) ) ->
+          enum_exhaustive_check
+            cx
+            ~trace
+            ~check_reason
+            ~enum_reason
+            ~enum
+            ~possible_checks
+            ~checks
+            ~default_case
+        (* Resolving the case tests. *)
+        | ( _,
+            EnumExhaustiveCheckT
+              ( check_reason,
+                EnumExhaustiveCheckPossiblyValid
+                  {
+                    tool = EnumResolveCaseTest { discriminant_reason; discriminant_enum; check };
+                    possible_checks;
+                    checks;
+                    default_case;
+                  } ) ) ->
+          let (EnumCheck { member_name; _ }) = check in
+          let { enum_id = enum_id_discriminant; members; _ } = discriminant_enum in
+          let checks =
+            match l with
+            | DefT (_, _, EnumObjectT { enum_id = enum_id_check; _ })
+              when ALoc.equal_id enum_id_discriminant enum_id_check && SMap.mem member_name members
+              ->
+              check :: checks
+            (* If the check is not the same enum type, ignore it and continue. The user will
+             * still get an error as the comparison between discriminant and case test will fail. *)
+            | _ -> checks
+          in
+          enum_exhaustive_check
             cx
-            { Context.check_reason; enum_reason; enum; exhaustive_check }
+            ~trace
+            ~check_reason
+            ~enum_reason:discriminant_reason
+            ~enum:discriminant_enum
+            ~possible_checks
+            ~checks
+            ~default_case
         | ( DefT (_, _, EnumT { enum_name; members; _ }),
             EnumExhaustiveCheckT (_, EnumExhaustiveCheckInvalid reasons) ) ->
           let example_member = SMap.choose_opt members |> Base.Option.map ~f:fst in
           List.iter
             (fun reason ->
               add_output cx (Error_message.EEnumInvalidCheck { reason; enum_name; example_member }))
             reasons
-        | (_, EnumExhaustiveCheckT _) -> () (* ignore non-enum exhaustive checks *)
+        (* Ignore non-enum exhaustive checks. *)
+        | ( _,
+            EnumExhaustiveCheckT
+              ( _,
+                ( EnumExhaustiveCheckInvalid _
+                | EnumExhaustiveCheckPossiblyValid { tool = EnumResolveDiscriminant; _ } ) ) ) ->
+          ()
         (**************************************************************************)
         (* TestPropT is emitted for property reads in the context of branch tests.
        Such tests are always non-strict, in that we don't immediately report an
@@ -7505,6 +7552,7 @@ struct
     | (_, ChoiceKitUseT _)
     | (_, CondT _)
     | (_, DestructuringT _)
+    | (_, EnumExhaustiveCheckT _)
     | (_, MakeExactT _)
     | (_, ObjKitT _)
     | (_, ReposLowerT _)
@@ -7632,7 +7680,6 @@ struct
     | (_, TypeAppVarianceCheckT _)
     | (_, TypeCastT _)
     | (_, EnumCastT _)
-    | (_, EnumExhaustiveCheckT _)
     | (_, UnaryMinusT _)
     | (_, VarianceCheckT _)
     | (_, ModuleExportsAssignT _)
@@ -8588,13 +8635,59 @@ struct
     in
     reposition cx ?trace (aloc_of_reason reason) result
 
-  (***************)
-  (* enums utils *)
-  (***************)
+  (*********)
+  (* enums *)
+  (*********)
   and enum_proto cx trace ~reason (enum_reason, trust, enum) =
     let enum_t = DefT (enum_reason, trust, EnumT enum) in
     let { representation_t; _ } = enum in
     get_builtin_typeapp cx ~trace reason "$EnumProto" [enum_t; representation_t]
+
+  and enum_exhaustive_check
+      cx ~trace ~check_reason ~enum_reason ~enum ~possible_checks ~checks ~default_case =
+    match possible_checks with
+    (* No possible checks left to resolve, analyze the exhaustive check. *)
+    | [] ->
+      let { members; _ } = enum in
+      let check_member (members_remaining, seen) (EnumCheck { reason; member_name }) =
+        if not @@ SMap.mem member_name members_remaining then
+          add_output
+            cx
+            ~trace
+            (Error_message.EEnumMemberAlreadyChecked
+               { reason; prev_check_reason = SMap.find member_name seen; enum_reason; member_name });
+        (SMap.remove member_name members_remaining, SMap.add member_name reason seen)
+      in
+      let (left_over, _) = List.fold_left check_member (members, SMap.empty) checks in
+      (match (SMap.is_empty left_over, default_case) with
+      | (false, None) ->
+        add_output
+          cx
+          ~trace
+          (Error_message.EEnumNotAllChecked
+             { reason = check_reason; enum_reason; left_to_check = SMap.keys left_over })
+      | (true, Some default_case_reason) ->
+        add_output
+          cx
+          ~trace
+          (Error_message.EEnumAllMembersAlreadyChecked { reason = default_case_reason; enum_reason })
+      | _ -> ())
+    (* There are still possible checks to resolve, continue to resolve them. *)
+    | (obj_t, check) :: rest_possible_checks ->
+      let exhaustive_check =
+        EnumExhaustiveCheckT
+          ( check_reason,
+            EnumExhaustiveCheckPossiblyValid
+              {
+                tool =
+                  EnumResolveCaseTest
+                    { discriminant_enum = enum; discriminant_reason = enum_reason; check };
+                possible_checks = rest_possible_checks;
+                checks;
+                default_case;
+              } )
+      in
+      rec_flow cx trace (obj_t, exhaustive_check)
     (*******************************************************)
     (* Entry points into the process of trying different   *)
     (* branches of union and intersection types.           *)

diff --git a/src/typing/merge_js.ml b/src/typing/merge_js.ml
@@ -201,9 +201,6 @@ let detect_unnecessary_invariants cx =
 let detect_invalid_type_assert_calls cx file_sigs cxs tasts =
   if Context.type_asserts cx then Type_asserts.detect_invalid_calls ~full_cx:cx file_sigs cxs tasts
 
-let detect_invalid_enum_exhaustive_checks cx =
-  List.iter (Enum_exhaustive_check.detect_invalid_check cx) (Context.enum_exhaustive_checks cx)
-
 let force_annotations leader_cx other_cxs =
   Base.List.iter
     ~f:(fun cx ->
@@ -438,7 +435,6 @@ let merge_component
     detect_unnecessary_optional_chains cx;
     detect_unnecessary_invariants cx;
     detect_invalid_type_assert_calls cx file_sigs cxs tasts;
-    detect_invalid_enum_exhaustive_checks cx;
 
     force_annotations cx other_cxs;
 

diff --git a/src/typing/statement.ml b/src/typing/statement.ml
@@ -8386,30 +8386,39 @@ and enum_exhaustive_check_of_switch_cases cases_ast =
           when is_valid_enum_member_name name ->
           (match acc with
           | EnumExhaustiveCheckInvalid _ -> acc
-          | EnumExhaustiveCheckPossiblyValid { checks; default_case } ->
+          | EnumExhaustiveCheckPossiblyValid { tool; possible_checks; checks; default_case } ->
             let reason = mk_reason (RCustom "case") case_test_loc in
-            let check = EnumCheck { reason; member_name = name; obj_t } in
-            EnumExhaustiveCheckPossiblyValid { checks = check :: checks; default_case })
+            let possible_check = (obj_t, EnumCheck { reason; member_name = name }) in
+            EnumExhaustiveCheckPossiblyValid
+              { tool; possible_checks = possible_check :: possible_checks; checks; default_case })
         | (default_case_loc, { Case.test = None; _ }) ->
           (match acc with
           | EnumExhaustiveCheckInvalid _ -> acc
-          | EnumExhaustiveCheckPossiblyValid { checks; default_case = _ } ->
+          | EnumExhaustiveCheckPossiblyValid { tool; possible_checks; checks; default_case = _ } ->
             EnumExhaustiveCheckPossiblyValid
-              { checks; default_case = Some (mk_reason (RCustom "default case") default_case_loc) })
+              {
+                tool;
+                possible_checks;
+                checks;
+                default_case = Some (mk_reason (RCustom "default case") default_case_loc);
+              })
         | (_, { Case.test = Some ((case_test_loc, _), _); _ }) ->
           let case_reason = Reason.mk_reason (Reason.RCustom "case") case_test_loc in
           (match acc with
           | EnumExhaustiveCheckInvalid invalid_checks ->
             EnumExhaustiveCheckInvalid (case_reason :: invalid_checks)
           | EnumExhaustiveCheckPossiblyValid _ -> EnumExhaustiveCheckInvalid [case_reason]))
-      (EnumExhaustiveCheckPossiblyValid { checks = []; default_case = None })
+      (EnumExhaustiveCheckPossiblyValid
+         { tool = EnumResolveDiscriminant; possible_checks = []; checks = []; default_case = None })
       cases_ast
   in
   match exhaustive_check with
   | EnumExhaustiveCheckInvalid invalid_checks ->
     EnumExhaustiveCheckInvalid (List.rev invalid_checks)
-  | EnumExhaustiveCheckPossiblyValid { checks; default_case } ->
-    EnumExhaustiveCheckPossiblyValid { checks = List.rev checks; default_case }
+  | EnumExhaustiveCheckPossiblyValid _ ->
+    (* As we process `possible_checks` into `checks`, we reverse the list back
+     * into the correct order. *)
+    exhaustive_check
 
 and mk_enum cx ~enum_reason enum =
   let open Ast.Statement.EnumDeclaration in