Implement new code actions (erlang-ls#1212)

* If function is unused, provide action to export it
* If variable is unbound, provide action to fix spelling
* If module name doesn't match filename, provide action to fix it

This also includes refactoring of els_code_action_provider and fixing
the unused variable code action that didn't work properly.

apps/els_core/include/els_core.hrl

@@ -568,7 +568,7 @@
  , context := code_action_context()
  }.
 
--type code_action() :: #{ title := string()
+-type code_action() :: #{ title := binary()
  , kind => code_action_kind()
  , diagnostics => [els_diagnostics:diagnostic()]
  , edit => workspace_edit()

apps/els_core/src/els_protocol.erl

@@ -84,7 +84,7 @@ range(#{ from := {FromL, FromC}, to := {ToL, ToC} }) ->
 %%==============================================================================
 -spec content(binary()) -> binary().
 content(Body) ->
-els_utils:to_binary([headers(Body), "\r\n", Body]).
+ els_utils:to_binary([headers(Body), "\r\n", Body]).
 
 -spec headers(binary()) -> iolist().
 headers(Body) ->

apps/els_core/src/els_utils.erl

@@ -20,6 +20,7 @@
  , function_signature/1
  , base64_encode_term/1
  , base64_decode_term/1
+ , levenshtein_distance/2
  ]).
 
 
@@ -449,3 +450,27 @@ base64_encode_term(Term) ->
 -spec base64_decode_term(binary()) -> any().
 base64_decode_term(Base64) ->
  binary_to_term(base64:decode(Base64)).
+
+-spec levenshtein_distance(binary(), binary()) -> integer().
+levenshtein_distance(S, T) ->
+ {Distance, _} = levenshtein_distance(to_list(S), to_list(T), #{}),
+ Distance.
+
+-spec levenshtein_distance(string(), string(), map()) -> {integer(), map()}.
+levenshtein_distance([] = S, T, Cache) ->
+ {length(T), maps:put({S, T}, length(T), Cache)};
+levenshtein_distance(S, [] = T, Cache) ->
+ {length(S), maps:put({S, T}, length(S), Cache)};
+levenshtein_distance([X|S], [X|T], Cache) ->
+ levenshtein_distance(S, T, Cache);
+levenshtein_distance([_SH|ST] = S, [_TH|TT] = T, Cache) ->
+ case maps:find({S, T}, Cache) of
+ {ok, Distance} ->
+ {Distance, Cache};
+ error ->
+ {L1, C1} = levenshtein_distance(S, TT, Cache),
+ {L2, C2} = levenshtein_distance(ST, T, C1),
+ {L3, C3} = levenshtein_distance(ST, TT, C2),
+ L = 1 + lists:min([L1, L2, L3]),
+ {L, maps:put({S, T}, L, C3)}
+ end.

apps/els_lsp/priv/code_navigation/src/code_action.erl

@@ -0,0 +1,15 @@
+-module(code_action_oops).
+
+-export([function_a/0]).
+
+function_a() ->
+ A = 123,
+ function_b().
+
+function_b() ->
+ ok.
+
+function_c() ->
+ Foo = 1,
+ Bar = 2,
+ Foo + Barf.

apps/els_lsp/src/els_code_action_provider.erl

@@ -29,79 +29,133 @@ handle_request({document_codeaction, Params}, State) ->
 %% Internal Functions
 %%==============================================================================
 
-
 %% @doc Result: `(Command | CodeAction)[] | null'
 -spec code_actions(uri(), range(), code_action_context()) -> [map()].
-code_actions(Uri, _Range, Context) ->
- #{ <<"diagnostics">> := Diagnostics } = Context,
- Actions0 = [ make_code_action(Uri, D) || D <- Diagnostics],
- Actions = lists:flatten(Actions0),
- Actions.
-
-%% @doc Note: if the start and end line of the range are the same, the line
-%% is simply added.
--spec replace_lines_action(uri(), binary(), binary(), binary(), range())
- -> map().
-replace_lines_action(Uri, Title, Kind, Lines, Range) ->
- #{ <<"start">> := #{ <<"character">> := _StartCol
- , <<"line">> := StartLine }
- , <<"end">> := #{ <<"character">> := _EndCol
- , <<"line">> := EndLine }
- } = Range,
+code_actions(Uri, _Range, #{<<"diagnostics">> := Diagnostics}) ->
+ lists:flatten([make_code_action(Uri, D) || D <- Diagnostics]).
+
+-spec make_code_action(uri(), map()) -> [map()].
+make_code_action(Uri, #{<<"message">> := Message, <<"range">> := Range}) ->
+ make_code_action(
+ [ {"function (.*) is unused", fun action_export_function/3}
+ , {"variable '(.*)' is unused", fun action_ignore_variable/3}
+ , {"variable '(.*)' is unbound", fun action_suggest_variable/3}
+ , {"Module name '(.*)' does not match file name '(.*)'",
+ fun action_fix_module_name/3}
+ ], Uri, Range, Message).
+
+-spec make_code_action([{string(), Fun}], uri(), range(), binary()) -> [map()]
+ when Fun :: fun((uri(), range(), [binary()]) -> [map()]).
+make_code_action([], _Uri, _Range, _Message) ->
+ [];
+make_code_action([{RE, Fun}|Rest], Uri, Range, Message) ->
+ Actions = case re:run(Message, RE, [{capture, all_but_first, binary}]) of
+ {match, Matches} ->
+ Fun(Uri, Range, Matches);
+ nomatch ->
+ []
+ end,
+ Actions ++ make_code_action(Rest, Uri, Range, Message).
+
+-spec action_export_function(uri(), range(), [binary()]) -> [map()].
+action_export_function(Uri, _Range, [UnusedFun]) ->
+ {ok, Document} = els_utils:lookup_document(Uri),
+ case els_poi:sort(els_dt_document:pois(Document, [module, export])) of
+ [] ->
+ [];
+ POIs ->
+ #{range := #{to := {Line, _Col}}} = lists:last(POIs),
+ Pos = {Line + 1, 1},
+ [ make_edit_action( Uri
+ , <<"Export ", UnusedFun/binary>>
+ , ?CODE_ACTION_KIND_QUICKFIX
+ , <<"-export([", UnusedFun/binary, "]).\n">>
+ , els_protocol:range(#{from => Pos, to => Pos})) ]
+ end.
+
+-spec action_ignore_variable(uri(), range(), [binary()]) -> [map()].
+action_ignore_variable(Uri, Range, [UnusedVariable]) ->
+ {ok, Document} = els_utils:lookup_document(Uri),
+ POIs = els_poi:sort(els_dt_document:pois(Document, [variable])),
+ case ensure_range(els_range:to_poi_range(Range), UnusedVariable, POIs) of
+ {ok, VarRange} ->
+ [ make_edit_action( Uri
+ , <<"Add '_' to '", UnusedVariable/binary, "'">>
+ , ?CODE_ACTION_KIND_QUICKFIX
+ , <<"_", UnusedVariable/binary>>
+ , els_protocol:range(VarRange)) ];
+ error ->
+ []
+ end.
+
+-spec action_suggest_variable(uri(), range(), [binary()]) -> [map()].
+action_suggest_variable(Uri, Range, [Var]) ->
+ %% Supply a quickfix to replace an unbound variable with the most similar
+ %% variable name in scope.
+ {ok, Document} = els_utils:lookup_document(Uri),
+ POIs = els_poi:sort(els_dt_document:pois(Document, [variable])),
+ case ensure_range(els_range:to_poi_range(Range), Var, POIs) of
+ {ok, VarRange} ->
+ ScopeRange = els_scope:variable_scope_range(VarRange, Document),
+ VarsInScope = [atom_to_binary(Id, utf8) ||
+ #{range := R, id := Id} <- POIs,
+ els_range:in(R, ScopeRange),
+ els_range:compare(R, VarRange)],
+ case [{els_utils:levenshtein_distance(V, Var), V} ||
+ V <- VarsInScope,
+ V =/= Var,
+ binary:at(Var, 0) =:= binary:at(V, 0)]
+ of
+ [] ->
+ [];
+ VariableDistances ->
+ {_, SimilarVariable} = lists:min(VariableDistances),
+ [ make_edit_action( Uri
+ , <<"Did you mean '", SimilarVariable/binary, "'?">>
+ , ?CODE_ACTION_KIND_QUICKFIX
+ , SimilarVariable
+ , els_protocol:range(VarRange)) ]
+ end;
+ error ->
+ []
+ end.
+
+-spec action_fix_module_name(uri(), range(), [binary()]) -> [map()].
+action_fix_module_name(Uri, Range0, [ModName, FileName]) ->
+ {ok, Document} = els_utils:lookup_document(Uri),
+ POIs = els_poi:sort(els_dt_document:pois(Document, [module])),
+ case ensure_range(els_range:to_poi_range(Range0), ModName, POIs) of
+ {ok, Range} ->
+ [ make_edit_action( Uri
+ , <<"Change to -module(", FileName/binary, ").">>
+ , ?CODE_ACTION_KIND_QUICKFIX
+ , FileName
+ , els_protocol:range(Range)) ];
+ error ->
+ []
+ end.
+
+-spec ensure_range(poi_range(), binary(), [poi()]) -> {ok, poi_range()} | error.
+ensure_range(#{from := {Line, _}}, SubjectId, POIs) ->
+ SubjectAtom = binary_to_atom(SubjectId, utf8),
+ Ranges = [R || #{range := R, id := Id} <- POIs,
+ els_range:in(R, #{from => {Line, 1}, to => {Line + 1, 1}}),
+ Id =:= SubjectAtom],
+ case Ranges of
+ [] ->
+ error;
+ [Range|_] ->
+ {ok, Range}
+ end.
+
+-spec make_edit_action(uri(), binary(), binary(), binary(), range())
+ -> map().
+make_edit_action(Uri, Title, Kind, Text, Range) ->
  #{ title => Title
  , kind => Kind
- , command =>
- els_command:make_command( Title
- , <<"replace-lines">>
- , [#{ uri => Uri
- , lines => Lines
- , from => StartLine
- , to => EndLine }])
+ , edit => edit(Uri, Text, Range)
  }.
 
--spec make_code_action(uri(), els_diagnostics:diagnostic()) -> [map()].
-make_code_action(Uri, #{ <<"message">> := Message
- , <<"range">> := Range } = _Diagnostic) ->
- unused_variable_action(Uri, Range, Message).
-
-%%------------------------------------------------------------------------------
-
--spec unused_variable_action(uri(), range(), binary()) -> [map()].
-unused_variable_action(Uri, Range, Message) ->
- %% Processing messages like "variable 'Foo' is unused"
- case re:run(Message, "variable '(.*)' is unused"
- , [{capture, all_but_first, binary}]) of
- {match, [UnusedVariable]} ->
- make_unused_variable_action(Uri, Range, UnusedVariable);
- _ -> []
- end.
-
--spec make_unused_variable_action(uri(), range(), binary()) -> [map()].
-make_unused_variable_action(Uri, Range, UnusedVariable) ->
- #{ <<"start">> := #{ <<"character">> := _StartCol
- , <<"line">> := StartLine }
- , <<"end">> := _End
- } = Range,
- %% processing messages like "variable 'Foo' is unused"
- {ok, #{text := Bin}} = els_utils:lookup_document(Uri),
- Line = els_utils:to_list(els_text:line(Bin, StartLine)),
-
- {ok, Tokens, _} = erl_scan:string(Line, 1, [return, text]),
- UnusedString = els_utils:to_list(UnusedVariable),
- Replace =
- fun(Tok) ->
- case Tok of
- {var, [{text, UnusedString}, _], _} -> "_" ++ UnusedString;
- {var, [{text, VarName}, _], _} -> VarName;
- {_, [{text, Text }, _], _} -> Text;
- {_, [{text, Text }, _]} -> Text
- end
- end,
- UpdatedLine = lists:flatten(lists:map(Replace, Tokens)) ++ "\n",
- [ replace_lines_action( Uri
- , <<"Add '_' to '", UnusedVariable/binary, "'">>
- , ?CODE_ACTION_KIND_QUICKFIX
- , els_utils:to_binary(UpdatedLine)
- , Range)].
-
-%%------------------------------------------------------------------------------
+-spec edit(uri(), binary(), range()) -> workspace_edit().
+edit(Uri, Text, Range) ->
+ #{changes => #{Uri => [#{newText => Text, range => Range}]}}.

apps/els_lsp/src/els_execute_command_provider.erl

@@ -24,8 +24,7 @@ is_enabled() -> true.
 
 -spec options() -> map().
 options() ->
- #{ commands => [ els_command:with_prefix(<<"replace-lines">>)
- , els_command:with_prefix(<<"server-info">>)
+ #{ commands => [ els_command:with_prefix(<<"server-info">>)
  , els_command:with_prefix(<<"ct-run-test">>)
  , els_command:with_prefix(<<"show-behaviour-usages">>)
  , els_command:with_prefix(<<"suggest-spec">>)
@@ -45,17 +44,6 @@ handle_request({workspace_executecommand, Params}, State) ->
 %%==============================================================================
 
 -spec execute_command(els_command:command_id(), [any()]) -> [map()].
-execute_command(<<"replace-lines">>
- , [#{ <<"uri">> := Uri
- , <<"lines">> := Lines
- , <<"from">> := LineFrom
- , <<"to">> := LineTo }]) ->
- Method = <<"workspace/applyEdit">>,
- Params = #{ edit =>
- els_text_edit:edit_replace_text(Uri, Lines, LineFrom, LineTo)
- },
- els_server:send_request(Method, Params),
- [];
 execute_command(<<"server-info">>, _Arguments) ->
  {ok, Version} = application:get_key(?APP, vsn),
  BinVersion = list_to_binary(Version),