Misleading unused clause warning on 1.20-dev

[tmjoen]

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 — be17d71e450c71acc461ef9b5b2c0da0333d8c5e)

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
     │
 646 │   defp 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]

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]

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

[josevalim]

@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]

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

[josevalim]

FWIW, here is a minimal example to reproduce it:

defmodule Hello do
  def example(changeset) do
    maybe_mark_for_deletion(changeset, OMG)
  end

  defp maybe_mark_for_deletion(%{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
end

[josevalim]

In order to fix this, we will need to break apart the type the function accepts and the type the function actually succeeds.

[josevalim]

We will tone down type inference for the v1.19 release but this bug will be present on the v1.20-dev branch.