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

Misleading unused clause warning on 1.19-dev #14401

Open
tmjoen opened this issue Apr 5, 2025 · 4 comments
Open

Misleading unused clause warning on 1.19-dev #14401

tmjoen opened this issue Apr 5, 2025 · 4 comments

Comments

@tmjoen
Copy link

tmjoen commented Apr 5, 2025

Elixir and Erlang/OTP versions

Erlang/OTP 27 [erts-15.2] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit] [dtrace]

Elixir 1.19.0-dev (compiled with Erlang/OTP 27)

(running off main — be17d71)

Operating system

MacOS 14.5

Current behavior

I was testing the new parallel deps compilation in Elixir 1.19-dev main and looking at the deprecation warnings when I got this one:

     warning: this clause of defp maybe_mark_for_deletion/2 is never used
     │
 646defp maybe_mark_for_deletion(%Ecto.Changeset{changes: %{marked_as_deleted: true}} = changeset, module) do
     │        ~
     │
     └─ lib/my_app/blueprint.ex:646:8: MyApp.Blueprint.maybe_mark_for_deletion/2

This is the code:

  defp maybe_mark_for_deletion(%Ecto.Changeset{changes: %{marked_as_deleted: true}} = changeset, module) do
    if module.__allow_mark_as_deleted__ do
      %{changeset | action: :delete}
    else
      changeset
    end
  end

  defp maybe_mark_for_deletion(changeset, _) do
    changeset
  end

It gets called from a run_changeset pipeline → maybe_mark_for_deletion(changeset, module)

My code is in a library and run_changeset gets called with user code, so how would the compiler know that the clause never gets used?

I noticed that module.__allow_mark_as_deleted__ should have parens, since it's a function. When I added them (module.__allow_mark_as_deleted__()), the warning disappeared.

Expected behavior

The function clause is in fact in use, so wouldn't expect that warning.

@josevalim
Copy link
Member

Thank you. Because of the missing parenthesis, we are inferring that it expects a map, and you never give it a map. One potential fix here is to improve the error message to say the types the clause is expecting, and then mention it is never invoked with matching types.

In other words, we are barking at the wrong tree (but we are correct in the barking).

@tmjoen
Copy link
Author

tmjoen commented Apr 5, 2025

Haha, that's a great way of putting it! Thank you for taking a look

@josevalim
Copy link
Member

@tmjoen how did you find it was missing parenthesis? Did you get a warning? I am wondering if we should simply change the message to say "this function is either unused or will emit warnings when invoked". I am worried printing the inferred types can be too confusing (because they can be quite complex).

@tmjoen
Copy link
Author

tmjoen commented Apr 5, 2025

I didn't get a warning about the parentheses, I just spotted it when looking at the function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants