From ee55c48c96807ce73836ef3523d050e9d59d1493 Mon Sep 17 00:00:00 2001 From: Hakan Nilsson Date: Tue, 9 Jan 2024 15:35:55 +0100 Subject: [PATCH] Handle recursive deps for behaviours Fixes #1044 --- .../src/diagnostics_behaviour_recursive.erl | 12 +++++ .../diagnostics_behaviour_recursive_impl.erl | 5 ++ apps/els_lsp/src/els_compiler_diagnostics.erl | 3 +- apps/els_lsp/src/els_diagnostics_utils.erl | 54 +++++++++++++++++-- apps/els_lsp/test/els_diagnostics_SUITE.erl | 11 ++++ 5 files changed, 80 insertions(+), 5 deletions(-) create mode 100644 apps/els_lsp/priv/code_navigation/src/diagnostics_behaviour_recursive.erl create mode 100644 apps/els_lsp/priv/code_navigation/src/diagnostics_behaviour_recursive_impl.erl diff --git a/apps/els_lsp/priv/code_navigation/src/diagnostics_behaviour_recursive.erl b/apps/els_lsp/priv/code_navigation/src/diagnostics_behaviour_recursive.erl new file mode 100644 index 000000000..07bf3c5bc --- /dev/null +++ b/apps/els_lsp/priv/code_navigation/src/diagnostics_behaviour_recursive.erl @@ -0,0 +1,12 @@ +-module(diagnostics_behaviour_recursive). +-behaviour(diagnostics_behaviour). + +-export([one/0]). +-export([two/0]). + +-callback three() -> ok. + +one() -> + ok. +two() -> + ok. diff --git a/apps/els_lsp/priv/code_navigation/src/diagnostics_behaviour_recursive_impl.erl b/apps/els_lsp/priv/code_navigation/src/diagnostics_behaviour_recursive_impl.erl new file mode 100644 index 000000000..da92e2aad --- /dev/null +++ b/apps/els_lsp/priv/code_navigation/src/diagnostics_behaviour_recursive_impl.erl @@ -0,0 +1,5 @@ +-module(diagnostics_behaviour_recursive_impl). +-behaviour(diagnostics_behaviour_recursive). +-export([three/0]). +three() -> + ok. diff --git a/apps/els_lsp/src/els_compiler_diagnostics.erl b/apps/els_lsp/src/els_compiler_diagnostics.erl index 26c42d441..11ad22ea8 100644 --- a/apps/els_lsp/src/els_compiler_diagnostics.erl +++ b/apps/els_lsp/src/els_compiler_diagnostics.erl @@ -791,8 +791,7 @@ module_name_check(Path) -> %% @doc Load a dependency, return the old version of the code (if any), %% so it can be restored. -spec load_dependency(atom(), string()) -> - {{atom(), binary(), file:filename()}, [els_diagnostics:diagnostic()]} - | error. + {{atom(), binary(), file:filename()} | error, [els_diagnostics:diagnostic()]}. load_dependency(Module, IncludingPath) -> Old = code:get_object_code(Module), Diagnostics = diff --git a/apps/els_lsp/src/els_diagnostics_utils.erl b/apps/els_lsp/src/els_diagnostics_utils.erl index bd6d9039c..c1b49744d 100644 --- a/apps/els_lsp/src/els_diagnostics_utils.erl +++ b/apps/els_lsp/src/els_diagnostics_utils.erl @@ -21,7 +21,7 @@ -spec dependencies(uri()) -> [atom()]. dependencies(Uri) -> - dependencies([Uri], [], sets:new()). + lists:reverse(dependencies([Uri], [], sets:new())). -spec included_uris(els_dt_document:item()) -> [uri()]. included_uris(Document) -> @@ -108,6 +108,15 @@ dependencies([Uri | Uris], Acc, AlreadyProcessed) -> IncludedUris, AlreadyProcessed ), + BeUris = lists:usort( + lists:flatten( + [be_deps(Id) || #{id := Id} <- Behaviours] + ) + ), + FilteredBeUris = exclude_already_processed( + BeUris, + AlreadyProcessed + ), PTUris = lists:usort( lists:flatten( [pt_deps(Id) || #{id := Id} <- ParseTransforms] @@ -118,9 +127,10 @@ dependencies([Uri | Uris], Acc, AlreadyProcessed) -> AlreadyProcessed ), dependencies( - Uris ++ FilteredIncludedUris ++ FilteredPTUris, + Uris ++ FilteredIncludedUris ++ FilteredPTUris ++ FilteredBeUris, Acc ++ [Id || #{id := Id} <- Behaviours ++ ParseTransforms] ++ - [els_uri:module(FPTUri) || FPTUri <- FilteredPTUris], + [els_uri:module(FPTUri) || FPTUri <- FilteredPTUris] ++ + [els_uri:module(BeUri) || BeUri <- FilteredBeUris], sets:add_element(Uri, AlreadyProcessed) ); {error, _Error} -> @@ -150,6 +160,27 @@ pt_deps(Module) -> [] end. +-spec be_deps(atom()) -> [uri()]. +be_deps(Module) -> + case els_utils:find_module(Module) of + {ok, Uri} -> + case els_utils:lookup_document(Uri) of + {ok, Document} -> + Behaviours = els_dt_document:pois( + Document, + [ + behaviour + ] + ), + behaviours_to_uris(Behaviours); + {error, _Error} -> + [] + end; + {error, Error} -> + ?LOG_INFO("Find module failed [module=~p] [error=~p]", [Module, Error]), + [] + end. + -spec applications_to_uris([els_poi:poi()]) -> [uri()]. applications_to_uris(Applications) -> Modules = [M || #{id := {M, _F, _A}} <- Applications], @@ -167,6 +198,23 @@ applications_to_uris(Applications) -> end, lists:foldl(Fun, [], Modules). +-spec behaviours_to_uris([els_poi:poi()]) -> [uri()]. +behaviours_to_uris(Behaviours) -> + Modules = [M || #{id := M} <- Behaviours], + Fun = fun(M, Acc) -> + case els_utils:find_module(M) of + {ok, Uri} -> + [Uri | Acc]; + {error, Error} -> + ?LOG_INFO( + "Could not find module [module=~p] [error=~p]", + [M, Error] + ), + Acc + end + end, + lists:foldl(Fun, [], Modules). + -spec included_uris([atom()], [uri()]) -> [uri()]. included_uris([], Acc) -> lists:usort(Acc); diff --git a/apps/els_lsp/test/els_diagnostics_SUITE.erl b/apps/els_lsp/test/els_diagnostics_SUITE.erl index a7b172b88..4a9d6cb64 100644 --- a/apps/els_lsp/test/els_diagnostics_SUITE.erl +++ b/apps/els_lsp/test/els_diagnostics_SUITE.erl @@ -17,6 +17,7 @@ bound_var_in_pattern_cannot_parse/1, compiler/1, compiler_with_behaviour/1, + compiler_with_behaviour_recursive/1, compiler_with_broken_behaviour/1, compiler_with_custom_macros/1, compiler_with_parse_transform/1, @@ -401,6 +402,16 @@ compiler(_Config) -> Hints = [], els_test:run_diagnostics_test(Path, Source, Errors, Warnings, Hints). +-spec compiler_with_behaviour_recursive(config()) -> ok. +compiler_with_behaviour_recursive(_Config) -> + %% Test that recursive deps are handled for behaviours + Path = src_path("diagnostics_behaviour_recursive_impl.erl"), + Source = <<"Compiler">>, + Errors = [], + Warnings = [], + Hints = [], + els_test:run_diagnostics_test(Path, Source, Errors, Warnings, Hints). + -spec compiler_with_behaviour(config()) -> ok. compiler_with_behaviour(_Config) -> Path = src_path("diagnostics_behaviour_impl.erl"),