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

Get dialyzer warnings working in umbrella apps. #149

Merged
merged 2 commits into from
Mar 12, 2020

Conversation

hworld
Copy link
Contributor

@hworld hworld commented Mar 9, 2020

Should fix #59

I added a simple test, however I couldn't actually get it to fail with the current code, with the current .tool-versions of elixir/erlang in the project. I have tested the fix locally as a vscode extension, though.

Copy link
Member

@axelson axelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good, just one small comment on the code.

Although testing this with Elixir 1.10.2 on an umbrella project results in multiple errors like:

07:09:03.759 [error] Task #PID<0.115.0> started from #PID<0.1955.0> terminating
** (MatchError) no match of right hand side value: false
    (elixir 1.10.2) src/elixir_erl_var.erl:14: :elixir_erl_var.translate/4
    (elixir 1.10.2) src/elixir_erl_pass.erl:464: anonymous fn/5 in :elixir_erl_pass.translate_map/4
    (stdlib 3.10) lists.erl:1354: :lists.mapfoldl/3
    (elixir 1.10.2) src/elixir_erl_pass.erl:462: :elixir_erl_pass.translate_map/4
    (stdlib 3.10) lists.erl:1354: :lists.mapfoldl/3
    (elixir 1.10.2) src/elixir_erl_clauses.erl:20: :elixir_erl_clauses.match/3
    (elixir 1.10.2) src/elixir_erl_clauses.erl:28: :elixir_erl_clauses.clause/6
    (elixir 1.10.2) src/elixir_erl.erl:225: :elixir_erl.translate_clause/3
Function: &:erlang.apply/2
    Args: [#Function<8.15981531/1 in ElixirLS.LanguageServer.Dialyzer.update_stale/4>, [".elixir_ls/build/test/consolidated/Elixir.Inspect.beam"]]

beam files:

  • .elixir_ls/build/test/consolidated/Elixir.Phoenix.Param.beam
  • .elixir_ls/build/test/consolidated/Elixir.Jason.Encoder.beam
  • .elixir_ls/build/test/consolidated/Elixir.Collectable.beam
  • .elixir_ls/build/test/consolidated/Elixir.Inspect.beam
  • .elixir_ls/build/test/consolidated/Elixir.Phoenix.HTML.FormData.beam

In my testing this error doesn't appear on a non-umbrella project on 1.9 or 1.10, or on an umbrella project running 1.9. And the errors do prevent the dialyzer warnings from being reported.

So this error appears to be related to dialyzer and protocol consolidation. It is possible that this is an Elixir bug, although it's quite possible that this is caused by ElixirLS using some private Elixir module/function. This error doesn't necessarily block this PR, but it would be great to resolve it with this PR since I haven't been able to trigger this error without using this PR.

apps/language_server/lib/language_server/dialyzer.ex Outdated Show resolved Hide resolved
@asummers
Copy link
Contributor

@axelson Does that happen on a fresh build of the project? Dialyzer has issues with protocol consolidation but I'm wondering if you just need to clear build or something on the language bump.

@hworld
Copy link
Contributor Author

hworld commented Mar 11, 2020

I've updated the PR with the helper functions below. I've actually written this using Elixir 1.10.2 and OTP 22.2.8 and tested on an umbrella app with 3 apps and quite a few dependencies and it's been working just fine. I'm not sure how to reproduce what you've been seeing @axelson.

@axelson
Copy link
Member

axelson commented Mar 12, 2020

@asummers it appears that you are correct and after a rm -rf .elixir_ls this code is working great!

Thanks @hworld! ❤️

@axelson axelson merged commit 7e9dd6b into elixir-lsp:master Mar 12, 2020
axelson added a commit that referenced this pull request Nov 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ElixirLS Dialyzer Warnings Support for Umbrellas?
3 participants