diff --git a/src/typing/context.ml b/src/typing/context.ml index a771eaa19ab..6022d21ea35 100644 --- 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 index 1c9ecbd910a..55bec17f20a 100644 --- 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 deleted file mode 100644 index 4a0f4c4dd4d..00000000000 --- a/src/typing/enum_exhaustive_check.ml +++ /dev/null @@ -1,70 +0,0 @@ -(* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - *) - -open Type - -type enum_search_result = - | Empty - | SingleEnum of (Reason.t * enum_t) - | Other - -let search_for_enum_object_type cx t = - let rec f ((seen_ids, result) as acc) = function - | DefT (reason, _, EnumObjectT enum) -> - let result = - match result with - | Empty -> SingleEnum (reason, enum) - | SingleEnum _ - | Other -> - Other - in - (seen_ids, result) - | OpenT (_, id) -> - if ISet.mem id seen_ids then - acc - else - List.fold_left f (ISet.add id seen_ids, result) (Flow_js.possible_types cx id) - | AnnotT (_, t, _) - | ReposT (_, t) -> - f acc t - | _ -> (seen_ids, Other) - in - let (_, result) = f (ISet.empty, Empty) t in - result - -let detect_invalid_check - cx - { - Context.check_reason; - enum_reason; - enum = { members; enum_id; _ }; - exhaustive_check = { checks; default_case }; - } = - let check_member (members_remaining, seen) (EnumCheck { reason; member_name; obj_t }) = - match search_for_enum_object_type cx obj_t with - | SingleEnum (_, { enum_id = check_enum_id; _ }) - when ALoc.equal_id enum_id check_enum_id && SMap.mem member_name members -> - if not @@ SMap.mem member_name members_remaining then - Flow_js.add_output - cx - (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) - | _ -> (members_remaining, 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) -> - Flow_js.add_output - cx - (Error_message.EEnumNotAllChecked - { reason = check_reason; enum_reason; left_to_check = SMap.keys left_over }) - | (true, Some default_case_reason) -> - Flow_js.add_output - cx - (Error_message.EEnumAllMembersAlreadyChecked { reason = default_case_reason; enum_reason }) - | _ -> () diff --git a/src/typing/enum_exhaustive_check.mli b/src/typing/enum_exhaustive_check.mli deleted file mode 100644 index 5df5f11b2ba..00000000000 --- a/src/typing/enum_exhaustive_check.mli +++ /dev/null @@ -1,8 +0,0 @@ -(* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - *) - -val detect_invalid_check : Context.t -> Context.enum_exhaustive_check_with_context -> unit diff --git a/src/typing/flow_js.ml b/src/typing/flow_js.ml index 7ade71e826a..e429ac75bbe 100644 --- a/src/typing/flow_js.ml +++ b/src/typing/flow_js.ml @@ -6512,12 +6512,53 @@ 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 @@ -6525,7 +6566,13 @@ struct (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 index 6bde02a1b87..a405d286aba 100644 --- 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 index a40d3c3db51..12518bcc1ed 100644 --- 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 diff --git a/src/typing/type.ml b/src/typing/type.ml index 40007c10fa3..7240f07c84d 100644 --- a/src/typing/type.ml +++ b/src/typing/type.ml @@ -782,18 +782,30 @@ module rec TypeTerm : sig and enum_check_t = | EnumCheck of { - reason: Reason.t; + reason: reason; member_name: string; - obj_t: t; } - and enum_exhaustive_check_t = { - checks: enum_check_t list; - default_case: reason option; - } + (* Valid state transitions are: + * EnumResolveDiscriminant -> EnumResolveCaseTest (with discriminant info populated) + * EnumResolveCaseTest -> EnumResolveCaseTest *) + and enum_exhaustive_check_tool_t = + | EnumResolveDiscriminant + | EnumResolveCaseTest of { + discriminant_enum: enum_t; + discriminant_reason: reason; + check: enum_check_t; + } and enum_possible_exhaustive_check_t = - | EnumExhaustiveCheckPossiblyValid of enum_exhaustive_check_t + | EnumExhaustiveCheckPossiblyValid of { + tool: enum_exhaustive_check_tool_t; + (* We only convert a "possible check" into a "check" if it has the same + * enum type as the discriminant. *) + possible_checks: (t * enum_check_t) list; + checks: enum_check_t list; + default_case: reason option; + } | EnumExhaustiveCheckInvalid of reason list and cond_context = diff --git a/src/typing/type_mapper.ml b/src/typing/type_mapper.ml index 59178a6670a..e518bf3ac0c 100644 --- a/src/typing/type_mapper.ml +++ b/src/typing/type_mapper.ml @@ -1111,20 +1111,22 @@ class virtual ['a] t_with_uses = EnumCastT { use_op; enum = (reason, trust, enum') } | EnumExhaustiveCheckT (r, exhaustive_check) -> (match exhaustive_check with - | EnumExhaustiveCheckPossiblyValid { checks; default_case } -> - let map_check (EnumCheck { reason; member_name; obj_t } as check) = + | EnumExhaustiveCheckPossiblyValid { tool; possible_checks; checks; default_case } -> + let map_possible_check ((obj_t, check) as possible_check) = let obj_t' = self#type_ cx map_cx obj_t in if obj_t' == obj_t then - check + possible_check else - EnumCheck { reason; member_name; obj_t = obj_t' } + (obj_t', check) in - let checks' = ListUtils.ident_map map_check checks in - if checks' == checks then + let possible_checks' = ListUtils.ident_map map_possible_check possible_checks in + if possible_checks' == possible_checks then t else EnumExhaustiveCheckT - (r, EnumExhaustiveCheckPossiblyValid { checks = checks'; default_case }) + ( r, + EnumExhaustiveCheckPossiblyValid + { tool; possible_checks = possible_checks'; checks; default_case } ) | EnumExhaustiveCheckInvalid _ -> t) | FilterOptionalT (use_op, t') -> let t'' = self#type_ cx map_cx t' in diff --git a/src/typing/type_visitor.ml b/src/typing/type_visitor.ml index 12f6f9a8c30..59355ba47ca 100644 --- a/src/typing/type_visitor.ml +++ b/src/typing/type_visitor.ml @@ -354,11 +354,11 @@ class ['a] t = self#type_ cx pole_TODO acc representation_t | EnumExhaustiveCheckT (_, exhaustive_check) -> (match exhaustive_check with - | EnumExhaustiveCheckPossiblyValid { checks; _ } -> + | EnumExhaustiveCheckPossiblyValid { possible_checks; _ } -> List.fold_left - (fun acc (EnumCheck { obj_t; _ }) -> self#type_ cx pole_TODO acc obj_t) + (fun acc (obj_t, _) -> self#type_ cx pole_TODO acc obj_t) acc - checks + possible_checks | EnumExhaustiveCheckInvalid _ -> acc) | FilterOptionalT (_, t) -> self#type_ cx pole_TODO acc t | FilterMaybeT (_, t) -> self#type_ cx pole_TODO acc t diff --git a/tests/enums/enums.exp b/tests/enums/enums.exp index ed3ba450118..3067cf5bea5 100644 --- a/tests/enums/enums.exp +++ b/tests/enums/enums.exp @@ -1859,8 +1859,9 @@ References: Error --------------------------------------------------------------------------------------- exhaustive-check.js:176:11 -Incomplete exhaustive check: the member `B` of enum `E` [1] has not been considered in check of `x`. -[invalid-exhaustive-check] +All branches are incompatible: [incompatible-use] + - Either incomplete exhaustive check: the member `B` of enum `E` [1] has not been considered in check of `x`. + - Or incomplete exhaustive check: the member `B` of enum `E` [2] has not been considered in check of `x`. exhaustive-check.js:176:11 176| switch (x) { // Error @@ -1870,14 +1871,110 @@ References: exhaustive-check.js:170:16 170| function f7(x: E & E) { ^ [1] + exhaustive-check.js:170:20 + 170| function f7(x: E & E) { + ^ [2] + + +Error --------------------------------------------------------------------------------------- exhaustive-check.js:188:11 + +Incomplete exhaustive check: the member `A` of enum `M` [1] has not been considered in check of `x`. +[invalid-exhaustive-check] + + exhaustive-check.js:188:11 + 188| switch (x) { // Error + ^ + +References: + exhaustive-check.js:187:16 + 187| function f8(x: M | N, X: typeof M | typeof N) { + ^ [1] + + +Error --------------------------------------------------------------------------------------- exhaustive-check.js:188:11 + +Incomplete exhaustive check: the member `A` of enum `N` [1] has not been considered in check of `x`. +[invalid-exhaustive-check] + + exhaustive-check.js:188:11 + 188| switch (x) { // Error + ^ + +References: + exhaustive-check.js:187:20 + 187| function f8(x: M | N, X: typeof M | typeof N) { + ^ [1] + + +Error --------------------------------------------------------------------------------------- exhaustive-check.js:189:10 + +Invalid check of case test against switch discriminant [1] because `M` [2] is incompatible with `N` [3]. +[incompatible-type] + + exhaustive-check.js:189:10 + 189| case X.A: break; // Error + ^^^ + +References: + exhaustive-check.js:188:11 + 188| switch (x) { // Error + ^ [1] + exhaustive-check.js:181:1 + v------- + 181| enum M { + 182| A, + 183| } + ^ [2] + exhaustive-check.js:187:20 + 187| function f8(x: M | N, X: typeof M | typeof N) { + ^ [3] + + +Error --------------------------------------------------------------------------------------- exhaustive-check.js:189:10 + +Invalid check of case test against switch discriminant [1] because `N` [2] is incompatible with `M` [3]. +[incompatible-type] + + exhaustive-check.js:189:10 + 189| case X.A: break; // Error + ^^^ + +References: + exhaustive-check.js:188:11 + 188| switch (x) { // Error + ^ [1] + exhaustive-check.js:184:1 + v------- + 184| enum N { + 185| A, + 186| } + ^ [2] + exhaustive-check.js:187:16 + 187| function f8(x: M | N, X: typeof M | typeof N) { + ^ [3] + + +Error --------------------------------------------------------------------------------------- exhaustive-check.js:209:11 + +Incomplete exhaustive check: the members `B` and `A` of enum `E` [1] have not been considered in check of `x`. +[invalid-exhaustive-check] + + exhaustive-check.js:209:11 + 209| switch (x) { // Error + ^ + +References: + exhaustive-check.js:208:17 + 208| function f11(x: E, X: empty) { + ^ [1] -Error --------------------------------------------------------------------------------------- exhaustive-check.js:184:10 +Error --------------------------------------------------------------------------------------- exhaustive-check.js:217:10 Cannot access `XXX` because `XXX` is not a member of enum `E` [1]. [invalid-enum-access] - exhaustive-check.js:184:10 - 184| case E.XXX: break; // Error + exhaustive-check.js:217:10 + 217| case E.XXX: break; // Error ^^^ References: @@ -3389,7 +3486,7 @@ References: -Found 185 errors +Found 190 errors Only showing the most relevant union/intersection branches. To see all branches, re-run Flow with --show-all-branches diff --git a/tests/enums/exhaustive-check.js b/tests/enums/exhaustive-check.js index ffe9e148bb8..ab98735f5a9 100644 --- a/tests/enums/exhaustive-check.js +++ b/tests/enums/exhaustive-check.js @@ -116,7 +116,7 @@ switch (s) { case E.B: break; // Error } -// Discriminant is union +// Unions and intersections function f1(x?: E) { switch (x) { case E.A: break; // Error @@ -178,6 +178,39 @@ function f7(x: E & E) { } } +enum M { + A, +} +enum N { + A, +} +function f8(x: M | N, X: typeof M | typeof N) { + switch (x) { // Error + case X.A: break; // Error + } +} + +function f9(x: E | F, X: typeof E & typeof F) { + switch (x) { + case X.A: break; + case X.B: break; + } +} + +// Empty +function f10(x: empty) { + switch (x) { + case E.A: break; + case E.B: break; + } +} + +function f11(x: E, X: empty) { + switch (x) { // Error + case X.A: break; + } +} + // Invalid enum member switch (x) { case E.A: break;