Better support for goto definition when parsing is incomplete (erlang…

…-ls#1224)

* Better support for goto definition when parsing is incomplete

* Remove els_code_actions

* Use els_parser:parse_incomplete_text to preserve ranges

* Fix invalid spec

* Break out functions to els_incomplete_parser

apps/els_core/src/els_text.erl

@@ -7,6 +7,7 @@
  , line/2
  , line/3
  , range/3
+ , split_at_line/2
  , tokens/1
  , apply_edits/2
  ]).
@@ -43,6 +44,12 @@ range(Text, StartLoc, EndLoc) ->
  EndPos = pos(LineStarts, EndLoc),
  binary:part(Text, StartPos, EndPos - StartPos).
 
+-spec split_at_line(text(), line_num()) -> {text(), text()}.
+split_at_line(Text, Line) ->
+ StartPos = pos(line_starts(Text), {Line + 1, 1}),
+ <<Left:StartPos/binary, Right/binary>> = Text,
+ {Left, Right}.
+
 %% @doc Return tokens from text.
 -spec tokens(text()) -> [any()].
 tokens(Text) ->

apps/els_lsp/priv/code_navigation/src/code_navigation_broken.erl

@@ -0,0 +1,19 @@
+-module(code_navigation_broken).
+
+function_a() ->
+ ok.
+
+function_b() ->
+ function_a() % missing comma, breaks parsing of this function!
+ function_a(),
+ case function_a() of
+ ok ->
+ function_a(),
+ case function_a() of
+ ok ->
+ ok
+ end
+ end,
+ function_a(
+ ),
+ function_a().

apps/els_lsp/src/els_code_navigation.erl

@@ -31,7 +31,8 @@ goto_definition( Uri
  %% first occurrence of the variable in variable scope.
  case find_in_scope(Uri, Var) of
  [Var|_] -> {error, already_at_definition};
- [POI|_] -> {ok, Uri, POI}
+ [POI|_] -> {ok, Uri, POI};
+ [] -> {error, nothing_in_scope} % Probably due to parse error
  end;
 goto_definition( _Uri
  , #{ kind := Kind, id := {M, F, A} }

apps/els_lsp/src/els_definition_provider.erl

@@ -30,7 +30,14 @@ handle_request({definition, Params}, State) ->
  POIs = els_dt_document:get_element_at_pos(Document, Line + 1, Character + 1),
  case goto_definition(Uri, POIs) of
  null ->
- els_references_provider:handle_request({references, Params}, State);
+ #{text := Text} = Document,
+ IncompletePOIs = match_incomplete(Text, {Line, Character}),
+ case goto_definition(Uri, IncompletePOIs) of
+ null ->
+ els_references_provider:handle_request({references, Params}, State);
+ GoTo ->
+ {GoTo, State}
+ end;
  GoTo ->
  {GoTo, State}
  end.
@@ -45,3 +52,47 @@ goto_definition(Uri, [POI|Rest]) ->
  _ ->
  goto_definition(Uri, Rest)
  end.
+
+-spec match_incomplete(binary(), pos()) -> [poi()].
+match_incomplete(Text, Pos) ->
+ %% Try parsing subsets of text to find a matching POI at Pos
+ match_after(Text, Pos) ++ match_line(Text, Pos).
+
+-spec match_after(binary(), pos()) -> [poi()].
+match_after(Text, {Line, Character}) ->
+ %% Try to parse current line and the lines after it
+ POIs = els_incomplete_parser:parse_after(Text, Line),
+ MatchingPOIs = match_pois(POIs, {1, Character + 1}),
+ fix_line_offsets(MatchingPOIs, Line).
+
+-spec match_line(binary(), pos()) -> [poi()].
+match_line(Text, {Line, Character}) ->
+ %% Try to parse only current line
+ POIs = els_incomplete_parser:parse_line(Text, Line),
+ MatchingPOIs = match_pois(POIs, {1, Character + 1}),
+ fix_line_offsets(MatchingPOIs, Line).
+
+-spec match_pois([poi()], pos()) -> [poi()].
+match_pois(POIs, Pos) ->
+ els_poi:sort(els_poi:match_pos(POIs, Pos)).
+
+-spec fix_line_offsets([poi()], integer()) -> [poi()].
+fix_line_offsets(POIs, Offset) ->
+ [fix_line_offset(POI, Offset) || POI <- POIs].
+
+-spec fix_line_offset(poi(), integer()) -> poi().
+fix_line_offset(#{range := #{from := {FromL, FromC},
+ to := {ToL, ToC}}} = POI, Offset) ->
+ %% TODO: Fix other ranges too
+ POI#{range => #{from => {FromL + Offset, FromC},
+ to => {ToL + Offset, ToC}
+ }}.
+
+-ifdef(TEST).
+-include_lib("eunit/include/eunit.hrl").
+fix_line_offset_test() ->
+ In = #{range => #{from => {1, 16}, to => {1, 32}}},
+ ?assertMatch( #{range := #{from := {66, 16}, to := {66, 32}}}
+ , fix_line_offset(In, 65)).
+
+-endif.

apps/els_lsp/src/els_incomplete_parser.erl

@@ -0,0 +1,30 @@
+-module(els_incomplete_parser).
+-export([parse_after/2]).
+-export([parse_line/2]).
+
+-include("els_lsp.hrl").
+-include_lib("kernel/include/logger.hrl").
+
+-spec parse_after(binary(), integer()) -> [poi()].
+parse_after(Text, Line) ->
+ {_, AfterText} = els_text:split_at_line(Text, Line),
+ {ok, POIs} = els_parser:parse(AfterText),
+ POIs.
+
+-spec parse_line(binary(), integer()) -> [poi()].
+parse_line(Text, Line) ->
+ LineText0 = string:trim(els_text:line(Text, Line), trailing, ",;"),
+ case els_parser:parse(LineText0) of
+ {ok, []} ->
+ LineStr = els_utils:to_list(LineText0),
+ case lists:reverse(LineStr) of
+ "fo " ++ _ -> %% Kludge to parse "case foo() of"
+ LineText1 = <<LineText0/binary, " _ -> _ end">>,
+ {ok, POIs} = els_parser:parse(LineText1),
+ POIs;
+ _ ->
+ []
+ end;
+ {ok, POIs} ->
+ POIs
+ end.

apps/els_lsp/src/els_parser.erl

@@ -6,12 +6,14 @@
 %%==============================================================================
 %% Exports
 %%==============================================================================
--export([ parse/1]).
+-export([ parse/1
+ , parse_incomplete_text/2
+ , points_of_interest/1
+ ]).
 
 %% For manual use only, to test the parser
 -export([ parse_file/1
  , parse_text/1
- , parse_incomplete_text/2
  ]).
 
 %%==============================================================================

apps/els_lsp/test/els_completion_SUITE.erl

@@ -423,6 +423,10 @@ default_completions(Config) ->
  , kind => ?COMPLETION_ITEM_KIND_MODULE
  , label => <<"code_navigation_undefined">>
  }
+ , #{ insertTextFormat => ?INSERT_TEXT_FORMAT_PLAIN_TEXT
+ , kind => ?COMPLETION_ITEM_KIND_MODULE
+ , label => <<"code_navigation_broken">>
+ }
  , #{ insertTextFormat => ?INSERT_TEXT_FORMAT_PLAIN_TEXT
  , kind => ?COMPLETION_ITEM_KIND_MODULE
  , label => <<"code_action">>

apps/els_lsp/test/els_definition_SUITE.erl

@@ -49,6 +49,7 @@
  , variable/1
  , opaque_application_remote/1
  , opaque_application_user/1
+ , parse_incomplete/1
  ]).
 
 %%==============================================================================
@@ -505,3 +506,23 @@ opaque_application_user(Config) ->
  ?assertEqual( els_protocol:range(#{from => {20, 1}, to => {20, 34}})
  , Range),
  ok.
+
+-spec parse_incomplete(config()) -> ok.
+parse_incomplete(Config) ->
+ Uri = ?config(code_navigation_broken_uri, Config),
+ Range = els_protocol:range(#{from => {3, 1}, to => {3, 11}}),
+ ?assertMatch( #{result := #{range := Range, uri := Uri}}
+ , els_client:definition(Uri, 7, 3)),
+ ?assertMatch( #{result := #{range := Range, uri := Uri}}
+ , els_client:definition(Uri, 8, 3)),
+ ?assertMatch( #{result := #{range := Range, uri := Uri}}
+ , els_client:definition(Uri, 9, 8)),
+ ?assertMatch( #{result := #{range := Range, uri := Uri}}
+ , els_client:definition(Uri, 11, 7)),
+ ?assertMatch( #{result := #{range := Range, uri := Uri}}
+ , els_client:definition(Uri, 12, 12)),
+ ?assertMatch( #{result := #{range := Range, uri := Uri}}
+ , els_client:definition(Uri, 17, 3)),
+ ?assertMatch( #{result := #{range := Range, uri := Uri}}
+ , els_client:definition(Uri, 19, 3)),
+ ok.

apps/els_lsp/test/els_workspace_symbol_SUITE.erl

@@ -66,6 +66,7 @@ query_multiple(Config) ->
  ExtraUri = ?config(code_navigation_extra_uri, Config),
  TypesUri = ?config(code_navigation_types_uri, Config),
  UndefUri = ?config(code_navigation_undefined_uri, Config),
+ BrokenUri = ?config(code_navigation_broken_uri, Config),
  #{result := Result} = els_client:workspace_symbol(Query),
  Expected = [ #{ kind => ?SYMBOLKIND_MODULE
  , location =>
@@ -107,6 +108,16 @@ query_multiple(Config) ->
  }
  , name => <<"code_navigation_undefined">>
  }
+ , #{ kind => ?SYMBOLKIND_MODULE
+ , location =>
+ #{ range =>
+ #{ 'end' => #{character => 0, line => 0}
+ , start => #{character => 0, line => 0}
+ }
+ , uri => BrokenUri
+ }
+ , name => <<"code_navigation_broken">>
+ }
  ],
  ?assertEqual(lists:sort(Expected), lists:sort(Result)),
  ok.