Merge pull request #1099 from erlang-ls/1098-detect-unused-record-fields

[#1098] Detect unused record fields

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

@@ -0,0 +1,11 @@
+-module(diagnostics_unused_record_fields).
+
+-export([main/1]).
+
+-record(used_field, {field_a, field_b = 42}).
+-record(unused_field, {field_c, field_d}).
+
+main(#used_field{field_a = A, field_b = B}) ->
+  {A, B};
+main(R) ->
+  R#unused_field.field_c.

apps/els_lsp/src/els_diagnostics.erl

@@ -64,6 +64,7 @@ available_diagnostics() ->
   , <<"elvis">>
   , <<"unused_includes">>
   , <<"unused_macros">>
+  , <<"unused_record_fields">>
   ].
 
 -spec default_diagnostics() -> [diagnostic_id()].

apps/els_lsp/src/els_unused_record_fields_diagnostics.erl

@@ -0,0 +1,68 @@
+%%==============================================================================
+%% Unused Includes diagnostics
+%%==============================================================================
+-module(els_unused_record_fields_diagnostics).
+
+%%==============================================================================
+%% Behaviours
+%%==============================================================================
+-behaviour(els_diagnostics).
+
+%%==============================================================================
+%% Exports
+%%==============================================================================
+-export([ is_default/0
+        , run/1
+        , source/0
+        ]).
+
+%%==============================================================================
+%% Includes
+%%==============================================================================
+-include("els_lsp.hrl").
+
+%%==============================================================================
+%% Callback Functions
+%%==============================================================================
+
+-spec is_default() -> boolean().
+is_default() ->
+  true.
+
+-spec run(uri()) -> [els_diagnostics:diagnostic()].
+run(Uri) ->
+  case filename:extension(Uri) of
+    <<".erl">> ->
+      case els_utils:lookup_document(Uri) of
+        {error, _Error} ->
+          [];
+        {ok, Document} ->
+          UnusedRecordFields = find_unused_record_fields(Document),
+          [make_diagnostic(POI) || POI <- UnusedRecordFields ]
+      end;
+    _ ->
+      []
+  end.
+
+-spec source() -> binary().
+source() ->
+  <<"UnusedRecordFields">>.
+
+%%==============================================================================
+%% Internal Functions
+%%==============================================================================
+-spec find_unused_record_fields(els_dt_document:item()) -> [poi()].
+find_unused_record_fields(Document) ->
+  Definitions = els_dt_document:pois(Document, [record_def_field]),
+  Usages = els_dt_document:pois(Document, [record_field]),
+  UsagesIds = lists:usort([Id || #{id := Id} <- Usages]),
+  [POI || #{id := Id} = POI <- Definitions, not lists:member(Id, UsagesIds)].
+
+-spec make_diagnostic(poi()) -> els_diagnostics:diagnostic().
+make_diagnostic(#{id := {RecName, RecField}, range := POIRange}) ->
+  Range = els_protocol:range(POIRange),
+  FullName = els_utils:to_binary(io_lib:format("#~p.~p", [RecName, RecField])),
+  Message = <<"Unused record field: ", FullName/binary>>,
+  Severity = ?DIAGNOSTIC_WARNING,
+  Source = source(),
+  els_diagnostics:make_diagnostic(Range, Message, Severity, Source).

apps/els_lsp/test/els_diagnostics_SUITE.erl

@@ -38,6 +38,7 @@
         , unused_includes_compiler_attribute/1
         , exclude_unused_includes/1
         , unused_macros/1
+        , unused_record_fields/1
         ]).
 
 %%==============================================================================
@@ -279,10 +280,12 @@ compiler_with_broken_behaviour(Config) ->
   els_mock_diagnostics:subscribe(),
   ok = els_client:did_save(Uri),
   Diagnostics = els_mock_diagnostics:wait_until_complete(),
-  ?assertEqual(24, length(Diagnostics)),
-  Warnings = [D || #{severity := ?DIAGNOSTIC_WARNING} = D <- Diagnostics],
-  ?assertEqual(18, length(Warnings)),
-  Errors = [D || #{severity := ?DIAGNOSTIC_ERROR} = D <- Diagnostics],
+  CompilerDiagnostics = [D || #{source := <<"Compiler">>} = D <- Diagnostics],
+  ?assertEqual(22, length(CompilerDiagnostics)),
+  Warnings = [D || #{severity := ?DIAGNOSTIC_WARNING} = D
+                     <- CompilerDiagnostics],
+  ?assertEqual(16, length(Warnings)),
+  Errors = [D || #{severity := ?DIAGNOSTIC_ERROR} = D <- CompilerDiagnostics],
   ?assertEqual(6, length(Errors)),
   [BehaviourError | _ ] = Errors,
   ExpectedError =
@@ -608,18 +611,7 @@ crossref(Config) ->
                      , start => #{character => 2, line => 6}}
                 , severity => 1
                 , source => <<"CrossRef">>
-                }] ++
-             fixcolumns(
-               [#{ message =>
-                       <<"function non_existing/0 undefined">>
-                 , range =>
-                       #{ 'end' => #{character => 14, line => 6}
-                        , start => #{character => 2, line => 6}}
-                 , severity => 1
-                 , source => <<"Compiler">>
-                 , code => <<"L1227">>
-                 }
-               ], Config),
+                }],
   do_crossref_test(Config, diagnostics_xref_uri, Expected).
 
 
@@ -629,8 +621,9 @@ do_crossref_test(Config, TestModule, ExpectedDiagnostics) ->
   els_mock_diagnostics:subscribe(),
   ok = els_client:did_save(Uri),
   Diagnostics = els_mock_diagnostics:wait_until_complete(),
+  CrossRefDiagnostics = [D || #{source := <<"CrossRef">>} = D <- Diagnostics],
   F = fun(#{message := M1}, #{message := M2}) -> M1 =< M2 end,
-  ?assertEqual(ExpectedDiagnostics, lists:sort(F, Diagnostics)),
+  ?assertEqual(ExpectedDiagnostics, lists:sort(F, CrossRefDiagnostics)),
   ok.
 
 %% #641
@@ -674,14 +667,7 @@ crossref_autoimport_disabled(Config) ->
   %% This testcase cannot be run from an Erlang source tree version,
   %% it needs a released version.
 
-  Expected =
-    fixcolumns(
-      [#{message =>
-         <<"function atom_to_list/1 undefined">>,
-       range =>
-         #{'end' => #{character => 22, line => 6},
-           start => #{character => 4, line => 6}},
-       severity => 1, source => <<"Compiler">>, code => <<"L1227">>}], Config),
+  Expected = [],
   do_crossref_test(Config, diagnostics_autoimport_disabled_uri, Expected).
 
 -spec unused_includes(config()) -> ok.
@@ -773,6 +759,25 @@ unused_macros(Config) ->
   ?assertEqual(Expected, lists:sort(F, Diagnostics)),
   ok.
 
+-spec unused_record_fields(config()) -> ok.
+unused_record_fields(Config) ->
+  Uri = ?config(diagnostics_unused_record_fields_uri, Config),
+  els_mock_diagnostics:subscribe(),
+  ok = els_client:did_save(Uri),
+  Diagnostics = els_mock_diagnostics:wait_until_complete(),
+  Expected = [ #{ message => <<"Unused record field: #unused_field.field_d">>
+                , range =>
+                    #{ 'end' => #{character => 39, line => 5}
+                     , start => #{character => 32, line => 5}
+                     }
+                , severity => 2
+                , source => <<"UnusedRecordFields">>
+                }
+             ],
+  F = fun(#{message := M1}, #{message := M2}) -> M1 =< M2 end,
+  ?assertEqual(Expected, lists:sort(F, Diagnostics)),
+  ok.
+
 %%==============================================================================
 %% Internal Functions
 %%==============================================================================

apps/els_lsp/test/els_initialization_SUITE.erl

@@ -128,6 +128,7 @@ initialize_diagnostics_custom(Config) ->
              , <<"dialyzer">>
              , <<"unused_includes">>
              , <<"unused_macros">>
+             , <<"unused_record_fields">>
              ],
   Result = els_diagnostics:enabled_diagnostics(),
   ?assertEqual(Expected, Result),
@@ -148,6 +149,7 @@ initialize_diagnostics_invalid(Config) ->
              , <<"elvis">>
              , <<"unused_includes">>
              , <<"unused_macros">>
+             , <<"unused_record_fields">>
              ],
   ?assertEqual(Expected, Result),
   ok.

apps/els_lsp/test/els_test_utils.erl

@@ -156,6 +156,7 @@ sources() ->
   , diagnostics_unused_includes
   , diagnostics_unused_includes_compiler_attribute
   , diagnostics_unused_macros
+  , diagnostics_unused_record_fields
   , elvis_diagnostics
   , execute_command_suggest_spec
   , format_input