Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong "unused record field" when using record field as index #1391

Closed
Alberdi opened this issue Oct 21, 2022 · 0 comments · Fixed by #1392
Closed

Wrong "unused record field" when using record field as index #1391

Alberdi opened this issue Oct 21, 2022 · 0 comments · Fixed by #1392
Labels
bug Something isn't working

Comments

@Alberdi
Copy link

Alberdi commented Oct 21, 2022

Describe the bug
Using a record field as index doesn't count towards marking it as "used".

To Reproduce
Write the following record definition and function:

-record(used_field, {field_c}).

main(R) ->
  element(#used_field.field_c, R).

Expected behavior
No warnings.

Actual behavior
One warning: "Unused record field: #used_field.field_c"

Context

  • erlang_ls version (tag/sha): 0.44.1/b151f9d9a4a1dde1118ae2822226500e2fa69125
  • Editor used: NVIM v0.6.1
  • LSP client used: Nvim LSP client

The following change cause the unused_record_fields test from els_diagnostics_SUITE to fail, even though it shouldn't:

diff --git a/apps/els_lsp/priv/code_navigation/src/diagnostics_unused_record_fields.erl b/apps/els_lsp/priv/code_navigation/src/diagnostics_unused_record_fields.erl
index f10efc7..5ddfacd 100644
--- a/apps/els_lsp/priv/code_navigation/src/diagnostics_unused_record_fields.erl
+++ b/apps/els_lsp/priv/code_navigation/src/diagnostics_unused_record_fields.erl
@@ -8,4 +8,4 @@
 main(#used_field{field_a = A, field_b = B}) ->
   {A, B};
 main(R) ->
-  R#unused_field.field_c.
+  element(#unused_field.field_c, R).

Not knowing anything about the internals of erlang_ls (yet?) I've the tried the following without any effect:

diff --git a/apps/els_lsp/src/els_unused_record_fields_diagnostics.erl b/apps/els_lsp/src/els_unused_record_fields_diagnostics.erl
index 4e92e46..7b2e0e3 100644
--- a/apps/els_lsp/src/els_unused_record_fields_diagnostics.erl
+++ b/apps/els_lsp/src/els_unused_record_fields_diagnostics.erl
@@ -55,7 +55,7 @@ source() ->
 -spec find_unused_record_fields(els_dt_document:item()) -> [els_poi:poi()].
 find_unused_record_fields(Document) ->
     Definitions = els_dt_document:pois(Document, [record_def_field]),
-    Usages = els_dt_document:pois(Document, [record_field]),
+    Usages = els_dt_document:pois(Document, [record_field, record_index]),
     UsagesIds = lists:usort([Id || #{id := Id} <- Usages]),
     [POI || #{id := Id} = POI <- Definitions, not lists:member(Id, UsagesIds)].
@Alberdi Alberdi added the bug Something isn't working label Oct 21, 2022
plux added a commit to plux/erlang_ls that referenced this issue Oct 21, 2022
robertoaloi pushed a commit that referenced this issue Nov 7, 2022
* [#1391] Add support for record index in els_parser

* Fix typo in test suite
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant