From 51c877da0af975bd769ee9407e9b38c25db18ff1 Mon Sep 17 00:00:00 2001 From: Hakan Nilsson Date: Tue, 15 Oct 2024 09:28:21 +0200 Subject: [PATCH 1/3] Add support for checking if a function is exported in crossref diagnostics --- .../code_navigation/src/diagnostics_xref.erl | 3 ++ apps/els_lsp/src/els_crossref_diagnostics.erl | 36 ++++++++++++------- apps/els_lsp/src/els_dt_functions.erl | 21 ++++++----- apps/els_lsp/src/els_indexing.erl | 11 +++--- apps/els_lsp/test/els_diagnostics_SUITE.erl | 8 +++++ 5 files changed, 55 insertions(+), 24 deletions(-) diff --git a/apps/els_lsp/priv/code_navigation/src/diagnostics_xref.erl b/apps/els_lsp/priv/code_navigation/src/diagnostics_xref.erl index 1e716ea2b..49eed564f 100644 --- a/apps/els_lsp/priv/code_navigation/src/diagnostics_xref.erl +++ b/apps/els_lsp/priv/code_navigation/src/diagnostics_xref.erl @@ -12,3 +12,6 @@ existing() -> dynamic_call(Foo, Bar) -> Foo:bar(), foo:Bar(). + +not_exported() -> + lists:map_1(foo, [1,2,3]). diff --git a/apps/els_lsp/src/els_crossref_diagnostics.erl b/apps/els_lsp/src/els_crossref_diagnostics.erl index 5e6b03f50..dc1d60c3b 100644 --- a/apps/els_lsp/src/els_crossref_diagnostics.erl +++ b/apps/els_lsp/src/els_crossref_diagnostics.erl @@ -18,6 +18,8 @@ source/0 ]). +-type missing_reason() :: module | function | export. + %%============================================================================== %% Includes %%============================================================================== @@ -114,19 +116,23 @@ make_diagnostic({missing, Kind}, #{id := Id} = POI) -> make_diagnostic(true, _) -> []. --spec range(module | function, els_poi:poi()) -> els_poi:poi_range(). +-spec range(missing_reason(), els_poi:poi()) -> els_poi:poi_range(). range(module, #{data := #{mod_range := Range}}) -> Range; range(function, #{data := #{name_range := Range}}) -> Range; +range(export, #{data := #{name_range := Range}}) -> + Range; range(_, #{range := Range}) -> Range. --spec error_msg(module | function, els_poi:poi_id()) -> binary(). +-spec error_msg(missing_reason(), els_poi:poi_id()) -> binary(). error_msg(module, {M, _F, _A}) -> els_utils:to_binary(io_lib:format("Cannot find module ~p", [M])); error_msg(function, Id) -> - els_utils:to_binary(io_lib:format("Cannot find definition for function ~s", [id_str(Id)])). + els_utils:to_binary(io_lib:format("Cannot find definition for function ~s", [id_str(Id)])); +error_msg(export, Id) -> + els_utils:to_binary(io_lib:format("Function ~s is not exported.", [id_str(Id)])). -spec id_str(els_poi:poi_id()) -> string(). id_str(Id) -> @@ -136,7 +142,7 @@ id_str(Id) -> end. -spec has_definition(els_poi:poi(), els_dt_document:item(), _) -> - true | {missing, function | module}. + true | {missing, missing_reason()}. has_definition(#{data := #{imported := true}}, _Document, _Opts) -> %% Call to a bif true; @@ -177,7 +183,9 @@ has_definition( case function_lookup(MFA) of true -> true; - false -> + {missing, export} -> + true; + {missing, function} -> case els_code_navigation:goto_definition(Uri, POI) of {ok, _Defs} -> true; @@ -189,12 +197,14 @@ has_definition(#{id := {M, _F, _A} = MFA} = POI, _Document, _Opts) -> case function_lookup(MFA) of true -> true; - false -> + {missing, export} -> + {missing, export}; + {missing, function} -> case els_utils:find_module(M) of {ok, Uri} -> case els_code_navigation:goto_definition(Uri, POI) of {ok, _Defs} -> - true; + function_lookup(MFA); {error, _Error} -> {missing, function} end; @@ -205,13 +215,15 @@ has_definition(#{id := {M, _F, _A} = MFA} = POI, _Document, _Opts) -> has_definition(_POI, #{uri := _Uri}, _Opts) -> true. --spec function_lookup(mfa()) -> boolean(). +-spec function_lookup(mfa()) -> true | {missing, missing_reason()}. function_lookup(MFA) -> - case els_db:lookup(els_dt_functions:name(), MFA) of + case els_dt_functions:lookup(MFA) of {ok, []} -> - false; - {ok, _} -> - true + {missing, function}; + {ok, [#{is_exported := true}]} -> + true; + {ok, [#{is_exported := false}]} -> + {missing, export} end. -spec lager_definition(atom(), integer()) -> boolean(). diff --git a/apps/els_lsp/src/els_dt_functions.erl b/apps/els_lsp/src/els_dt_functions.erl index 8b7b955e7..5446be45b 100644 --- a/apps/els_lsp/src/els_dt_functions.erl +++ b/apps/els_lsp/src/els_dt_functions.erl @@ -37,13 +37,15 @@ -record(els_dt_functions, { mfa :: mfa() | '_' | {atom(), '_', '_'}, - version :: version() | '_' + version :: version() | '_', + is_exported :: boolean() | '_' }). -type els_dt_functions() :: #els_dt_functions{}. -type version() :: null | integer(). -type item() :: #{ mfa := mfa(), - version := version() + version := version(), + is_exported := boolean() }. -export_type([item/0]). @@ -65,21 +67,25 @@ opts() -> -spec from_item(item()) -> els_dt_functions(). from_item(#{ mfa := MFA, - version := Version + version := Version, + is_exported := IsExported }) -> #els_dt_functions{ mfa = MFA, - version = Version + version = Version, + is_exported = IsExported }. -spec to_item(els_dt_functions()) -> item(). to_item(#els_dt_functions{ mfa = MFA, - version = Version + version = Version, + is_exported = IsExported }) -> #{ mfa => MFA, - version => Version + version => Version, + is_exported => IsExported }. -spec insert(item()) -> ok | {error, any()}. @@ -96,8 +102,7 @@ versioned_insert(#{mfa := MFA, version := Version} = Map) -> els_db:conditional_write(name(), MFA, Record, Condition). -spec lookup(mfa()) -> {ok, [item()]}. -lookup({M, _F, _A} = MFA) -> - {ok, _Uris} = els_utils:find_modules(M), +lookup(MFA) -> {ok, Items} = els_db:lookup(name(), MFA), {ok, [to_item(Item) || Item <- Items]}. diff --git a/apps/els_lsp/src/els_indexing.erl b/apps/els_lsp/src/els_indexing.erl index 99eca4ee1..773737d10 100644 --- a/apps/els_lsp/src/els_indexing.erl +++ b/apps/els_lsp/src/els_indexing.erl @@ -137,14 +137,17 @@ index_signature(M, Text, #{id := {F, A}, range := Range, data := #{args := Args} -spec index_functions(atom(), uri(), [els_poi:poi()], version()) -> ok. index_functions(M, Uri, POIs, Version) -> ok = els_dt_functions:versioned_delete_by_uri(Uri, Version), - [index_function(M, POI, Version) || #{kind := function} = POI <- POIs], + Exports = [{F, A} || #{id := {F, A}, kind := export_entry} <- POIs], + [index_function(M, POI, Exports, Version) || #{kind := function} = POI <- POIs], ok. --spec index_function(atom(), els_poi:poi(), version()) -> ok. -index_function(M, #{id := {F, A}}, Version) -> +-spec index_function(atom(), els_poi:poi(), els_poi:poi_id(), version()) -> ok. +index_function(M, #{id := {F, A}}, Exports, Version) -> + IsExported = lists:member({F, A}, Exports), els_dt_functions:versioned_insert(#{ mfa => {M, F, A}, - version => Version + version => Version, + is_exported => IsExported }). -spec index_references(atom(), uri(), [els_poi:poi()], version()) -> ok. diff --git a/apps/els_lsp/test/els_diagnostics_SUITE.erl b/apps/els_lsp/test/els_diagnostics_SUITE.erl index 64f44925b..011b5cc8f 100644 --- a/apps/els_lsp/test/els_diagnostics_SUITE.erl +++ b/apps/els_lsp/test/els_diagnostics_SUITE.erl @@ -786,6 +786,10 @@ crossref(_Config) -> #{ message => <<"Cannot find definition for function lists:map/3">>, range => {{5, 8}, {5, 11}} + }, + #{ + message => <<"Function lists:map_1/2 is not exported.">>, + range => {{16, 10}, {16, 15}} } ], Warnings = [], @@ -802,6 +806,10 @@ crossref_compiler_enabled(_Config) -> #{ message => <<"Cannot find definition for function lists:map/3">>, range => {{5, 8}, {5, 11}} + }, + #{ + message => <<"Function lists:map_1/2 is not exported.">>, + range => {{16, 10}, {16, 15}} } ], Warnings = [], From 54d00d953a04f88e2facb4eb80e51019b77c80d0 Mon Sep 17 00:00:00 2001 From: Hakan Nilsson Date: Tue, 15 Oct 2024 10:12:50 +0200 Subject: [PATCH 2/3] Add code action to export function --- apps/els_lsp/src/els_code_action_provider.erl | 2 + apps/els_lsp/src/els_code_actions.erl | 27 ++++++++++ apps/els_lsp/test/els_code_action_SUITE.erl | 49 +++++++++++++++++++ 3 files changed, 78 insertions(+) diff --git a/apps/els_lsp/src/els_code_action_provider.erl b/apps/els_lsp/src/els_code_action_provider.erl index 2f0a4286a..f48bd07b5 100644 --- a/apps/els_lsp/src/els_code_action_provider.erl +++ b/apps/els_lsp/src/els_code_action_provider.erl @@ -62,6 +62,8 @@ make_code_actions( {"function (.*) undefined", fun els_code_actions:suggest_function/4}, {"Cannot find definition for function (.*)", fun els_code_actions:suggest_function/4}, {"Cannot find module (.*)", fun els_code_actions:suggest_module/4}, + {"Function (.+):(.+) is not exported.", + fun els_code_actions:export_external_function/4}, {"Unused file: (.*)", fun els_code_actions:remove_unused/4}, {"Atom typo\\? Did you mean: (.*)", fun els_code_actions:fix_atom_typo/4}, {"undefined callback function (.*) \\\(behaviour '(.*)'\\\)", diff --git a/apps/els_lsp/src/els_code_actions.erl b/apps/els_lsp/src/els_code_actions.erl index c4e8fb5d8..9ae1e6530 100644 --- a/apps/els_lsp/src/els_code_actions.erl +++ b/apps/els_lsp/src/els_code_actions.erl @@ -3,6 +3,7 @@ extract_function/2, create_function/4, export_function/4, + export_external_function/4, fix_module_name/4, ignore_variable/4, remove_macro/4, @@ -84,6 +85,32 @@ export_function(Uri, _Range, _Data, [UnusedFun]) -> ] end. +-spec export_external_function(uri(), range(), binary(), [binary()]) -> [map()]. +export_external_function(_Uri, _Range, _Data, [ModBin, FABin]) -> + Mod = binary_to_atom(ModBin, utf8), + case els_utils:find_module(Mod) of + {ok, Uri} -> + {ok, Document} = els_utils:lookup_document(Uri), + case els_poi:sort(els_dt_document:pois(Document, [module, export])) of + [] -> + []; + POIs -> + #{range := #{to := {Line, _Col}}} = lists:last(POIs), + Pos = {Line + 1, 1}, + [ + make_edit_action( + Uri, + <<"Export ", FABin/binary>>, + ?CODE_ACTION_KIND_QUICKFIX, + <<"-export([", FABin/binary, "]).\n">>, + els_protocol:range(#{from => Pos, to => Pos}) + ) + ] + end; + {error, not_found} -> + [] + end. + -spec ignore_variable(uri(), range(), binary(), [binary()]) -> [map()]. ignore_variable(Uri, Range, _Data, [UnusedVariable]) -> {ok, Document} = els_utils:lookup_document(Uri), diff --git a/apps/els_lsp/test/els_code_action_SUITE.erl b/apps/els_lsp/test/els_code_action_SUITE.erl index 1e1a51bd5..68f1f2f3c 100644 --- a/apps/els_lsp/test/els_code_action_SUITE.erl +++ b/apps/els_lsp/test/els_code_action_SUITE.erl @@ -14,6 +14,7 @@ -export([ add_underscore_to_unused_var/1, export_unused_function/1, + export_external_function/1, suggest_variable/1, fix_module_name/1, remove_unused_macro/1, @@ -158,6 +159,54 @@ export_unused_function(Config) -> ?assertEqual(Expected, Result), ok. +-spec export_external_function(config()) -> ok. +export_external_function(Config) -> + Uri = ?config(code_navigation_uri, Config), + %% Don't care + Range = els_protocol:range(#{ + from => {1, 1}, + to => {1, 2} + }), + Diag = #{ + message => <<"Function code_action:function_c/0 is not exported.">>, + range => Range, + severity => 2, + source => <<"CrossRef">> + }, + #{result := Result} = els_client:document_codeaction(Uri, Range, [Diag]), + TargetUri = ?config(code_action_uri, Config), + Expected = + [ + #{ + edit => #{ + changes => + #{ + binary_to_atom(TargetUri, utf8) => + [ + #{ + range => + #{ + 'end' => #{ + character => 0, + line => ?COMMENTS_LINES + 3 + }, + start => #{ + character => 0, + line => ?COMMENTS_LINES + 3 + } + }, + newText => <<"-export([function_c/0]).\n">> + } + ] + } + }, + kind => <<"quickfix">>, + title => <<"Export function_c/0">> + } + ], + ?assertEqual(Expected, Result), + ok. + -spec suggest_variable(config()) -> ok. suggest_variable(Config) -> Uri = ?config(code_action_uri, Config), From c891f18c51825af317fc4e29bbe118a39a3aaf39 Mon Sep 17 00:00:00 2001 From: Hakan Nilsson Date: Tue, 15 Oct 2024 14:48:48 +0200 Subject: [PATCH 3/3] Fix test --- .../priv/code_navigation/src/diagnostics_xref.erl | 2 +- apps/els_lsp/test/els_diagnostics_SUITE.erl | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/apps/els_lsp/priv/code_navigation/src/diagnostics_xref.erl b/apps/els_lsp/priv/code_navigation/src/diagnostics_xref.erl index 49eed564f..b8be8f865 100644 --- a/apps/els_lsp/priv/code_navigation/src/diagnostics_xref.erl +++ b/apps/els_lsp/priv/code_navigation/src/diagnostics_xref.erl @@ -14,4 +14,4 @@ dynamic_call(Foo, Bar) -> foo:Bar(). not_exported() -> - lists:map_1(foo, [1,2,3]). + code_navigation:function_c(). diff --git a/apps/els_lsp/test/els_diagnostics_SUITE.erl b/apps/els_lsp/test/els_diagnostics_SUITE.erl index 011b5cc8f..070062cdb 100644 --- a/apps/els_lsp/test/els_diagnostics_SUITE.erl +++ b/apps/els_lsp/test/els_diagnostics_SUITE.erl @@ -787,9 +787,11 @@ crossref(_Config) -> message => <<"Cannot find definition for function lists:map/3">>, range => {{5, 8}, {5, 11}} }, + #{ - message => <<"Function lists:map_1/2 is not exported.">>, - range => {{16, 10}, {16, 15}} + message => + <<"Function code_navigation:function_c/0 is not exported.">>, + range => {{16, 20}, {16, 30}} } ], Warnings = [], @@ -808,8 +810,9 @@ crossref_compiler_enabled(_Config) -> range => {{5, 8}, {5, 11}} }, #{ - message => <<"Function lists:map_1/2 is not exported.">>, - range => {{16, 10}, {16, 15}} + message => + <<"Function code_navigation:function_c/0 is not exported.">>, + range => {{16, 20}, {16, 30}} } ], Warnings = [],