Remove fragile heuristics from editable_range

apps/els_lsp/src/els_parser.erl

@@ -147,8 +147,11 @@ application(Tree) ->
         false -> [poi(Pos, application, {F, A})]
       end;
     MFA ->
-      Pos = erl_syntax:get_pos(erl_syntax:application_operator(Tree)),
-      [poi(Pos, application, MFA)]
+      ModFunTree = erl_syntax:application_operator(Tree),
+      Pos = erl_syntax:get_pos(ModFunTree),
+      FunTree = erl_syntax:module_qualifier_body(ModFunTree),
+      [poi(Pos, application, MFA,
+           #{name_range => els_range:range(erl_syntax:get_pos(FunTree))})]
   end.
 
 -spec application_mfa(tree()) ->
@@ -271,9 +274,7 @@ attribute(Tree) ->
         {true, TypeName} ->
           Id = {TypeName, length(TypeArgs)},
           [poi(Pos, type_definition, Id,
-                 #{ name => els_range:range(
-                                erl_syntax:get_pos(Type),
-                                type_definition, Id, undefined),
+                 #{ name_range => els_range:range(erl_syntax:get_pos(Type)),
                     args => type_args(TypeArgs)})];
         _ ->
           []
@@ -283,20 +284,18 @@ attribute(Tree) ->
       case spec_function_name(FATree) of
         {F, A} ->
           [FTree, _] = erl_syntax:tuple_elements(FATree),
-          Anno = erl_syntax:get_pos(FTree),
-          %% FIXME this is weird:
-          %% starts at '-', ends at the end of callback function name
-          Start = get_start_location(Tree),
-          CallbackAnno = erl_anno:set_location(Start, Anno),
-          [poi(CallbackAnno, callback, {F, A})];
+          [poi(Pos, callback, {F, A},
+               #{name_range => els_range:range(erl_syntax:get_pos(FTree))})];
         undefined ->
           []
       end;
     {spec, [ArgTuple]} ->
       [FATree | _] = erl_syntax:tuple_elements(ArgTuple),
       case spec_function_name(FATree) of
         {F, A} ->
-          [poi(Pos, spec, {F, A})];
+          [FTree, _] = erl_syntax:tuple_elements(FATree),
+          [poi(Pos, spec, {F, A},
+               #{name_range => els_range:range(erl_syntax:get_pos(FTree))})];
         undefined ->
           [poi(Pos, spec, undefined)]
       end;
@@ -357,7 +356,9 @@ find_export_entry_pois(EntryPoiKind, Exports) ->
   lists:flatten(
     [ case get_name_arity(FATree) of
         {F, A} ->
-          poi(erl_syntax:get_pos(FATree), EntryPoiKind, {F, A});
+          FTree = erl_syntax:arity_qualifier_body(FATree),
+          poi(erl_syntax:get_pos(FATree), EntryPoiKind, {F, A},
+              #{name_range => els_range:range(erl_syntax:get_pos(FTree))});
         false ->
           []
       end
@@ -369,7 +370,9 @@ find_import_entry_pois(M, Imports) ->
   lists:flatten(
     [ case get_name_arity(FATree) of
         {F, A} ->
-          poi(erl_syntax:get_pos(FATree), import_entry, {M, F, A});
+          FTree = erl_syntax:arity_qualifier_body(FATree),
+          poi(erl_syntax:get_pos(FATree), import_entry, {M, F, A},
+              #{name_range => els_range:range(erl_syntax:get_pos(FTree))});
         false ->
           []
       end
@@ -480,7 +483,18 @@ implicit_fun(Tree) ->
             end,
   case FunSpec of
     undefined -> [];
-    _ -> [poi(erl_syntax:get_pos(Tree), implicit_fun, FunSpec)]
+    _ ->
+      NameTree = erl_syntax:implicit_fun_name(Tree),
+      FunTree =
+        case FunSpec of
+          {_, _, _} ->
+            erl_syntax:arity_qualifier_body(
+              erl_syntax:module_qualifier_body(NameTree));
+          {_, _} ->
+            erl_syntax:arity_qualifier_body(NameTree)
+        end,
+      [poi(erl_syntax:get_pos(Tree), implicit_fun, FunSpec,
+           #{name_range => els_range:range(erl_syntax:get_pos(FunTree))})]
   end.
 
 -spec macro(tree()) -> [poi()].
@@ -639,8 +653,11 @@ type_application(Tree) ->
     {Module, {Name, Arity}} ->
       %% remote type
       Id = {Module, Name, Arity},
-      Pos = erl_syntax:get_pos(erl_syntax:type_application_name(Tree)),
-      [poi(Pos, type_application, Id)];
+      ModTypeTree = erl_syntax:type_application_name(Tree),
+      Pos = erl_syntax:get_pos(ModTypeTree),
+      TypeTree = erl_syntax:module_qualifier_body(ModTypeTree),
+      [poi(Pos, type_application, Id,
+           #{name_range => els_range:range(erl_syntax:get_pos(TypeTree))})];
     {Name, Arity} when Type =:= user_type_application ->
       %% user-defined local type
       Id = {Name, Arity},

apps/els_lsp/src/els_range.erl

@@ -5,6 +5,7 @@
 -export([ compare/2
         , in/2
         , range/4
+        , range/1
         , to_poi_range/1
         ]).
 
@@ -36,6 +37,10 @@ range({Line, Column}, function_clause, {F, _A, _Index}, _Data) ->
   To = plus(From, atom_to_string(F)),
   #{ from => From, to => To };
 range(Anno, _Type, _Id, _Data) ->
+  range(Anno).
+
+-spec range(erl_anno:anno()) -> poi_range().
+range(Anno) ->
   From = erl_anno:location(Anno),
   %% To = erl_anno:end_location(Anno),
   To = proplists:get_value(end_location, erl_anno:to_term(Anno)),

apps/els_lsp/src/els_rename_provider.erl

@@ -110,53 +110,19 @@ workspace_edits(_Uri, _POIs, _NewName) ->
   null.
 
 -spec editable_range(poi()) -> range().
-editable_range(#{kind := callback, range := Range}) ->
-  #{ from := {FromL, FromC} } = Range,
-  EditFromC = FromC + string:length("-callback "),
-  els_protocol:range(Range#{ from := {FromL, EditFromC } });
-editable_range(#{kind := export_entry, id := {F, _A}, range := Range}) ->
-  #{ from := {FromL, FromC} } = Range,
-  EditToC = FromC + string:length(atom_to_string(F)),
-  els_protocol:range(Range#{ to := {FromL, EditToC} });
-editable_range(#{kind := export_type_entry, id := {T, _A}, range := Range}) ->
-  #{ from := {FromL, FromC} } = Range,
-  EditToC = FromC + length(atom_to_string(T)),
-  els_protocol:range(Range#{ to := {FromL, EditToC} });
-editable_range(#{kind := spec, id := {F, _A}, range := Range}) ->
-  #{ from := {FromL, FromC}, to := {_ToL, _ToC} } = Range,
-  EditFromC = FromC + string:length("-spec "),
-  EditToC = EditFromC + string:length(atom_to_string(F)),
-  els_protocol:range(Range#{ from := {FromL, EditFromC}
-                           , to := {FromL, EditToC} });
-editable_range(#{kind := implicit_fun, id := {M, F, _A}, range := Range}) ->
-  #{ from := {FromL, FromC}, to := {_ToL, _ToC} } = Range,
-  EditFromC = FromC + length("fun " ++ atom_to_string(M) ++ ":"),
-  EditToC = EditFromC + length(atom_to_string(F)),
-  els_protocol:range(#{ from => {FromL, EditFromC}
-                      , to => {FromL, EditToC} });
-editable_range(#{kind := implicit_fun, id := {F, _A}, range := Range}) ->
-  #{ from := {FromL, FromC}, to := {_ToL, _ToC} } = Range,
-  EditFromC = FromC + length("fun "),
-  EditToC = EditFromC + length(atom_to_string(F)),
-  els_protocol:range(#{ from => {FromL, EditFromC}
-                      , to => {FromL, EditToC} });
-editable_range(#{kind := import_entry, id := {_M, F, _A}, range := Range}) ->
-  #{ from := {FromL, FromC} } = Range,
-  EditToC = FromC + length(atom_to_string(F)),
-  els_protocol:range(Range#{ to := {FromL, EditToC} });
-editable_range(#{kind := application, id := {M, F, _A}, range := Range}) ->
-  #{ from := {FromL, FromC}, to := {_ToL, _ToC} } = Range,
-  EditFromC = FromC + length(atom_to_string(M) ++ ":"),
-  EditToC = EditFromC + length(atom_to_string(F)),
-  els_protocol:range(#{ from => {FromL, EditFromC}
-                      , to => {FromL, EditToC} });
-editable_range(#{kind := type_application, id := {M, T, _A}, range := Range}) ->
-  #{ from := {FromL, FromC}, to := {_ToL, _ToC} } = Range,
-  EditFromC = FromC + length(atom_to_string(M) ++ ":"),
-  EditToC = EditFromC + length(atom_to_string(T)),
-  els_protocol:range(#{ from => {FromL, EditFromC}
-                      , to => {FromL, EditToC} });
-editable_range(#{kind := type_definition, data := #{ name := Range }}) ->
+editable_range(#{kind := Kind, data := #{name_range := Range}})
+  when Kind =:= application;
+       Kind =:= implicit_fun;
+       Kind =:= callback;
+       Kind =:= spec;
+       Kind =:= export_entry;
+       Kind =:= export_type_entry;
+       Kind =:= import_entry;
+       Kind =:= type_application;
+       Kind =:= type_definition ->
+  %% application POI of a local call and
+  %% type_application POI of a built-in type don't have name_range data
+  %% they are handled by the next clause
   els_protocol:range(Range);
 editable_range(#{kind := _Kind, range := Range}) ->
   els_protocol:range(Range).
@@ -305,13 +271,4 @@ import_changes(_Uri, _POI, _NewName) ->
 
 -spec change(poi(), binary()) -> text_edit().
 change(POI, NewName) ->
-  %% TODO: editable_range expects only `fun foo/0' in source so
-  %% so for example if `fun   foo/0' is in the source it will result in invalid
-  %% changes as the text that the POI corresponds to is lost.
-  %% This is a deep fundamental flaw in how POIs are parsed and need to be
-  %% addressed in the parser
   #{range => editable_range(POI), newText => NewName}.
-
--spec atom_to_string(atom()) -> string().
-atom_to_string(Atom) ->
-  io_lib:write(Atom).

apps/els_lsp/test/els_document_highlight_SUITE.erl

@@ -331,7 +331,7 @@ behaviour(Config) ->
 callback(Config) ->
   Uri = ?config(rename_uri, Config),
   #{result := Locations} = els_client:document_highlight(Uri, 3, 10),
-  ExpectedLocations = [ #{range => #{from => {3, 1}, to => {3, 20}}}
+  ExpectedLocations = [ #{range => #{from => {3, 1}, to => {3, 34}}}
                       ],
   assert_locations(ExpectedLocations, Locations),
   ok.