From 93dcb78f8ffae1cb5de94104f832c9d9e14c5318 Mon Sep 17 00:00:00 2001 From: Benjamin Krenn Date: Sun, 19 Nov 2023 19:30:04 +0100 Subject: [PATCH 1/5] Add goto definition for implemented behaviour callbacks --- apps/els_lsp/src/els_code_navigation.erl | 11 +++- apps/els_lsp/src/els_definition_provider.erl | 62 +++++++++++++++++--- apps/els_lsp/test/els_definition_SUITE.erl | 13 ++++ 3 files changed, 77 insertions(+), 9 deletions(-) diff --git a/apps/els_lsp/src/els_code_navigation.erl b/apps/els_lsp/src/els_code_navigation.erl index 86e8630c4..65e3a5442 100644 --- a/apps/els_lsp/src/els_code_navigation.erl +++ b/apps/els_lsp/src/els_code_navigation.erl @@ -17,14 +17,19 @@ %% Includes %%============================================================================== -include("els_lsp.hrl"). --include_lib("kernel/include/logger.hrl"). + +%%============================================================================== +%% Type definitions +%%============================================================================== +-type goto_definition() :: [{uri(), els_poi:poi()}]. +-export_type([goto_definition/0]). %%============================================================================== %% API %%============================================================================== -spec goto_definition(uri(), els_poi:poi()) -> - {ok, [{uri(), els_poi:poi()}]} | {error, any()}. + {ok, goto_definition()} | {error, any()}. goto_definition( Uri, Var = #{kind := variable} @@ -133,6 +138,8 @@ goto_definition(_Uri, #{kind := parse_transform, id := Module}) -> {ok, Uri} -> defs_to_res(find(Uri, module, Module)); {error, Error} -> {error, Error} end; +goto_definition(Uri, #{kind := callback, id := Id}) -> + defs_to_res(find(Uri, callback, Id)); goto_definition(_Filename, _) -> {error, not_found}. diff --git a/apps/els_lsp/src/els_definition_provider.erl b/apps/els_lsp/src/els_definition_provider.erl index 2a1855c2c..11deebda1 100644 --- a/apps/els_lsp/src/els_definition_provider.erl +++ b/apps/els_lsp/src/els_definition_provider.erl @@ -7,6 +7,7 @@ ]). -include("els_lsp.hrl"). +-include_lib("kernel/include/logger.hrl"). %%============================================================================== %% els_provider functions @@ -22,6 +23,7 @@ handle_request({definition, Params}) -> } = Params, {ok, Document} = els_utils:lookup_document(Uri), POIs = els_dt_document:get_element_at_pos(Document, Line + 1, Character + 1), + ?LOG_WARNING(#{pois => POIs}), case goto_definition(Uri, POIs) of null -> #{text := Text} = Document, @@ -39,16 +41,37 @@ handle_request({definition, Params}) -> -spec goto_definition(uri(), [els_poi:poi()]) -> [map()] | null. goto_definition(_Uri, []) -> null; +goto_definition(Uri, [#{id := FunId, kind := function} = POI | Rest]) -> + {ok, Document} = els_utils:lookup_document(Uri), + BehaviourPOIs = els_dt_document:pois(Document, [behaviour]), + ?LOG_WARNING(#{bpois => BehaviourPOIs}), + case BehaviourPOIs of + [] -> + %% cursor is not over a function - continue + case els_code_navigation:goto_definition(Uri, POI) of + {ok, Definitions} -> + goto_definitions_to_goto(Definitions); + _ -> + goto_definition(Uri, Rest) + end; + Behaviours -> + case does_implement_behaviour(FunId, Behaviours) of + false -> + %% no matching callback for this behaviour so proceed + goto_definition(Uri, Rest); + {true, BehaviourModuleUri, MatchingCallback} -> + {ok, Definitions} = els_code_navigation:goto_definition( + BehaviourModuleUri, + MatchingCallback + ), + goto_definitions_to_goto(Definitions) + end + end; + goto_definition(Uri, [POI | Rest]) -> case els_code_navigation:goto_definition(Uri, POI) of {ok, Definitions} -> - lists:map( - fun({DefUri, DefPOI}) -> - #{range := Range} = DefPOI, - #{uri => DefUri, range => els_protocol:range(Range)} - end, - Definitions - ); + goto_definitions_to_goto(Definitions); _ -> goto_definition(Uri, Rest) end. @@ -98,6 +121,31 @@ fix_line_offset( } }. +-spec goto_definitions_to_goto(Definitions) -> Result when + Definitions :: els_code_navigation:goto_definition(), + Result :: [map()]. +goto_definitions_to_goto(Definitions) -> + lists:map(fun({DefUri, DefPOI}) -> + #{range := Range} = DefPOI, + #{uri => DefUri, range => els_protocol:range(Range)} + end, Definitions). + +-spec does_implement_behaviour(any(), list()) -> {true, any()} | false. +does_implement_behaviour(_, []) -> false; +does_implement_behaviour(FunId, [#{id := ModuleId, kind := behaviour} | Rest]) -> + {ok, BehaviourModuleUri} = els_utils:find_module(ModuleId), + {ok, BehaviourModuleDocument} = els_utils:lookup_document(BehaviourModuleUri), + DefinedCallbacks = els_dt_document:pois(BehaviourModuleDocument, + [callback]), + MaybeMatchingCallback = lists:filter(fun(#{id := CallbackId}) -> + CallbackId =:= FunId + end, DefinedCallbacks), + case MaybeMatchingCallback of + [] -> does_implement_behaviour(FunId, Rest); + [H | _] -> {true, BehaviourModuleUri, H} + end. + + -ifdef(TEST). -include_lib("eunit/include/eunit.hrl"). fix_line_offset_test() -> diff --git a/apps/els_lsp/test/els_definition_SUITE.erl b/apps/els_lsp/test/els_definition_SUITE.erl index dcc14f568..7ee5e95a7 100644 --- a/apps/els_lsp/test/els_definition_SUITE.erl +++ b/apps/els_lsp/test/els_definition_SUITE.erl @@ -19,6 +19,7 @@ application_remote/1, atom/1, behaviour/1, + behaviour_callback_definition/1, definition_after_closing/1, duplicate_definition/1, export_entry/1, @@ -179,6 +180,18 @@ behaviour(Config) -> ), ok. +-spec behaviour_callback_definition(config()) -> ok. +behaviour_callback_definition(Config) -> + Uri = ?config(code_navigation_uri, Config), + Def = els_client:definition(Uri, 28, 5), + #{result := [#{range := Range, uri := DefUri}]} = Def, + ?assertEqual(?config(behaviour_a_uri, Config), DefUri), + ?assertEqual( + els_protocol:range(#{from => {3, 1}, to => {3, 30}}), + Range + ), + ok. + -spec testcase(config()) -> ok. testcase(Config) -> Uri = ?config(sample_SUITE_uri, Config), From bf25cdd8c24e6691965fcec38b56c3b63479ac82 Mon Sep 17 00:00:00 2001 From: Benjamin Krenn Date: Sun, 19 Nov 2023 20:06:42 +0100 Subject: [PATCH 2/5] Fix formatting --- apps/els_lsp/src/els_definition_provider.erl | 67 +++++++++++--------- 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/apps/els_lsp/src/els_definition_provider.erl b/apps/els_lsp/src/els_definition_provider.erl index 11deebda1..74a5539d3 100644 --- a/apps/els_lsp/src/els_definition_provider.erl +++ b/apps/els_lsp/src/els_definition_provider.erl @@ -23,7 +23,7 @@ handle_request({definition, Params}) -> } = Params, {ok, Document} = els_utils:lookup_document(Uri), POIs = els_dt_document:get_element_at_pos(Document, Line + 1, Character + 1), - ?LOG_WARNING(#{pois => POIs}), + ?LOG_WARNING(#{pois => POIs}), case goto_definition(Uri, POIs) of null -> #{text := Text} = Document, @@ -48,12 +48,12 @@ goto_definition(Uri, [#{id := FunId, kind := function} = POI | Rest]) -> case BehaviourPOIs of [] -> %% cursor is not over a function - continue - case els_code_navigation:goto_definition(Uri, POI) of - {ok, Definitions} -> - goto_definitions_to_goto(Definitions); - _ -> - goto_definition(Uri, Rest) - end; + case els_code_navigation:goto_definition(Uri, POI) of + {ok, Definitions} -> + goto_definitions_to_goto(Definitions); + _ -> + goto_definition(Uri, Rest) + end; Behaviours -> case does_implement_behaviour(FunId, Behaviours) of false -> @@ -61,13 +61,12 @@ goto_definition(Uri, [#{id := FunId, kind := function} = POI | Rest]) -> goto_definition(Uri, Rest); {true, BehaviourModuleUri, MatchingCallback} -> {ok, Definitions} = els_code_navigation:goto_definition( - BehaviourModuleUri, - MatchingCallback - ), + BehaviourModuleUri, + MatchingCallback + ), goto_definitions_to_goto(Definitions) end end; - goto_definition(Uri, [POI | Rest]) -> case els_code_navigation:goto_definition(Uri, POI) of {ok, Definitions} -> @@ -122,29 +121,37 @@ fix_line_offset( }. -spec goto_definitions_to_goto(Definitions) -> Result when - Definitions :: els_code_navigation:goto_definition(), - Result :: [map()]. + Definitions :: els_code_navigation:goto_definition(), + Result :: [map()]. goto_definitions_to_goto(Definitions) -> - lists:map(fun({DefUri, DefPOI}) -> - #{range := Range} = DefPOI, - #{uri => DefUri, range => els_protocol:range(Range)} - end, Definitions). + lists:map( + fun({DefUri, DefPOI}) -> + #{range := Range} = DefPOI, + #{uri => DefUri, range => els_protocol:range(Range)} + end, + Definitions + ). -spec does_implement_behaviour(any(), list()) -> {true, any()} | false. -does_implement_behaviour(_, []) -> false; +does_implement_behaviour(_, []) -> + false; does_implement_behaviour(FunId, [#{id := ModuleId, kind := behaviour} | Rest]) -> - {ok, BehaviourModuleUri} = els_utils:find_module(ModuleId), - {ok, BehaviourModuleDocument} = els_utils:lookup_document(BehaviourModuleUri), - DefinedCallbacks = els_dt_document:pois(BehaviourModuleDocument, - [callback]), - MaybeMatchingCallback = lists:filter(fun(#{id := CallbackId}) -> - CallbackId =:= FunId - end, DefinedCallbacks), - case MaybeMatchingCallback of - [] -> does_implement_behaviour(FunId, Rest); - [H | _] -> {true, BehaviourModuleUri, H} - end. - + {ok, BehaviourModuleUri} = els_utils:find_module(ModuleId), + {ok, BehaviourModuleDocument} = els_utils:lookup_document(BehaviourModuleUri), + DefinedCallbacks = els_dt_document:pois( + BehaviourModuleDocument, + [callback] + ), + MaybeMatchingCallback = lists:filter( + fun(#{id := CallbackId}) -> + CallbackId =:= FunId + end, + DefinedCallbacks + ), + case MaybeMatchingCallback of + [] -> does_implement_behaviour(FunId, Rest); + [H | _] -> {true, BehaviourModuleUri, H} + end. -ifdef(TEST). -include_lib("eunit/include/eunit.hrl"). From a6263f1fe3abaf111187563e14115d412217d448 Mon Sep 17 00:00:00 2001 From: Benjamin Krenn Date: Sat, 25 Nov 2023 23:20:05 +0100 Subject: [PATCH 3/5] Remove unnecessary log statements and logger import --- apps/els_lsp/src/els_definition_provider.erl | 3 --- 1 file changed, 3 deletions(-) diff --git a/apps/els_lsp/src/els_definition_provider.erl b/apps/els_lsp/src/els_definition_provider.erl index 74a5539d3..dad692502 100644 --- a/apps/els_lsp/src/els_definition_provider.erl +++ b/apps/els_lsp/src/els_definition_provider.erl @@ -7,7 +7,6 @@ ]). -include("els_lsp.hrl"). --include_lib("kernel/include/logger.hrl"). %%============================================================================== %% els_provider functions @@ -23,7 +22,6 @@ handle_request({definition, Params}) -> } = Params, {ok, Document} = els_utils:lookup_document(Uri), POIs = els_dt_document:get_element_at_pos(Document, Line + 1, Character + 1), - ?LOG_WARNING(#{pois => POIs}), case goto_definition(Uri, POIs) of null -> #{text := Text} = Document, @@ -44,7 +42,6 @@ goto_definition(_Uri, []) -> goto_definition(Uri, [#{id := FunId, kind := function} = POI | Rest]) -> {ok, Document} = els_utils:lookup_document(Uri), BehaviourPOIs = els_dt_document:pois(Document, [behaviour]), - ?LOG_WARNING(#{bpois => BehaviourPOIs}), case BehaviourPOIs of [] -> %% cursor is not over a function - continue From 7bf484c02930e9c579e9386535bedd68e1dcb523 Mon Sep 17 00:00:00 2001 From: Benjamin Krenn Date: Sun, 17 Dec 2023 22:45:53 +0100 Subject: [PATCH 4/5] Format els_dodger with erlfmt 1.3.0 --- apps/els_core/src/els_dodger.erl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/els_core/src/els_dodger.erl b/apps/els_core/src/els_dodger.erl index 3277ecdc6..a9cd3fded 100644 --- a/apps/els_core/src/els_dodger.erl +++ b/apps/els_core/src/els_dodger.erl @@ -567,7 +567,7 @@ quickscan_form([{'-', _L}, {'if', La} | _Ts]) -> kill_form(La); quickscan_form([{'-', _L}, {atom, La, elif} | _Ts]) -> kill_form(La); -quickscan_form([{'-', _L}, {atom, La, else} | _Ts]) -> +quickscan_form([{'-', _L}, {atom, La, 'else'} | _Ts]) -> kill_form(La); quickscan_form([{'-', _L}, {atom, La, endif} | _Ts]) -> kill_form(La); @@ -791,13 +791,13 @@ scan_form([{'-', _L}, {atom, La, elif} | Ts], Opt) -> {atom, La, 'elif'} | scan_macros(Ts, Opt) ]; -scan_form([{'-', _L}, {atom, La, else} | Ts], Opt) -> +scan_form([{'-', _L}, {atom, La, 'else'} | Ts], Opt) -> [ {atom, La, ?pp_form}, {'(', La}, {')', La}, {'->', La}, - {atom, La, else} + {atom, La, 'else'} | scan_macros(Ts, Opt) ]; scan_form([{'-', _L}, {atom, La, endif} | Ts], Opt) -> From 351680da9c6466e21293ed37cc4a941a5ef1e590 Mon Sep 17 00:00:00 2001 From: Benjamin Krenn Date: Sun, 17 Dec 2023 23:17:40 +0100 Subject: [PATCH 5/5] Fix wrong function specification --- apps/els_lsp/src/els_definition_provider.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/els_lsp/src/els_definition_provider.erl b/apps/els_lsp/src/els_definition_provider.erl index dad692502..060cc582e 100644 --- a/apps/els_lsp/src/els_definition_provider.erl +++ b/apps/els_lsp/src/els_definition_provider.erl @@ -129,7 +129,7 @@ goto_definitions_to_goto(Definitions) -> Definitions ). --spec does_implement_behaviour(any(), list()) -> {true, any()} | false. +-spec does_implement_behaviour(any(), list()) -> {true, uri(), els_poi:poi()} | false. does_implement_behaviour(_, []) -> false; does_implement_behaviour(FunId, [#{id := ModuleId, kind := behaviour} | Rest]) ->