Fix out of bounds crashes in completion (#1388)

erlang-ls · Oct 5, 2022 · b151f9d · b151f9d
1 parent bd7e901
commit b151f9d
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 18 deletions.
diff --git a/apps/els_core/src/els_text.erl b/apps/els_core/src/els_text.erl
@@ -7,6 +7,7 @@
     last_token/1,
     line/2,
     line/3,
+    get_char/3,
     range/3,
     split_at_line/2,
     tokens/1,
@@ -35,6 +36,18 @@ line(Text, LineNum, ColumnNum) ->
     Line = line(Text, LineNum),
     binary:part(Line, {0, ColumnNum}).
 
+-spec get_char(text(), line_num(), column_num()) ->
+    {ok, char()} | {error, out_of_range}.
+get_char(Text, Line, Column) ->
+    LineStarts = line_starts(Text),
+    Pos = pos(LineStarts, {Line, Column}),
+    case Pos < size(Text) of
+        true ->
+            {ok, binary:at(Text, Pos)};
+        false ->
+            {error, out_of_range}
+    end.
+
 %% @doc Extract a snippet from a text, from [StartLoc..EndLoc).
 -spec range(text(), {line_num(), column_num()}, {line_num(), column_num()}) ->
     text().

diff --git a/apps/els_lsp/priv/code_navigation/src/code_completion_fail.erl b/apps/els_lsp/priv/code_navigation/src/code_completion_fail.erl
@@ -0,0 +1,2 @@
+-module(s).
+a
diff --git a/apps/els_lsp/src/els_completion_provider.erl b/apps/els_lsp/src/els_completion_provider.erl
@@ -352,20 +352,20 @@ complete_record_field(
     complete_record_field(Document, {Line, Col}, <<"key=val}.">>).
 
 -spec complete_record_field(map(), pos(), binary()) -> items().
-complete_record_field(#{text := Text} = Document, Pos, Suffix) ->
-    T0 = els_text:range(Text, {1, 1}, Pos),
+complete_record_field(#{text := Text0} = Document, Pos, Suffix) ->
+    Prefix0 = els_text:range(Text0, {1, 1}, Pos),
     POIs = els_dt_document:pois(Document, [function]),
-    Line =
+    %% Look for record start between current pos and end of last function
+    Prefix =
         case els_scope:pois_before(POIs, #{from => Pos, to => Pos}) of
-            [#{range := #{to := {L, _}}} | _] ->
-                L;
+            [#{range := #{to := {Line, _}}} | _] ->
+                {_, Prefix1} = els_text:split_at_line(Prefix0, Line),
+                Prefix1;
             _ ->
-                %% No function before
-                1
+                %% No function before, consider all the text
+                Prefix0
         end,
-    %% Just look at lines after last function
-    {_, T} = els_text:split_at_line(T0, Line),
-    case parse_record(els_text:strip_comments(T), Suffix) of
+    case parse_record(els_text:strip_comments(Prefix), Suffix) of
         {ok, Id} ->
             record_fields_with_var(Document, Id);
         error ->
@@ -726,13 +726,9 @@ completion_context(#{text := Text} = Document, Line, Column, Tokens) ->
             true ->
                 arity_only;
             false ->
-                NextChar = els_text:range(
-                    Text,
-                    {Line, Column + 1},
-                    {Line, Column + 2}
-                ),
-                case NextChar of
-                    <<"(">> ->
+                case els_text:get_char(Text, Line, Column + 1) of
+                    {ok, $(} ->
+                        %% Don't inlude args if next character is a '('
                         no_args;
                     _ ->
                         args

diff --git a/apps/els_lsp/test/els_completion_SUITE.erl b/apps/els_lsp/test/els_completion_SUITE.erl
@@ -50,7 +50,8 @@
     resolve_opaque_application_remote_self/1,
     resolve_type_application_remote_external/1,
     resolve_opaque_application_remote_external/1,
-    resolve_type_application_remote_otp/1
+    resolve_type_application_remote_otp/1,
+    completion_request_fails/1
 ]).
 
 %%==============================================================================
@@ -534,6 +535,11 @@ default_completions(Config) ->
             insertTextFormat => ?INSERT_TEXT_FORMAT_PLAIN_TEXT,
             kind => ?COMPLETION_ITEM_KIND_MODULE,
             label => <<"code_action">>
+        },
+        #{
+            insertTextFormat => ?INSERT_TEXT_FORMAT_PLAIN_TEXT,
+            kind => ?COMPLETION_ITEM_KIND_MODULE,
+            label => <<"code_completion_fail">>
         }
         | Functions
     ],
@@ -1939,6 +1945,18 @@ resolve_type_application_remote_otp(Config) ->
     },
     ?assertEqual(Expected, Result).
 
+%% Issue #1387
+completion_request_fails(Config) ->
+    Uri = ?config(code_completion_fail_uri, Config),
+    TriggerKind = ?COMPLETION_TRIGGER_KIND_INVOKED,
+    %% Complete at -module(s|).
+    #{result := Result1} = els_client:completion(Uri, 1, 10, TriggerKind, <<>>),
+    ?assertNotEqual(null, Result1),
+    %% Complete at a|, file doesn't end with a newline!
+    #{result := Result2} = els_client:completion(Uri, 2, 2, TriggerKind, <<>>),
+    ?assertNotEqual(null, Result2),
+    ok.
+
 select_completionitems(CompletionItems, Kind, Label) ->
     [CI || #{kind := K, label := L} = CI <- CompletionItems, L =:= Label, K =:= Kind].