Add Fix for erlang-ls#1202 remove unused include (erlang-ls#1227)

* Add Fix for erlang-ls#1202 remove unused include

* Fixing lint line length

* Fixing failing tests

* Fixing Var name

* Update diagnostic type spec

* Update diagnostic type spec

* Fixing dialyzer errors

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

@@ -17,3 +17,5 @@ function_c() ->
  Foo + Barf.
 
 -define(TIMEOUT, 200).
+
+-include_lib("stdlib/include/assert.hrl").

apps/els_lsp/src/els_code_action_provider.erl

@@ -35,31 +35,35 @@ code_actions(Uri, _Range, #{<<"diagnostics">> := Diagnostics}) ->
  lists:flatten([make_code_action(Uri, D) || D <- Diagnostics]).
 
 -spec make_code_action(uri(), map()) -> [map()].
-make_code_action(Uri, #{<<"message">> := Message, <<"range">> := Range}) ->
+make_code_action(Uri,
+ #{<<"message">> := Message, <<"range">> := Range} = D) ->
+ Data = maps:get(<<"data">>, D, <<>>),
  make_code_action(
- [ {"function (.*) is unused", fun action_export_function/3}
- , {"variable '(.*)' is unused", fun action_ignore_variable/3}
- , {"variable '(.*)' is unbound", fun action_suggest_variable/3}
+ [ {"function (.*) is unused", fun action_export_function/4}
+ , {"variable '(.*)' is unused", fun action_ignore_variable/4}
+ , {"variable '(.*)' is unbound", fun action_suggest_variable/4}
  , {"Module name '(.*)' does not match file name '(.*)'",
- fun action_fix_module_name/3}
- , {"Unused macro: (.*)", fun action_remove_macro/3}
- ], Uri, Range, Message).
-
--spec make_code_action([{string(), Fun}], uri(), range(), binary()) -> [map()]
- when Fun :: fun((uri(), range(), [binary()]) -> [map()]).
-make_code_action([], _Uri, _Range, _Message) ->
+ fun action_fix_module_name/4}
+ , {"Unused macro: (.*)", fun action_remove_macro/4}
+ , {"Unused file: (.*)", fun action_remove_unused/4}
+ ], Uri, Range, Data, Message).
+
+-spec make_code_action([{string(), Fun}], uri(), range(), binary(), binary())
+ -> [map()]
+ when Fun :: fun((uri(), range(), binary(), [binary()]) -> [map()]).
+make_code_action([], _Uri, _Range, _Data, _Message) ->
  [];
-make_code_action([{RE, Fun}|Rest], Uri, Range, Message) ->
+make_code_action([{RE, Fun}|Rest], Uri, Range, Data, Message) ->
  Actions = case re:run(Message, RE, [{capture, all_but_first, binary}]) of
  {match, Matches} ->
- Fun(Uri, Range, Matches);
+ Fun(Uri, Range, Data, Matches);
  nomatch ->
  []
  end,
- Actions ++ make_code_action(Rest, Uri, Range, Message).
+ Actions ++ make_code_action(Rest, Uri, Range, Data, Message).
 
--spec action_export_function(uri(), range(), [binary()]) -> [map()].
-action_export_function(Uri, _Range, [UnusedFun]) ->
+-spec action_export_function(uri(), range(), binary(), [binary()]) -> [map()].
+action_export_function(Uri, _Range, _Data, [UnusedFun]) ->
  {ok, Document} = els_utils:lookup_document(Uri),
  case els_poi:sort(els_dt_document:pois(Document, [module, export])) of
  [] ->
@@ -74,8 +78,8 @@ action_export_function(Uri, _Range, [UnusedFun]) ->
  , els_protocol:range(#{from => Pos, to => Pos})) ]
  end.
 
--spec action_ignore_variable(uri(), range(), [binary()]) -> [map()].
-action_ignore_variable(Uri, Range, [UnusedVariable]) ->
+-spec action_ignore_variable(uri(), range(), binary(), [binary()]) -> [map()].
+action_ignore_variable(Uri, Range, _Data, [UnusedVariable]) ->
  {ok, Document} = els_utils:lookup_document(Uri),
  POIs = els_poi:sort(els_dt_document:pois(Document, [variable])),
  case ensure_range(els_range:to_poi_range(Range), UnusedVariable, POIs) of
@@ -89,8 +93,8 @@ action_ignore_variable(Uri, Range, [UnusedVariable]) ->
  []
  end.
 
--spec action_suggest_variable(uri(), range(), [binary()]) -> [map()].
-action_suggest_variable(Uri, Range, [Var]) ->
+-spec action_suggest_variable(uri(), range(), binary(), [binary()]) -> [map()].
+action_suggest_variable(Uri, Range, _Data, [Var]) ->
  %% Supply a quickfix to replace an unbound variable with the most similar
  %% variable name in scope.
  {ok, Document} = els_utils:lookup_document(Uri),
@@ -115,8 +119,8 @@ action_suggest_variable(Uri, Range, [Var]) ->
  []
  end.
 
--spec action_fix_module_name(uri(), range(), [binary()]) -> [map()].
-action_fix_module_name(Uri, Range0, [ModName, FileName]) ->
+-spec action_fix_module_name(uri(), range(), binary(), [binary()]) -> [map()].
+action_fix_module_name(Uri, Range0, _Data, [ModName, FileName]) ->
  {ok, Document} = els_utils:lookup_document(Uri),
  POIs = els_poi:sort(els_dt_document:pois(Document, [module])),
  case ensure_range(els_range:to_poi_range(Range0), ModName, POIs) of
@@ -130,8 +134,8 @@ action_fix_module_name(Uri, Range0, [ModName, FileName]) ->
  []
  end.
 
-- spec action_remove_macro(uri(), range(), [binary()]) -> [map()].
-action_remove_macro(Uri, Range, [Macro]) ->
+- spec action_remove_macro(uri(), range(), binary(), [binary()]) -> [map()].
+action_remove_macro(Uri, Range, _Data, [Macro]) ->
  %% Supply a quickfix to remove the unused Macro
  {ok, Document} = els_utils:lookup_document(Uri),
  POIs = els_poi:sort(els_dt_document:pois(Document, [define])),
@@ -147,6 +151,21 @@ action_remove_macro(Uri, Range, [Macro]) ->
  []
  end.
 
+-spec action_remove_unused(uri(), range(), binary(), [binary()]) -> [map()].
+action_remove_unused(Uri, _Range0, Data, [Import]) ->
+ {ok, Document} = els_utils:lookup_document(Uri),
+ case els_range:inclusion_range(Data, Document) of
+ {ok, UnusedRange} ->
+ LineRange = els_range:line(UnusedRange),
+ [ make_edit_action( Uri
+ , <<"Remove unused -include_lib(", Import/binary, ").">>
+ , ?CODE_ACTION_KIND_QUICKFIX
+ , <<>>
+ , els_protocol:range(LineRange)) ];
+ error ->
+ []
+ end.
+
 -spec ensure_range(poi_range(), binary(), [poi()]) -> {ok, poi_range()} | error.
 ensure_range(#{from := {Line, _}}, SubjectId, POIs) ->
  SubjectAtom = binary_to_atom(SubjectId, utf8),

apps/els_lsp/src/els_diagnostics.erl

@@ -18,6 +18,7 @@
  , source => binary()
  , message := binary()
  , relatedInformation => [related_info()]
+ , data => binary()
  }.
 -type diagnostic_id() :: binary().
 -type related_info() :: #{ location := location()
@@ -48,6 +49,7 @@
  , default_diagnostics/0
  , enabled_diagnostics/0
  , make_diagnostic/4
+ , make_diagnostic/5
  , run_diagnostics/1
  ]).
 
@@ -81,14 +83,25 @@ enabled_diagnostics() ->
  Disabled = maps:get("disabled", Config, []),
  lists:usort((Default ++ valid(Enabled)) -- valid(Disabled)).
 
--spec make_diagnostic(range(), binary(), severity(), binary()) -> diagnostic().
+-spec make_diagnostic(range(), binary(), severity(), binary()) ->
+ diagnostic().
 make_diagnostic(Range, Message, Severity, Source) ->
  #{ range => Range
  , message => Message
  , severity => Severity
  , source => Source
  }.
 
+-spec make_diagnostic(range(), binary(), severity(), binary(), binary())
+ -> diagnostic().
+make_diagnostic(Range, Message, Severity, Source, Data) ->
+ #{ range => Range
+ , message => Message
+ , severity => Severity
+ , source => Source
+ , data => Data
+ }.
+
 -spec run_diagnostics(uri()) -> [pid()].
 run_diagnostics(Uri) ->
  [run_diagnostic(Uri, Id) || Id <- enabled_diagnostics()].

apps/els_lsp/src/els_range.erl

@@ -8,6 +8,7 @@
  , range/1
  , line/1
  , to_poi_range/1
+ , inclusion_range/2
  ]).
 
 -spec compare(poi_range(), poi_range()) -> boolean().
@@ -66,6 +67,19 @@ to_poi_range(#{<<"start">> := Start, <<"end">> := End}) ->
  , to => {LineEnd + 1, CharEnd + 1}
  }.
 
+-spec inclusion_range(uri(), els_dt_document:item()) ->
+ {ok, poi_range()} | error.
+inclusion_range(Uri, Document) ->
+ Path = binary_to_list(els_uri:path(Uri)),
+ case
+ els_compiler_diagnostics:inclusion_range(Path, Document, include) ++
+ els_compiler_diagnostics:inclusion_range(Path, Document, include_lib) of
+ [Range|_] ->
+ {ok, Range};
+ [] ->
+ error
+ end.
+
 -spec plus(pos(), string()) -> pos().
 plus({Line, Column}, String) ->
  {Line, Column + string:length(String)}.

apps/els_lsp/src/els_unused_includes_diagnostics.erl

@@ -41,6 +41,7 @@ run(Uri) ->
  , <<"Unused file: ", (filename:basename(UI))/binary>>
  , ?DIAGNOSTIC_WARNING
  , source()
+ , <<UI/binary>> %% Additional data with complete path
  ) || UI <- UnusedIncludes ]
  end.
 
@@ -112,11 +113,8 @@ expand_includes(Document) ->
 
 -spec inclusion_range(uri(), els_dt_document:item()) -> poi_range().
 inclusion_range(Uri, Document) ->
- Path = binary_to_list(els_uri:path(Uri)),
- case
- els_compiler_diagnostics:inclusion_range(Path, Document, include) ++
- els_compiler_diagnostics:inclusion_range(Path, Document, include_lib) of
- [Range|_] ->
+ case els_range:inclusion_range(Uri, Document) of
+ {ok, Range} ->
  Range;
  _ ->
  #{from => {1, 1}, to => {2, 1}}

apps/els_lsp/test/els_code_action_SUITE.erl

@@ -15,6 +15,7 @@
  , suggest_variable/1
  , fix_module_name/1
  , remove_unused_macro/1
+ , remove_unused_import/1
  ]).
 
 %%==============================================================================
@@ -194,3 +195,33 @@ remove_unused_macro(Config) ->
  ],
  ?assertEqual(Expected, Result),
  ok.
+
+-spec remove_unused_import(config()) -> ok.
+remove_unused_import(Config) ->
+ Uri = ?config(code_action_uri, Config),
+ Range = els_protocol:range(#{from => {?COMMENTS_LINES + 19, 15}
+ , to => {?COMMENTS_LINES + 19, 40}}),
+ LineRange = els_range:line(#{from => {?COMMENTS_LINES + 19, 15}
+ , to => {?COMMENTS_LINES + 19, 40}}),
+ {ok, FileName} = els_utils:find_header(
+ els_utils:filename_to_atom("stdlib/include/assert.hrl")),
+ Diag = #{ message => <<"Unused file: assert.hrl">>
+ , range => Range
+ , severity => 2
+ , source => <<"UnusedIncludes">>
+ , data => FileName
+ },
+ #{result := Result} = els_client:document_codeaction(Uri, Range, [Diag]),
+ Expected =
+ [ #{ edit => #{changes =>
+ #{ binary_to_atom(Uri, utf8) =>
+ [#{ range => els_protocol:range(LineRange)
+ , newText => <<>>
+ }]
+ }}
+ , kind => <<"quickfix">>
+ , title => <<"Remove unused -include_lib(assert.hrl).">>
+ }
+ ],
+ ?assertEqual(Expected, Result),
+ ok.

apps/els_lsp/test/els_diagnostics_SUITE.erl

@@ -598,8 +598,11 @@ unused_includes(_Config) ->
  Path = src_path("diagnostics_unused_includes.erl"),
  Source = <<"UnusedIncludes">>,
  Errors = [],
+ {ok, FileName} = els_utils:find_header(
+ els_utils:filename_to_atom("et/include/et.hrl")),
  Warnings = [#{ message => <<"Unused file: et.hrl">>
  , range => {{3, 0}, {3, 34}}
+ , data => FileName
  }
  ],
  Hints = [],
@@ -610,8 +613,11 @@ unused_includes_compiler_attribute(_Config) ->
  Path = src_path("diagnostics_unused_includes_compiler_attribute.erl"),
  Source = <<"UnusedIncludes">>,
  Errors = [],
+ {ok, FileName} = els_utils:find_header(
+ els_utils:filename_to_atom("kernel/include/file.hrl")),
  Warnings = [ #{ message => <<"Unused file: file.hrl">>
  , range => {{3, 0}, {3, 40}}
+ , data => FileName
  }
  ],
  Hints = [],