Skip to content

Commit

Permalink
Refactor folding_ranges onto POIs, Add support for records. (#1268)
Browse files Browse the repository at this point in the history 
Co-authored-by: Tanoy Kumar Sinha <[email protected]>
  • Loading branch information
@tks2103
tks2103 and Tanoy Kumar Sinha committed Apr 21, 2022
1 parent 8ca5170 commit 0f6962e
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 118 deletions.
11 changes: 11 additions & 0 deletions apps/els_lsp/priv/code_navigation/src/folding_ranges.erl
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
-module(folding_ranges).

function_foldable() ->
?assertEqual(2.0, 4/2).

-record(unfoldable_record, { field_a }).

-record(foldable_record, { field_a
, field_b
, field_c
}).
5 changes: 3 additions & 2 deletions apps/els_lsp/src/els_folding_range_provider.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ is_enabled() -> true.
handle_request({document_foldingrange, Params}, _State) ->
#{ <<"textDocument">> := #{<<"uri">> := Uri} } = Params,
{ok, Document} = els_utils:lookup_document(Uri),
POIs = els_dt_document:pois(Document, [folding_range]),
Response = case [folding_range(Range) || #{range := Range} <- POIs] of
POIs = els_dt_document:pois(Document, [function, record]),
Response = case [folding_range(Range)
|| #{data := #{folding_range := Range = #{}}} <- POIs] of
[] -> null;
Ranges -> Ranges
end,
Expand Down
38 changes: 22 additions & 16 deletions apps/els_lsp/src/els_parser.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,9 +385,16 @@ attribute(Tree) ->
-spec record_attribute_pois(tree(), tree(), atom(), tree()) -> [poi()].
record_attribute_pois(Tree, Record, RecordName, Fields) ->
FieldList = record_def_field_name_list(Fields),
ValueRange = #{ from => get_start_location(Tree),
to => get_end_location(Tree)},
Data = #{field_list => FieldList, value_range => ValueRange},
{StartLine, StartColumn} = get_start_location(Tree),
{EndLine, EndColumn} = get_end_location(Tree),
ValueRange = #{ from => {StartLine, StartColumn}
, to => {EndLine, EndColumn}
},
FoldingRange = exceeds_one_line(StartLine, EndLine),
Data = #{ field_list => FieldList
, value_range => ValueRange
, folding_range => FoldingRange
},
[poi(erl_syntax:get_pos(Record), record, RecordName, Data)
| record_def_fields(Fields, RecordName)].

Expand Down Expand Up @@ -494,27 +501,17 @@ function(Tree) ->
)
|| {I, Clause} <- IndexedClauses,
erl_syntax:type(Clause) =:= clause],
{StartLine, StartColumn} = StartLocation = get_start_location(Tree),
{StartLine, StartColumn} = get_start_location(Tree),
{EndLine, _EndColumn} = get_end_location(Tree),
%% It only makes sense to fold a function if the function contains
%% at least one line apart from its signature.
FoldingRanges = case EndLine > StartLine of
true ->
Range = #{ from => {StartLine, ?END_OF_LINE}
, to => {EndLine, ?END_OF_LINE}
},
[ els_poi:new(Range, folding_range, StartLocation) ];
false ->
[]
end,
FoldingRange = exceeds_one_line(StartLine, EndLine),
FunctionPOI = poi(erl_syntax:get_pos(FunName), function, {F, A},
#{ args => Args
, wrapping_range => #{ from => {StartLine, StartColumn}
, to => {EndLine + 1, 0}
}
, folding_range => FoldingRange
}),
lists:append([ [ FunctionPOI ]
, FoldingRanges
, ClausesPOIs
]).

Expand Down Expand Up @@ -1044,6 +1041,15 @@ skip_function_entries(FunList) ->
[FunList]
end.

%% Helpers for determining valid Folding Ranges
-spec exceeds_one_line(erl_anno:line(), erl_anno:line()) ->
poi_range() | oneliner.
exceeds_one_line(StartLine, EndLine) when EndLine > StartLine ->
#{ from => {StartLine, ?END_OF_LINE}
, to => {EndLine, ?END_OF_LINE}
};
exceeds_one_line(_, _) -> oneliner.

%% Skip visiting atoms of record names as they are already
%% represented as `record_expr' pois
-spec skip_record_name_atom(tree()) -> [tree()].
Expand Down
35 changes: 30 additions & 5 deletions apps/els_lsp/test/els_call_hierarchy_SUITE.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ incoming_calls(Config) ->
, wrapping_range => #{ from => {7, 1}
, to => {17, 0}
}
, folding_range => #{ from => {7, ?END_OF_LINE}
, to => {16, ?END_OF_LINE}
}
}
, id => {function_a, 1}
, kind => function
Expand Down Expand Up @@ -99,7 +102,10 @@ incoming_calls(Config) ->
#{ args => [{1, "Arg1"}]
, wrapping_range =>
#{ from => {7, 1}
, to => {14, 0}}}
, to => {14, 0}}
, folding_range =>
#{ from => {7, ?END_OF_LINE}
, to => {13, ?END_OF_LINE}}}
, id => {function_a, 1}
, kind => function
, range => #{from => {7, 1}, to => {7, 11}}}})
Expand Down Expand Up @@ -130,6 +136,10 @@ incoming_calls(Config) ->
#{ from => {7, 1}
, to => {17, 0}
}
, folding_range =>
#{ from => {7, ?END_OF_LINE}
, to => {16, ?END_OF_LINE}
}
}
, id => {function_a, 1}
, kind => function
Expand Down Expand Up @@ -178,6 +188,9 @@ outgoing_calls(Config) ->
, wrapping_range => #{ from => {7, 1}
, to => {17, 0}
}
, folding_range => #{ from => {7, ?END_OF_LINE}
, to => {16, ?END_OF_LINE}
}
}
, id => {function_a, 1}
, kind => function
Expand Down Expand Up @@ -205,7 +218,10 @@ outgoing_calls(Config) ->
#{ args => [{1, "Arg1"}]
, wrapping_range => #{ from => {7, 1}
, to => {17, 0}
}}
}
, folding_range => #{ from => {7, ?END_OF_LINE}
, to => {16, ?END_OF_LINE}
}}
, id => {function_a, 1}
, kind => function
, range => #{ from => {7, 1}
Expand All @@ -217,7 +233,10 @@ outgoing_calls(Config) ->
#{ args => []
, wrapping_range => #{ from => {18, 1}
, to => {20, 0}
}}
}
, folding_range => #{ from => {18, ?END_OF_LINE}
, to => {19, ?END_OF_LINE}
}}
, id => {function_b, 0}
, kind => function
, range => #{ from => {18, 1}
Expand All @@ -229,7 +248,10 @@ outgoing_calls(Config) ->
#{ args => [{1, "Arg1"}]
, wrapping_range => #{ from => {7, 1}
, to => {14, 0}
}}
}
, folding_range => #{ from => {7, ?END_OF_LINE}
, to => {13, ?END_OF_LINE}
}}
, id => {function_a, 1}
, kind => function
, range => #{ from => {7, 1}
Expand All @@ -241,7 +263,10 @@ outgoing_calls(Config) ->
#{ args => []
, wrapping_range => #{ from => {18, 1}
, to => {20, 0}
}}
}
, folding_range => #{ from => {18, ?END_OF_LINE}
, to => {19, ?END_OF_LINE}
}}
, id => {function_b, 0}
, kind => function
, range => #{ from => {18, 1}
Expand Down
100 changes: 5 additions & 95 deletions apps/els_lsp/test/els_foldingrange_SUITE.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,106 +60,16 @@ end_per_testcase(TestCase, Config) ->
-spec folding_range(config()) -> ok.
folding_range(Config) ->
#{result := Result} =
els_client:folding_range(?config(code_navigation_uri, Config)),
els_client:folding_range(?config(folding_ranges_uri, Config)),
Expected = [ #{ endCharacter => ?END_OF_LINE
, endLine => 22
, endLine => 3
, startCharacter => ?END_OF_LINE
, startLine => 20
, startLine => 2
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 25
, endLine => 10
, startCharacter => ?END_OF_LINE
, startLine => 24
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 28
, startCharacter => ?END_OF_LINE
, startLine => 27
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 34
, startCharacter => ?END_OF_LINE
, startLine => 30
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 39
, startCharacter => ?END_OF_LINE
, startLine => 38
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 42
, startCharacter => ?END_OF_LINE
, startLine => 41
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 47
, startCharacter => ?END_OF_LINE
, startLine => 46
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 52
, startCharacter => ?END_OF_LINE
, startLine => 49
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 56
, startCharacter => ?END_OF_LINE
, startLine => 55
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 67
, startCharacter => ?END_OF_LINE
, startLine => 66
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 75
, startCharacter => ?END_OF_LINE
, startLine => 73
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 80
, startCharacter => ?END_OF_LINE
, startLine => 78
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 85
, startCharacter => ?END_OF_LINE
, startLine => 83
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 89
, startCharacter => ?END_OF_LINE
, startLine => 88
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 93
, startCharacter => ?END_OF_LINE
, startLine => 92
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 100
, startCharacter => ?END_OF_LINE
, startLine => 97
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 107
, startCharacter => ?END_OF_LINE
, startLine => 102
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 115
, startCharacter => ?END_OF_LINE
, startLine => 113
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 120
, startCharacter => ?END_OF_LINE
, startLine => 119
}
, #{ endCharacter => ?END_OF_LINE
, endLine => 123
, startCharacter => ?END_OF_LINE
, startLine => 122
, startLine => 7
}
],
?assertEqual(Expected, Result),
Expand Down

0 comments on commit 0f6962e

Please sign in to comment.