Skip to content

Commit

Permalink
Fix regression: completion for incomplete export enrty
Browse files Browse the repository at this point in the history
The unexpected atom (an incomplete function name) in the export attribute caused
the below crash in the parser. This way the completion provider failed to
determine that the location is in an export context, and used function snippet
with arguments instead of '/arity'.

For example when hitting enter on a completion erlang_ls used to insert
`foo(Arg)` instead of `foo/1`.

```
1> els_parser:parse("-export([foo]).").
2021-06-06T16:40:30.522244+02:00 warning: Please report error parsing form error:{try_clause,foo}:[{els_parser,'-skip_function_entries/1-fun-0-',1,[{file,"/Users/ext.pgomori/git/erlang_ls/apps/els_lsp/src/els_parser.erl"},{line,839}]},{lists,'-filter/2-lc$^0/1-0-',2,[{file,"lists.erl"},{line,1286}]},{els_parser,attribute_subtrees,2,[{file,"/Users/ext.pgomori/git/erlang_ls/apps/els_lsp/src/els_parser.erl"},{line,792}]},{els_parser,fold,3,[{file,"/Users/ext.pgomori/git/erlang_ls/apps/els_lsp/src/els_parser.erl"},{line,687}]},{els_parser,'-parse_forms/1-lc$^0/1-0-',1,[{file,"/Users/ext.pgomori/git/erlang_ls/apps/els_lsp/src/els_parser.erl"},{line,59}]},{els_parser,parse,1,[{file,"/Users/ext.pgomori/git/erlang_ls/apps/els_lsp/src/els_parser.erl"},{line,30}]},{erl_eval,do_apply,6,[{file,"erl_eval.erl"},{line,684}]},{shell,exprs,7,[{file,"shell.erl"},{line,686}]}], {attribute,#{end_location => {1,16},location => {1,1}},{atom,#{end_location => {1,8},location => {1,2},text => "export"},export},[{list,#{end_location => {1,14},location => {1,9}},[{atom,#{end_location => {1,13},location => {1,10},text => "foo"},foo}]}]}
```
  • Loading branch information
gomoripeti committed Jun 6, 2021
1 parent ed67dd0 commit c897127
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 10 deletions.
11 changes: 11 additions & 0 deletions apps/els_lsp/priv/code_navigation/src/completion_incomplete.erl
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
-module(completion_incomplete).

-export([ function_exported/0
, function_
]).

function_exported() ->
ok.

function_unexported() ->
ok.
41 changes: 31 additions & 10 deletions apps/els_lsp/src/els_parser.erl
Original file line number Diff line number Diff line change
Expand Up @@ -353,10 +353,10 @@ find_export_pois(Tree, AttrName, Arg) ->
-> [poi()].
find_export_entry_pois(EntryPoiKind, Exports) ->
lists:flatten(
[ try erl_syntax_lib:analyze_function_name(FATree) of
[ case get_name_arity(FATree) of
{F, A} ->
poi(erl_syntax:get_pos(FATree), EntryPoiKind, {F, A})
catch throw:syntax_error ->
poi(erl_syntax:get_pos(FATree), EntryPoiKind, {F, A});
false ->
[]
end
|| FATree <- Exports
Expand All @@ -365,10 +365,10 @@ find_export_entry_pois(EntryPoiKind, Exports) ->
-spec find_import_entry_pois(atom(), [tree()]) -> [poi()].
find_import_entry_pois(M, Imports) ->
lists:flatten(
[ try erl_syntax_lib:analyze_function_name(FATree) of
[ case get_name_arity(FATree) of
{F, A} ->
poi(erl_syntax:get_pos(FATree), import_entry, {M, F, A})
catch throw:syntax_error ->
poi(erl_syntax:get_pos(FATree), import_entry, {M, F, A});
false ->
[]
end
|| FATree <- Imports
Expand Down Expand Up @@ -668,6 +668,28 @@ is_atom_node(Tree) ->
false
end.

-spec get_name_arity(tree()) -> {atom(), integer()} | false.
get_name_arity(Tree) ->
case erl_syntax:type(Tree) of
arity_qualifier ->
A = erl_syntax:arity_qualifier_argument(Tree),
case erl_syntax:type(A) of
integer ->
F = erl_syntax:arity_qualifier_body(Tree),
case is_atom_node(F) of
{true, Name} ->
Arity = erl_syntax:integer_value(A),
{Name, Arity};
false ->
false
end;
_ ->
false
end;
_ ->
false
end.

-spec poi(pos() | {pos(), pos()} | erl_anno:anno(), poi_kind(), any()) -> poi().
poi(Pos, Kind, Id) ->
poi(Pos, Kind, Id, undefined).
Expand Down Expand Up @@ -836,10 +858,9 @@ skip_function_entries(FunList) ->
list ->
lists:filter(
fun(FATree) ->
try erl_syntax_lib:analyze_function_name(FATree) of
{_, _} -> false
catch
throw:syntax_error -> true
case get_name_arity(FATree) of
{_, _} -> false;
false -> true
end
end, erl_syntax:list_elements(FunList));
_ ->
Expand Down
31 changes: 31 additions & 0 deletions apps/els_lsp/test/els_completion_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
, attribute_include/1
, attribute_include_lib/1
, attribute_export/1
, attribute_export_incomplete/1
, attribute_export_type/1
, default_completions/1
, empty_completions/1
Expand Down Expand Up @@ -273,6 +274,36 @@ attribute_export(Config) ->
[?assertNot(lists:member(E, Completions)) || E <- NotExpected],
ok.

-spec attribute_export_incomplete(config()) -> ok.
attribute_export_incomplete(Config) ->
Uri = ?config(completion_incomplete_uri, Config),
TriggerKindInvoked = ?COMPLETION_TRIGGER_KIND_INVOKED,
Expected = [#{ insertTextFormat => ?INSERT_TEXT_FORMAT_PLAIN_TEXT
, kind => ?COMPLETION_ITEM_KIND_FUNCTION
, label => <<"function_unexported/0">>
, data =>
#{ arity => 0
, function => <<"function_unexported">>
, module => <<"completion_incomplete">>
}
}
],
NotExpected = [#{ insertTextFormat => ?INSERT_TEXT_FORMAT_PLAIN_TEXT
, kind => ?COMPLETION_ITEM_KIND_FUNCTION
, label => <<"function_exported/0">>
, data =>
#{ arity => 0
, function => <<"function_exported">>
, module => <<"completion_incomplete">>
}
}
],
#{result := Completions} =
els_client:completion(Uri, 4, 18, TriggerKindInvoked, <<"">>),
[?assert(lists:member(E, Completions)) || E <- Expected],
[?assertNot(lists:member(E, Completions)) || E <- NotExpected],
ok.

-spec attribute_export_type(config()) -> ok.
attribute_export_type(Config) ->
Uri = ?config(completion_attributes_uri, Config),
Expand Down
1 change: 1 addition & 0 deletions apps/els_lsp/test/els_test_utils.erl
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ sources() ->
, completion_resolve_2
, completion_snippets
, completion_attributes
, completion_incomplete
, diagnostics
, diagnostics_bound_var_in_pattern
, diagnostics_autoimport
Expand Down

0 comments on commit c897127

Please sign in to comment.