search

jeremyjh / dialyxir

Mix tasks to simplify use of Dialyzer in Elixir projects.
Apache License 2.0
1.69k stars 141 forks source link

Ignore_warnings file not ignoring warnings after upgrade to OTP 24 #448

Open imricardoramos opened 2 years ago

imricardoramos commented 2 years ago

Precheck

Environment

Elixir 1.12.3 (compiled with Erlang/OTP 24)


* Which version of Dialyxir are you using? (cat mix.lock | grep dialyxir):

"dialyxir": {:hex, :dialyxir, "1.1.0", "c5aab0d6e71e5522e77beff7ba9e08f8e02bad90dfbeffae60eaf0cb47e29488", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "07ea8e49c45f15264ebe6d5b93799d4dd56a44036cf42d0ad9c960bc266c0b9a"},

### Current behavior
**TLDR:** Warnings are emitted even after placing filters in the ignore file.

While working in another big project, we silenced an Dialyzer warning (due to the use of [money ecto type](https://hexdocs.pm/money/Money.Ecto.Composite.Type.html)) through the use of the [ignore_warnings](https://github.com/jeremyjh/dialyxir#ignore-file) file. 

After updating to OTP 24, the warnings showed up again when running `mix dialyzer`, this is preventing us from updating to OTP 24, since it's failing at our CI.

[This repo](https://github.com/imricardoramos/dialyxir-test) attempts to provide a reproducible example of the warning through a wrong spec, although it's not the only warning we're getting (and in a different scenario).

Dialyzer output:

❯ mix dialyzer Finding suitable PLTs Checking PLT... [:compiler, :elixir, :kernel, :logger, :money, :stdlib] PLT is up to date! ignore_warnings: .dialyzer_ignore.exs

Starting Dialyzer [ check_plt: false, init_plt: '.../dialyxir_test/_build/dev/dialyxir_erlang-24.0.2_elixir-1.12.3_deps-dev.plt', files: [...], warnings: [:unknown] ] Total errors: 1, Skipped: 0, Unnecessary Skips: 1 done in 0m1.19s lib/dialyxir_test.ex:19:unknown_type Unknown type: Money.Ecto.Composite.Type.t/0.

done (warnings were emitted) Halting VM with exit status 2

ignore file (we use line 19 for this example, but the actual error is on line 0):

This file should be empty ALWAYS. Use it only for emergencies.

[ {":19:unknown_type Unknown type: Money.Ecto.Composite.Type.t/0."}, ]

mix file:

defmodule DialyxirTest.MixProject do use Mix.Project

def project do [ app: :dialyxir_test, version: "0.1.0", elixir: "~> 1.12", start_permanent: Mix.env() == :prod, deps: deps(), dialyzer: [ ignore_warnings: ".dialyzer_ignore.exs", ] ] end

Run "mix help compile.app" to learn about applications.

def application do [ extra_applications: [:logger] ] end

Run "mix help deps" to learn about dependencies.

defp deps do [ {:money, "~> 1.9.0"}, {:dialyxir, "~> 1.0", only: [:dev], runtime: false},

{:dep_from_hexpm, "~> 0.3.0"},

  # {:dep_from_git, git: "https://github.com/elixir-lang/my_dep.git", tag: "0.1.0"}
]

end end


### Expected behavior
It should ignore the warnings specified in the ignore file.
octowombat commented 2 years ago

I forked the project since I too have this issue when using dialyxir on Elixir 1.13 and OTP24. On my fork I tried different combinations of Elixir/OTP pairings whilst running the CI steps of this project under GitHub actions. The results I got were that the builds fail when the library switches to 1.13

image

and this is the specific log of the job for Elixir 1.13 / OTP 22.3

mix dialyzer --format short
  shell: /usr/bin/bash -e {0}
  env:
    INSTALL_DIR_FOR_OTP: /home/runner/work/_temp/.setup-beam/otp
    INSTALL_DIR_FOR_ELIXIR: /home/runner/work/_temp/.setup-beam/elixir
    MIX_ENV: prod
Finding suitable PLTs
Checking PLT...
[:dialyzer, :elixir, :erlex, :kernel, :mix, :stdlib]
Looking up modules in dialyxir_erlang-22.[3](https://github.com/octowombat/dialyxir/runs/5456638012?check_suite_focus=true#step:11:3).[4](https://github.com/octowombat/dialyxir/runs/5456638012?check_suite_focus=true#step:11:4).24_elixir-1.13.3_deps-prod.plt
Looking up modules in dialyxir_erlang-22.3.4.24_elixir-1.13.3.plt
Looking up modules in dialyxir_erlang-22.3.4.24.plt
Finding applications for dialyxir_erlang-22.3.4.24.plt
Finding modules for dialyxir_erlang-22.3.4.24.plt
Creating dialyxir_erlang-22.3.4.24.plt
Looking up modules in dialyxir_erlang-22.3.4.24.plt
Removing 3 modules from dialyxir_erlang-22.3.4.24.plt
Checking 18 modules in dialyxir_erlang-22.3.4.24.plt
Adding 176 modules to dialyxir_erlang-22.3.4.24.plt
done in 1m2.66s
Finding applications for dialyxir_erlang-22.3.4.24_elixir-1.13.3.plt
Finding modules for dialyxir_erlang-22.3.4.24_elixir-1.13.3.plt
Copying dialyxir_erlang-22.3.4.24.plt to dialyxir_erlang-22.3.4.24_elixir-1.13.3.plt
Looking up modules in dialyxir_erlang-22.3.4.24_elixir-1.13.3.plt
Checking 194 modules in dialyxir_erlang-22.3.4.24_elixir-1.13.3.plt
Adding 247 modules to dialyxir_erlang-22.3.4.24_elixir-1.13.3.plt
done in 0m41.48s
Finding applications for dialyxir_erlang-22.3.4.24_elixir-1.13.3_deps-prod.plt
Finding modules for dialyxir_erlang-22.3.4.24_elixir-1.13.3_deps-prod.plt
Copying dialyxir_erlang-22.3.4.24_elixir-1.13.3.plt to dialyxir_erlang-22.3.4.24_elixir-1.13.3_deps-prod.plt
Looking up modules in dialyxir_erlang-22.3.4.24_elixir-1.13.3_deps-prod.plt
Checking 441 modules in dialyxir_erlang-22.3.4.24_elixir-1.13.3_deps-prod.plt
Adding 121 modules to dialyxir_erlang-22.3.4.24_elixir-1.13.3_deps-prod.plt
done in 0m22.48s
ignore_warnings: .dialyzer_ignore.exs

Starting Dialyzer
[
  check_plt: false,
  init_plt: '/home/runner/work/dialyxir/dialyxir/_build/prod/dialyxir_erlang-22.3.4.24_elixir-1.13.3_deps-prod.plt',
  files: ['/home/runner/work/dialyxir/dialyxir/_build/prod/lib/dialyxir/ebin/Elixir.Dialyxir.Project.beam',
   '/home/runner/work/dialyxir/dialyxir/_build/prod/lib/dialyxir/ebin/Elixir.Dialyxir.Warnings.OverlappingContract.beam',
   '/home/runner/work/dialyxir/dialyxir/_build/prod/lib/dialyxir/ebin/Elixir.Dialyxir.Warnings.FunctionApplicationNoFunction.beam',
   '/home/runner/work/dialyxir/dialyxir/_build/prod/lib/dialyxir/ebin/Elixir.Dialyxir.WarningHelpers.beam',
   '/home/runner/work/dialyxir/dialyxir/_build/prod/lib/dialyxir/ebin/Elixir.Mix.Tasks.Dialyzer.Explain.beam',
   ...],
  warnings: [:unmatched_returns, :error_handling, :underspecs, :unknown]
]
Total errors: 3, Skipped: 2, Unnecessary Skips: 0
done in 0m1.38s
lib/dialyxir/project.ex:4[5](https://github.com/octowombat/dialyxir/runs/5456638012?check_suite_focus=true#step:11:5):unmatched_return The expression produces multiple types, but none are matched.
done (warnings were emitted)
Halting VM with exit status 2
Error: Process completed with exit code 2.

I am going to therefore investigate from the release of 1.13 and the command mix dialyzer --format short.

octowombat commented 2 years ago

From what I have found so far using a fork from this repo and no other changes, just using asdf to install local matrix of OTP and Elixir releases is that when I run mix dialyzer --format short (sticking with OTP 24.2) ;

  1. For Elixir 1.12.3 it produces no warnings on the project at all, notably not the above :unmatched_returns warning and thus the .dialyzer_ignore.exs file is not brought into play.
  2. From Elixir 1.13.0 through to 1.13.2 it produces the warning (in red) lib/dialyxir/project.ex:45:unmatched_return The expression produces multiple types, but none are matched.
  3. If I add the short form or warning as a term into the .dialyzer_ignore.exs verbatim, namely add a line thus {"lib/dialyxir/project.ex:45:unmatched_return The expression produces multiple types, but none are matched."} and re-run mix dialyzer --format short the warning file is honoured and no warning appears.
  4. If I replace this line by the alternative of {"lib/dialyxir/project.ex", :unmatched_return} it equally returns no warnings and the warning file is honoured once again as does adding the line number, {"lib/dialyxir/project.ex", :unmatched_return, 45}.
  5. HOWEVER, if I try to use the shorthand version hinted at in the documentation (which I have to confess I usually always do), {":0:unmatched_return The expression produces multiple types, but none are matched."}, then running mix dialyzer --format short does produce the red warning.

So the question is, in point 1 above, why in the case of this project (dog-fooding itself!) does the warning not appear until Elixir 1.13.x?

The dialyzer block in the mix.exs file has the following:

      dialyzer: [
        plt_apps: [:dialyzer, :elixir, :kernel, :mix, :stdlib, :erlex],
        ignore_warnings: ".dialyzer_ignore.exs",
        flags: [:unmatched_returns, :error_handling, :underspecs]
      ]

which I thought would produce the warning from specifying the flag (in this case :unmatched_returns). Quoting from the Erlang manual:

-Wunmatched_returns () Include warnings for function calls that ignore a structured return value or do not match against one of many possible return values. However, no warnings are included if the possible return values are a union of atoms or a union of numbers. denotes options that turn on warnings rather than turning them off.

Let's look more closely then at this case of Project.ex:45. The line in question is as follows

Mix.Tasks.Deps.Loadpaths.run([])

When we use Elixir 1.13.x, this produces a list of paths to dependencies such as 

["/Volumes/Development/projects/dialyxir/_build/dev/lib/earmark_parser/ebin",
 "/Volumes/Development/projects/dialyxir/_build/dev/lib/erlex/ebin",
 "/Volumes/Development/projects/dialyxir/_build/dev/lib/nimble_parsec/ebin",
 "/Volumes/Development/projects/dialyxir/_build/dev/lib/makeup/ebin",
 "/Volumes/Development/projects/dialyxir/_build/dev/lib/makeup_elixir/ebin",
 "/Volumes/Development/projects/dialyxir/_build/dev/lib/ex_doc/ebin"]

BUT when using Elixir 1.12.3 (for example), it produces simply :ok which means it must have only atom return values according to the above Erlang docs so as not to produce the dialyzer warning in this case. This change came about in Elixir 1.13 from my reading of the git blame results.

So for the dog-food example of dialyxir itself, this change exposes a warning that wasn't exposed previously and as such the CI on this project will fail unless we add the correct form of warning into the ignore file (assuming of course we want to ignore the warning). And for the rest of us, the changes to Elixir 1.13 may also expose new warnings and if we want to ignore them now, we can use mix dialyzer --format short or mix dialyzer --format ignore_file and grab the correct form of entry to add to our .dialyzer_ignore.exs files.

I hope this helps!

dennym commented 1 year ago

Any progress on this one? For me my dialyzer is acting up with ExUnit when running in MIX_ENV=test. For now I can find an entry for my ignore dotfile via mix dialyzer --format ignore_file but they basically just ignore a quite generic error {"test/support/data_case.ex", :unknown_function}. Tested on OTP25 with 1.13.2 and 1.14.

connorjacobsen commented 1 year ago

Does this change fix it for you? https://github.com/jeremyjh/dialyxir/issues/444#issuecomment-940223772

If so, I have a fork with the change implemented here: https://github.com/connorjacobsen/dialyxir/commit/499b54b4872ca396002ec517e46c8d602c1e113d

Nezteb commented 1 year ago

This doesn't seem to be an issue for me using:

❯ cat .tool-versions 
elixir 1.14.1-otp-25
erlang 25.1.1

We have Dialyxir running in CI using an ignore file and everything works as expected. 🤔

jeremyjh commented 1 year ago

@nezteb It looks like it may be an issue with ignore files using --format dialyzer in a dialyzer.ignore-warnings file ? But not in the term format in an exs file.