Skip to content

Commit

Permalink
Implement "Extract into Method" refactoring
Browse files Browse the repository at this point in the history
Summary:
Steps to use:
- select some code that you want to extract into a method
- cmd-'.' or click the lightbulb
- select "Extract into method"

Example:
https://pxl.cl/2FHsS

## Advice for reviewing

As shown in the large test suite, there is a lot of behavior here, including:
- handling of plain functions, iterators, async functions, async iterators
- modifiers, parameters, and return type for the extracted function
- whether an expression or a set of statements is selected

To play with this feature, I recommend opening up some real code, and pasting in some of the examples from the test suite.

In the implementation, the code is split into two main parts:
- find the information needed to provide the refactoring
- generate the refactoring–I recommend reading this first, since otherwise it may not be obvious what all the information collected is *for*

## Next

the meta-only section below gives more context for these changes and follow-on work.

Reviewed By: ljw1004

Differential Revision: D44757198

fbshipit-source-id: 9a94d2a3b5ddf8765e26c9ca100877c8affaaf6a
  • Loading branch information
mheiber authored and facebook-github-bot committed May 17, 2023
1 parent df2e31d commit 3614df0
Show file tree
Hide file tree
Showing 69 changed files with 1,895 additions and 0 deletions.
5 changes: 5 additions & 0 deletions hphp/hack/src/hh_single_type_check.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2043,6 +2043,11 @@ let handle_mode
let range = find_ide_range src in
let path = Relative_path.to_absolute path in
let resolve =
(* Simulate the resolution flow:
If server replies to textDocument/codeAction with neither 'edit' nor 'command',
and the user selects the code action
then the client sends codeAction/resolve to resolve the code action
*)
Lsp.CodeAction.(
function
| Action { title; _ } ->
Expand Down
1 change: 1 addition & 0 deletions hphp/hack/src/server/codeActionsService.ml
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ let find
actions_for_errors errors path classish_starts ~start_line ~start_col
@ override_method_commands_or_actions
@ variable_actions
@ Extract_method.find ~range:lsp_range ~path ~entry ctx
@ CodeActionsServiceFlipAroundComma.find ~range:lsp_range ~path ~entry ctx

let go
Expand Down
1 change: 1 addition & 0 deletions hphp/hack/src/server/dune
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@
collections
decl_provider
errors
extract_method
full_fidelity
lsp
pos
Expand Down
13 changes: 13 additions & 0 deletions hphp/hack/src/server/extract_method/dune
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
(library
(name extract_method)
(wrapped true)
(libraries
annotated_ast
ast
full_fidelity
lsp
pos
provider_context
tast_env
tast_provider
utils_core))
38 changes: 38 additions & 0 deletions hphp/hack/src/server/extract_method/extract_method.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
(*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the "hack" directory of this source tree.
*
*)
open Hh_prelude

let find ~(range : Lsp.range) ~path ~entry ctx =
let is_selection =
Lsp.(
range.start.line < range.end_.line
|| range.start.line = range.end_.line
&& range.start.character < range.end_.character)
in
if is_selection then
match entry.Provider_context.source_text with
| Some source_text ->
let selection =
let line_to_offset line =
Full_fidelity_source_text.position_to_offset source_text (line, 0)
in
Lsp_helpers.lsp_range_to_pos
~line_to_offset
entry.Provider_context.path
range
in
let candidate_opt =
Extract_method_traverse.find_candidate ~selection ~entry ctx
in
let to_lsp =
Extract_method_to_lsp.command_or_action_of_candidate ~source_text ~path
in
Option.(candidate_opt >>| to_lsp |> to_list)
| None -> []
else
[]
13 changes: 13 additions & 0 deletions hphp/hack/src/server/extract_method/extract_method.mli
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
(*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the "hack" directory of this source tree.
*
*)
val find :
range:Lsp.range ->
path:string ->
entry:Provider_context.entry ->
Provider_context.t ->
Lsp.CodeAction.resolvable_command_or_action list
297 changes: 297 additions & 0 deletions hphp/hack/src/server/extract_method/extract_method_to_lsp.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,297 @@
(*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the "hack" directory of this source tree.
*
*)
open Hh_prelude
module T = Extract_method_types
module SyntaxTree =
Full_fidelity_syntax_tree.WithSyntax (Full_fidelity_positioned_syntax)

let tree_from_string s =
let source_text = Full_fidelity_source_text.make Relative_path.default s in
let env = Full_fidelity_parser_env.make ~mode:FileInfo.Mstrict () in
let tree = SyntaxTree.make ~env source_text in

if List.is_empty (SyntaxTree.all_errors tree) then
Some tree
else
None

let hackfmt src =
let prefix = "<?hh\n" in
match tree_from_string (prefix ^ src) with
| Some tree ->
tree |> Libhackfmt.format_tree |> String.chop_prefix_if_exists ~prefix
| None -> src

let indent ~(indent_amount : int) (s : string) : string =
let indentation = String.make indent_amount ' ' in
s
|> String.split_lines
|> List.map ~f:(fun line -> indentation ^ line)
|> String.concat ~sep:"\n"

let return_type_string_of_candidate
~(return : (string * T.ty_string) list)
T.{ selection_kind; is_async; iterator_kind; _ } =
let wrap_return_type =
match (is_async, iterator_kind) with
| (true, Some T.Iterator) -> Fn.const "AsyncIterator<_, _, _>"
| (true, Some T.KeyedIterator) -> Fn.const "AsyncKeyedIterator<_, _, _>"
| (false, Some T.Iterator) -> Fn.const "Iterator<_>"
| (false, Some T.KeyedIterator) -> Fn.const "KeyedIterator<_>"
| (true, None) -> Format.sprintf "Awaitable<%s>"
| (false, None) -> Fn.id
in
match selection_kind with
| T.SelectionKindExpression (T.Ty return_type_string) ->
wrap_return_type return_type_string
| T.SelectionKindStatement ->
wrap_return_type
@@
(match return with
| [] -> "void"
| [(_, T.Ty s)] -> s
| _ ->
return
|> List.map ~f:(fun (_, T.Ty s) -> s)
|> String.concat ~sep:", "
|> Format.sprintf "(%s)")

let body_string_of_candidate
~source_text
~(return : (string * T.ty_string) list)
T.{ selection_kind; pos; iterator_kind; method_pos; _ } =
let raw_body_string =
let (first_line, first_col) = Pos.line_column pos in
let exp_offset =
Full_fidelity_source_text.position_to_offset
source_text
(first_line, first_col + 1)
in
Full_fidelity_source_text.sub source_text exp_offset (Pos.length pos)
in
match selection_kind with
| T.SelectionKindExpression _ -> Format.sprintf "return %s;" raw_body_string
| T.SelectionKindStatement ->
if Option.is_some iterator_kind then
raw_body_string
else
let method_indent_amount = snd @@ Pos.line_column method_pos in
let format_as_return : string -> string =
let whitespace = String.make (2 * method_indent_amount) ' ' in
Format.sprintf "\n%sreturn %s;" whitespace
in
let return_string =
match return with
| [] -> ""
| [(var_name, _)] -> format_as_return var_name
| _ ->
return
|> List.map ~f:fst
|> String.concat ~sep:", "
|> Format.sprintf "tuple(%s)"
|> format_as_return
in
raw_body_string ^ return_string

let method_string_of_candidate
~source_text
~(params : (string * T.ty_string) list)
~(return : (string * T.ty_string) list)
(T.{ placeholder_name; is_static; is_async; method_pos; _ } as candidate) =
let return_type_string = return_type_string_of_candidate ~return candidate in
let body_string = body_string_of_candidate ~source_text ~return candidate in
let add_modifiers : string -> string =
let static_string =
if is_static then
"static "
else
""
in
let function_kind_string =
if is_async then
"async "
else
""
in
Format.sprintf "private %s%s%s" static_string function_kind_string
in
let params_string =
params
|> List.map ~f:(fun (name, T.Ty shown_ty) ->
Format.sprintf "%s %s" shown_ty name)
|> String.concat ~sep:", "
in
(* we format as a function before adding modifiers, since a function is hackfmt-able (a valid top-level form) *)
let raw_function_string =
Format.sprintf
"function %s(%s): %s {\n%s\n}"
placeholder_name
params_string
return_type_string
body_string
in
let indent_amount = snd @@ Pos.line_column method_pos in
let add_suffix s = s ^ "\n\n" in
raw_function_string
|> hackfmt
|> add_modifiers
|> indent ~indent_amount
|> add_suffix

let method_call_string_of_candidate
~(params : (string * T.ty_string) list)
~(return : (string * T.ty_string) list)
T.
{
placeholder_name;
is_static;
selection_kind;
is_async;
iterator_kind;
pos;
method_pos;
_;
} =
let args_string = params |> List.map ~f:fst |> String.concat ~sep:", " in
let receiver_string =
if is_static then
"self::"
else
"$this->"
in
let call_expr =
Format.sprintf "%s%s(%s)" receiver_string placeholder_name args_string
in
match iterator_kind with
| None ->
(* examples:
- `foo($arg1)`
- `await foo($arg1, $arg2)`
*)
let call_expr =
if is_async then
Format.sprintf "await %s" call_expr
else
call_expr
in
(match selection_kind with
| T.SelectionKindExpression _ -> call_expr
| T.SelectionKindStatement ->
let fmt_assignment lhs_string =
Format.sprintf "%s = %s;" lhs_string call_expr
in
(match return with
| [] -> call_expr ^ ";"
| [(var_name, _)] -> fmt_assignment var_name
| _ ->
return
|> List.map ~f:fst
|> String.concat ~sep:", "
|> Format.sprintf "list(%s)"
|> fmt_assignment))
| Some iterator_kind ->
(* example:
foreach(self::foo() as $value__) {
}
*)
let await_string =
if is_async then
"await "
else
""
in
let as_string =
match iterator_kind with
| T.Iterator -> "$value__"
| T.KeyedIterator -> "$key__ => $value__"
in
let comment_and_whitespace =
(* generate comments like: "/* TODO: assign to $x, $y */"
TODO(T152359779): do more work for the user to handle assignments
*)
let indent_amount = snd @@ Pos.line_column method_pos in
let call_site_indent_amount = snd @@ Pos.line_column pos in
let outer_indent = String.make call_site_indent_amount ' ' in
let inner_indent =
String.make (call_site_indent_amount + indent_amount) ' '
in
let of_var_name_string var_names_string =
Format.sprintf
"\n%s/* TODO: assign to %s */\n%s\n%s"
inner_indent
var_names_string
inner_indent
outer_indent
in
match return with
| [] -> Format.sprintf "\n%s\n%s" inner_indent outer_indent
| [(var_name, _)] -> of_var_name_string var_name
| _ ->
return
|> List.map ~f:fst
|> String.concat ~sep:", "
|> of_var_name_string
in
Format.sprintf
"foreach (%s %sas %s) {%s}"
call_expr
await_string
as_string
comment_and_whitespace

let edit_of_candidate
~source_text ~path (T.{ method_pos; params; return; pos; _ } as candidate) :
Lsp.WorkspaceEdit.t =
let type_assoc_list_of map =
map
|> String.Map.to_alist ~key_order:`Increasing
|> List.dedup_and_sort ~compare:(fun (s1, _) (s2, _) ->
String.compare s1 s2)
in
let params = type_assoc_list_of params in
let return = type_assoc_list_of return in
let change_add_call =
let call_string =
method_call_string_of_candidate ~params ~return candidate
in
{
Lsp.TextEdit.range =
Lsp_helpers.hack_pos_to_lsp_range ~equal:Relative_path.equal pos;
newText = call_string;
}
in
let change_add_method =
let line = (fst @@ Pos.line_column method_pos) - 1 in
let character = 0 in
let method_string =
method_string_of_candidate ~source_text ~params ~return candidate
in
Lsp.
{
Lsp.TextEdit.range =
{ start = { line; character }; end_ = { line; character } };
newText = method_string;
}
in
let changes = SMap.singleton path [change_add_method; change_add_call] in
Lsp.WorkspaceEdit.{ changes }

let command_or_action_of_candidate ~source_text ~path candidate =
let action =
Lsp.CodeAction.UnresolvedEdit
(lazy (edit_of_candidate ~source_text ~path candidate))
in
Lsp.CodeAction.Action
Lsp.CodeAction.
{
title = "Extract into method";
kind = Lsp.CodeActionKind.refactor;
diagnostics = [];
action;
}
Loading

0 comments on commit 3614df0

Please sign in to comment.