Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle recursive deps for behaviours #1477

Merged
merged 1 commit into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-module(diagnostics_behaviour_recursive).
-behaviour(diagnostics_behaviour).

-export([one/0]).
-export([two/0]).

-callback three() -> ok.

one() ->
ok.
two() ->
ok.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-module(diagnostics_behaviour_recursive_impl).
-behaviour(diagnostics_behaviour_recursive).
-export([three/0]).
three() ->
ok.
3 changes: 1 addition & 2 deletions apps/els_lsp/src/els_compiler_diagnostics.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
54 changes: 51 additions & 3 deletions apps/els_lsp/src/els_diagnostics_utils.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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) ->
Expand Down Expand Up @@ -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]
Expand All @@ -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} ->
Expand Down Expand Up @@ -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],
Expand All @@ -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);
Expand Down
11 changes: 11 additions & 0 deletions apps/els_lsp/test/els_diagnostics_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"),
Expand Down
Loading