diff --git a/apps/els_lsp/src/els_code_navigation.erl b/apps/els_lsp/src/els_code_navigation.erl index d32eb324e..15ddb5cb5 100644 --- a/apps/els_lsp/src/els_code_navigation.erl +++ b/apps/els_lsp/src/els_code_navigation.erl @@ -8,7 +8,9 @@ %%============================================================================== %% API --export([ goto_definition/2 ]). +-export([ goto_definition/2 + , find_in_scope/2 + ]). %%============================================================================== %% Includes @@ -23,28 +25,13 @@ -spec goto_definition(uri(), poi()) -> {ok, uri(), poi()} | {error, any()}. goto_definition( Uri - , Var = #{kind := variable, id := VarId, range := VarRange} + , Var = #{kind := variable} ) -> %% This will naively try to find the definition of a variable by finding the - %% first occurrence of the variable in the function clause. - {ok, Document} = els_utils:lookup_document(Uri), - FunPOIs = els_poi:sort(els_dt_document:pois(Document, [function_clause])), - VarPOIs = els_poi:sort(els_dt_document:pois(Document, [variable])), - %% Find the function clause we are in - case [Range || #{range := Range} <- FunPOIs, - els_range:compare(Range, VarRange)] of - [] -> {error, not_in_function_clause}; - FunRanges -> - FunRange = lists:last(FunRanges), - %% Find the first occurrence of the variable in the function clause - [POI|_] = [P || P = #{range := Range, id := Id} <- VarPOIs, - els_range:compare(Range, VarRange), - els_range:compare(FunRange, Range), - Id =:= VarId], - case POI of - Var -> {error, already_at_definition}; - POI -> {ok, Uri, POI} - end + %% first occurrence of the variable in variable scope. + case find_in_scope(Uri, Var) of + [Var|_] -> {error, already_at_definition}; + [POI|_] -> {ok, Uri, POI} end; goto_definition( _Uri , #{ kind := Kind, id := {M, F, A} } @@ -213,3 +200,12 @@ maybe_imported(Document, function, {F, A}) -> end; maybe_imported(_Document, _Kind, _Data) -> {error, not_found}. + +-spec find_in_scope(uri(), poi()) -> [poi()]. +find_in_scope(Uri, #{kind := variable, id := VarId, range := VarRange}) -> + {ok, Document} = els_utils:lookup_document(Uri), + VarPOIs = els_poi:sort(els_dt_document:pois(Document, [variable])), + ScopeRange = els_scope:variable_scope_range(VarRange, Document), + [POI || #{range := Range, id := Id} = POI <- VarPOIs, + els_range:in(Range, ScopeRange), + Id =:= VarId]. diff --git a/apps/els_lsp/src/els_references_provider.erl b/apps/els_lsp/src/els_references_provider.erl index 7f81b8f6e..8faa40c3a 100644 --- a/apps/els_lsp/src/els_references_provider.erl +++ b/apps/els_lsp/src/els_references_provider.erl @@ -67,6 +67,9 @@ find_references(Uri, #{ kind := Kind {M, F, A} -> {M, F, A} end, find_references_for_id(Kind, Key); +find_references(Uri, #{kind := variable} = Var) -> + POIs = els_code_navigation:find_in_scope(Uri, Var), + [location(Uri, Range) || #{range := Range} = POI <- POIs, POI =/= Var]; find_references(Uri, #{ kind := Kind , id := Id }) when Kind =:= function_clause -> diff --git a/apps/els_lsp/src/els_rename_provider.erl b/apps/els_lsp/src/els_rename_provider.erl index f7586e047..7db3af35b 100644 --- a/apps/els_lsp/src/els_rename_provider.erl +++ b/apps/els_lsp/src/els_rename_provider.erl @@ -130,20 +130,9 @@ editable_range(#{kind := _Kind, range := Range}) -> els_protocol:range(Range). -spec changes(uri(), poi(), binary()) -> #{uri() => [text_edit()]} | null. -changes(Uri, #{kind := variable, id := VarId, range := VarRange}, NewName) -> - %% Rename variable in function clause scope - case els_utils:lookup_document(Uri) of - {ok, Document} -> - FunRange = variable_scope_range(VarRange, Document), - Changes = [#{range => editable_range(POI), newText => NewName} || - POI <- els_dt_document:pois(Document, [variable]), - maps:get(id, POI) =:= VarId, - els_range:in(maps:get(range, POI), FunRange) - ], - #{Uri => Changes}; - {error, _} -> - null - end; +changes(Uri, #{kind := variable} = Var, NewName) -> + POIs = els_code_navigation:find_in_scope(Uri, Var), + #{Uri => [#{range => editable_range(P), newText => NewName} || P <- POIs]}; changes(Uri, #{kind := type_definition, id := {Name, A}}, NewName) -> ?LOG_INFO("Renaming type ~p/~p to ~s", [Name, A, NewName]), {ok, Doc} = els_utils:lookup_document(Uri), @@ -222,87 +211,6 @@ new_name(#{kind := record_expr}, NewName) -> new_name(_, NewName) -> NewName. --spec variable_scope_range(poi_range(), els_dt_document:item()) -> poi_range(). -variable_scope_range(VarRange, Document) -> - Attributes = [spec, callback, define, record, type_definition], - AttrPOIs = els_dt_document:pois(Document, Attributes), - case pois_match(AttrPOIs, VarRange) of - [#{range := Range}] -> - %% Inside attribute, simple. - Range; - [] -> - %% If variable is not inside an attribute we need to figure out where the - %% scope of the variable begins and ends. - %% The scope of variables inside functions are limited by function clauses - %% The scope of variables outside of function are limited by top-level - %% POIs (attributes and functions) before and after. - FunPOIs = els_poi:sort(els_dt_document:pois(Document, [function])), - POIs = els_poi:sort(els_dt_document:pois(Document, [ function_clause - | Attributes - ])), - CurrentFunRange = case pois_match(FunPOIs, VarRange) of - [] -> undefined; - [POI] -> range(POI) - end, - IsInsideFunction = CurrentFunRange /= undefined, - BeforeFunRanges = [range(POI) || POI <- pois_before(FunPOIs, VarRange)], - %% Find where scope should begin - From = - case [R || #{range := R} <- pois_before(POIs, VarRange)] of - [] -> - %% No POIs before - {0, 0}; - [BeforeRange|_] when IsInsideFunction -> - %% Inside function, use beginning of closest function clause - maps:get(from, BeforeRange); - [BeforeRange|_] when BeforeFunRanges == [] -> - %% No function before, use end of closest POI - maps:get(to, BeforeRange); - [BeforeRange|_] -> - %% Use end of closest POI, including functions. - max(maps:get(to, hd(BeforeFunRanges)), - maps:get(to, BeforeRange)) - end, - %% Find when scope should end - To = - case [R || #{range := R} <- pois_after(POIs, VarRange)] of - [] when IsInsideFunction -> - %% No POIs after, use end of function - maps:get(to, CurrentFunRange); - [] -> - %% No POIs after, use end of document - {999999999, 999999999}; - [AfterRange|_] when IsInsideFunction -> - %% Inside function, use closest of end of function *OR* - %% beginning of the next function clause - min(maps:get(to, CurrentFunRange), maps:get(from, AfterRange)); - [AfterRange|_] -> - %% Use beginning of next POI - maps:get(from, AfterRange) - end, - #{from => From, to => To} - end. - --spec pois_before([poi()], poi_range()) -> [poi()]. -pois_before(POIs, VarRange) -> - %% Reverse since we are typically interested in the last POI - lists:reverse([POI || POI <- POIs, els_range:compare(range(POI), VarRange)]). - --spec pois_after([poi()], poi_range()) -> [poi()]. -pois_after(POIs, VarRange) -> - [POI || POI <- POIs, els_range:compare(VarRange, range(POI))]. - --spec pois_match([poi()], poi_range()) -> [poi()]. -pois_match(POIs, Range) -> - [POI || POI <- POIs, els_range:in(Range, range(POI))]. - --spec range(poi()) -> poi_range(). -range(#{kind := function, data := #{wrapping_range := Range}}) -> - Range; -range(#{range := Range}) -> - Range. - - -spec convert_references_to_pois([els_dt_references:item()], [poi_kind()]) -> [{uri(), poi()}]. convert_references_to_pois(Refs, Kinds) -> diff --git a/apps/els_lsp/src/els_scope.erl b/apps/els_lsp/src/els_scope.erl index ab12a1d55..ab63e748b 100644 --- a/apps/els_lsp/src/els_scope.erl +++ b/apps/els_lsp/src/els_scope.erl @@ -3,6 +3,7 @@ -export([ local_and_included_pois/2 , local_and_includer_pois/2 + , variable_scope_range/2 ]). -include("els_lsp.hrl"). @@ -68,3 +69,86 @@ find_includers(Uri) -> find_includers(Kind, Id) -> {ok, Items} = els_dt_references:find_by_id(Kind, Id), [Uri || #{uri := Uri} <- Items]. + +%% @doc Find the rough scope of a variable, this is based on heuristics and +%% won't always be correct. +%% `VarRange' is expected to be the range of the variable. +-spec variable_scope_range(poi_range(), els_dt_document:item()) -> poi_range(). +variable_scope_range(VarRange, Document) -> + Attributes = [spec, callback, define, record, type_definition], + AttrPOIs = els_dt_document:pois(Document, Attributes), + case pois_match(AttrPOIs, VarRange) of + [#{range := Range}] -> + %% Inside attribute, simple. + Range; + [] -> + %% If variable is not inside an attribute we need to figure out where the + %% scope of the variable begins and ends. + %% The scope of variables inside functions are limited by function clauses + %% The scope of variables outside of function are limited by top-level + %% POIs (attributes and functions) before and after. + FunPOIs = els_poi:sort(els_dt_document:pois(Document, [function])), + POIs = els_poi:sort(els_dt_document:pois(Document, [ function_clause + | Attributes + ])), + CurrentFunRange = case pois_match(FunPOIs, VarRange) of + [] -> undefined; + [POI] -> range(POI) + end, + IsInsideFunction = CurrentFunRange /= undefined, + BeforeFunRanges = [range(POI) || POI <- pois_before(FunPOIs, VarRange)], + %% Find where scope should begin + From = + case [R || #{range := R} <- pois_before(POIs, VarRange)] of + [] -> + %% No POIs before + {0, 0}; + [BeforeRange|_] when IsInsideFunction -> + %% Inside function, use beginning of closest function clause + maps:get(from, BeforeRange); + [BeforeRange|_] when BeforeFunRanges == [] -> + %% No function before, use end of closest POI + maps:get(to, BeforeRange); + [BeforeRange|_] -> + %% Use end of closest POI, including functions. + max(maps:get(to, hd(BeforeFunRanges)), + maps:get(to, BeforeRange)) + end, + %% Find when scope should end + To = + case [R || #{range := R} <- pois_after(POIs, VarRange)] of + [] when IsInsideFunction -> + %% No POIs after, use end of function + maps:get(to, CurrentFunRange); + [] -> + %% No POIs after, use end of document + {999999999, 999999999}; + [AfterRange|_] when IsInsideFunction -> + %% Inside function, use closest of end of function *OR* + %% beginning of the next function clause + min(maps:get(to, CurrentFunRange), maps:get(from, AfterRange)); + [AfterRange|_] -> + %% Use beginning of next POI + maps:get(from, AfterRange) + end, + #{from => From, to => To} + end. + +-spec pois_before([poi()], poi_range()) -> [poi()]. +pois_before(POIs, VarRange) -> + %% Reverse since we are typically interested in the last POI + lists:reverse([POI || POI <- POIs, els_range:compare(range(POI), VarRange)]). + +-spec pois_after([poi()], poi_range()) -> [poi()]. +pois_after(POIs, VarRange) -> + [POI || POI <- POIs, els_range:compare(VarRange, range(POI))]. + +-spec pois_match([poi()], poi_range()) -> [poi()]. +pois_match(POIs, Range) -> + [POI || POI <- POIs, els_range:in(Range, range(POI))]. + +-spec range(poi()) -> poi_range(). +range(#{kind := function, data := #{wrapping_range := Range}}) -> + Range; +range(#{range := Range}) -> + Range. diff --git a/apps/els_lsp/test/els_definition_SUITE.erl b/apps/els_lsp/test/els_definition_SUITE.erl index a498b8176..4b23e6034 100644 --- a/apps/els_lsp/test/els_definition_SUITE.erl +++ b/apps/els_lsp/test/els_definition_SUITE.erl @@ -463,10 +463,12 @@ variable(Config) -> Def1 = els_client:definition(Uri, 105, 10), Def2 = els_client:definition(Uri, 107, 10), Def3 = els_client:definition(Uri, 108, 10), + Def4 = els_client:definition(Uri, 19, 36), #{result := #{range := Range0, uri := DefUri0}} = Def0, #{result := #{range := Range1, uri := DefUri0}} = Def1, #{result := #{range := Range2, uri := DefUri0}} = Def2, #{result := #{range := Range3, uri := DefUri0}} = Def3, + #{result := #{range := Range4, uri := DefUri0}} = Def4, ?assertEqual(?config(code_navigation_uri, Config), DefUri0), ?assertEqual( els_protocol:range(#{from => {103, 12}, to => {103, 15}}) @@ -477,6 +479,9 @@ variable(Config) -> , Range2), ?assertEqual( els_protocol:range(#{from => {106, 12}, to => {106, 15}}) , Range3), + %% Inside macro + ?assertEqual( els_protocol:range(#{from => {19, 17}, to => {19, 18}}) + , Range4), ok. diff --git a/apps/els_lsp/test/els_rename_SUITE.erl b/apps/els_lsp/test/els_rename_SUITE.erl index 301ae691b..cce615297 100644 --- a/apps/els_lsp/test/els_rename_SUITE.erl +++ b/apps/els_lsp/test/els_rename_SUITE.erl @@ -479,7 +479,7 @@ assert_changes(#{ changes := ExpectedChanges }, #{ changes := Changes }) -> lists:sort(maps:to_list(ExpectedChanges))), [ begin ?assertEqual(ExpectedKey, Key), - ?assertEqual(Expected, Change) + ?assertEqual(lists:sort(Expected), lists:sort(Change)) end || {{Key, Change}, {ExpectedKey, Expected}} <- Pairs ],