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 1e716ea2..b8be8f86 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() -> + code_navigation:function_c(). diff --git a/apps/els_lsp/src/els_code_action_provider.erl b/apps/els_lsp/src/els_code_action_provider.erl index 2f0a4286..f48bd07b 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 c4e8fb5d..9ae1e653 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/src/els_crossref_diagnostics.erl b/apps/els_lsp/src/els_crossref_diagnostics.erl index 5e6b03f5..dc1d60c3 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 8b7b955e..5446be45 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 99eca4ee..773737d1 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_code_action_SUITE.erl b/apps/els_lsp/test/els_code_action_SUITE.erl index 1e1a51bd..68f1f2f3 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), diff --git a/apps/els_lsp/test/els_diagnostics_SUITE.erl b/apps/els_lsp/test/els_diagnostics_SUITE.erl index 64f44925..070062cd 100644 --- a/apps/els_lsp/test/els_diagnostics_SUITE.erl +++ b/apps/els_lsp/test/els_diagnostics_SUITE.erl @@ -786,6 +786,12 @@ crossref(_Config) -> #{ message => <<"Cannot find definition for function lists:map/3">>, range => {{5, 8}, {5, 11}} + }, + + #{ + message => + <<"Function code_navigation:function_c/0 is not exported.">>, + range => {{16, 20}, {16, 30}} } ], Warnings = [], @@ -802,6 +808,11 @@ crossref_compiler_enabled(_Config) -> #{ message => <<"Cannot find definition for function lists:map/3">>, range => {{5, 8}, {5, 11}} + }, + #{ + message => + <<"Function code_navigation:function_c/0 is not exported.">>, + range => {{16, 20}, {16, 30}} } ], Warnings = [],