Skip to content

Commit 0258e38

Browse files
SamChou19815meta-codesync[bot]
SamChou19815
authored and
meta-codesync[bot]
committed
[flow] Provide a quickfix to change match coded like a switch to more idiomatic form
Summary: This diff implements a quickfix to change matches that are coded like a switch to a more idiomatic forms. It's attempting to target code like this ``` type DataPoint = | {type: 'avg', p50: null, avg: number} | {type: 'p50', p50: number, avg: null}; const v: number = match (dataPoint.type) { 'avg' => dataPoint.avg, 'p50' => dataPoint.p50, } ``` which is written with the style of switch on disjoint union, but won't type check under match. A better way to write it is ``` type DataPoint = | {type: 'avg', p50: null, avg: number} | {type: 'p50', p50: number, avg: null}; const v: number = match (dataPoint) { {type:'avg', const avg} => avg, {type:'p50',const p50} => p50, } ``` This diff detects this pattern by checking the following: - whether the match root is a member expression - whether all the cases are just simple literals - whether there are property accesses of the form `<match_root>.prop` in case bodies. I think the detection is probably good in 90% of cases, but just in case I missed something and the user is actually doing something totally reasonable, I decide to only offer it as a quickfix at the location of errors within match. Changelog: [internal] Reviewed By: gkz Differential Revision: D85988965 fbshipit-source-id: 80b1682d9b3098a3b5a666e0167f1812d725163d
1 parent 2a8927a commit 0258e38

File tree

8 files changed

+525
-0
lines changed

8 files changed

+525
-0
lines changed

‎newtests/lsp/code-action/quickfix/match/__snapshots__/fix-match-coded-like-switch-1.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"method": "textDocument/codeAction",
3+
"result": []
4+
}

‎newtests/lsp/code-action/quickfix/match/__snapshots__/fix-match-coded-like-switch-2.json‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
{
2+
"method": "textDocument/codeAction",
3+
"result": [
4+
{
5+
"title": "Refactor `match` coded like a switch",
6+
"kind": "quickfix",
7+
"diagnostics": [],
8+
"edit": {
9+
"changes": {
10+
"<PLACEHOLDER_PROJECT_URL>/fix-match-coded-like-switch.js": [
11+
{
12+
"range": {
13+
"start": {
14+
"line": 8,
15+
"character": 7
16+
},
17+
"end": {
18+
"line": 8,
19+
"character": 21
20+
}
21+
},
22+
"newText": "dataPoint"
23+
},
24+
{
25+
"range": {
26+
"start": {
27+
"line": 9,
28+
"character": 0
29+
},
30+
"end": {
31+
"line": 10,
32+
"character": 9
33+
}
34+
},
35+
"newText": "{\n type: // ^\n \"avg\",\n const avg,\n ...,\n} as dataPoint"
36+
},
37+
{
38+
"range": {
39+
"start": {
40+
"line": 11,
41+
"character": 8
42+
},
43+
"end": {
44+
"line": 11,
45+
"character": 21
46+
}
47+
},
48+
"newText": "(avg)"
49+
},
50+
{
51+
"range": {
52+
"start": {
53+
"line": 14,
54+
"character": 4
55+
},
56+
"end": {
57+
"line": 14,
58+
"character": 9
59+
}
60+
},
61+
"newText": "{type: \"p50\", const p50, ...} as dataPoint"
62+
},
63+
{
64+
"range": {
65+
"start": {
66+
"line": 15,
67+
"character": 8
68+
},
69+
"end": {
70+
"line": 15,
71+
"character": 21
72+
}
73+
},
74+
"newText": "(p50)"
75+
}
76+
]
77+
}
78+
},
79+
"command": {
80+
"title": "",
81+
"command": "log:org.flow:<PLACEHOLDER_PROJECT_URL>",
82+
"arguments": [
83+
"textDocument/codeAction",
84+
"refactor_switch_to_match",
85+
"Refactor `match` coded like a switch"
86+
]
87+
}
88+
}
89+
]
90+
}

‎newtests/lsp/code-action/quickfix/match/__snapshots__/fix-match-coded-like-switch-3.json‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
{
2+
"method": "textDocument/codeAction",
3+
"result": [
4+
{
5+
"title": "Refactor `match` coded like a switch",
6+
"kind": "quickfix",
7+
"diagnostics": [],
8+
"edit": {
9+
"changes": {
10+
"<PLACEHOLDER_PROJECT_URL>/fix-match-coded-like-switch.js": [
11+
{
12+
"range": {
13+
"start": {
14+
"line": 18,
15+
"character": 25
16+
},
17+
"end": {
18+
"line": 18,
19+
"character": 39
20+
}
21+
},
22+
"newText": "dataPoint"
23+
},
24+
{
25+
"range": {
26+
"start": {
27+
"line": 19,
28+
"character": 4
29+
},
30+
"end": {
31+
"line": 19,
32+
"character": 9
33+
}
34+
},
35+
"newText": "{type: \"avg\", const avg, ...} as dataPoint"
36+
},
37+
{
38+
"range": {
39+
"start": {
40+
"line": 19,
41+
"character": 13
42+
},
43+
"end": {
44+
"line": 19,
45+
"character": 26
46+
}
47+
},
48+
"newText": "avg"
49+
},
50+
{
51+
"range": {
52+
"start": {
53+
"line": 21,
54+
"character": 4
55+
},
56+
"end": {
57+
"line": 21,
58+
"character": 9
59+
}
60+
},
61+
"newText": "{type: \"p50\", const p50, ...} as dataPoint"
62+
},
63+
{
64+
"range": {
65+
"start": {
66+
"line": 21,
67+
"character": 13
68+
},
69+
"end": {
70+
"line": 21,
71+
"character": 26
72+
}
73+
},
74+
"newText": "p50"
75+
}
76+
]
77+
}
78+
},
79+
"command": {
80+
"title": "",
81+
"command": "log:org.flow:<PLACEHOLDER_PROJECT_URL>",
82+
"arguments": [
83+
"textDocument/codeAction",
84+
"refactor_switch_to_match",
85+
"Refactor `match` coded like a switch"
86+
]
87+
}
88+
}
89+
]
90+
}

‎newtests/lsp/code-action/quickfix/match/fix-match-coded-like-switch.js.ignored‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// @flow
2+
3+
type DataPoint =
4+
| {type: 'avg', p50: null, avg: number}
5+
| {type: 'p50', p50: number, avg: null};
6+
7+
declare const dataPoint: DataPoint;
8+
9+
match (dataPoint.type) {
10+
// ^
11+
'avg' => {
12+
dataPoint.avg as number;
13+
// ^
14+
},
15+
'p50' => {
16+
dataPoint.p50 as number;
17+
},
18+
}
19+
const v: number = match (dataPoint.type) {
20+
'avg' => dataPoint.avg,
21+
// ^
22+
'p50' => dataPoint.p50,
23+
}

‎newtests/lsp/code-action/quickfix/match/test.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import type {SuiteType} from '../../../../Tester';
77
const path = require('path');
88
const {suite, test} = require('../../../../Tester');
9+
const {generateSimpleTests} = require('../../test-utils');
910

1011
module.exports = (suite(
1112
({
@@ -584,5 +585,19 @@ module.exports = (suite(
584585
['textDocument/publishDiagnostics', ...lspIgnoreStatusAndCancellation],
585586
),
586587
]),
588+
test('match coded like switch', [
589+
...generateSimpleTests(
590+
'quickfix',
591+
{
592+
addFile,
593+
lspIgnoreStatusAndCancellation,
594+
lspStartAndConnect,
595+
lspRequestAndWaitUntilResponse,
596+
},
597+
__dirname,
598+
'fix-match-coded-like-switch.js',
599+
'fix-match-coded-like-switch',
600+
),
601+
]),
587602
],
588603
): SuiteType);

‎src/services/code_action/code_action_service.ml‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,43 @@ let refactor_switch_to_match_statement_actions ~cx ~ast ~options ~only uri loc =
501501
else
502502
[]
503503

504+
let refactor_match_coded_like_switch ~cx ~ast ~options ~only ~loc_of_aloc uri loc =
505+
if
506+
Context.enable_pattern_matching cx
507+
&& (include_quick_fixes only || include_rewrite_refactors only)
508+
&& Flow_error.ErrorSet.exists
509+
(fun error ->
510+
let { Flow_intermediate_error_types.loc = error_loc; _ } =
511+
Flow_intermediate_error.make_intermediate_error ~loc_of_aloc error
512+
in
513+
Loc.contains error_loc loc)
514+
(Context.errors cx)
515+
then
516+
match Refactor_match_discriminant.refactor ast loc with
517+
| Some ast' ->
518+
Flow_ast_differ.program ast ast'
519+
|> Replacement_printer.mk_loc_patch_ast_differ ~opts:(layout_options options)
520+
|> flow_loc_patch_to_lsp_edits
521+
|> fun edits ->
522+
let open Lsp in
523+
let title = "Refactor `match` coded like a switch" in
524+
[
525+
CodeAction.Action
526+
{
527+
CodeAction.title;
528+
kind = CodeActionKind.quickfix;
529+
diagnostics = [];
530+
action =
531+
CodeAction.BothEditThenCommand
532+
( WorkspaceEdit.{ changes = UriMap.singleton uri edits },
533+
mk_log_command ~title ~diagnostic_title:"refactor_switch_to_match"
534+
);
535+
};
536+
]
537+
| None -> []
538+
else
539+
[]
540+
504541
let add_jsx_props_code_actions ~snippets_enabled ~cx ~ast ~typed_ast ~options uri loc =
505542
match Refactor_add_jsx_props.fill_props cx ~snippets_enabled ~ast ~tast:typed_ast loc with
506543
| None -> []
@@ -1892,6 +1929,7 @@ let code_actions_at_loc
18921929
@ convert_type_to_readonly_form_code_actions ~options ~ast ~only uri loc
18931930
@ refactor_arrow_function_code_actions ~ast ~scope_info ~options ~only uri loc
18941931
@ refactor_switch_to_match_statement_actions ~cx ~ast ~options ~only uri loc
1932+
@ refactor_match_coded_like_switch ~cx ~ast ~options ~only ~loc_of_aloc uri loc
18951933
@ add_jsx_props_code_actions
18961934
~snippets_enabled:(Lsp_helpers.supports_experimental_snippet_text_edit lsp_init_params)
18971935
~cx

0 commit comments

Comments
 (0)