Skip to content

Commit eb6888e

Browse files
jbrown215facebook-github-bot
authored andcommitted
[flow][apply-code-actions] Suggest Imports
Summary: Exposes import suggestions for each unbound name error in the file. The JSON output maps each name to an array of suggestions for that name. --pretty prettifies the output. The tests need a `sed` to replace filenames because we generate new directories to run the tests. Changelog: [internal] Reviewed By: gkz Differential Revision: D69545414 ------------------------------------------------------------------------ (from 1172968109c207a6f38d7fca79dd8503db33058f) fbshipit-source-id: bafb367b3ef17c21f30aa233da24c819a564a7e8
1 parent ffd6440 commit eb6888e

File tree

11 files changed

+190
-30
lines changed

11 files changed

+190
-30
lines changed

src/commands/applyCodeActionCommand.ml

+26-17
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,6 @@ open CommandSpec
1111
(* This module implements the flow command `apply-code-action` which exposes LSP code-actions via the CLI *)
1212
let handle_error ?(code = Exit.Unknown_error) msg = Exit.(exit ~msg code)
1313

14-
let handle_ok in_place patch source_path input =
15-
let write_patch content =
16-
let output_channel =
17-
if in_place then
18-
open_out source_path
19-
else
20-
stdout
21-
in
22-
output_string output_channel @@ Replacement_printer.print patch content;
23-
close_out output_channel
24-
in
25-
match File_input.content_of_file_input input with
26-
| Ok content -> write_patch content
27-
| Error msg -> handle_error msg
28-
2914
module SourceAddMissingImports = struct
3015
let spec =
3116
{
@@ -48,6 +33,21 @@ module SourceAddMissingImports = struct
4833
);
4934
}
5035

36+
let handle_ok in_place patch source_path input =
37+
let write_patch content =
38+
let output_channel =
39+
if in_place then
40+
open_out source_path
41+
else
42+
stdout
43+
in
44+
output_string output_channel @@ Replacement_printer.print patch content;
45+
close_out output_channel
46+
in
47+
match File_input.content_of_file_input input with
48+
| Ok content -> write_patch content
49+
| Error msg -> handle_error msg
50+
5151
let main base_flags connect_params _json _pretty root_arg path wait_for_recheck in_place file () =
5252
let source_path = expand_path file in
5353
let input = get_file_from_filename_or_stdin ~cmd:spec.name path (Some source_path) in
@@ -85,7 +85,16 @@ module SuggestImports = struct
8585
);
8686
}
8787

88-
let main base_flags connect_params _json _pretty root_arg path wait_for_recheck in_place file () =
88+
let handle_ok pretty lsp_result =
89+
let result_json =
90+
Hh_json.JSON_Object
91+
(SMap.bindings lsp_result
92+
|> List.map (fun (name, result) -> (name, Lsp_fmt.print_codeActionResult ~key:"" result))
93+
)
94+
in
95+
print_endline (Hh_json.json_to_string ~sort_keys:pretty ~pretty result_json)
96+
97+
let main base_flags connect_params _json pretty root_arg path wait_for_recheck _in_place file () =
8998
let source_path = expand_path file in
9099
let input = get_file_from_filename_or_stdin ~cmd:spec.name path (Some source_path) in
91100
let root = get_the_root ~base_flags ~input root_arg in
@@ -96,7 +105,7 @@ module SuggestImports = struct
96105
in
97106
let result = connect_and_make_request flowconfig_name connect_params root request in
98107
match result with
99-
| ServerProt.Response.APPLY_CODE_ACTION (Ok patch) -> handle_ok in_place patch source_path input
108+
| ServerProt.Response.SUGGEST_IMPORTS (Ok result) -> handle_ok pretty result
100109
| _ -> handle_error "Flow: invalid server response"
101110

102111
let command = CommandSpec.command spec main

src/hack_forked/utils/lsp/lsp_fmt.mli

+2
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ val parse_documentOnTypeFormatting : Hh_json.json option -> Lsp.DocumentOnTypeFo
106106

107107
val print_documentOnTypeFormatting : Lsp.DocumentOnTypeFormatting.result -> Hh_json.json
108108

109+
val print_codeActionResult : key:string -> Lsp.CodeAction.result -> Hh_json.json
110+
109111
val parse_initialize : Hh_json.json option -> Lsp.Initialize.params
110112

111113
val error_of_exn : exn -> Lsp.Error.t

src/server/command_handler/commandHandler.ml

+24-1
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,18 @@ let autofix_imports_cli ~options ~env ~profiling ~loc_of_aloc ~module_system_inf
544544
~file_key
545545
~file_content
546546

547+
let suggest_imports_cli ~options ~env ~profiling ~loc_of_aloc ~module_system_info ~input =
548+
let file_key = file_key_of_file_input ~options ~env input in
549+
File_input.content_of_file_input input >>= fun file_content ->
550+
Code_action_service.suggest_imports_cli
551+
~options
552+
~profiling
553+
~env
554+
~loc_of_aloc
555+
~module_system_info
556+
~file_key
557+
~file_content
558+
547559
let autocomplete
548560
~trigger_character
549561
~reader
@@ -1519,7 +1531,18 @@ let handle_apply_code_action ~options ~reader ~profiling ~env action file_input
15191531
in
15201532
Lwt.return (ServerProt.Response.APPLY_CODE_ACTION result, None)
15211533
| SuggestImports ->
1522-
Lwt.return (ServerProt.Response.APPLY_CODE_ACTION (Error "Not yet implemented"), None)
1534+
let result =
1535+
try_with (fun () ->
1536+
suggest_imports_cli
1537+
~options
1538+
~profiling
1539+
~env
1540+
~loc_of_aloc:(Parsing_heaps.Reader.loc_of_aloc ~reader)
1541+
~module_system_info:(mk_module_system_info ~options ~reader)
1542+
~input:file_input
1543+
)
1544+
in
1545+
Lwt.return (ServerProt.Response.SUGGEST_IMPORTS result, None)
15231546
)
15241547

15251548
let handle_autocomplete

src/server/protocol/serverProt.ml

+4
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,8 @@ module Response = struct
329329

330330
type rage_response = (string * string) list
331331

332+
type suggest_imports_response = (Lsp.CodeAction.command_or_action list SMap.t, string) result
333+
332334
type graph_response = (graph_response_subgraph, string) result
333335

334336
and graph_response_subgraph = (string * string list) list
@@ -369,6 +371,7 @@ module Response = struct
369371
lazy_stats: lazy_stats;
370372
}
371373
| SAVE_STATE of (string, string) result
374+
| SUGGEST_IMPORTS of suggest_imports_response
372375

373376
let to_string = function
374377
| APPLY_CODE_ACTION _ -> "apply-code-action response"
@@ -390,4 +393,5 @@ module Response = struct
390393
| RAGE _ -> "rage response"
391394
| STATUS _ -> "status response"
392395
| SAVE_STATE _ -> "save_state response"
396+
| SUGGEST_IMPORTS _ -> "suggest imports response"
393397
end

src/services/code_action/code_action_service.ml

+35
Original file line numberDiff line numberDiff line change
@@ -1611,6 +1611,41 @@ let with_type_checked_file ~options ~profiling ~env ~file_key ~file_content ~f =
16111611
| Ok (Parse_artifacts { ast; _ }, Typecheck_artifacts { cx; _ }) -> f ~cx ~ast
16121612
| _ -> Error "Failed to parse or check file"
16131613

1614+
let suggest_imports_cli
1615+
~options ~profiling ~env ~loc_of_aloc ~module_system_info ~file_key ~file_content =
1616+
let uri = File_key.to_string file_key |> Lsp_helpers.path_to_lsp_uri ~default_path:"" in
1617+
let get_edits ~cx ~ast =
1618+
let errors = Context.errors cx in
1619+
let { ServerEnv.exports; _ } = env in
1620+
let (imports, _) =
1621+
Flow_error.ErrorSet.fold
1622+
(fun error (imports, unbound_names) ->
1623+
match
1624+
Flow_error.msg_of_error error |> Error_message.map_loc_of_error_message loc_of_aloc
1625+
with
1626+
| Error_message.EBuiltinNameLookupFailed { loc = error_loc; name }
1627+
when Options.autoimports options ->
1628+
let ranked_imports =
1629+
suggest_imports
1630+
~layout_options:(Code_action_utils.layout_options options)
1631+
~module_system_info
1632+
~ast
1633+
~diagnostics:[]
1634+
~imports_ranked_usage:true
1635+
~exports
1636+
~name
1637+
uri
1638+
error_loc
1639+
in
1640+
(SMap.add name ranked_imports imports, SSet.add name unbound_names)
1641+
| _ -> (imports, unbound_names))
1642+
errors
1643+
(SMap.empty, SSet.empty)
1644+
in
1645+
Ok imports
1646+
in
1647+
with_type_checked_file ~options ~profiling ~env ~file_key ~file_content ~f:get_edits
1648+
16141649
let autofix_imports_cli
16151650
~options ~profiling ~env ~loc_of_aloc ~module_system_info ~file_key ~file_content =
16161651
let src_dir = File_key.to_string file_key |> Filename.dirname |> Base.Option.return in

src/services/code_action/code_action_service.mli

+17-7
Original file line numberDiff line numberDiff line change
@@ -54,25 +54,35 @@ val code_actions_at_loc :
5454
loc:Loc.t ->
5555
(Lsp.CodeAction.command_or_action list, string) result
5656

57-
val autofix_imports_lsp :
57+
val autofix_imports_cli :
5858
options:Options.t ->
59+
profiling:Profiling_js.running ->
5960
env:ServerEnv.env ->
6061
loc_of_aloc:(ALoc.t -> Loc.t) ->
6162
module_system_info:Lsp_module_system_info.t ->
62-
cx:Context.t ->
63-
ast:(Loc.t, Loc.t) Flow_ast.Program.t ->
64-
uri:Lsp.DocumentUri.t ->
65-
Lsp.TextEdit.t list
63+
file_key:File_key.t ->
64+
file_content:string ->
65+
(Replacement_printer.patch, string) result
6666

67-
val autofix_imports_cli :
67+
val suggest_imports_cli :
6868
options:Options.t ->
6969
profiling:Profiling_js.running ->
7070
env:ServerEnv.env ->
7171
loc_of_aloc:(ALoc.t -> Loc.t) ->
7272
module_system_info:Lsp_module_system_info.t ->
7373
file_key:File_key.t ->
7474
file_content:string ->
75-
(Replacement_printer.patch, string) result
75+
(Lsp.CodeAction.command_or_action list SMap.t, string) result
76+
77+
val autofix_imports_lsp :
78+
options:Options.t ->
79+
env:ServerEnv.env ->
80+
loc_of_aloc:(ALoc.t -> Loc.t) ->
81+
module_system_info:Lsp_module_system_info.t ->
82+
cx:Context.t ->
83+
ast:(Loc.t, Loc.t) Flow_ast.Program.t ->
84+
uri:Lsp.DocumentUri.t ->
85+
Lsp.TextEdit.t list
7686

7787
val autofix_exports :
7888
options:Options.t ->
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// @flow
2+
3+
export const foo = 3;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// @flow
2+
3+
export const foo = 3;

tests/apply_code_action_command/apply_code_action_command.exp

+69-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import OtherModule from "./OtherModule";
55

66
OtherModule;
77
> Confirm no errors
8+
Found 0 errors
89
> apply-code-action 'source.addMissingImports' tmp/multi.js
910
// @flow
1011

@@ -13,4 +14,71 @@ import OtherModule from "./OtherModule";
1314
OtherModule;
1415
OtherModule;
1516
> Confirm no errors
16-
No errors!
17+
Found 0 errors
18+
> apply-code-action suggestImports tmp/suggest_imports.js
19+
{
20+
"OtherModule":[
21+
{
22+
"command":{
23+
"arguments":["textDocument/codeAction","import","Import default from ./OtherModule"],
24+
"command":"log:",
25+
"title":""
26+
},
27+
"diagnostics":[],
28+
"edit":{
29+
"changes":{
30+
"file:tmp/suggest_imports.js": [
31+
{
32+
"newText":"import OtherModule from \"./OtherModule\";\n\n",
33+
"range":{"end":{"character":0,"line":1},"start":{"character":0,"line":1}}
34+
}
35+
]
36+
}
37+
},
38+
"kind":"quickfix",
39+
"title":"Import default from ./OtherModule"
40+
}
41+
],
42+
"foo":[
43+
{
44+
"command":{
45+
"arguments":["textDocument/codeAction","import","Import from ./ExportFoo1"],
46+
"command":"log:",
47+
"title":""
48+
},
49+
"diagnostics":[],
50+
"edit":{
51+
"changes":{
52+
"file:tmp/suggest_imports.js": [
53+
{
54+
"newText":"import { foo } from \"./ExportFoo1\";\n\n",
55+
"range":{"end":{"character":0,"line":1},"start":{"character":0,"line":1}}
56+
}
57+
]
58+
}
59+
},
60+
"kind":"quickfix",
61+
"title":"Import from ./ExportFoo1"
62+
},
63+
{
64+
"command":{
65+
"arguments":["textDocument/codeAction","import","Import from ./ExportFoo2"],
66+
"command":"log:",
67+
"title":""
68+
},
69+
"diagnostics":[],
70+
"edit":{
71+
"changes":{
72+
"file:tmp/suggest_imports.js": [
73+
{
74+
"newText":"import { foo } from \"./ExportFoo2\";\n\n",
75+
"range":{"end":{"character":0,"line":1},"start":{"character":0,"line":1}}
76+
}
77+
]
78+
}
79+
},
80+
"kind":"quickfix",
81+
"title":"Import from ./ExportFoo2"
82+
}
83+
]
84+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// @flow
2+
OtherModule;
3+
foo;

tests/apply_code_action_command/test.sh

+4-4
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ echo '> apply-code-action '\''source.addMissingImports'\'' tmp/a.js'
1414
assert_ok "$FLOW" apply-code-action 'source.addMissingImports' tmp/a.js
1515
assert_ok "$FLOW" apply-code-action 'source.addMissingImports' --in-place tmp/a.js
1616
echo '> Confirm no errors'
17-
assert_ok "$FLOW" force-recheck tmp/a.js
17+
assert_ok "$FLOW" focus-check tmp/a.js
1818
echo '> apply-code-action '\''source.addMissingImports'\'' tmp/multi.js'
1919
assert_ok "$FLOW" apply-code-action 'source.addMissingImports' tmp/multi.js
2020
assert_ok "$FLOW" apply-code-action 'source.addMissingImports' --in-place tmp/multi.js
2121
echo '> Confirm no errors'
22-
assert_ok "$FLOW" force-recheck tmp/multi.js
23-
24-
assert_ok "$FLOW" status tmp
22+
assert_ok "$FLOW" focus-check tmp/multi.js
23+
echo '> apply-code-action suggestImports tmp/suggest_imports.js'
24+
"$FLOW" apply-code-action suggestImports --pretty tmp/suggest_imports.js | sed -e 's/file:.*:/file:tmp\/suggest_imports.js": /'

0 commit comments

Comments
 (0)