From ab00aec18adb9d8acf6bb5ea83f7835b03b9687e Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Wed, 31 Jul 2019 15:13:03 +0200 Subject: [PATCH 01/22] [#22] Add skeleton for code navigation test suite --- test/erlang_ls_code_navigation_SUITE.erl | 69 ++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 test/erlang_ls_code_navigation_SUITE.erl diff --git a/test/erlang_ls_code_navigation_SUITE.erl b/test/erlang_ls_code_navigation_SUITE.erl new file mode 100644 index 000000000..c9e691d96 --- /dev/null +++ b/test/erlang_ls_code_navigation_SUITE.erl @@ -0,0 +1,69 @@ +%%============================================================================== +%% Unit Tests for Code Navigation +%%============================================================================== +-module(erlang_ls_code_navigation_SUITE). + +%% CT Callbacks +-export([ suite/0 + , init_per_suite/1 + , end_per_suite/1 + , init_per_testcase/2 + , end_per_testcase/2 + , all/0 + ]). + +%% Test cases +-export([ main/1 + ]). + +%%============================================================================== +%% Includes +%%============================================================================== +-include_lib("common_test/include/ct.hrl"). +-include_lib("stdlib/include/assert.hrl"). + +%%============================================================================== +%% Types +%%============================================================================== +-type config() :: [{atom(), any()}]. + +%%============================================================================== +%% CT Callbacks +%%============================================================================== +-spec suite() -> [tuple()]. +suite() -> + [{timetrap, {seconds, 30}}]. + +-spec init_per_suite(config()) -> config(). +init_per_suite(Config) -> + Config. + +-spec end_per_suite(config()) -> ok. +end_per_suite(_Config) -> + ok. + +-spec init_per_testcase(atom(), config()) -> config(). +init_per_testcase(_TestCase, Config) -> + Config. + +-spec end_per_testcase(atom(), config()) -> ok. +end_per_testcase(_TestCase, _Config) -> + ok. + +-spec all() -> [atom()]. +all() -> + [main]. + +%%============================================================================== +%% Testcases +%%============================================================================== + +-spec main(config()) -> ok. +main(_Config) -> + %% First stream allowed + ?assert(true), + ok. + +%%============================================================================== +%% Internal Functions +%%============================================================================== From 8eec62570ac2a28d935c8497e9ac7054c7816de1 Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Wed, 31 Jul 2019 15:13:34 +0200 Subject: [PATCH 02/22] [#22] Run CT as part of CI --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index e0e3e7011..d2f6ab4bd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ otp_release: - 22.0 - 21.3 script: - - rebar3 as test do compile, proper --cover, xref, cover, coveralls send + - rebar3 as test do compile, ct, proper --cover, xref, cover, coveralls send cache: directories: - "$HOME/.cache/rebar3" From 191198d8cf43b47dfedcb14214e7c517c400ebcd Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Wed, 31 Jul 2019 17:00:16 +0200 Subject: [PATCH 03/22] [#22] Add basic tests for code navigation --- src/erlang_ls_server.erl | 6 +++ test/erlang_ls_code_navigation_SUITE.erl | 48 ++++++++++++++++--- .../src/code_navigation.erl | 14 ++++++ 3 files changed, 61 insertions(+), 7 deletions(-) create mode 100644 test/erlang_ls_code_navigation_SUITE_data/src/code_navigation.erl diff --git a/src/erlang_ls_server.erl b/src/erlang_ls_server.erl index 6efe9ff38..b5c7a328b 100644 --- a/src/erlang_ls_server.erl +++ b/src/erlang_ls_server.erl @@ -26,6 +26,11 @@ -export([ connected/3 ]). +%% TODO: Move to separate module +-export([ definition/1 + , search/3 + ]). + %%============================================================================== %% Includes %%============================================================================== @@ -202,6 +207,7 @@ send_notification(Socket, Method, Params) -> lager:debug("[SERVER] Sending notification [notification=~p]", [Notification]), gen_tcp:send(Socket, Notification). +%% TODO: definition/2 should probably not take the Uri, but a path. -spec definition(uri(), erlang_ls_parser:poi()) -> null | map(). definition(_Uri, #{ info := {application, {M, _F, _A}} = Info }) -> case annotated_tree(erlang_ls_uri:filename(M)) of diff --git a/test/erlang_ls_code_navigation_SUITE.erl b/test/erlang_ls_code_navigation_SUITE.erl index c9e691d96..92aded7bd 100644 --- a/test/erlang_ls_code_navigation_SUITE.erl +++ b/test/erlang_ls_code_navigation_SUITE.erl @@ -13,7 +13,8 @@ ]). %% Test cases --export([ main/1 +-export([ macro/1 + , record/1 ]). %%============================================================================== @@ -36,7 +37,13 @@ suite() -> -spec init_per_suite(config()) -> config(). init_per_suite(Config) -> - Config. + DataDir = ?config(data_dir, Config), + DataDirBin = list_to_binary(DataDir), + [ {data_dir_bin, DataDirBin} + , {include_path, [ filename:join([DataDirBin, "src"]) + , filename:join([DataDirBin, "include"]) + ]} + |Config]. -spec end_per_suite(config()) -> ok. end_per_suite(_Config) -> @@ -52,18 +59,45 @@ end_per_testcase(_TestCase, _Config) -> -spec all() -> [atom()]. all() -> - [main]. + [macro, record]. %%============================================================================== %% Testcases %%============================================================================== +-spec macro(config()) -> ok. +macro(Config) -> + FileName = <<"code_navigation.erl">>, + Thing = {macro, 'MACRO_A'}, + Definition = definition(?config(data_dir_bin, Config), Thing), + ?assertEqual( Definition + , erlang_ls_server:search( FileName + , ?config(include_path, Config) + , erlang_ls_server:definition(Thing))), + ok. --spec main(config()) -> ok. -main(_Config) -> - %% First stream allowed - ?assert(true), +-spec record(config()) -> ok. +record(Config) -> + FileName = <<"code_navigation.erl">>, + Thing = {record_expr, "record_a"}, + Definition = definition(?config(data_dir_bin, Config), Thing), + ?assertEqual( Definition + , erlang_ls_server:search( FileName + , ?config(include_path, Config) + , erlang_ls_server:definition(Thing))), ok. %%============================================================================== %% Internal Functions %%============================================================================== +definition(DataDir, {record_expr, "record_a"}) -> + FilePath = filename:join([DataDir, <<"src">>, <<"code_navigation.erl">>]), + #{ range => erlang_ls_protocol:range(#{from => {4, 0}, to => {4, 0}}) + , uri => erlang_ls_uri:uri(FilePath) + }; +definition(DataDir, {macro, 'MACRO_A'}) -> + FilePath = filename:join([DataDir, <<"src">>, <<"code_navigation.erl">>]), + #{ range => erlang_ls_protocol:range(#{from => {6, 0}, to => {6, 0}}) + , uri => erlang_ls_uri:uri(FilePath) + }. + +%% TODO: Armonize strings vs atoms in definitions diff --git a/test/erlang_ls_code_navigation_SUITE_data/src/code_navigation.erl b/test/erlang_ls_code_navigation_SUITE_data/src/code_navigation.erl new file mode 100644 index 000000000..e760c5704 --- /dev/null +++ b/test/erlang_ls_code_navigation_SUITE_data/src/code_navigation.erl @@ -0,0 +1,14 @@ +-module(code_navigation). + +-export([ function_a/0 ]). + +-record(record_a, {field_a, field_b}). + +-define(MACRO_A, macro_a). + +function_a() -> + function_b(), + #record_a{}. + +function_b() -> + ?MACRO_A. From d63a39682003193f3626cd993479550e4caf4399 Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Thu, 1 Aug 2019 14:39:31 +0200 Subject: [PATCH 04/22] [#22] Use apply to avoid dialyzer issue --- src/erlang_ls_parser.erl | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/erlang_ls_parser.erl b/src/erlang_ls_parser.erl index 88eb0a299..8fb7a4f91 100644 --- a/src/erlang_ls_parser.erl +++ b/src/erlang_ls_parser.erl @@ -24,17 +24,6 @@ , range/0 ]). -%%============================================================================== -%% Dialyzer Exceptions -%%============================================================================== -%% The specs for the epp_dodger API are slightly incorrect. -%% A bug has been reported (see https://bugs.erlang.org/browse/ERL-1005) -%% Meanwhile, let's skip checking this module. --dialyzer(no_contracts). --dialyzer(no_return). --dialyzer(no_unused). --dialyzer(no_fail_call). - %% TODO: Generate random filename %% TODO: Ideally avoid writing to file at all (require epp changes) -define(TMP_PATH, "/tmp/erlang_ls_tmp"). @@ -52,7 +41,10 @@ parse_file(Path) -> {ok, IoDevice} -> %% Providing `{1, 1}` as the initial location ensures %% that the returned forms include column numbers, as well. - {ok, Forms} = epp_dodger:parse(IoDevice, {1, 1}), + %% The specs for the epp_dodger API are slightly incorrect. + %% A bug has been reported (see https://bugs.erlang.org/browse/ERL-1005) + %% Meanwhile, let's trick Dialyzer with an apply. + {ok, Forms} = erlang:apply(epp_dodger, parse, [IoDevice, {1, 1}]), Tree = erl_syntax:form_list(Forms), ok = file:close(IoDevice), {ok, Tree}; From cc6659441332c63fcebc02b7ee839f46fd1a3739 Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Tue, 30 Jul 2019 17:12:29 +0200 Subject: [PATCH 05/22] [#22] Export missing type --- src/erlang_ls_parser.erl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/erlang_ls_parser.erl b/src/erlang_ls_parser.erl index 8fb7a4f91..5adfd95b1 100644 --- a/src/erlang_ls_parser.erl +++ b/src/erlang_ls_parser.erl @@ -22,6 +22,7 @@ -export_type([ poi/0 , range/0 + , syntax_tree/0 ]). %% TODO: Generate random filename From 1e0351d9001c06631889868ba38b890d2b48406e Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Thu, 1 Aug 2019 14:48:51 +0200 Subject: [PATCH 06/22] [#22] Address Dialyzer warnings --- src/erlang_ls_parser.erl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/erlang_ls_parser.erl b/src/erlang_ls_parser.erl index 5adfd95b1..76a14e000 100644 --- a/src/erlang_ls_parser.erl +++ b/src/erlang_ls_parser.erl @@ -27,7 +27,7 @@ %% TODO: Generate random filename %% TODO: Ideally avoid writing to file at all (require epp changes) --define(TMP_PATH, "/tmp/erlang_ls_tmp"). +-define(TMP_PATH, <<"/tmp/erlang_ls_tmp">>). -spec parse(binary()) -> {ok, syntax_tree()}. parse(Text) -> @@ -36,7 +36,7 @@ parse(Text) -> ok = file:write_file(?TMP_PATH, Text), parse_file(?TMP_PATH). --spec parse_file(string()) -> {ok, syntax_tree()} | {error, any()}. +-spec parse_file(binary()) -> {ok, syntax_tree()} | {error, any()}. parse_file(Path) -> case file:open(Path, [read]) of {ok, IoDevice} -> @@ -127,12 +127,12 @@ get_range(_Tree, {_Line, _Column}, {spec, _Spec}) -> %% parsing in `epp_dodger`. This requires fixing. #{ from => {0, 0}, to => {0, 0} }. --spec find_poi_by_info(syntax_tree(), any()) -> poi(). +-spec find_poi_by_info(syntax_tree(), any()) -> [poi()]. find_poi_by_info(Tree, Info0) -> [POI || #{info := Info} = POI <- list_poi(Tree), Info0 =:= Info]. %% TODO: Rename --spec find_poi_by_info_key(syntax_tree(), atom()) -> poi(). +-spec find_poi_by_info_key(syntax_tree(), atom()) -> [poi()]. find_poi_by_info_key(Tree, Key0) -> [POI || #{info := {Key, _}} = POI <- list_poi(Tree), Key0 =:= Key]. From 36b66b1c8180e0794ca5ed35ab403b2fa95a7012 Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Thu, 1 Aug 2019 14:49:50 +0200 Subject: [PATCH 07/22] [#22] Re-enable Dialyzer in CI --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index d2f6ab4bd..c1611774a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ otp_release: - 22.0 - 21.3 script: - - rebar3 as test do compile, ct, proper --cover, xref, cover, coveralls send + - rebar3 as test do compile, ct, proper --cover, dialyzer, xref, cover, coveralls send cache: directories: - "$HOME/.cache/rebar3" From 1fbd8e96fbb302554ac51b96d514a3b7b1a9d18c Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Thu, 1 Aug 2019 15:30:25 +0200 Subject: [PATCH 08/22] [#22] Add unit test for behaviour navigation --- src/erlang_ls_parser.erl | 8 +++++ src/erlang_ls_server.erl | 23 ++++++------- test/erlang_ls_code_navigation_SUITE.erl | 32 ++++++++++++++----- .../src/code_navigation.erl | 8 +++++ 4 files changed, 49 insertions(+), 22 deletions(-) diff --git a/src/erlang_ls_parser.erl b/src/erlang_ls_parser.erl index 76a14e000..90d8d535d 100644 --- a/src/erlang_ls_parser.erl +++ b/src/erlang_ls_parser.erl @@ -113,6 +113,10 @@ get_range(_Tree, {Line, Column}, {macro, Macro}) -> From = {Line, Column}, To = {Line, Column + length(atom_to_list(Macro))}, #{ from => From, to => To }; +get_range(_Tree, {Line, Column}, {module, _}) -> + From = {Line - 1, Column - 1}, + To = From, + #{ from => From, to => To }; get_range(_Tree, {Line, Column}, {record_expr, Record}) -> From = {Line, Column - 1}, To = {Line, Column + length(Record) - 1}, @@ -209,6 +213,10 @@ analyze(Tree, attribute) -> [poi(Tree, {behaviour, Behaviour})]; {export, Exports} -> [poi(Tree, {exports_entry, {F, A}}) || {F, A} <- Exports]; + {module, {Module, _Args}} -> + [poi(Tree, {module, Module})]; + {module, Module} -> + [poi(Tree, {module, Module})]; preprocessor -> Name = erl_syntax:atom_value(erl_syntax:attribute_name(Tree)), case {Name, erl_syntax:attribute_arguments(Tree)} of diff --git a/src/erlang_ls_server.erl b/src/erlang_ls_server.erl index b5c7a328b..487cc0d51 100644 --- a/src/erlang_ls_server.erl +++ b/src/erlang_ls_server.erl @@ -239,18 +239,9 @@ definition(Uri, #{ info := {application, {_F, _A}} = Info }) -> {error, _Error} -> null end; -definition(_Uri, #{ info := {behaviour, Behaviour} }) -> - case annotated_tree(erlang_ls_uri:filename(Behaviour)) of - {ok, Uri, _AnnotatedTree} -> - #{ uri => Uri - %% TODO: We could point to the module attribute, instead - , range => erlang_ls_protocol:range(#{ from => {0, 0} - , to => {0, 0} - }) - }; - {error, _Error} -> - null - end; +definition(_Uri, #{ info := {behaviour, Behaviour} = Info }) -> + Filename = erlang_ls_uri:filename(Behaviour), + search(Filename, full_path(), definition(Info)); %% TODO: Eventually search everywhere and suggest a code lens to include a file definition(Uri, #{ info := {macro, _Define} = Info }) -> Filename = erlang_ls_uri:basename(Uri), @@ -292,6 +283,8 @@ definition({application, {_M, F, A}}) -> {function, {F, A}}; definition({application, {F, A}}) -> {function, {F, A}}; +definition({behaviour, Behaviour}) -> + {module, Behaviour}; definition({macro, Define}) -> {define, Define}; definition({record_expr, Record}) -> @@ -300,8 +293,7 @@ definition({record_expr, Record}) -> -spec annotated_tree(binary()) -> {ok, uri(), erlang_ls_parser:syntax_tree()} | {error, any()}. annotated_tree(Filename) -> - Path = lists:append( [ app_path() , deps_path() , otp_path() ]), - annotated_tree(Filename, Path). + annotated_tree(Filename, full_path()). -spec annotated_tree(binary(), [string()]) -> {ok, uri(), erlang_ls_parser:syntax_tree()} | {error, any()}. @@ -333,6 +325,9 @@ app_path() -> deps_path() -> filelib:wildcard(filename:join([?DEPS_PATH, "*/src"])). +full_path() -> + lists:append( [ app_path() , deps_path() , otp_path() ]). + %% Look for a definition recursively in a file and its includes. -spec search(binary(), [string()], any()) -> null | map(). search(Filename, Path, Thing) -> diff --git a/test/erlang_ls_code_navigation_SUITE.erl b/test/erlang_ls_code_navigation_SUITE.erl index 92aded7bd..e5bcac56c 100644 --- a/test/erlang_ls_code_navigation_SUITE.erl +++ b/test/erlang_ls_code_navigation_SUITE.erl @@ -13,7 +13,8 @@ ]). %% Test cases --export([ macro/1 +-export([ behaviour/1 + , macro/1 , record/1 ]). @@ -59,11 +60,23 @@ end_per_testcase(_TestCase, _Config) -> -spec all() -> [atom()]. all() -> - [macro, record]. + [behaviour, macro, record]. %%============================================================================== %% Testcases %%============================================================================== +%% TODO: Refactor API +-spec behaviour(config()) -> ok. +behaviour(Config) -> + FileName = <<"behaviour_a.erl">>, + Thing = {behaviour, 'behaviour_a'}, + Definition = definition(?config(data_dir_bin, Config), Thing), + ?assertEqual( Definition + , erlang_ls_server:search( FileName + , ?config(include_path, Config) + , erlang_ls_server:definition(Thing))), + ok. + -spec macro(config()) -> ok. macro(Config) -> FileName = <<"code_navigation.erl">>, @@ -89,15 +102,18 @@ record(Config) -> %%============================================================================== %% Internal Functions %%============================================================================== -definition(DataDir, {record_expr, "record_a"}) -> - FilePath = filename:join([DataDir, <<"src">>, <<"code_navigation.erl">>]), - #{ range => erlang_ls_protocol:range(#{from => {4, 0}, to => {4, 0}}) +definition(DataDir, {behaviour, 'behaviour_a'}) -> + FilePath = filename:join([DataDir, <<"src">>, <<"behaviour_a.erl">>]), + #{ range => erlang_ls_protocol:range(#{from => {0, 1}, to => {0, 1}}) , uri => erlang_ls_uri:uri(FilePath) }; definition(DataDir, {macro, 'MACRO_A'}) -> FilePath = filename:join([DataDir, <<"src">>, <<"code_navigation.erl">>]), - #{ range => erlang_ls_protocol:range(#{from => {6, 0}, to => {6, 0}}) + #{ range => erlang_ls_protocol:range(#{from => {11, 0}, to => {11, 0}}) + , uri => erlang_ls_uri:uri(FilePath) + }; +definition(DataDir, {record_expr, "record_a"}) -> + FilePath = filename:join([DataDir, <<"src">>, <<"code_navigation.erl">>]), + #{ range => erlang_ls_protocol:range(#{from => {9, 0}, to => {9, 0}}) , uri => erlang_ls_uri:uri(FilePath) }. - -%% TODO: Armonize strings vs atoms in definitions diff --git a/test/erlang_ls_code_navigation_SUITE_data/src/code_navigation.erl b/test/erlang_ls_code_navigation_SUITE_data/src/code_navigation.erl index e760c5704..fd9272a14 100644 --- a/test/erlang_ls_code_navigation_SUITE_data/src/code_navigation.erl +++ b/test/erlang_ls_code_navigation_SUITE_data/src/code_navigation.erl @@ -1,7 +1,12 @@ -module(code_navigation). +-behaviour(behaviour_a). + -export([ function_a/0 ]). +%% behaviour_a callbacks +-export([ callback_a/0 ]). + -record(record_a, {field_a, field_b}). -define(MACRO_A, macro_a). @@ -12,3 +17,6 @@ function_a() -> function_b() -> ?MACRO_A. + +callback_a() -> + ok. From 471b8ef11902da54e25e1077a9009ebaef44e4bd Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Thu, 1 Aug 2019 15:56:20 +0200 Subject: [PATCH 09/22] [#22] Remove un-necessary parameter --- src/erlang_ls_parser.erl | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/erlang_ls_parser.erl b/src/erlang_ls_parser.erl index 90d8d535d..01015fdcd 100644 --- a/src/erlang_ls_parser.erl +++ b/src/erlang_ls_parser.erl @@ -1,4 +1,3 @@ -%% TODO: Rename into erlang_ls_syntax -module(erlang_ls_parser). -export([ annotate/1 @@ -75,58 +74,58 @@ postorder_update(F, Tree) -> end). -spec get_range(syntax_tree(), pos(), {atom(), any()}) -> range(). -get_range(_Tree, {Line, Column}, {application, {M, F, _A}}) -> +get_range({Line, Column}, {application, {M, F, _A}}) -> CFrom = Column - length(atom_to_list(M)), From = {Line, CFrom}, CTo = Column + length(atom_to_list(F)), To = {Line, CTo}, #{ from => From, to => To }; -get_range(_Tree, {Line, Column}, {application, {F, _A}}) -> +get_range({Line, Column}, {application, {F, _A}}) -> From = {Line, Column}, To = {Line, Column + length(atom_to_list(F))}, #{ from => From, to => To }; -get_range(_Tree, {Line, Column}, {behaviour, Behaviour}) -> +get_range({Line, Column}, {behaviour, Behaviour}) -> From = {Line, Column - 1}, To = {Line, Column + length("behaviour") + length(atom_to_list(Behaviour))}, #{ from => From, to => To }; -get_range(_Tree, {_Line, _Column}, {exports_entry, {_F, _A}}) -> +get_range({_Line, _Column}, {exports_entry, {_F, _A}}) -> %% TODO: The location information for the arity qualifiers are lost during %% parsing in `epp_dodger`. This requires fixing. #{ from => {0, 0}, to => {0, 0} }; -get_range(_Tree, {Line, Column}, {function, {F, _A}}) -> +get_range({Line, Column}, {function, {F, _A}}) -> From = {Line - 1, Column - 1}, To = {Line - 1, Column + length(atom_to_list(F)) - 1}, #{ from => From, to => To }; -get_range(_Tree, {Line, _Column}, {define, _Define}) -> +get_range({Line, _Column}, {define, _Define}) -> From = {Line - 1, 0}, To = From, #{ from => From, to => To }; -get_range(_Tree, {Line, Column}, {include, Include}) -> +get_range({Line, Column}, {include, Include}) -> From = {Line, Column - 1}, To = {Line, Column + length("include") + length(Include)}, #{ from => From, to => To }; -get_range(_Tree, {Line, Column}, {include_lib, Include}) -> +get_range({Line, Column}, {include_lib, Include}) -> From = {Line, Column - 1}, To = {Line, Column + length("include_lib") + length(Include)}, #{ from => From, to => To }; -get_range(_Tree, {Line, Column}, {macro, Macro}) -> +get_range({Line, Column}, {macro, Macro}) -> From = {Line, Column}, To = {Line, Column + length(atom_to_list(Macro))}, #{ from => From, to => To }; -get_range(_Tree, {Line, Column}, {module, _}) -> +get_range({Line, Column}, {module, _}) -> From = {Line - 1, Column - 1}, To = From, #{ from => From, to => To }; -get_range(_Tree, {Line, Column}, {record_expr, Record}) -> +get_range({Line, Column}, {record_expr, Record}) -> From = {Line, Column - 1}, To = {Line, Column + length(Record) - 1}, #{ from => From, to => To }; %% TODO: Distinguish between usage poi and definition poi -get_range(_Tree, {Line, _Column}, {record, _Record}) -> +get_range({Line, _Column}, {record, _Record}) -> From = {Line - 1, 0}, To = From, #{ from => From, to => To }; -get_range(_Tree, {_Line, _Column}, {spec, _Spec}) -> +get_range({_Line, _Column}, {spec, _Spec}) -> %% TODO: The location information for the arity qualifiers are lost during %% parsing in `epp_dodger`. This requires fixing. #{ from => {0, 0}, to => {0, 0} }. @@ -251,7 +250,7 @@ analyze(_Tree, _) -> -spec poi(syntax_tree(), any()) -> poi(). poi(Tree, Info) -> Pos = erl_syntax:get_pos(Tree), - Range = get_range(Tree, Pos, Info), + Range = get_range(Pos, Info), #{ type => poi , info => Info , range => Range From 733aa91b66d0319a41735018c4a9ed5278320532 Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Thu, 1 Aug 2019 16:13:12 +0200 Subject: [PATCH 10/22] [#22] Introduce erlang_ls_tree module --- src/erlang_ls_buffer.erl | 2 +- src/erlang_ls_parser.erl | 117 +------------------------------- src/erlang_ls_tree.erl | 140 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 117 deletions(-) create mode 100644 src/erlang_ls_tree.erl diff --git a/src/erlang_ls_buffer.erl b/src/erlang_ls_buffer.erl index 9fc6b08c7..8f3bf16e9 100644 --- a/src/erlang_ls_buffer.erl +++ b/src/erlang_ls_buffer.erl @@ -144,7 +144,7 @@ do_get_mfa(Text, Line, _Character) -> do_get_element_at_pos(Text, Line, Column) -> %% TODO: Cache tree {ok, Tree} = erlang_ls_parser:parse(Text), - AnnotatedTree = erlang_ls_parser:annotate(Tree), + AnnotatedTree = erlang_ls_tree:annotate(Tree), erlang_ls_parser:find_poi_by_pos(AnnotatedTree, {Line, Column}). -spec get_line_text(binary(), integer()) -> binary(). diff --git a/src/erlang_ls_parser.erl b/src/erlang_ls_parser.erl index 01015fdcd..b70cba133 100644 --- a/src/erlang_ls_parser.erl +++ b/src/erlang_ls_parser.erl @@ -1,17 +1,13 @@ -module(erlang_ls_parser). --export([ annotate/1 - , annotate_node/1 - , find_poi_by_info/2 +-export([ find_poi_by_info/2 , find_poi_by_info_key/2 , find_poi_by_pos/2 , list_poi/1 , parse/1 , parse_file/1 - , postorder_update/2 ]). --type syntax_tree() :: erl_syntax:syntaxTree(). -type line() :: non_neg_integer(). -type column() :: non_neg_integer(). -type pos() :: {line(), column()}. @@ -21,7 +17,6 @@ -export_type([ poi/0 , range/0 - , syntax_tree/0 ]). %% TODO: Generate random filename @@ -52,27 +47,6 @@ parse_file(Path) -> {error, Error} end. --spec annotate(syntax_tree()) -> syntax_tree(). -annotate(Tree) -> - postorder_update(fun annotate_node/1, Tree). - -%% Create annotations for the points of interest (aka `poi`) in the -%% tree. --spec annotate_node(syntax_tree()) -> syntax_tree(). -annotate_node(Tree) -> - lists:foldl(fun erl_syntax:add_ann/2, Tree, analyze(Tree)). - -%% Extracted from the `erl_syntax` documentation. --spec postorder_update(fun(), syntax_tree()) -> syntax_tree(). -postorder_update(F, Tree) -> - F(case erl_syntax:subtrees(Tree) of - [] -> Tree; - List -> erl_syntax:update_tree(Tree, - [[postorder_update(F, Subtree) - || Subtree <- Group] - || Group <- List]) - end). - -spec get_range(syntax_tree(), pos(), {atom(), any()}) -> range(). get_range({Line, Column}, {application, {M, F, _A}}) -> CFrom = Column - length(atom_to_list(M)), @@ -158,95 +132,6 @@ list_poi(Tree) -> matches_pos(Pos, #{from := From, to := To}) -> (From =< Pos) andalso (Pos =< To). --spec analyze(syntax_tree()) -> [poi()]. -analyze(Tree) -> - Type = erl_syntax:type(Tree), - try analyze(Tree, Type) - catch - Class:Reason -> - lager:warning("Could not analyze tree: ~p:~p", [Class, Reason]), - [] - end. - --spec analyze(syntax_tree(), any()) -> [poi()]. -analyze(Tree, application) -> - case erl_syntax_lib:analyze_application(Tree) of - {M, {F, A}} -> - %% Remote call - [poi(Tree, {application, {M, F, A}})]; - {F, A} -> - case lists:member({F, A}, erlang:module_info(exports)) of - true -> - %% Call to a function from the `erlang` module - [poi(Tree, {application, {erlang, F, A}})]; - false -> - %% Local call - [poi(Tree, {application, {F, A}})] - end; - A when is_integer(A) -> - %% If the function is not explicitly named (e.g. a variable is - %% used as the module qualifier or the function name), only the - %% arity A is returned. - %% In the special case where the macro `?MODULE` is used as the - %% module qualifier, we can consider it as a local call. - Operator = erl_syntax:application_operator(Tree), - try { erl_syntax:variable_name( - erl_syntax:macro_name( - erl_syntax:module_qualifier_argument(Operator))) - , erl_syntax:atom_value( - erl_syntax:module_qualifier_body(Operator)) - } of - {'MODULE', F} -> - [poi(Tree, {application, {'MODULE', F, A}})] - catch _:_ -> - [] - end - end; -analyze(Tree, attribute) -> - case erl_syntax_lib:analyze_attribute(Tree) of - %% Yes, Erlang allows both British and American spellings for - %% keywords. - {behavior, {behavior, Behaviour}} -> - [poi(Tree, {behaviour, Behaviour})]; - {behaviour, {behaviour, Behaviour}} -> - [poi(Tree, {behaviour, Behaviour})]; - {export, Exports} -> - [poi(Tree, {exports_entry, {F, A}}) || {F, A} <- Exports]; - {module, {Module, _Args}} -> - [poi(Tree, {module, Module})]; - {module, Module} -> - [poi(Tree, {module, Module})]; - preprocessor -> - Name = erl_syntax:atom_value(erl_syntax:attribute_name(Tree)), - case {Name, erl_syntax:attribute_arguments(Tree)} of - {define, [Define|_]} -> - [poi(Tree, {define, erl_syntax:variable_name(Define)})]; - {include, [String]} -> - [poi(Tree, {include, erl_syntax:string_literal(String)})]; - {include_lib, [String]} -> - [poi(Tree, {include_lib, erl_syntax:string_literal(String)})]; - _ -> - [] - end; - {record, {Record, _Fields}} -> - [poi(Tree, {record, atom_to_list(Record)})]; - {spec, Spec} -> - [poi(Tree, {spec, Spec})]; - _ -> - [] - end; -analyze(Tree, function) -> - {F, A} = erl_syntax_lib:analyze_function(Tree), - [poi(Tree, {function, {F, A}})]; -analyze(Tree, macro) -> - Macro = erl_syntax:variable_name(erl_syntax:macro_name(Tree)), - [poi(Tree, {macro, Macro})]; -analyze(Tree, record_expr) -> - Record = erl_syntax:atom_name(erl_syntax:record_expr_type(Tree)), - [poi(Tree, {record_expr, Record})]; -analyze(_Tree, _) -> - []. - -spec poi(syntax_tree(), any()) -> poi(). poi(Tree, Info) -> Pos = erl_syntax:get_pos(Tree), diff --git a/src/erlang_ls_tree.erl b/src/erlang_ls_tree.erl new file mode 100644 index 000000000..ae6bd065c --- /dev/null +++ b/src/erlang_ls_tree.erl @@ -0,0 +1,140 @@ +%%============================================================================== +%% Library to handle syntax trees annotated with points of interest +%%============================================================================== +-module(erlang_ls_tree). + +%%============================================================================== +%% Exports +%%============================================================================== +-export([ annotate/1 + , annotate_node/1 + , postorder_update/2 + , points_of_interest/1 + ]). + +%%============================================================================== +%% Types +%%============================================================================== +-type tree() :: erl_syntax:syntaxTree(). + +-export_type([ tree/0 ]). + +%%============================================================================== +%% API +%%============================================================================== +%% @edoc Given a syntax tree, it returns a new one, annotated with all +%% the identified _points of interest_ (a.k.a. _poi_). +-spec annotate(tree()) -> tree(). +annotate(Tree) -> + postorder_update(fun annotate_node/1, Tree). + +%% @edoc Add an annotation to the root of the given `Tree` for each +%% point of interest found. +-spec annotate_node(tree()) -> tree(). +annotate_node(Tree) -> + lists:foldl(fun erl_syntax:add_ann/2, Tree, points_of_interest(Tree)). + +%% @edoc Traverse the given `Tree`, applying the function `F` to all +%% nodes in the tree, in post-order. Extracted from the `erl_syntax` +%% documentation. +-spec postorder_update(fun(), syntax_tree()) -> syntax_tree(). +postorder_update(F, Tree) -> + F(case erl_syntax:subtrees(Tree) of + [] -> Tree; + List -> erl_syntax:update_tree(Tree, + [[postorder_update(F, Subtree) + || Subtree <- Group] + || Group <- List]) + end). + +%% @edoc Return the list of points of interest for a given `Tree`. +-spec points_of_interest(tree()) -> [poi()]. +points_of_interest(Tree) -> + Type = erl_syntax:type(Tree), + try points_of_interest(Tree, Type) + catch + Class:Reason -> + lager:warning("Could not analyze tree: ~p:~p", [Class, Reason]), + [] + end. + +%% @edoc Return the list of points of interest of a specific `Type` +%% for a given `Tree`. +-spec points_of_interest(syntax_tree(), any()) -> [poi()]. +points_of_interest(Tree, application) -> + case erl_syntax_lib:analyze_application(Tree) of + {M, {F, A}} -> + %% Remote call + [poi(Tree, {application, {M, F, A}})]; + {F, A} -> + case lists:member({F, A}, erlang:module_info(exports)) of + true -> + %% Call to a function from the `erlang` module + [poi(Tree, {application, {erlang, F, A}})]; + false -> + %% Local call + [poi(Tree, {application, {F, A}})] + end; + A when is_integer(A) -> + %% If the function is not explicitly named (e.g. a variable is + %% used as the module qualifier or the function name), only the + %% arity A is returned. + %% In the special case where the macro `?MODULE` is used as the + %% module qualifier, we can consider it as a local call. + Operator = erl_syntax:application_operator(Tree), + try { erl_syntax:variable_name( + erl_syntax:macro_name( + erl_syntax:module_qualifier_argument(Operator))) + , erl_syntax:atom_value( + erl_syntax:module_qualifier_body(Operator)) + } of + {'MODULE', F} -> + [poi(Tree, {application, {'MODULE', F, A}})] + catch _:_ -> + [] + end + end; +points_of_interest(Tree, attribute) -> + case erl_syntax_lib:analyze_attribute(Tree) of + %% Yes, Erlang allows both British and American spellings for + %% keywords. + {behavior, {behavior, Behaviour}} -> + [poi(Tree, {behaviour, Behaviour})]; + {behaviour, {behaviour, Behaviour}} -> + [poi(Tree, {behaviour, Behaviour})]; + {export, Exports} -> + [poi(Tree, {exports_entry, {F, A}}) || {F, A} <- Exports]; + {module, {Module, _Args}} -> + [poi(Tree, {module, Module})]; + {module, Module} -> + [poi(Tree, {module, Module})]; + preprocessor -> + Name = erl_syntax:atom_value(erl_syntax:attribute_name(Tree)), + case {Name, erl_syntax:attribute_arguments(Tree)} of + {define, [Define|_]} -> + [poi(Tree, {define, erl_syntax:variable_name(Define)})]; + {include, [String]} -> + [poi(Tree, {include, erl_syntax:string_literal(String)})]; + {include_lib, [String]} -> + [poi(Tree, {include_lib, erl_syntax:string_literal(String)})]; + _ -> + [] + end; + {record, {Record, _Fields}} -> + [poi(Tree, {record, atom_to_list(Record)})]; + {spec, Spec} -> + [poi(Tree, {spec, Spec})]; + _ -> + [] + end; +points_of_interest(Tree, function) -> + {F, A} = erl_syntax_lib:analyze_function(Tree), + [poi(Tree, {function, {F, A}})]; +points_of_interest(Tree, macro) -> + Macro = erl_syntax:variable_name(erl_syntax:macro_name(Tree)), + [poi(Tree, {macro, Macro})]; +points_of_interest(Tree, record_expr) -> + Record = erl_syntax:atom_name(erl_syntax:record_expr_type(Tree)), + [poi(Tree, {record_expr, Record})]; +points_of_interest(_Tree, _) -> + []. From b1fead438c239c77d388df4e39fbbe205b3495cd Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Thu, 1 Aug 2019 16:13:33 +0200 Subject: [PATCH 11/22] [#22] Add list of tests TODO --- test/erlang_ls_code_navigation_SUITE.erl | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/erlang_ls_code_navigation_SUITE.erl b/test/erlang_ls_code_navigation_SUITE.erl index e5bcac56c..c6cad79ee 100644 --- a/test/erlang_ls_code_navigation_SUITE.erl +++ b/test/erlang_ls_code_navigation_SUITE.erl @@ -117,3 +117,11 @@ definition(DataDir, {record_expr, "record_a"}) -> #{ range => erlang_ls_protocol:range(#{from => {9, 0}, to => {9, 0}}) , uri => erlang_ls_uri:uri(FilePath) }. +%% TODO: API should be refactored. I should search for something, not the definition. +%% TODO: include +%% TODO: include_lib +%% TODO: recursive record +%% TODO: recursive macro +%% TODO: local function +%% TODO: remote function +%% TODO: bif From 0a9d9ba193828c00f26bc9af91cebe58476ee289 Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Thu, 1 Aug 2019 16:14:08 +0200 Subject: [PATCH 12/22] [#22] Add behaviour data file for tests --- test/erlang_ls_code_navigation_SUITE_data/src/behaviour_a.erl | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 test/erlang_ls_code_navigation_SUITE_data/src/behaviour_a.erl diff --git a/test/erlang_ls_code_navigation_SUITE_data/src/behaviour_a.erl b/test/erlang_ls_code_navigation_SUITE_data/src/behaviour_a.erl new file mode 100644 index 000000000..83db351d9 --- /dev/null +++ b/test/erlang_ls_code_navigation_SUITE_data/src/behaviour_a.erl @@ -0,0 +1,3 @@ +-module(behaviour_a). + +-callback callback_a() -> ok. From 5f3bdd47e706a305637d4e12b9933c6a1d955578 Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Mon, 5 Aug 2019 18:24:07 +0200 Subject: [PATCH 13/22] [#22] Move functions to poi module, refactor --- src/erlang_ls_buffer.erl | 2 +- src/erlang_ls_parser.erl | 95 +++++--------------------------------- src/erlang_ls_poi.erl | 92 ++++++++++++++++++++++++++++++++++++ src/erlang_ls_protocol.erl | 2 +- src/erlang_ls_server.erl | 12 ++--- src/erlang_ls_tree.erl | 43 +++++++++-------- 6 files changed, 134 insertions(+), 112 deletions(-) create mode 100644 src/erlang_ls_poi.erl diff --git a/src/erlang_ls_buffer.erl b/src/erlang_ls_buffer.erl index 8f3bf16e9..c6a5e3330 100644 --- a/src/erlang_ls_buffer.erl +++ b/src/erlang_ls_buffer.erl @@ -140,7 +140,7 @@ do_get_mfa(Text, Line, _Character) -> {M, F, A}. -spec do_get_element_at_pos(binary(), non_neg_integer(), non_neg_integer()) -> - [erlang_ls_parser:poi()]. + [erlang_ls_poi:poi()]. do_get_element_at_pos(Text, Line, Column) -> %% TODO: Cache tree {ok, Tree} = erlang_ls_parser:parse(Text), diff --git a/src/erlang_ls_parser.erl b/src/erlang_ls_parser.erl index b70cba133..769f6d2d9 100644 --- a/src/erlang_ls_parser.erl +++ b/src/erlang_ls_parser.erl @@ -8,29 +8,19 @@ , parse_file/1 ]). --type line() :: non_neg_integer(). --type column() :: non_neg_integer(). --type pos() :: {line(), column()}. --type range() :: #{ from := pos(), to := pos() }. -%% Point of Interest --type poi() :: #{ type := atom(), info => any(), range := range()}. - --export_type([ poi/0 - , range/0 - ]). - %% TODO: Generate random filename %% TODO: Ideally avoid writing to file at all (require epp changes) -define(TMP_PATH, <<"/tmp/erlang_ls_tmp">>). --spec parse(binary()) -> {ok, syntax_tree()}. +-spec parse(binary()) -> {ok, erlang_ls_tree:tree()}. parse(Text) -> %% epp_dodger only works with source files, %% so let's use a temporary file. ok = file:write_file(?TMP_PATH, Text), parse_file(?TMP_PATH). --spec parse_file(binary()) -> {ok, syntax_tree()} | {error, any()}. +-spec parse_file(binary()) -> + {ok, erlang_ls_tree:tree()} | {error, any()}. parse_file(Path) -> case file:open(Path, [read]) of {ok, IoDevice} -> @@ -47,77 +37,23 @@ parse_file(Path) -> {error, Error} end. --spec get_range(syntax_tree(), pos(), {atom(), any()}) -> range(). -get_range({Line, Column}, {application, {M, F, _A}}) -> - CFrom = Column - length(atom_to_list(M)), - From = {Line, CFrom}, - CTo = Column + length(atom_to_list(F)), - To = {Line, CTo}, - #{ from => From, to => To }; -get_range({Line, Column}, {application, {F, _A}}) -> - From = {Line, Column}, - To = {Line, Column + length(atom_to_list(F))}, - #{ from => From, to => To }; -get_range({Line, Column}, {behaviour, Behaviour}) -> - From = {Line, Column - 1}, - To = {Line, Column + length("behaviour") + length(atom_to_list(Behaviour))}, - #{ from => From, to => To }; -get_range({_Line, _Column}, {exports_entry, {_F, _A}}) -> - %% TODO: The location information for the arity qualifiers are lost during - %% parsing in `epp_dodger`. This requires fixing. - #{ from => {0, 0}, to => {0, 0} }; -get_range({Line, Column}, {function, {F, _A}}) -> - From = {Line - 1, Column - 1}, - To = {Line - 1, Column + length(atom_to_list(F)) - 1}, - #{ from => From, to => To }; -get_range({Line, _Column}, {define, _Define}) -> - From = {Line - 1, 0}, - To = From, - #{ from => From, to => To }; -get_range({Line, Column}, {include, Include}) -> - From = {Line, Column - 1}, - To = {Line, Column + length("include") + length(Include)}, - #{ from => From, to => To }; -get_range({Line, Column}, {include_lib, Include}) -> - From = {Line, Column - 1}, - To = {Line, Column + length("include_lib") + length(Include)}, - #{ from => From, to => To }; -get_range({Line, Column}, {macro, Macro}) -> - From = {Line, Column}, - To = {Line, Column + length(atom_to_list(Macro))}, - #{ from => From, to => To }; -get_range({Line, Column}, {module, _}) -> - From = {Line - 1, Column - 1}, - To = From, - #{ from => From, to => To }; -get_range({Line, Column}, {record_expr, Record}) -> - From = {Line, Column - 1}, - To = {Line, Column + length(Record) - 1}, - #{ from => From, to => To }; -%% TODO: Distinguish between usage poi and definition poi -get_range({Line, _Column}, {record, _Record}) -> - From = {Line - 1, 0}, - To = From, - #{ from => From, to => To }; -get_range({_Line, _Column}, {spec, _Spec}) -> - %% TODO: The location information for the arity qualifiers are lost during - %% parsing in `epp_dodger`. This requires fixing. - #{ from => {0, 0}, to => {0, 0} }. - --spec find_poi_by_info(syntax_tree(), any()) -> [poi()]. +-spec find_poi_by_info(erlang_ls_tree:tree(), any()) -> + [erlang_ls_poi:poi()]. find_poi_by_info(Tree, Info0) -> [POI || #{info := Info} = POI <- list_poi(Tree), Info0 =:= Info]. %% TODO: Rename --spec find_poi_by_info_key(syntax_tree(), atom()) -> [poi()]. +-spec find_poi_by_info_key(erlang_ls_tree:tree(), atom()) -> + [erlang_ls_poi:poi()]. find_poi_by_info_key(Tree, Key0) -> [POI || #{info := {Key, _}} = POI <- list_poi(Tree), Key0 =:= Key]. --spec find_poi_by_pos(syntax_tree(), pos()) -> [poi()]. +-spec find_poi_by_pos(erlang_ls_tree:tree(), erlang_ls_poi:pos()) -> + [erlang_ls_poi:poi()]. find_poi_by_pos(Tree, Pos) -> [POI || #{range := Range} = POI <- list_poi(Tree), matches_pos(Pos, Range)]. --spec list_poi(syntax_tree()) -> [poi()]. +-spec list_poi(erlang_ls_tree:tree()) -> [erlang_ls_poi:poi()]. list_poi(Tree) -> F = fun(T, Acc) -> Annotations = erl_syntax:get_ann(T), @@ -128,15 +64,6 @@ list_poi(Tree) -> end, erl_syntax_lib:fold(F, [], Tree). --spec matches_pos(pos(), range()) -> boolean(). +-spec matches_pos(erlang_ls_poi:pos(), erlang_ls_poi:range()) -> boolean(). matches_pos(Pos, #{from := From, to := To}) -> (From =< Pos) andalso (Pos =< To). - --spec poi(syntax_tree(), any()) -> poi(). -poi(Tree, Info) -> - Pos = erl_syntax:get_pos(Tree), - Range = get_range(Pos, Info), - #{ type => poi - , info => Info - , range => Range - }. diff --git a/src/erlang_ls_poi.erl b/src/erlang_ls_poi.erl new file mode 100644 index 000000000..f9a393d55 --- /dev/null +++ b/src/erlang_ls_poi.erl @@ -0,0 +1,92 @@ +%%============================================================================== +%% The Point Of Interest (a.k.a. _poi_) Data Structure +%%============================================================================== +-module(erlang_ls_poi). + +-type line() :: non_neg_integer(). +-type column() :: non_neg_integer(). +-type pos() :: {line(), column()}. +-type range() :: #{ from := pos(), to := pos() }. +-type poi() :: #{ type := atom() + , info => any() + , range := range() + }. + +-export_type([ poi/0 + , pos/0 + , range/0 + ]). + + +-export([ poi/2 ]). + +%% @edoc Constructor for a Point of Interest. +-spec poi(erlang_ls_tree:tree(), any()) -> poi(). +poi(Tree, Info) -> + Pos = erl_syntax:get_pos(Tree), + Range = get_range(Pos, Info), + #{ type => poi + , info => Info + , range => Range + }. + +%%============================================================================== +%% Internal Functions +%%============================================================================== + +-spec get_range(pos(), {atom(), any()}) -> range(). +get_range({Line, Column}, {application, {M, F, _A}}) -> + CFrom = Column - length(atom_to_list(M)), + From = {Line, CFrom}, + CTo = Column + length(atom_to_list(F)), + To = {Line, CTo}, + #{ from => From, to => To }; +get_range({Line, Column}, {application, {F, _A}}) -> + From = {Line, Column}, + To = {Line, Column + length(atom_to_list(F))}, + #{ from => From, to => To }; +get_range({Line, Column}, {behaviour, Behaviour}) -> + From = {Line, Column - 1}, + To = {Line, Column + length("behaviour") + length(atom_to_list(Behaviour))}, + #{ from => From, to => To }; +get_range({_Line, _Column}, {exports_entry, {_F, _A}}) -> + %% TODO: The location information for the arity qualifiers are lost during + %% parsing in `epp_dodger`. This requires fixing. + #{ from => {0, 0}, to => {0, 0} }; +get_range({Line, Column}, {function, {F, _A}}) -> + From = {Line - 1, Column - 1}, + To = {Line - 1, Column + length(atom_to_list(F)) - 1}, + #{ from => From, to => To }; +get_range({Line, _Column}, {define, _Define}) -> + From = {Line - 1, 0}, + To = From, + #{ from => From, to => To }; +get_range({Line, Column}, {include, Include}) -> + From = {Line, Column - 1}, + To = {Line, Column + length("include") + length(Include)}, + #{ from => From, to => To }; +get_range({Line, Column}, {include_lib, Include}) -> + From = {Line, Column - 1}, + To = {Line, Column + length("include_lib") + length(Include)}, + #{ from => From, to => To }; +get_range({Line, Column}, {macro, Macro}) -> + From = {Line, Column}, + To = {Line, Column + length(atom_to_list(Macro))}, + #{ from => From, to => To }; +get_range({Line, Column}, {module, _}) -> + From = {Line - 1, Column - 1}, + To = From, + #{ from => From, to => To }; +get_range({Line, Column}, {record_expr, Record}) -> + From = {Line, Column - 1}, + To = {Line, Column + length(Record) - 1}, + #{ from => From, to => To }; +%% TODO: Distinguish between usage poi and definition poi +get_range({Line, _Column}, {record, _Record}) -> + From = {Line - 1, 0}, + To = From, + #{ from => From, to => To }; +get_range({_Line, _Column}, {spec, _Spec}) -> + %% TODO: The location information for the arity qualifiers are lost during + %% parsing in `epp_dodger`. This requires fixing. + #{ from => {0, 0}, to => {0, 0} }. diff --git a/src/erlang_ls_protocol.erl b/src/erlang_ls_protocol.erl index ca4a64f46..5c7d337ff 100644 --- a/src/erlang_ls_protocol.erl +++ b/src/erlang_ls_protocol.erl @@ -52,7 +52,7 @@ response(RequestId, Result) -> %%============================================================================== %% Data Structures %%============================================================================== --spec range(erlang_ls_parser:range()) -> range(). +-spec range(erlang_ls_poi:range()) -> range(). range(#{ from := {FromL, FromC}, to := {ToL, ToC} }) -> #{ start => #{line => FromL, character => FromC} , 'end' => #{line => ToL, character => ToC} diff --git a/src/erlang_ls_server.erl b/src/erlang_ls_server.erl index 487cc0d51..5ffa9f270 100644 --- a/src/erlang_ls_server.erl +++ b/src/erlang_ls_server.erl @@ -208,7 +208,7 @@ send_notification(Socket, Method, Params) -> gen_tcp:send(Socket, Notification). %% TODO: definition/2 should probably not take the Uri, but a path. --spec definition(uri(), erlang_ls_parser:poi()) -> null | map(). +-spec definition(uri(), erlang_ls_poi:poi()) -> null | map(). definition(_Uri, #{ info := {application, {M, _F, _A}} = Info }) -> case annotated_tree(erlang_ls_uri:filename(M)) of {ok, Uri, AnnotatedTree} -> @@ -291,12 +291,12 @@ definition({record_expr, Record}) -> {record, Record}. -spec annotated_tree(binary()) -> - {ok, uri(), erlang_ls_parser:syntax_tree()} | {error, any()}. + {ok, uri(), erlang_ls_tree:tree()} | {error, any()}. annotated_tree(Filename) -> annotated_tree(Filename, full_path()). -spec annotated_tree(binary(), [string()]) -> - {ok, uri(), erlang_ls_parser:syntax_tree()} | {error, any()}. + {ok, uri(), erlang_ls_tree:tree()} | {error, any()}. annotated_tree(Filename, Path) -> case file:path_open(Path, Filename, [read]) of {ok, IoDevice, FullName} -> @@ -304,7 +304,7 @@ annotated_tree(Filename, Path) -> file:close(IoDevice), {ok, Tree} = erlang_ls_parser:parse_file(FullName), Uri = erlang_ls_uri:uri(FullName), - {ok, Uri, erlang_ls_parser:annotate(Tree)}; + {ok, Uri, erlang_ls_tree:annotate(Tree)}; {error, Error} -> {error, Error} end. @@ -346,7 +346,7 @@ search(Filename, Path, Thing) -> end. %% Look for a definition in a given tree --spec find(uri(), erlang_ls_parser:syntax_tree(), any()) -> null | map(). +-spec find(uri(), erlang_ls_tree:tree(), any()) -> null | map(). find(Uri, AnnotatedTree, Thing) -> case erlang_ls_parser:find_poi_by_info(AnnotatedTree, Thing) of [#{ range := Range }|_] -> @@ -355,7 +355,7 @@ find(Uri, AnnotatedTree, Thing) -> null end. --spec search_in_includes([erlang_ls_parser:poi()], string()) -> null | map(). +-spec search_in_includes([erlang_ls_poi:poi()], string()) -> null | map(). search_in_includes([], _Thing) -> null; search_in_includes([#{info := Info}|T], Thing) -> diff --git a/src/erlang_ls_tree.erl b/src/erlang_ls_tree.erl index ae6bd065c..dc6462279 100644 --- a/src/erlang_ls_tree.erl +++ b/src/erlang_ls_tree.erl @@ -37,7 +37,7 @@ annotate_node(Tree) -> %% @edoc Traverse the given `Tree`, applying the function `F` to all %% nodes in the tree, in post-order. Extracted from the `erl_syntax` %% documentation. --spec postorder_update(fun(), syntax_tree()) -> syntax_tree(). +-spec postorder_update(fun(), tree()) -> tree(). postorder_update(F, Tree) -> F(case erl_syntax:subtrees(Tree) of [] -> Tree; @@ -48,7 +48,7 @@ postorder_update(F, Tree) -> end). %% @edoc Return the list of points of interest for a given `Tree`. --spec points_of_interest(tree()) -> [poi()]. +-spec points_of_interest(tree()) -> [erlang_ls_poi:poi()]. points_of_interest(Tree) -> Type = erl_syntax:type(Tree), try points_of_interest(Tree, Type) @@ -60,20 +60,20 @@ points_of_interest(Tree) -> %% @edoc Return the list of points of interest of a specific `Type` %% for a given `Tree`. --spec points_of_interest(syntax_tree(), any()) -> [poi()]. +-spec points_of_interest(tree(), any()) -> [erlang_ls_poi:poi()]. points_of_interest(Tree, application) -> case erl_syntax_lib:analyze_application(Tree) of {M, {F, A}} -> %% Remote call - [poi(Tree, {application, {M, F, A}})]; + [erlang_ls_poi:poi(Tree, {application, {M, F, A}})]; {F, A} -> case lists:member({F, A}, erlang:module_info(exports)) of true -> %% Call to a function from the `erlang` module - [poi(Tree, {application, {erlang, F, A}})]; + [erlang_ls_poi:poi(Tree, {application, {erlang, F, A}})]; false -> %% Local call - [poi(Tree, {application, {F, A}})] + [erlang_ls_poi:poi(Tree, {application, {F, A}})] end; A when is_integer(A) -> %% If the function is not explicitly named (e.g. a variable is @@ -89,7 +89,7 @@ points_of_interest(Tree, application) -> erl_syntax:module_qualifier_body(Operator)) } of {'MODULE', F} -> - [poi(Tree, {application, {'MODULE', F, A}})] + [erlang_ls_poi:poi(Tree, {application, {'MODULE', F, A}})] catch _:_ -> [] end @@ -99,42 +99,45 @@ points_of_interest(Tree, attribute) -> %% Yes, Erlang allows both British and American spellings for %% keywords. {behavior, {behavior, Behaviour}} -> - [poi(Tree, {behaviour, Behaviour})]; + [erlang_ls_poi:poi(Tree, {behaviour, Behaviour})]; {behaviour, {behaviour, Behaviour}} -> - [poi(Tree, {behaviour, Behaviour})]; + [erlang_ls_poi:poi(Tree, {behaviour, Behaviour})]; {export, Exports} -> - [poi(Tree, {exports_entry, {F, A}}) || {F, A} <- Exports]; + [erlang_ls_poi:poi(Tree, {exports_entry, {F, A}}) || {F, A} <- Exports]; {module, {Module, _Args}} -> - [poi(Tree, {module, Module})]; + [erlang_ls_poi:poi(Tree, {module, Module})]; {module, Module} -> - [poi(Tree, {module, Module})]; + [erlang_ls_poi:poi(Tree, {module, Module})]; preprocessor -> Name = erl_syntax:atom_value(erl_syntax:attribute_name(Tree)), case {Name, erl_syntax:attribute_arguments(Tree)} of {define, [Define|_]} -> - [poi(Tree, {define, erl_syntax:variable_name(Define)})]; + [erlang_ls_poi:poi( Tree + , {define, erl_syntax:variable_name(Define)})]; {include, [String]} -> - [poi(Tree, {include, erl_syntax:string_literal(String)})]; + [erlang_ls_poi:poi( Tree + , {include, erl_syntax:string_literal(String)})]; {include_lib, [String]} -> - [poi(Tree, {include_lib, erl_syntax:string_literal(String)})]; + [erlang_ls_poi:poi( Tree + , {include_lib, erl_syntax:string_literal(String)})]; _ -> [] end; {record, {Record, _Fields}} -> - [poi(Tree, {record, atom_to_list(Record)})]; + [erlang_ls_poi:poi(Tree, {record, atom_to_list(Record)})]; {spec, Spec} -> - [poi(Tree, {spec, Spec})]; + [erlang_ls_poi:poi(Tree, {spec, Spec})]; _ -> [] end; points_of_interest(Tree, function) -> {F, A} = erl_syntax_lib:analyze_function(Tree), - [poi(Tree, {function, {F, A}})]; + [erlang_ls_poi:poi(Tree, {function, {F, A}})]; points_of_interest(Tree, macro) -> Macro = erl_syntax:variable_name(erl_syntax:macro_name(Tree)), - [poi(Tree, {macro, Macro})]; + [erlang_ls_poi:poi(Tree, {macro, Macro})]; points_of_interest(Tree, record_expr) -> Record = erl_syntax:atom_name(erl_syntax:record_expr_type(Tree)), - [poi(Tree, {record_expr, Record})]; + [erlang_ls_poi:poi(Tree, {record_expr, Record})]; points_of_interest(_Tree, _) -> []. From 90cae6b5025687baf0e822987ebb0e4ce35c527d Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Mon, 5 Aug 2019 18:43:47 +0200 Subject: [PATCH 14/22] [#22] Introduce code navigation module --- src/erlang_ls_code_navigation.erl | 202 +++++++++++++++++++++++ src/erlang_ls_server.erl | 177 +------------------- test/erlang_ls_code_navigation_SUITE.erl | 18 +- 3 files changed, 212 insertions(+), 185 deletions(-) create mode 100644 src/erlang_ls_code_navigation.erl diff --git a/src/erlang_ls_code_navigation.erl b/src/erlang_ls_code_navigation.erl new file mode 100644 index 000000000..49d5bbc89 --- /dev/null +++ b/src/erlang_ls_code_navigation.erl @@ -0,0 +1,202 @@ +%%============================================================================== +%% Code Navigation +%%============================================================================== +-module(erlang_ls_code_navigation). + +%%============================================================================== +%% Exports +%%============================================================================== + +%% API +-export([ goto_definition/2 ]). + +%% TODO: Refactor API +-export([ definition/1 + , search/3 + ]). + +%%============================================================================== +%% Includes +%%============================================================================== +-include("erlang_ls.hrl"). + +%%============================================================================== +%% Macros +%%============================================================================== +-define(OTP_INCLUDE_PATH, "/usr/local/Cellar/erlang/21.2.4/lib/erlang/lib"). +%% TODO: Implement support for workspaces +-define(ERLANG_LS_PATH, "/Users/robert.aloi/git/github/erlang-ls/erlang_ls"). +-define(TEST_APP_PATH, "/Users/robert.aloi/git/github/erlang-ls/test"). +-define(DEPS_PATH, "/Users/robert.aloi/git/github/erlang-ls/erlang_ls/_build/debug/lib"). + +%%============================================================================== +%% API +%%============================================================================== + +%% TODO: Specify type instead of generic map +%% TODO: goto_definition/2 should probably not take the Uri, but a path. +-spec goto_definition(uri(), erlang_ls_poi:poi()) -> null | map(). +goto_definition(_Uri, #{ info := {application, {M, _F, _A}} = Info }) -> + case annotated_tree(erlang_ls_uri:filename(M)) of + {ok, Uri, AnnotatedTree} -> + case erlang_ls_parser:find_poi_by_info(AnnotatedTree, definition(Info)) of + [#{ range := Range }] -> + %% TODO: Use API to create types + #{ uri => Uri + , range => erlang_ls_protocol:range(Range) + }; + [] -> + null + end; + {error, _Error} -> + null + end; +goto_definition(Uri, #{ info := {application, {_F, _A}} = Info }) -> + case annotated_tree(erlang_ls_uri:basename(Uri)) of + {ok, Uri, AnnotatedTree} -> + case erlang_ls_parser:find_poi_by_info(AnnotatedTree, definition(Info)) of + [#{ range := Range }] -> + %% TODO: Use API to create types + #{ uri => Uri + , range => erlang_ls_protocol:range(Range) + }; + [] -> + null + end; + {error, _Error} -> + null + end; +goto_definition(_Uri, #{ info := {behaviour, Behaviour} = Info }) -> + Filename = erlang_ls_uri:filename(Behaviour), + search(Filename, full_path(), definition(Info)); +%% TODO: Eventually search everywhere and suggest a code lens to include a file +goto_definition(Uri, #{ info := {macro, _Define} = Info }) -> + Filename = erlang_ls_uri:basename(Uri), + search(Filename, app_path(), definition(Info)); +goto_definition(Uri, #{ info := {record_expr, _Record} = Info }) -> + Filename = erlang_ls_uri:basename(Uri), + search(Filename, app_path(), Info); +goto_definition(_Uri, #{ info := {include, Include0} }) -> + Include = list_to_binary(string:trim(Include0, both, [$"])), + case annotated_tree(Include) of + {ok, Uri, _AnnotatedTree} -> + #{ uri => Uri + %% TODO: We could point to the module attribute, instead + , range => erlang_ls_protocol:range(#{ from => {0, 0} + , to => {0, 0} + }) + }; + {error, _Error} -> + null + end; +goto_definition(_Uri, #{ info := {include_lib, Include0} }) -> + Include = list_to_binary(lists:last(filename:split(string:trim(Include0, both, [$"])))), + case annotated_tree(Include) of + {ok, Uri, _AnnotatedTree} -> + #{ uri => Uri + %% TODO: We could point to the module attribute, instead + , range => erlang_ls_protocol:range(#{ from => {0, 0} + , to => {0, 0} + }) + }; + {error, _Error} -> + null + end; +goto_definition(_Uri, _) -> + null. + +%% TODO: Move to tree module +-spec annotated_tree(binary()) -> + {ok, uri(), erlang_ls_tree:tree()} | {error, any()}. +annotated_tree(Filename) -> + annotated_tree(Filename, full_path()). + +-spec annotated_tree(binary(), [string()]) -> + {ok, uri(), erlang_ls_tree:tree()} | {error, any()}. +annotated_tree(Filename, Path) -> + case file:path_open(Path, Filename, [read]) of + {ok, IoDevice, FullName} -> + %% TODO: Avoid opening file twice + file:close(IoDevice), + {ok, Tree} = erlang_ls_parser:parse_file(FullName), + Uri = erlang_ls_uri:uri(FullName), + {ok, Uri, erlang_ls_tree:annotate(Tree)}; + {error, Error} -> + {error, Error} + end. + +-spec definition({atom(), any()}) -> {atom(), any()}. +definition({application, {_M, F, A}}) -> + {function, {F, A}}; +definition({application, {F, A}}) -> + {function, {F, A}}; +definition({behaviour, Behaviour}) -> + {module, Behaviour}; +definition({macro, Define}) -> + {define, Define}; +definition({record_expr, Record}) -> + {record, Record}. + + +-spec otp_path() -> [string()]. +otp_path() -> + filelib:wildcard(filename:join([?OTP_INCLUDE_PATH, "*/src"])). + +-spec app_path() -> [string()]. +app_path() -> + [ filename:join([?TEST_APP_PATH, "src"]) + , filename:join([?TEST_APP_PATH, "include"]) + , filename:join([?ERLANG_LS_PATH, "src"]) + , filename:join([?TEST_APP_PATH, "include"]) + ]. + +-spec deps_path() -> [string()]. +deps_path() -> + filelib:wildcard(filename:join([?DEPS_PATH, "*/src"])). + +full_path() -> + lists:append( [ app_path() , deps_path() , otp_path() ]). + + +%% Look for a definition recursively in a file and its includes. +-spec search(binary(), [string()], any()) -> null | map(). +search(Filename, Path, Thing) -> + case annotated_tree(Filename, Path) of + {ok, Uri, AnnotatedTree} -> + case find(Uri, AnnotatedTree, Thing) of + null -> + Includes = erlang_ls_parser:find_poi_by_info_key(AnnotatedTree, include), + IncludeLibs = erlang_ls_parser:find_poi_by_info_key(AnnotatedTree, include_lib), + search_in_includes(Includes ++ IncludeLibs, Thing); + Def -> + Def + end; + {error, _Error} -> + null + end. + +%% Look for a definition in a given tree +-spec find(uri(), erlang_ls_tree:tree(), any()) -> null | map(). +find(Uri, AnnotatedTree, Thing) -> + case erlang_ls_parser:find_poi_by_info(AnnotatedTree, Thing) of + [#{ range := Range }|_] -> + #{ uri => Uri, range => erlang_ls_protocol:range(Range) }; + [] -> + null + end. + +-spec search_in_includes([erlang_ls_poi:poi()], string()) -> null | map(). +search_in_includes([], _Thing) -> + null; +search_in_includes([#{info := Info}|T], Thing) -> + Include = normalize_include(Info), + case search(list_to_binary(Include), app_path(), Thing) of + null -> search_in_includes(T, Thing); + Def -> Def + end. + +-spec normalize_include({atom(), string()}) -> string(). +normalize_include({include, Include}) -> + string:trim(Include, both, [$"]); +normalize_include({include_lib, Include}) -> + lists:last(filename:split(string:trim(Include, both, [$"]))). diff --git a/src/erlang_ls_server.erl b/src/erlang_ls_server.erl index 5ffa9f270..037ce751c 100644 --- a/src/erlang_ls_server.erl +++ b/src/erlang_ls_server.erl @@ -26,11 +26,6 @@ -export([ connected/3 ]). -%% TODO: Move to separate module --export([ definition/1 - , search/3 - ]). - %%============================================================================== %% Includes %%============================================================================== @@ -41,12 +36,6 @@ %%============================================================================== -record(state, {socket, buffer}). --define(OTP_INCLUDE_PATH, "/usr/local/Cellar/erlang/21.2.4/lib/erlang/lib"). -%% TODO: Implement support for workspaces --define(ERLANG_LS_PATH, "/Users/robert.aloi/git/github/erlang-ls/erlang_ls"). --define(TEST_APP_PATH, "/Users/robert.aloi/git/github/erlang-ls/test"). --define(DEPS_PATH, "/Users/robert.aloi/git/github/erlang-ls/erlang_ls/_build/debug/lib"). - %%============================================================================== %% Type Definitions %%============================================================================== @@ -188,7 +177,7 @@ handle_method(<<"textDocument/definition">>, Params) -> {ok, Buffer} = erlang_ls_buffer_server:get_buffer(Uri), case erlang_ls_buffer:get_element_at_pos(Buffer, Line + 1, Character + 1) of [POI|_] -> - {response, definition(Uri, POI)}; + {response, erlang_ls_code_navigation:goto_definition(Uri, POI)}; [] -> {response, null} end; @@ -206,167 +195,3 @@ send_notification(Socket, Method, Params) -> Notification = erlang_ls_protocol:notification(Method, Params), lager:debug("[SERVER] Sending notification [notification=~p]", [Notification]), gen_tcp:send(Socket, Notification). - -%% TODO: definition/2 should probably not take the Uri, but a path. --spec definition(uri(), erlang_ls_poi:poi()) -> null | map(). -definition(_Uri, #{ info := {application, {M, _F, _A}} = Info }) -> - case annotated_tree(erlang_ls_uri:filename(M)) of - {ok, Uri, AnnotatedTree} -> - case erlang_ls_parser:find_poi_by_info(AnnotatedTree, definition(Info)) of - [#{ range := Range }] -> - %% TODO: Use API to create types - #{ uri => Uri - , range => erlang_ls_protocol:range(Range) - }; - [] -> - null - end; - {error, _Error} -> - null - end; -definition(Uri, #{ info := {application, {_F, _A}} = Info }) -> - case annotated_tree(erlang_ls_uri:basename(Uri)) of - {ok, Uri, AnnotatedTree} -> - case erlang_ls_parser:find_poi_by_info(AnnotatedTree, definition(Info)) of - [#{ range := Range }] -> - %% TODO: Use API to create types - #{ uri => Uri - , range => erlang_ls_protocol:range(Range) - }; - [] -> - null - end; - {error, _Error} -> - null - end; -definition(_Uri, #{ info := {behaviour, Behaviour} = Info }) -> - Filename = erlang_ls_uri:filename(Behaviour), - search(Filename, full_path(), definition(Info)); -%% TODO: Eventually search everywhere and suggest a code lens to include a file -definition(Uri, #{ info := {macro, _Define} = Info }) -> - Filename = erlang_ls_uri:basename(Uri), - search(Filename, app_path(), definition(Info)); -definition(Uri, #{ info := {record_expr, _Record} = Info }) -> - Filename = erlang_ls_uri:basename(Uri), - search(Filename, app_path(), Info); -definition(_Uri, #{ info := {include, Include0} }) -> - Include = list_to_binary(string:trim(Include0, both, [$"])), - case annotated_tree(Include) of - {ok, Uri, _AnnotatedTree} -> - #{ uri => Uri - %% TODO: We could point to the module attribute, instead - , range => erlang_ls_protocol:range(#{ from => {0, 0} - , to => {0, 0} - }) - }; - {error, _Error} -> - null - end; -definition(_Uri, #{ info := {include_lib, Include0} }) -> - Include = list_to_binary(lists:last(filename:split(string:trim(Include0, both, [$"])))), - case annotated_tree(Include) of - {ok, Uri, _AnnotatedTree} -> - #{ uri => Uri - %% TODO: We could point to the module attribute, instead - , range => erlang_ls_protocol:range(#{ from => {0, 0} - , to => {0, 0} - }) - }; - {error, _Error} -> - null - end; -definition(_Uri, _) -> - null. - --spec definition({atom(), any()}) -> {atom(), any()}. -definition({application, {_M, F, A}}) -> - {function, {F, A}}; -definition({application, {F, A}}) -> - {function, {F, A}}; -definition({behaviour, Behaviour}) -> - {module, Behaviour}; -definition({macro, Define}) -> - {define, Define}; -definition({record_expr, Record}) -> - {record, Record}. - --spec annotated_tree(binary()) -> - {ok, uri(), erlang_ls_tree:tree()} | {error, any()}. -annotated_tree(Filename) -> - annotated_tree(Filename, full_path()). - --spec annotated_tree(binary(), [string()]) -> - {ok, uri(), erlang_ls_tree:tree()} | {error, any()}. -annotated_tree(Filename, Path) -> - case file:path_open(Path, Filename, [read]) of - {ok, IoDevice, FullName} -> - %% TODO: Avoid opening file twice - file:close(IoDevice), - {ok, Tree} = erlang_ls_parser:parse_file(FullName), - Uri = erlang_ls_uri:uri(FullName), - {ok, Uri, erlang_ls_tree:annotate(Tree)}; - {error, Error} -> - {error, Error} - end. - --spec otp_path() -> [string()]. -otp_path() -> - filelib:wildcard(filename:join([?OTP_INCLUDE_PATH, "*/src"])). - --spec app_path() -> [string()]. -app_path() -> - [ filename:join([?TEST_APP_PATH, "src"]) - , filename:join([?TEST_APP_PATH, "include"]) - , filename:join([?ERLANG_LS_PATH, "src"]) - , filename:join([?TEST_APP_PATH, "include"]) - ]. - --spec deps_path() -> [string()]. -deps_path() -> - filelib:wildcard(filename:join([?DEPS_PATH, "*/src"])). - -full_path() -> - lists:append( [ app_path() , deps_path() , otp_path() ]). - -%% Look for a definition recursively in a file and its includes. --spec search(binary(), [string()], any()) -> null | map(). -search(Filename, Path, Thing) -> - case annotated_tree(Filename, Path) of - {ok, Uri, AnnotatedTree} -> - case find(Uri, AnnotatedTree, Thing) of - null -> - Includes = erlang_ls_parser:find_poi_by_info_key(AnnotatedTree, include), - IncludeLibs = erlang_ls_parser:find_poi_by_info_key(AnnotatedTree, include_lib), - search_in_includes(Includes ++ IncludeLibs, Thing); - Def -> - Def - end; - {error, _Error} -> - null - end. - -%% Look for a definition in a given tree --spec find(uri(), erlang_ls_tree:tree(), any()) -> null | map(). -find(Uri, AnnotatedTree, Thing) -> - case erlang_ls_parser:find_poi_by_info(AnnotatedTree, Thing) of - [#{ range := Range }|_] -> - #{ uri => Uri, range => erlang_ls_protocol:range(Range) }; - [] -> - null - end. - --spec search_in_includes([erlang_ls_poi:poi()], string()) -> null | map(). -search_in_includes([], _Thing) -> - null; -search_in_includes([#{info := Info}|T], Thing) -> - Include = normalize_include(Info), - case search(list_to_binary(Include), app_path(), Thing) of - null -> search_in_includes(T, Thing); - Def -> Def - end. - --spec normalize_include({atom(), string()}) -> string(). -normalize_include({include, Include}) -> - string:trim(Include, both, [$"]); -normalize_include({include_lib, Include}) -> - lists:last(filename:split(string:trim(Include, both, [$"]))). diff --git a/test/erlang_ls_code_navigation_SUITE.erl b/test/erlang_ls_code_navigation_SUITE.erl index c6cad79ee..70b056294 100644 --- a/test/erlang_ls_code_navigation_SUITE.erl +++ b/test/erlang_ls_code_navigation_SUITE.erl @@ -72,9 +72,9 @@ behaviour(Config) -> Thing = {behaviour, 'behaviour_a'}, Definition = definition(?config(data_dir_bin, Config), Thing), ?assertEqual( Definition - , erlang_ls_server:search( FileName - , ?config(include_path, Config) - , erlang_ls_server:definition(Thing))), + , erlang_ls_code_navigation:search( FileName + , ?config(include_path, Config) + , erlang_ls_code_navigation:definition(Thing))), ok. -spec macro(config()) -> ok. @@ -83,9 +83,9 @@ macro(Config) -> Thing = {macro, 'MACRO_A'}, Definition = definition(?config(data_dir_bin, Config), Thing), ?assertEqual( Definition - , erlang_ls_server:search( FileName - , ?config(include_path, Config) - , erlang_ls_server:definition(Thing))), + , erlang_ls_code_navigation:search( FileName + , ?config(include_path, Config) + , erlang_ls_code_navigation:definition(Thing))), ok. -spec record(config()) -> ok. @@ -94,9 +94,9 @@ record(Config) -> Thing = {record_expr, "record_a"}, Definition = definition(?config(data_dir_bin, Config), Thing), ?assertEqual( Definition - , erlang_ls_server:search( FileName - , ?config(include_path, Config) - , erlang_ls_server:definition(Thing))), + , erlang_ls_code_navigation:search( FileName + , ?config(include_path, Config) + , erlang_ls_code_navigation:definition(Thing))), ok. %%============================================================================== From 73a768298cde5e9e14475485eca2f0232dec830d Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Mon, 5 Aug 2019 19:00:58 +0200 Subject: [PATCH 15/22] [#22] Move annotate_file function to tree module --- src/erlang_ls_code_navigation.erl | 31 +++++-------------------------- src/erlang_ls_completion.erl | 1 - src/erlang_ls_tree.erl | 26 +++++++++++++++++++++++++- 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/src/erlang_ls_code_navigation.erl b/src/erlang_ls_code_navigation.erl index 49d5bbc89..1db53b74a 100644 --- a/src/erlang_ls_code_navigation.erl +++ b/src/erlang_ls_code_navigation.erl @@ -37,7 +37,7 @@ %% TODO: goto_definition/2 should probably not take the Uri, but a path. -spec goto_definition(uri(), erlang_ls_poi:poi()) -> null | map(). goto_definition(_Uri, #{ info := {application, {M, _F, _A}} = Info }) -> - case annotated_tree(erlang_ls_uri:filename(M)) of + case erlang_ls_tree:annotate_file(erlang_ls_uri:filename(M), full_path()) of {ok, Uri, AnnotatedTree} -> case erlang_ls_parser:find_poi_by_info(AnnotatedTree, definition(Info)) of [#{ range := Range }] -> @@ -52,7 +52,7 @@ goto_definition(_Uri, #{ info := {application, {M, _F, _A}} = Info }) -> null end; goto_definition(Uri, #{ info := {application, {_F, _A}} = Info }) -> - case annotated_tree(erlang_ls_uri:basename(Uri)) of + case erlang_ls_tree:annotate_file(erlang_ls_uri:basename(Uri), full_path()) of {ok, Uri, AnnotatedTree} -> case erlang_ls_parser:find_poi_by_info(AnnotatedTree, definition(Info)) of [#{ range := Range }] -> @@ -78,7 +78,7 @@ goto_definition(Uri, #{ info := {record_expr, _Record} = Info }) -> search(Filename, app_path(), Info); goto_definition(_Uri, #{ info := {include, Include0} }) -> Include = list_to_binary(string:trim(Include0, both, [$"])), - case annotated_tree(Include) of + case erlang_ls_tree:annotate_file(Include, full_path()) of {ok, Uri, _AnnotatedTree} -> #{ uri => Uri %% TODO: We could point to the module attribute, instead @@ -91,7 +91,7 @@ goto_definition(_Uri, #{ info := {include, Include0} }) -> end; goto_definition(_Uri, #{ info := {include_lib, Include0} }) -> Include = list_to_binary(lists:last(filename:split(string:trim(Include0, both, [$"])))), - case annotated_tree(Include) of + case erlang_ls_tree:annotate_file(Include, full_path()) of {ok, Uri, _AnnotatedTree} -> #{ uri => Uri %% TODO: We could point to the module attribute, instead @@ -105,26 +105,6 @@ goto_definition(_Uri, #{ info := {include_lib, Include0} }) -> goto_definition(_Uri, _) -> null. -%% TODO: Move to tree module --spec annotated_tree(binary()) -> - {ok, uri(), erlang_ls_tree:tree()} | {error, any()}. -annotated_tree(Filename) -> - annotated_tree(Filename, full_path()). - --spec annotated_tree(binary(), [string()]) -> - {ok, uri(), erlang_ls_tree:tree()} | {error, any()}. -annotated_tree(Filename, Path) -> - case file:path_open(Path, Filename, [read]) of - {ok, IoDevice, FullName} -> - %% TODO: Avoid opening file twice - file:close(IoDevice), - {ok, Tree} = erlang_ls_parser:parse_file(FullName), - Uri = erlang_ls_uri:uri(FullName), - {ok, Uri, erlang_ls_tree:annotate(Tree)}; - {error, Error} -> - {error, Error} - end. - -spec definition({atom(), any()}) -> {atom(), any()}. definition({application, {_M, F, A}}) -> {function, {F, A}}; @@ -157,11 +137,10 @@ deps_path() -> full_path() -> lists:append( [ app_path() , deps_path() , otp_path() ]). - %% Look for a definition recursively in a file and its includes. -spec search(binary(), [string()], any()) -> null | map(). search(Filename, Path, Thing) -> - case annotated_tree(Filename, Path) of + case erlang_ls_tree:annotate_file(Filename, Path) of {ok, Uri, AnnotatedTree} -> case find(Uri, AnnotatedTree, Thing) of null -> diff --git a/src/erlang_ls_completion.erl b/src/erlang_ls_completion.erl index 090187098..d4466413e 100644 --- a/src/erlang_ls_completion.erl +++ b/src/erlang_ls_completion.erl @@ -3,7 +3,6 @@ -export([ record_definitions/0 ]). -%% TODO: Store these in an ETS table -spec record_definitions() -> [atom()]. record_definitions() -> lists:usort(lists:flatten([record_definitions(B) || B <- all_beams()])). diff --git a/src/erlang_ls_tree.erl b/src/erlang_ls_tree.erl index dc6462279..92fed9220 100644 --- a/src/erlang_ls_tree.erl +++ b/src/erlang_ls_tree.erl @@ -7,17 +7,26 @@ %% Exports %%============================================================================== -export([ annotate/1 + , annotate_file/2 , annotate_node/1 , postorder_update/2 , points_of_interest/1 ]). +%%============================================================================== +%% Includes +%%============================================================================== +-include("erlang_ls.hrl"). + %%============================================================================== %% Types %%============================================================================== -type tree() :: erl_syntax:syntaxTree(). +-type annotated_tree() :: tree(). --export_type([ tree/0 ]). +-export_type([ annotated_tree/0 + , tree/0 + ]). %%============================================================================== %% API @@ -28,6 +37,21 @@ annotate(Tree) -> postorder_update(fun annotate_node/1, Tree). +-spec annotate_file(binary(), [string()]) -> + {ok, uri(), annotated_tree()} | {error, any()}. +annotate_file(Filename, Path) -> + case file:path_open(Path, Filename, [read]) of + {ok, IoDevice, FullName} -> + %% TODO: Avoid opening file twice + file:close(IoDevice), + {ok, Tree} = erlang_ls_parser:parse_file(FullName), + Uri = erlang_ls_uri:uri(FullName), + {ok, Uri, annotate(Tree)}; + {error, Error} -> + {error, Error} + end. + + %% @edoc Add an annotation to the root of the given `Tree` for each %% point of interest found. -spec annotate_node(tree()) -> tree(). From d4a05393b68b70f52d3aa7a6a97224f72f5aba22 Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Mon, 5 Aug 2019 19:21:46 +0200 Subject: [PATCH 16/22] [#22] Move list/match functions to poi module --- src/erlang_ls_buffer.erl | 2 +- src/erlang_ls_code_navigation.erl | 10 ++++---- src/erlang_ls_parser.erl | 37 +--------------------------- src/erlang_ls_poi.erl | 40 +++++++++++++++++++++++++++++-- 4 files changed, 45 insertions(+), 44 deletions(-) diff --git a/src/erlang_ls_buffer.erl b/src/erlang_ls_buffer.erl index c6a5e3330..fe668e974 100644 --- a/src/erlang_ls_buffer.erl +++ b/src/erlang_ls_buffer.erl @@ -145,7 +145,7 @@ do_get_element_at_pos(Text, Line, Column) -> %% TODO: Cache tree {ok, Tree} = erlang_ls_parser:parse(Text), AnnotatedTree = erlang_ls_tree:annotate(Tree), - erlang_ls_parser:find_poi_by_pos(AnnotatedTree, {Line, Column}). + erlang_ls_poi:match_pos(AnnotatedTree, {Line, Column}). -spec get_line_text(binary(), integer()) -> binary(). get_line_text(Text, Line) -> diff --git a/src/erlang_ls_code_navigation.erl b/src/erlang_ls_code_navigation.erl index 1db53b74a..2f8992976 100644 --- a/src/erlang_ls_code_navigation.erl +++ b/src/erlang_ls_code_navigation.erl @@ -39,7 +39,7 @@ goto_definition(_Uri, #{ info := {application, {M, _F, _A}} = Info }) -> case erlang_ls_tree:annotate_file(erlang_ls_uri:filename(M), full_path()) of {ok, Uri, AnnotatedTree} -> - case erlang_ls_parser:find_poi_by_info(AnnotatedTree, definition(Info)) of + case erlang_ls_poi:match(AnnotatedTree, definition(Info)) of [#{ range := Range }] -> %% TODO: Use API to create types #{ uri => Uri @@ -54,7 +54,7 @@ goto_definition(_Uri, #{ info := {application, {M, _F, _A}} = Info }) -> goto_definition(Uri, #{ info := {application, {_F, _A}} = Info }) -> case erlang_ls_tree:annotate_file(erlang_ls_uri:basename(Uri), full_path()) of {ok, Uri, AnnotatedTree} -> - case erlang_ls_parser:find_poi_by_info(AnnotatedTree, definition(Info)) of + case erlang_ls_poi:match(AnnotatedTree, definition(Info)) of [#{ range := Range }] -> %% TODO: Use API to create types #{ uri => Uri @@ -144,8 +144,8 @@ search(Filename, Path, Thing) -> {ok, Uri, AnnotatedTree} -> case find(Uri, AnnotatedTree, Thing) of null -> - Includes = erlang_ls_parser:find_poi_by_info_key(AnnotatedTree, include), - IncludeLibs = erlang_ls_parser:find_poi_by_info_key(AnnotatedTree, include_lib), + Includes = erlang_ls_poi:match_key(AnnotatedTree, include), + IncludeLibs = erlang_ls_poi:match_key(AnnotatedTree, include_lib), search_in_includes(Includes ++ IncludeLibs, Thing); Def -> Def @@ -157,7 +157,7 @@ search(Filename, Path, Thing) -> %% Look for a definition in a given tree -spec find(uri(), erlang_ls_tree:tree(), any()) -> null | map(). find(Uri, AnnotatedTree, Thing) -> - case erlang_ls_parser:find_poi_by_info(AnnotatedTree, Thing) of + case erlang_ls_poi:match(AnnotatedTree, Thing) of [#{ range := Range }|_] -> #{ uri => Uri, range => erlang_ls_protocol:range(Range) }; [] -> diff --git a/src/erlang_ls_parser.erl b/src/erlang_ls_parser.erl index 769f6d2d9..3699b3c6a 100644 --- a/src/erlang_ls_parser.erl +++ b/src/erlang_ls_parser.erl @@ -1,10 +1,6 @@ -module(erlang_ls_parser). --export([ find_poi_by_info/2 - , find_poi_by_info_key/2 - , find_poi_by_pos/2 - , list_poi/1 - , parse/1 +-export([ parse/1 , parse_file/1 ]). @@ -36,34 +32,3 @@ parse_file(Path) -> {error, Error} -> {error, Error} end. - --spec find_poi_by_info(erlang_ls_tree:tree(), any()) -> - [erlang_ls_poi:poi()]. -find_poi_by_info(Tree, Info0) -> - [POI || #{info := Info} = POI <- list_poi(Tree), Info0 =:= Info]. - -%% TODO: Rename --spec find_poi_by_info_key(erlang_ls_tree:tree(), atom()) -> - [erlang_ls_poi:poi()]. -find_poi_by_info_key(Tree, Key0) -> - [POI || #{info := {Key, _}} = POI <- list_poi(Tree), Key0 =:= Key]. - --spec find_poi_by_pos(erlang_ls_tree:tree(), erlang_ls_poi:pos()) -> - [erlang_ls_poi:poi()]. -find_poi_by_pos(Tree, Pos) -> - [POI || #{range := Range} = POI <- list_poi(Tree), matches_pos(Pos, Range)]. - --spec list_poi(erlang_ls_tree:tree()) -> [erlang_ls_poi:poi()]. -list_poi(Tree) -> - F = fun(T, Acc) -> - Annotations = erl_syntax:get_ann(T), - case [POI || #{ type := poi } = POI <- Annotations] of - [] -> Acc; - L -> L ++ Acc - end - end, - erl_syntax_lib:fold(F, [], Tree). - --spec matches_pos(erlang_ls_poi:pos(), erlang_ls_poi:range()) -> boolean(). -matches_pos(Pos, #{from := From, to := To}) -> - (From =< Pos) andalso (Pos =< To). diff --git a/src/erlang_ls_poi.erl b/src/erlang_ls_poi.erl index f9a393d55..362e983f1 100644 --- a/src/erlang_ls_poi.erl +++ b/src/erlang_ls_poi.erl @@ -13,13 +13,21 @@ }. -export_type([ poi/0 - , pos/0 , range/0 ]). - -export([ poi/2 ]). +-export([ list/1 + , match/2 + , match_key/2 + , match_pos/2 + ]). + +%%============================================================================== +%% API +%%============================================================================== + %% @edoc Constructor for a Point of Interest. -spec poi(erlang_ls_tree:tree(), any()) -> poi(). poi(Tree, Info) -> @@ -30,6 +38,30 @@ poi(Tree, Info) -> , range => Range }. +%% @edoc List the Points of Interest for a given tree. +-spec list(erlang_ls_tree:tree()) -> [poi()]. +list(Tree) -> + F = fun(T, Acc) -> + Annotations = erl_syntax:get_ann(T), + case [POI || #{ type := poi } = POI <- Annotations] of + [] -> Acc; + L -> L ++ Acc + end + end, + erl_syntax_lib:fold(F, [], Tree). + +-spec match(erlang_ls_tree:tree(), any()) -> [poi()]. +match(Tree, Info0) -> + [POI || #{info := Info} = POI <- list(Tree), Info0 =:= Info]. + +-spec match_key(erlang_ls_tree:tree(), atom()) -> [poi()]. +match_key(Tree, Key0) -> + [POI || #{info := {Key, _}} = POI <- list(Tree), Key0 =:= Key]. + +-spec match_pos(erlang_ls_tree:tree(), pos()) -> [poi()]. +match_pos(Tree, Pos) -> + [POI || #{range := Range} = POI <- list(Tree), matches_pos(Pos, Range)]. + %%============================================================================== %% Internal Functions %%============================================================================== @@ -90,3 +122,7 @@ get_range({_Line, _Column}, {spec, _Spec}) -> %% TODO: The location information for the arity qualifiers are lost during %% parsing in `epp_dodger`. This requires fixing. #{ from => {0, 0}, to => {0, 0} }. + +-spec matches_pos(pos(), range()) -> boolean(). +matches_pos(Pos, #{from := From, to := To}) -> + (From =< Pos) andalso (Pos =< To). From a346a6bd9a26504805859ad8f574965a32a9b909 Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Tue, 6 Aug 2019 10:11:02 +0200 Subject: [PATCH 17/22] [#22] Do not expose Uri to code navigation module --- src/erlang_ls_code_navigation.erl | 27 ++++++++++++--------------- src/erlang_ls_server.erl | 3 ++- src/erlang_ls_uri.erl | 9 ++------- 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/erlang_ls_code_navigation.erl b/src/erlang_ls_code_navigation.erl index 2f8992976..8dd3f959e 100644 --- a/src/erlang_ls_code_navigation.erl +++ b/src/erlang_ls_code_navigation.erl @@ -34,9 +34,8 @@ %%============================================================================== %% TODO: Specify type instead of generic map -%% TODO: goto_definition/2 should probably not take the Uri, but a path. --spec goto_definition(uri(), erlang_ls_poi:poi()) -> null | map(). -goto_definition(_Uri, #{ info := {application, {M, _F, _A}} = Info }) -> +-spec goto_definition(binary(), erlang_ls_poi:poi()) -> null | map(). +goto_definition(_Filename, #{ info := {application, {M, _F, _A}} = Info }) -> case erlang_ls_tree:annotate_file(erlang_ls_uri:filename(M), full_path()) of {ok, Uri, AnnotatedTree} -> case erlang_ls_poi:match(AnnotatedTree, definition(Info)) of @@ -51,8 +50,8 @@ goto_definition(_Uri, #{ info := {application, {M, _F, _A}} = Info }) -> {error, _Error} -> null end; -goto_definition(Uri, #{ info := {application, {_F, _A}} = Info }) -> - case erlang_ls_tree:annotate_file(erlang_ls_uri:basename(Uri), full_path()) of +goto_definition(Filename, #{ info := {application, {_F, _A}} = Info }) -> + case erlang_ls_tree:annotate_file(filename:basename(Filename), full_path()) of {ok, Uri, AnnotatedTree} -> case erlang_ls_poi:match(AnnotatedTree, definition(Info)) of [#{ range := Range }] -> @@ -66,17 +65,15 @@ goto_definition(Uri, #{ info := {application, {_F, _A}} = Info }) -> {error, _Error} -> null end; -goto_definition(_Uri, #{ info := {behaviour, Behaviour} = Info }) -> +goto_definition(_Filename, #{ info := {behaviour, Behaviour} = Info }) -> Filename = erlang_ls_uri:filename(Behaviour), search(Filename, full_path(), definition(Info)); %% TODO: Eventually search everywhere and suggest a code lens to include a file -goto_definition(Uri, #{ info := {macro, _Define} = Info }) -> - Filename = erlang_ls_uri:basename(Uri), - search(Filename, app_path(), definition(Info)); -goto_definition(Uri, #{ info := {record_expr, _Record} = Info }) -> - Filename = erlang_ls_uri:basename(Uri), - search(Filename, app_path(), Info); -goto_definition(_Uri, #{ info := {include, Include0} }) -> +goto_definition(Filename, #{ info := {macro, _Define} = Info }) -> + search(filename:basename(Filename), app_path(), definition(Info)); +goto_definition(Filename, #{ info := {record_expr, _Record} = Info }) -> + search(filename:basename(Filename), app_path(), Info); +goto_definition(_Filename, #{ info := {include, Include0} }) -> Include = list_to_binary(string:trim(Include0, both, [$"])), case erlang_ls_tree:annotate_file(Include, full_path()) of {ok, Uri, _AnnotatedTree} -> @@ -89,7 +86,7 @@ goto_definition(_Uri, #{ info := {include, Include0} }) -> {error, _Error} -> null end; -goto_definition(_Uri, #{ info := {include_lib, Include0} }) -> +goto_definition(_Filename, #{ info := {include_lib, Include0} }) -> Include = list_to_binary(lists:last(filename:split(string:trim(Include0, both, [$"])))), case erlang_ls_tree:annotate_file(Include, full_path()) of {ok, Uri, _AnnotatedTree} -> @@ -102,7 +99,7 @@ goto_definition(_Uri, #{ info := {include_lib, Include0} }) -> {error, _Error} -> null end; -goto_definition(_Uri, _) -> +goto_definition(_Filename, _) -> null. -spec definition({atom(), any()}) -> {atom(), any()}. diff --git a/src/erlang_ls_server.erl b/src/erlang_ls_server.erl index 037ce751c..ebcd80ed4 100644 --- a/src/erlang_ls_server.erl +++ b/src/erlang_ls_server.erl @@ -177,7 +177,8 @@ handle_method(<<"textDocument/definition">>, Params) -> {ok, Buffer} = erlang_ls_buffer_server:get_buffer(Uri), case erlang_ls_buffer:get_element_at_pos(Buffer, Line + 1, Character + 1) of [POI|_] -> - {response, erlang_ls_code_navigation:goto_definition(Uri, POI)}; + Filename = erlang_ls_uri:path(Uri), + {response, erlang_ls_code_navigation:goto_definition(Filename, POI)}; [] -> {response, null} end; diff --git a/src/erlang_ls_uri.erl b/src/erlang_ls_uri.erl index 9f68412d0..4cb4f15e8 100644 --- a/src/erlang_ls_uri.erl +++ b/src/erlang_ls_uri.erl @@ -8,8 +8,7 @@ %%============================================================================== %% Exports %%============================================================================== --export([ basename/1 - , filename/1 +-export([ filename/1 , module/1 , path/1 , uri/1 @@ -20,17 +19,13 @@ %%============================================================================== -include("erlang_ls.hrl"). --spec basename(uri()) -> binary(). -basename(Uri) -> - filename:basename(path(Uri)). - -spec filename(atom()) -> binary(). filename(Module) -> list_to_binary(atom_to_list(Module) ++ ".erl"). -spec module(uri()) -> atom(). module(Uri) -> - binary_to_atom(filename:basename(basename(Uri), <<".erl">>), utf8). + binary_to_atom(filename:basename(path(Uri), <<".erl">>), utf8). -spec path(uri()) -> uri_path(). path(<<"file://", Path/binary>>) -> From 32883be84cbdf65c72cb29e2f5d4003c2403f82c Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Tue, 6 Aug 2019 10:16:07 +0200 Subject: [PATCH 18/22] [#22] Do not return Uri from tree module --- src/erlang_ls_code_navigation.erl | 19 ++++++++++--------- src/erlang_ls_tree.erl | 5 ++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/erlang_ls_code_navigation.erl b/src/erlang_ls_code_navigation.erl index 8dd3f959e..851048c2e 100644 --- a/src/erlang_ls_code_navigation.erl +++ b/src/erlang_ls_code_navigation.erl @@ -37,11 +37,11 @@ -spec goto_definition(binary(), erlang_ls_poi:poi()) -> null | map(). goto_definition(_Filename, #{ info := {application, {M, _F, _A}} = Info }) -> case erlang_ls_tree:annotate_file(erlang_ls_uri:filename(M), full_path()) of - {ok, Uri, AnnotatedTree} -> + {ok, FullName, AnnotatedTree} -> case erlang_ls_poi:match(AnnotatedTree, definition(Info)) of [#{ range := Range }] -> %% TODO: Use API to create types - #{ uri => Uri + #{ uri => erlang_ls_uri:uri(FullName) , range => erlang_ls_protocol:range(Range) }; [] -> @@ -52,11 +52,11 @@ goto_definition(_Filename, #{ info := {application, {M, _F, _A}} = Info }) -> end; goto_definition(Filename, #{ info := {application, {_F, _A}} = Info }) -> case erlang_ls_tree:annotate_file(filename:basename(Filename), full_path()) of - {ok, Uri, AnnotatedTree} -> + {ok, FullName, AnnotatedTree} -> case erlang_ls_poi:match(AnnotatedTree, definition(Info)) of [#{ range := Range }] -> %% TODO: Use API to create types - #{ uri => Uri + #{ uri => erlang_ls_uri:uri(FullName) , range => erlang_ls_protocol:range(Range) }; [] -> @@ -76,8 +76,8 @@ goto_definition(Filename, #{ info := {record_expr, _Record} = Info }) -> goto_definition(_Filename, #{ info := {include, Include0} }) -> Include = list_to_binary(string:trim(Include0, both, [$"])), case erlang_ls_tree:annotate_file(Include, full_path()) of - {ok, Uri, _AnnotatedTree} -> - #{ uri => Uri + {ok, FullName, _AnnotatedTree} -> + #{ uri => erlang_ls_uri:uri(FullName) %% TODO: We could point to the module attribute, instead , range => erlang_ls_protocol:range(#{ from => {0, 0} , to => {0, 0} @@ -89,8 +89,8 @@ goto_definition(_Filename, #{ info := {include, Include0} }) -> goto_definition(_Filename, #{ info := {include_lib, Include0} }) -> Include = list_to_binary(lists:last(filename:split(string:trim(Include0, both, [$"])))), case erlang_ls_tree:annotate_file(Include, full_path()) of - {ok, Uri, _AnnotatedTree} -> - #{ uri => Uri + {ok, FullName, _AnnotatedTree} -> + #{ uri => erlang_ls_uri:uri(FullName) %% TODO: We could point to the module attribute, instead , range => erlang_ls_protocol:range(#{ from => {0, 0} , to => {0, 0} @@ -138,7 +138,8 @@ full_path() -> -spec search(binary(), [string()], any()) -> null | map(). search(Filename, Path, Thing) -> case erlang_ls_tree:annotate_file(Filename, Path) of - {ok, Uri, AnnotatedTree} -> + {ok, FullName, AnnotatedTree} -> + Uri = erlang_ls_uri:uri(FullName), case find(Uri, AnnotatedTree, Thing) of null -> Includes = erlang_ls_poi:match_key(AnnotatedTree, include), diff --git a/src/erlang_ls_tree.erl b/src/erlang_ls_tree.erl index 92fed9220..9ce9a74c8 100644 --- a/src/erlang_ls_tree.erl +++ b/src/erlang_ls_tree.erl @@ -38,15 +38,14 @@ annotate(Tree) -> postorder_update(fun annotate_node/1, Tree). -spec annotate_file(binary(), [string()]) -> - {ok, uri(), annotated_tree()} | {error, any()}. + {ok, binary(), annotated_tree()} | {error, any()}. annotate_file(Filename, Path) -> case file:path_open(Path, Filename, [read]) of {ok, IoDevice, FullName} -> %% TODO: Avoid opening file twice file:close(IoDevice), {ok, Tree} = erlang_ls_parser:parse_file(FullName), - Uri = erlang_ls_uri:uri(FullName), - {ok, Uri, annotate(Tree)}; + {ok, FullName, annotate(Tree)}; {error, Error} -> {error, Error} end. From 54cca9bc075ddadbc5db63438513473975b6f5a1 Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Tue, 6 Aug 2019 11:11:23 +0200 Subject: [PATCH 19/22] [#22] Refactor navigation API --- src/erlang_ls_code_navigation.erl | 136 +++++++++++------------ src/erlang_ls_server.erl | 9 +- src/erlang_ls_uri.erl | 7 +- test/erlang_ls_code_navigation_SUITE.erl | 67 +++++------ 4 files changed, 103 insertions(+), 116 deletions(-) diff --git a/src/erlang_ls_code_navigation.erl b/src/erlang_ls_code_navigation.erl index 851048c2e..a641a2b39 100644 --- a/src/erlang_ls_code_navigation.erl +++ b/src/erlang_ls_code_navigation.erl @@ -8,11 +8,8 @@ %%============================================================================== %% API --export([ goto_definition/2 ]). - -%% TODO: Refactor API --export([ definition/1 - , search/3 +-export([ goto_definition/2 + , goto_definition/3 ]). %%============================================================================== @@ -33,74 +30,66 @@ %% API %%============================================================================== -%% TODO: Specify type instead of generic map --spec goto_definition(binary(), erlang_ls_poi:poi()) -> null | map(). -goto_definition(_Filename, #{ info := {application, {M, _F, _A}} = Info }) -> - case erlang_ls_tree:annotate_file(erlang_ls_uri:filename(M), full_path()) of +-spec goto_definition(binary(), erlang_ls_poi:poi()) -> + {ok, binary(), erlang_ls_poi:range()} | {error, any()}. +goto_definition(Filename, POI) -> + goto_definition(Filename, POI, full_path()). + +-spec goto_definition(binary(), erlang_ls_poi:poi(), [string()]) -> + {ok, binary(), erlang_ls_poi:range()} | {error, any()}. +goto_definition( _Filename + , #{ info := {application, {M, _F, _A}} = Info } + , Path) -> + case erlang_ls_tree:annotate_file(filename(M), Path) of {ok, FullName, AnnotatedTree} -> case erlang_ls_poi:match(AnnotatedTree, definition(Info)) of [#{ range := Range }] -> - %% TODO: Use API to create types - #{ uri => erlang_ls_uri:uri(FullName) - , range => erlang_ls_protocol:range(Range) - }; + {ok, FullName, Range}; [] -> - null + {error, not_found} end; - {error, _Error} -> - null + {error, Error} -> + {error, Error} end; -goto_definition(Filename, #{ info := {application, {_F, _A}} = Info }) -> - case erlang_ls_tree:annotate_file(filename:basename(Filename), full_path()) of +goto_definition( Filename + , #{ info := {application, {_F, _A}} = Info } + , Path) -> + case erlang_ls_tree:annotate_file(filename:basename(Filename), Path) of {ok, FullName, AnnotatedTree} -> case erlang_ls_poi:match(AnnotatedTree, definition(Info)) of [#{ range := Range }] -> - %% TODO: Use API to create types - #{ uri => erlang_ls_uri:uri(FullName) - , range => erlang_ls_protocol:range(Range) - }; + {ok, FullName, Range}; [] -> - null + {error, not_found} end; - {error, _Error} -> - null + {error, Error} -> + {error, Error} end; -goto_definition(_Filename, #{ info := {behaviour, Behaviour} = Info }) -> - Filename = erlang_ls_uri:filename(Behaviour), - search(Filename, full_path(), definition(Info)); +goto_definition(_Filename, #{ info := {behaviour, Behaviour} = Info }, Path) -> + search(filename(Behaviour), Path, definition(Info)); %% TODO: Eventually search everywhere and suggest a code lens to include a file -goto_definition(Filename, #{ info := {macro, _Define} = Info }) -> - search(filename:basename(Filename), app_path(), definition(Info)); -goto_definition(Filename, #{ info := {record_expr, _Record} = Info }) -> - search(filename:basename(Filename), app_path(), Info); -goto_definition(_Filename, #{ info := {include, Include0} }) -> +goto_definition(Filename, #{ info := {macro, _Define} = Info }, Path) -> + search(filename:basename(Filename), Path, definition(Info)); +goto_definition(Filename, #{ info := {record_expr, _Record} = Info }, Path) -> + search(filename:basename(Filename), Path, definition(Info)); +goto_definition(_Filename, #{ info := {include, Include0} }, Path) -> Include = list_to_binary(string:trim(Include0, both, [$"])), - case erlang_ls_tree:annotate_file(Include, full_path()) of + case erlang_ls_tree:annotate_file(Include, Path) of {ok, FullName, _AnnotatedTree} -> - #{ uri => erlang_ls_uri:uri(FullName) - %% TODO: We could point to the module attribute, instead - , range => erlang_ls_protocol:range(#{ from => {0, 0} - , to => {0, 0} - }) - }; - {error, _Error} -> - null + {ok, FullName, #{ from => {0, 0}, to => {0, 0} }}; + {error, Error} -> + {error, Error} end; -goto_definition(_Filename, #{ info := {include_lib, Include0} }) -> +goto_definition(_Filename, #{ info := {include_lib, Include0} }, Path) -> Include = list_to_binary(lists:last(filename:split(string:trim(Include0, both, [$"])))), - case erlang_ls_tree:annotate_file(Include, full_path()) of + case erlang_ls_tree:annotate_file(Include, Path) of {ok, FullName, _AnnotatedTree} -> - #{ uri => erlang_ls_uri:uri(FullName) - %% TODO: We could point to the module attribute, instead - , range => erlang_ls_protocol:range(#{ from => {0, 0} - , to => {0, 0} - }) - }; - {error, _Error} -> - null + {ok, FullName, #{ from => {0, 0}, to => {0, 0} }}; + {error, Error} -> + {error, Error} end; -goto_definition(_Filename, _) -> - null. +goto_definition(_Filename, _, _Path) -> + {error, not_found}. -spec definition({atom(), any()}) -> {atom(), any()}. definition({application, {_M, F, A}}) -> @@ -114,7 +103,6 @@ definition({macro, Define}) -> definition({record_expr, Record}) -> {record, Record}. - -spec otp_path() -> [string()]. otp_path() -> filelib:wildcard(filename:join([?OTP_INCLUDE_PATH, "*/src"])). @@ -135,41 +123,43 @@ full_path() -> lists:append( [ app_path() , deps_path() , otp_path() ]). %% Look for a definition recursively in a file and its includes. --spec search(binary(), [string()], any()) -> null | map(). +-spec search(binary(), [string()], any()) -> + {ok, binary(), erlang_ls_poi:range()} | {error, any()}. search(Filename, Path, Thing) -> case erlang_ls_tree:annotate_file(Filename, Path) of {ok, FullName, AnnotatedTree} -> - Uri = erlang_ls_uri:uri(FullName), - case find(Uri, AnnotatedTree, Thing) of - null -> + case find(AnnotatedTree, Thing) of + {error, not_found} -> Includes = erlang_ls_poi:match_key(AnnotatedTree, include), IncludeLibs = erlang_ls_poi:match_key(AnnotatedTree, include_lib), search_in_includes(Includes ++ IncludeLibs, Thing); - Def -> - Def + {ok, Range} -> + {ok, FullName, Range} end; - {error, _Error} -> - null + {error, Error} -> + {error, Error} end. %% Look for a definition in a given tree --spec find(uri(), erlang_ls_tree:tree(), any()) -> null | map(). -find(Uri, AnnotatedTree, Thing) -> +-spec find(erlang_ls_tree:tree(), any()) -> + {ok, erlang_ls_poi:range()} | {error, any()}. +find(AnnotatedTree, Thing) -> case erlang_ls_poi:match(AnnotatedTree, Thing) of [#{ range := Range }|_] -> - #{ uri => Uri, range => erlang_ls_protocol:range(Range) }; + {ok, Range}; [] -> - null + {error, not_found} end. --spec search_in_includes([erlang_ls_poi:poi()], string()) -> null | map(). +-spec search_in_includes([erlang_ls_poi:poi()], string()) -> + {ok, binary(), erlang_ls_poi:range()} | {error, any()}. search_in_includes([], _Thing) -> - null; + {error, not_found}; search_in_includes([#{info := Info}|T], Thing) -> Include = normalize_include(Info), case search(list_to_binary(Include), app_path(), Thing) of - null -> search_in_includes(T, Thing); - Def -> Def + {error, not_found} -> search_in_includes(T, Thing); + {ok, FullName, Range} -> {ok, FullName, Range} end. -spec normalize_include({atom(), string()}) -> string(). @@ -177,3 +167,7 @@ normalize_include({include, Include}) -> string:trim(Include, both, [$"]); normalize_include({include_lib, Include}) -> lists:last(filename:split(string:trim(Include, both, [$"]))). + +-spec filename(atom()) -> binary(). +filename(Module) -> + list_to_binary(atom_to_list(Module) ++ ".erl"). diff --git a/src/erlang_ls_server.erl b/src/erlang_ls_server.erl index ebcd80ed4..40d58cddf 100644 --- a/src/erlang_ls_server.erl +++ b/src/erlang_ls_server.erl @@ -178,7 +178,14 @@ handle_method(<<"textDocument/definition">>, Params) -> case erlang_ls_buffer:get_element_at_pos(Buffer, Line + 1, Character + 1) of [POI|_] -> Filename = erlang_ls_uri:path(Uri), - {response, erlang_ls_code_navigation:goto_definition(Filename, POI)}; + case erlang_ls_code_navigation:goto_definition(Filename, POI) of + {error, _Error} -> + {response, null}; + {ok, FullName, Range} -> + {response, #{ uri => erlang_ls_uri:uri(FullName) + , range => erlang_ls_protocol:range(Range) + }} + end; [] -> {response, null} end; diff --git a/src/erlang_ls_uri.erl b/src/erlang_ls_uri.erl index 4cb4f15e8..c444ab00f 100644 --- a/src/erlang_ls_uri.erl +++ b/src/erlang_ls_uri.erl @@ -8,8 +8,7 @@ %%============================================================================== %% Exports %%============================================================================== --export([ filename/1 - , module/1 +-export([ module/1 , path/1 , uri/1 ]). @@ -19,10 +18,6 @@ %%============================================================================== -include("erlang_ls.hrl"). --spec filename(atom()) -> binary(). -filename(Module) -> - list_to_binary(atom_to_list(Module) ++ ".erl"). - -spec module(uri()) -> atom(). module(Uri) -> binary_to_atom(filename:basename(path(Uri), <<".erl">>), utf8). diff --git a/test/erlang_ls_code_navigation_SUITE.erl b/test/erlang_ls_code_navigation_SUITE.erl index 70b056294..164a15789 100644 --- a/test/erlang_ls_code_navigation_SUITE.erl +++ b/test/erlang_ls_code_navigation_SUITE.erl @@ -65,59 +65,50 @@ all() -> %%============================================================================== %% Testcases %%============================================================================== -%% TODO: Refactor API -spec behaviour(config()) -> ok. behaviour(Config) -> - FileName = <<"behaviour_a.erl">>, - Thing = {behaviour, 'behaviour_a'}, - Definition = definition(?config(data_dir_bin, Config), Thing), - ?assertEqual( Definition - , erlang_ls_code_navigation:search( FileName - , ?config(include_path, Config) - , erlang_ls_code_navigation:definition(Thing))), + FileName = <<"behaviour_a.erl">>, + Thing = #{ info => {behaviour, 'behaviour_a'} }, + Path = ?config(include_path, Config), + {ok, FullName, Range} = goto_definition(FileName, Thing, Path), + ?assertEqual(full_path(src, FileName, Config), FullName), + ?assertEqual(#{from => {0, 1}, to => {0, 1}}, Range), ok. -spec macro(config()) -> ok. macro(Config) -> - FileName = <<"code_navigation.erl">>, - Thing = {macro, 'MACRO_A'}, - Definition = definition(?config(data_dir_bin, Config), Thing), - ?assertEqual( Definition - , erlang_ls_code_navigation:search( FileName - , ?config(include_path, Config) - , erlang_ls_code_navigation:definition(Thing))), + FileName = <<"code_navigation.erl">>, + Thing = #{ info => {macro, 'MACRO_A'} }, + Path = ?config(include_path, Config), + {ok, FullName, Range} = goto_definition(FileName, Thing, Path), + ?assertEqual(full_path(src, FileName, Config), FullName), + ?assertEqual(#{from => {11, 0}, to => {11, 0}}, Range), ok. +%% TODO: Additional constructors for POI +%% TODO: Navigation should return POI, not range -spec record(config()) -> ok. record(Config) -> - FileName = <<"code_navigation.erl">>, - Thing = {record_expr, "record_a"}, - Definition = definition(?config(data_dir_bin, Config), Thing), - ?assertEqual( Definition - , erlang_ls_code_navigation:search( FileName - , ?config(include_path, Config) - , erlang_ls_code_navigation:definition(Thing))), + FileName = <<"code_navigation.erl">>, + Thing = #{ info => {record_expr, "record_a"} }, + Path = ?config(include_path, Config), + {ok, FullName, Range} = goto_definition(FileName, Thing, Path), + ?assertEqual(full_path(src, FileName, Config), FullName), + ?assertEqual(#{from => {9, 0}, to => {9, 0}}, Range), ok. %%============================================================================== %% Internal Functions %%============================================================================== -definition(DataDir, {behaviour, 'behaviour_a'}) -> - FilePath = filename:join([DataDir, <<"src">>, <<"behaviour_a.erl">>]), - #{ range => erlang_ls_protocol:range(#{from => {0, 1}, to => {0, 1}}) - , uri => erlang_ls_uri:uri(FilePath) - }; -definition(DataDir, {macro, 'MACRO_A'}) -> - FilePath = filename:join([DataDir, <<"src">>, <<"code_navigation.erl">>]), - #{ range => erlang_ls_protocol:range(#{from => {11, 0}, to => {11, 0}}) - , uri => erlang_ls_uri:uri(FilePath) - }; -definition(DataDir, {record_expr, "record_a"}) -> - FilePath = filename:join([DataDir, <<"src">>, <<"code_navigation.erl">>]), - #{ range => erlang_ls_protocol:range(#{from => {9, 0}, to => {9, 0}}) - , uri => erlang_ls_uri:uri(FilePath) - }. -%% TODO: API should be refactored. I should search for something, not the definition. +full_path(Dir, FileName, Config) -> + filename:join([ ?config(data_dir_bin, Config) + , atom_to_binary(Dir, utf8) + , FileName + ]). + +goto_definition(FileName, Thing, Path) -> + erlang_ls_code_navigation:goto_definition(FileName, Thing, Path). + %% TODO: include %% TODO: include_lib %% TODO: recursive record From 856fc79e7b69267eb33ca4fd290ef1abbfada11b Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Tue, 6 Aug 2019 15:12:07 +0200 Subject: [PATCH 20/22] [#22] Basic test for include navigation --- .../include/code_navigation.hrl | 3 + .../code_navigation}/src/behaviour_a.erl | 0 .../code_navigation}/src/code_navigation.erl | 2 + test/erlang_ls_code_navigation_SUITE.erl | 62 +++++++++++-------- 4 files changed, 42 insertions(+), 25 deletions(-) create mode 100644 priv/code_navigation/include/code_navigation.hrl rename {test/erlang_ls_code_navigation_SUITE_data => priv/code_navigation}/src/behaviour_a.erl (100%) rename {test/erlang_ls_code_navigation_SUITE_data => priv/code_navigation}/src/code_navigation.erl (89%) diff --git a/priv/code_navigation/include/code_navigation.hrl b/priv/code_navigation/include/code_navigation.hrl new file mode 100644 index 000000000..cd234d92b --- /dev/null +++ b/priv/code_navigation/include/code_navigation.hrl @@ -0,0 +1,3 @@ +-record(included_record_a, {field_a, field_b}). + +-define(INCLUDED_MACRO_A, included_macro_a). diff --git a/test/erlang_ls_code_navigation_SUITE_data/src/behaviour_a.erl b/priv/code_navigation/src/behaviour_a.erl similarity index 100% rename from test/erlang_ls_code_navigation_SUITE_data/src/behaviour_a.erl rename to priv/code_navigation/src/behaviour_a.erl diff --git a/test/erlang_ls_code_navigation_SUITE_data/src/code_navigation.erl b/priv/code_navigation/src/code_navigation.erl similarity index 89% rename from test/erlang_ls_code_navigation_SUITE_data/src/code_navigation.erl rename to priv/code_navigation/src/code_navigation.erl index fd9272a14..5c72d0fa8 100644 --- a/test/erlang_ls_code_navigation_SUITE_data/src/code_navigation.erl +++ b/priv/code_navigation/src/code_navigation.erl @@ -7,6 +7,8 @@ %% behaviour_a callbacks -export([ callback_a/0 ]). +-include("code_navigation.hrl"). + -record(record_a, {field_a, field_b}). -define(MACRO_A, macro_a). diff --git a/test/erlang_ls_code_navigation_SUITE.erl b/test/erlang_ls_code_navigation_SUITE.erl index 164a15789..c2e0c48a0 100644 --- a/test/erlang_ls_code_navigation_SUITE.erl +++ b/test/erlang_ls_code_navigation_SUITE.erl @@ -14,6 +14,7 @@ %% Test cases -export([ behaviour/1 + , include/1 , macro/1 , record/1 ]). @@ -24,6 +25,11 @@ -include_lib("common_test/include/ct.hrl"). -include_lib("stdlib/include/assert.hrl"). +%%============================================================================== +%% Defines +%%============================================================================== +-define(TEST_APP, <<"code_navigation">>). + %%============================================================================== %% Types %%============================================================================== @@ -38,11 +44,11 @@ suite() -> -spec init_per_suite(config()) -> config(). init_per_suite(Config) -> - DataDir = ?config(data_dir, Config), - DataDirBin = list_to_binary(DataDir), - [ {data_dir_bin, DataDirBin} - , {include_path, [ filename:join([DataDirBin, "src"]) - , filename:join([DataDirBin, "include"]) + RootDir = code:priv_dir(erlang_ls), + AppDir = filename:join([list_to_binary(RootDir), ?TEST_APP]), + [ {app_dir, AppDir} + , {include_path, [ filename:join([AppDir, "src"]) + , filename:join([AppDir, "include"]) ]} |Config]. @@ -60,53 +66,59 @@ end_per_testcase(_TestCase, _Config) -> -spec all() -> [atom()]. all() -> - [behaviour, macro, record]. + [behaviour, include, macro, record]. %%============================================================================== %% Testcases %%============================================================================== -spec behaviour(config()) -> ok. behaviour(Config) -> - FileName = <<"behaviour_a.erl">>, - Thing = #{ info => {behaviour, 'behaviour_a'} }, - Path = ?config(include_path, Config), - {ok, FullName, Range} = goto_definition(FileName, Thing, Path), - ?assertEqual(full_path(src, FileName, Config), FullName), + Thing = #{ info => {behaviour, 'behaviour_a'} }, + Path = ?config(include_path, Config), + {ok, FullName, Range} = goto_def(<<"code_navigation.erl">>, Thing, Path), + ?assertEqual(full_path(src, <<"behaviour_a.erl">>, Config), FullName), ?assertEqual(#{from => {0, 1}, to => {0, 1}}, Range), ok. +-spec include(config()) -> ok. +include(Config) -> + Thing = #{ info => {include, "code_navigation.hrl"} }, + Path = ?config(include_path, Config), + {ok, FullName, Range} = goto_def(<<"code_navigation.erl">>, Thing, Path), + ?assertEqual(full_path(include, <<"code_navigation.hrl">>, Config), FullName), + ?assertEqual(#{from => {0, 0}, to => {0, 0}}, Range), + ok. + -spec macro(config()) -> ok. macro(Config) -> - FileName = <<"code_navigation.erl">>, - Thing = #{ info => {macro, 'MACRO_A'} }, - Path = ?config(include_path, Config), - {ok, FullName, Range} = goto_definition(FileName, Thing, Path), - ?assertEqual(full_path(src, FileName, Config), FullName), - ?assertEqual(#{from => {11, 0}, to => {11, 0}}, Range), + Thing = #{ info => {macro, 'MACRO_A'} }, + Path = ?config(include_path, Config), + {ok, FullName, Range} = goto_def(<<"code_navigation.erl">>, Thing, Path), + ?assertEqual(full_path(src, <<"code_navigation.erl">>, Config), FullName), + ?assertEqual(#{from => {13, 0}, to => {13, 0}}, Range), ok. %% TODO: Additional constructors for POI %% TODO: Navigation should return POI, not range -spec record(config()) -> ok. record(Config) -> - FileName = <<"code_navigation.erl">>, - Thing = #{ info => {record_expr, "record_a"} }, - Path = ?config(include_path, Config), - {ok, FullName, Range} = goto_definition(FileName, Thing, Path), - ?assertEqual(full_path(src, FileName, Config), FullName), - ?assertEqual(#{from => {9, 0}, to => {9, 0}}, Range), + Thing = #{ info => {record_expr, "record_a"} }, + Path = ?config(include_path, Config), + {ok, FullName, Range} = goto_def(<<"code_navigation.erl">>, Thing, Path), + ?assertEqual(full_path(src, <<"code_navigation.erl">>, Config), FullName), + ?assertEqual(#{from => {11, 0}, to => {11, 0}}, Range), ok. %%============================================================================== %% Internal Functions %%============================================================================== full_path(Dir, FileName, Config) -> - filename:join([ ?config(data_dir_bin, Config) + filename:join([ ?config(app_dir, Config) , atom_to_binary(Dir, utf8) , FileName ]). -goto_definition(FileName, Thing, Path) -> +goto_def(FileName, Thing, Path) -> erlang_ls_code_navigation:goto_definition(FileName, Thing, Path). %% TODO: include From ae6304d957b9342c7ab695e2c17a329df5379515 Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Tue, 6 Aug 2019 15:27:36 +0200 Subject: [PATCH 21/22] [#22] Add tests for code navigation --- priv/code_navigation/src/code_navigation.erl | 5 ++ .../src/code_navigation_extra.erl | 6 ++ src/erlang_ls_code_navigation.erl | 12 +-- test/erlang_ls_code_navigation_SUITE.erl | 86 +++++++++++++++---- 4 files changed, 87 insertions(+), 22 deletions(-) create mode 100644 priv/code_navigation/src/code_navigation_extra.erl diff --git a/priv/code_navigation/src/code_navigation.erl b/priv/code_navigation/src/code_navigation.erl index 5c72d0fa8..474ad168e 100644 --- a/priv/code_navigation/src/code_navigation.erl +++ b/priv/code_navigation/src/code_navigation.erl @@ -8,6 +8,7 @@ -export([ callback_a/0 ]). -include("code_navigation.hrl"). +-include_lib("code_navigation/include/code_navigation.hrl"). -record(record_a, {field_a, field_b}). @@ -22,3 +23,7 @@ function_b() -> callback_a() -> ok. + +function_c() -> + code_navigation_extra:do(test), + length([1, 2, 3]). diff --git a/priv/code_navigation/src/code_navigation_extra.erl b/priv/code_navigation/src/code_navigation_extra.erl new file mode 100644 index 000000000..724e62e14 --- /dev/null +++ b/priv/code_navigation/src/code_navigation_extra.erl @@ -0,0 +1,6 @@ +-module(code_navigation_extra). + +-export([ do/1 ]). + +do(_Config) -> + ok. diff --git a/src/erlang_ls_code_navigation.erl b/src/erlang_ls_code_navigation.erl index a641a2b39..b7017b03e 100644 --- a/src/erlang_ls_code_navigation.erl +++ b/src/erlang_ls_code_navigation.erl @@ -132,7 +132,7 @@ search(Filename, Path, Thing) -> {error, not_found} -> Includes = erlang_ls_poi:match_key(AnnotatedTree, include), IncludeLibs = erlang_ls_poi:match_key(AnnotatedTree, include_lib), - search_in_includes(Includes ++ IncludeLibs, Thing); + search_in_includes(Includes ++ IncludeLibs, Path, Thing); {ok, Range} -> {ok, FullName, Range} end; @@ -151,14 +151,14 @@ find(AnnotatedTree, Thing) -> {error, not_found} end. --spec search_in_includes([erlang_ls_poi:poi()], string()) -> +-spec search_in_includes([erlang_ls_poi:poi()], [string()], string()) -> {ok, binary(), erlang_ls_poi:range()} | {error, any()}. -search_in_includes([], _Thing) -> +search_in_includes([], _Path, _Thing) -> {error, not_found}; -search_in_includes([#{info := Info}|T], Thing) -> +search_in_includes([#{info := Info}|T], Path, Thing) -> Include = normalize_include(Info), - case search(list_to_binary(Include), app_path(), Thing) of - {error, not_found} -> search_in_includes(T, Thing); + case search(list_to_binary(Include), Path, Thing) of + {error, _Error} -> search_in_includes(T, Path, Thing); {ok, FullName, Range} -> {ok, FullName, Range} end. diff --git a/test/erlang_ls_code_navigation_SUITE.erl b/test/erlang_ls_code_navigation_SUITE.erl index c2e0c48a0..8fd401fcf 100644 --- a/test/erlang_ls_code_navigation_SUITE.erl +++ b/test/erlang_ls_code_navigation_SUITE.erl @@ -13,10 +13,15 @@ ]). %% Test cases --export([ behaviour/1 +-export([ application_local/1 + , application_remote/1 + , behaviour/1 , include/1 + , include_lib/1 , macro/1 + , macro_included/1 , record/1 + , record_included/1 ]). %%============================================================================== @@ -66,14 +71,41 @@ end_per_testcase(_TestCase, _Config) -> -spec all() -> [atom()]. all() -> - [behaviour, include, macro, record]. + [ application_local + , application_remote + , behaviour + , include + , include_lib + , macro + , macro_included + , record + , record_included + ]. %%============================================================================== %% Testcases %%============================================================================== +-spec application_local(config()) -> ok. +application_local(Config) -> + Thing = #{info => {application, {function_b, 0}}}, + Path = ?config(include_path, Config), + {ok, FullName, Range} = goto_def(<<"code_navigation.erl">>, Thing, Path), + ?assertEqual(full_path(src, <<"code_navigation.erl">>, Config), FullName), + ?assertEqual(#{from => {20, 0}, to => {20, 10}}, Range), + ok. + +-spec application_remote(config()) -> ok. +application_remote(Config) -> + Thing = #{info => {application, {code_navigation_extra, do, 1}}}, + Path = ?config(include_path, Config), + {ok, FullName, Range} = goto_def(<<"code_navigation.erl">>, Thing, Path), + ?assertEqual(full_path(src, <<"code_navigation_extra.erl">>, Config), FullName), + ?assertEqual(#{from => {4, 0}, to => {4, 2}}, Range), + ok. + -spec behaviour(config()) -> ok. behaviour(Config) -> - Thing = #{ info => {behaviour, 'behaviour_a'} }, + Thing = #{info => {behaviour, 'behaviour_a'}}, Path = ?config(include_path, Config), {ok, FullName, Range} = goto_def(<<"code_navigation.erl">>, Thing, Path), ?assertEqual(full_path(src, <<"behaviour_a.erl">>, Config), FullName), @@ -82,7 +114,16 @@ behaviour(Config) -> -spec include(config()) -> ok. include(Config) -> - Thing = #{ info => {include, "code_navigation.hrl"} }, + Thing = #{info => {include, "code_navigation.hrl"}}, + Path = ?config(include_path, Config), + {ok, FullName, Range} = goto_def(<<"code_navigation.erl">>, Thing, Path), + ?assertEqual(full_path(include, <<"code_navigation.hrl">>, Config), FullName), + ?assertEqual(#{from => {0, 0}, to => {0, 0}}, Range), + ok. + +-spec include_lib(config()) -> ok. +include_lib(Config) -> + Thing = #{info => {include_lib, "code_navigation/include/code_navigation.hrl"}}, Path = ?config(include_path, Config), {ok, FullName, Range} = goto_def(<<"code_navigation.erl">>, Thing, Path), ?assertEqual(full_path(include, <<"code_navigation.hrl">>, Config), FullName), @@ -91,40 +132,53 @@ include(Config) -> -spec macro(config()) -> ok. macro(Config) -> - Thing = #{ info => {macro, 'MACRO_A'} }, + Thing = #{info => {macro, 'MACRO_A'}}, Path = ?config(include_path, Config), {ok, FullName, Range} = goto_def(<<"code_navigation.erl">>, Thing, Path), ?assertEqual(full_path(src, <<"code_navigation.erl">>, Config), FullName), - ?assertEqual(#{from => {13, 0}, to => {13, 0}}, Range), + ?assertEqual(#{from => {14, 0}, to => {14, 0}}, Range), + ok. + +-spec macro_included(config()) -> ok. +macro_included(Config) -> + Thing = #{info => {macro, 'INCLUDED_MACRO_A'}}, + Path = ?config(include_path, Config), + {ok, FullName, Range} = goto_def(<<"code_navigation.erl">>, Thing, Path), + ?assertEqual(full_path(include, <<"code_navigation.hrl">>, Config), FullName), + ?assertEqual(#{from => {2, 0}, to => {2, 0}}, Range), ok. %% TODO: Additional constructors for POI %% TODO: Navigation should return POI, not range -spec record(config()) -> ok. record(Config) -> - Thing = #{ info => {record_expr, "record_a"} }, + Thing = #{info => {record_expr, "record_a"}}, Path = ?config(include_path, Config), {ok, FullName, Range} = goto_def(<<"code_navigation.erl">>, Thing, Path), ?assertEqual(full_path(src, <<"code_navigation.erl">>, Config), FullName), - ?assertEqual(#{from => {11, 0}, to => {11, 0}}, Range), + ?assertEqual(#{from => {12, 0}, to => {12, 0}}, Range), + ok. + +-spec record_included(config()) -> ok. +record_included(Config) -> + Thing = #{info => {record_expr, "included_record_a"}}, + Path = ?config(include_path, Config), + {ok, FullName, Range} = goto_def(<<"code_navigation.erl">>, Thing, Path), + ?assertEqual(full_path(include, <<"code_navigation.hrl">>, Config), FullName), + ?assertEqual(#{from => {0, 0}, to => {0, 0}}, Range), ok. %%============================================================================== %% Internal Functions %%============================================================================== +-spec full_path(binary(), binary(), config()) -> binary(). full_path(Dir, FileName, Config) -> filename:join([ ?config(app_dir, Config) , atom_to_binary(Dir, utf8) , FileName ]). +-spec goto_def(binary(), erlang_ls_poi:poi(), [string()]) -> + {ok, binary(), erlang_ls_poi:range()} | {error, any()}. goto_def(FileName, Thing, Path) -> erlang_ls_code_navigation:goto_definition(FileName, Thing, Path). - -%% TODO: include -%% TODO: include_lib -%% TODO: recursive record -%% TODO: recursive macro -%% TODO: local function -%% TODO: remote function -%% TODO: bif From 8abf59f24ebddaad9477d3c6e12365b0668facde Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Tue, 6 Aug 2019 15:57:57 +0200 Subject: [PATCH 22/22] [#22] Fix Dialyzer issue --- src/erlang_ls_code_navigation.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/erlang_ls_code_navigation.erl b/src/erlang_ls_code_navigation.erl index b7017b03e..ca8c7c0b5 100644 --- a/src/erlang_ls_code_navigation.erl +++ b/src/erlang_ls_code_navigation.erl @@ -151,7 +151,7 @@ find(AnnotatedTree, Thing) -> {error, not_found} end. --spec search_in_includes([erlang_ls_poi:poi()], [string()], string()) -> +-spec search_in_includes([erlang_ls_poi:poi()], [string()], any()) -> {ok, binary(), erlang_ls_poi:range()} | {error, any()}. search_in_includes([], _Path, _Thing) -> {error, not_found};