Skip to content

Commit c313dc3

Browse files
gkzfacebook-github-bot
authored andcommitted
[flow][match] Propagate hints through match expressions case bodies
Summary: We need to propagate hints through match expressions case bodies. The simple case is when we have some external hint. More interesting is getting hints from sibling case bodies. This can come out of order, and indeed this might be a common pattern with match expressions as people like to list their edge cases first. E.g. ``` declare const x: number; const out = match (x) { 0: [], const x: [x], }; ``` So, we consider every sibling case body, from top to bottom, as a "best effort hint". The logic for hints diverges the match statement and match expression logic, so I break these up again into seperate methods. Changelog: [internal] Reviewed By: panagosg7 Differential Revision: D70140524 fbshipit-source-id: 499d5e8864964c32c6bce372743dec2adb4781cf
1 parent b55e5c3 commit c313dc3

File tree

2 files changed

+133
-25
lines changed

2 files changed

+133
-25
lines changed

src/analysis/env_builder/name_def.ml

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2871,8 +2871,7 @@ class def_finder ~autocomplete_hooks ~react_jsx env_info toplevel_scope =
28712871
| Ast.Expression.Conditional expr -> this#visit_conditional ~hints expr
28722872
| Ast.Expression.AsConstExpression { Ast.Expression.AsConstExpression.expression; _ } ->
28732873
this#visit_expression ~hints ~cond expression
2874-
| Ast.Expression.Match x ->
2875-
this#visit_match ~on_case_body:(fun e -> ignore @@ this#expression e) x
2874+
| Ast.Expression.Match x -> this#visit_match_expression ~hints x
28762875
| Ast.Expression.Class _
28772876
| Ast.Expression.Identifier _
28782877
| Ast.Expression.Import _
@@ -3146,32 +3145,58 @@ class def_finder ~autocomplete_hooks ~react_jsx env_info toplevel_scope =
31463145

31473146
method! match_expression loc _ = fail loc "Should be visited by visit_match_expression"
31483147

3148+
method private visit_match_expression ~hints x =
3149+
let open Ast.Match in
3150+
let { arg; cases; match_keyword_loc; comments = _ } = x in
3151+
this#add_ordinary_binding
3152+
match_keyword_loc
3153+
(mk_reason RMatch match_keyword_loc)
3154+
(Binding (Root (Value { hints = []; expr = arg })));
3155+
let value_hints =
3156+
Base.List.foldi cases ~init:IMap.empty ~f:(fun i acc (_, { Case.body; _ }) ->
3157+
if expression_is_definitely_synthesizable ~autocomplete_hooks body then
3158+
let hint = Hint_t (ValueHint body, BestEffortHint) in
3159+
IMap.add i hint acc
3160+
else
3161+
acc
3162+
)
3163+
in
3164+
Base.List.iteri cases ~f:(fun i (case_loc, { Case.pattern; body; guard; comments = _ }) ->
3165+
let match_root =
3166+
(case_loc, Ast.Expression.Identifier (Flow_ast_utils.match_root_ident case_loc))
3167+
in
3168+
ignore @@ this#expression match_root;
3169+
let acc = Value { hints = []; expr = match_root } in
3170+
this#add_match_destructure_bindings acc pattern;
3171+
ignore @@ super#match_pattern pattern;
3172+
run_opt this#expression guard;
3173+
(* We use best-effort value hints for cases other than the current case.
3174+
Hints are ordered as the cases are in source, top to bottom. *)
3175+
let value_hints = value_hints |> IMap.remove i |> IMap.values |> List.rev in
3176+
let hints = Base.List.append hints value_hints in
3177+
this#visit_expression ~hints ~cond:NonConditionalContext body
3178+
)
3179+
31493180
method! match_statement _ x =
3150-
this#visit_match ~on_case_body:(fun b -> run_loc this#block b) x;
3181+
let open Ast.Match in
3182+
let { arg; cases; match_keyword_loc; comments = _ } = x in
3183+
this#add_ordinary_binding
3184+
match_keyword_loc
3185+
(mk_reason RMatch match_keyword_loc)
3186+
(Binding (Root (Value { hints = []; expr = arg })));
3187+
Base.List.iter cases ~f:(fun (case_loc, { Case.pattern; body; guard; comments = _ }) ->
3188+
let match_root =
3189+
(case_loc, Ast.Expression.Identifier (Flow_ast_utils.match_root_ident case_loc))
3190+
in
3191+
ignore @@ this#expression match_root;
3192+
let acc = Value { hints = []; expr = match_root } in
3193+
this#add_match_destructure_bindings acc pattern;
3194+
ignore @@ super#match_pattern pattern;
3195+
run_opt this#expression guard;
3196+
run_loc this#block body
3197+
);
31513198
x
31523199

3153-
method private visit_match
3154-
: 'B. on_case_body:('B -> unit) -> ('loc, 'loc, 'B) Ast.Match.t -> unit =
3155-
fun ~on_case_body x ->
3156-
let open Ast.Match in
3157-
let { arg; cases; match_keyword_loc; comments = _ } = x in
3158-
this#add_ordinary_binding
3159-
match_keyword_loc
3160-
(mk_reason RMatch match_keyword_loc)
3161-
(Binding (Root (Value { hints = []; expr = arg })));
3162-
Base.List.iter cases ~f:(function
3163-
| (case_loc, { Case.pattern; body; guard; comments = _ }) ->
3164-
let match_root =
3165-
(case_loc, Ast.Expression.Identifier (Flow_ast_utils.match_root_ident case_loc))
3166-
in
3167-
ignore @@ this#expression match_root;
3168-
let acc = Value { hints = []; expr = match_root } in
3169-
this#add_match_destructure_bindings acc pattern;
3170-
ignore @@ super#match_pattern pattern;
3171-
run_opt this#expression guard;
3172-
on_case_body body
3173-
)
3174-
31753200
method add_match_destructure_bindings root pattern =
31763201
let visit_binding loc name binding =
31773202
let binding = this#mk_hooklike_if_necessary (Flow_ast_utils.hook_name name) binding in

tests/match/hints.js

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// Annotation hint
2+
type F = string => boolean;
3+
{
4+
declare const x: 'a';
5+
6+
const out: F = match (x) {
7+
'a': y => true, // OK
8+
};
9+
}
10+
11+
// Sibling before
12+
{
13+
declare const x: 'a' | 'b';
14+
15+
const out = match (x) {
16+
'a': [1],
17+
'b': [], // Should be `Array<number>`
18+
};
19+
out as Array<number>; // OK
20+
}
21+
22+
// Sibling after
23+
{
24+
declare const x: 'a' | 'b';
25+
26+
const out = match (x) {
27+
'a': [], // Should be `Array<number>`
28+
'b': [1],
29+
};
30+
out as Array<number>; // OK
31+
}
32+
33+
// Multiple siblings, one valid
34+
{
35+
declare const x: 'a' | 'b' | 'c' | 'd';
36+
37+
const out = match (x) {
38+
'a': 1,
39+
'b': {},
40+
'c': [1],
41+
'd': [], // Should be `Array<number>`
42+
};
43+
out as number | {} | Array<number>; // OK
44+
}
45+
46+
// Multiple siblings, multiple valid
47+
{
48+
declare const x: 'a' | 'b' | 'c';
49+
50+
const out = match (x) {
51+
'a': [true],
52+
'b': [1],
53+
'c': [],
54+
};
55+
out as Array<boolean> | Array<number>; // OK
56+
}
57+
{
58+
declare const x: 'a' | 'b' | 'c';
59+
60+
const out = match (x) {
61+
'a': (x: number) => 1,
62+
'b': (x: string) => true,
63+
'c': x => x as number, // OK
64+
};
65+
}
66+
67+
// Cycles avoided
68+
{
69+
declare const x: 'a' | 'b';
70+
71+
const out: (x: number) => void = match (x) { // OK
72+
'a': (x) => {},
73+
'b': (x) => {},
74+
};
75+
}
76+
{
77+
declare const x: 'a' | 'b';
78+
79+
const out: Array<number> = match (x) { // OK
80+
'a': [],
81+
'b': [],
82+
};
83+
}

0 commit comments

Comments
 (0)