Skip to content

Commit

Permalink
Avoid false warnings on private clauses that raise (return none())
Browse files Browse the repository at this point in the history
  • Loading branch information
josevalim committed Jan 16, 2025
1 parent 6df3c08 commit 6a96204
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 12 deletions.
21 changes: 19 additions & 2 deletions lib/elixir/lib/module/types.ex
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,13 @@ defmodule Module.Types do
not Keyword.get(meta, :from_super, false),
reduce: context do
context ->
{_kind, _inferred, mapping} = Map.fetch!(context.local_sigs, fun_arity)
{_kind, info, mapping} = Map.fetch!(context.local_sigs, fun_arity)

clauses_indexes =
for type_index <- pending, {clause_index, ^type_index} <- mapping, do: clause_index
for type_index <- pending,
not skip_unused_clause?(info, type_index),
{clause_index, ^type_index} <- mapping,
do: clause_index

Enum.reduce(clauses_indexes, context, fn clause_index, context ->
{meta, _args, _guards, _body} = Enum.fetch!(clauses, clause_index)
Expand All @@ -244,6 +247,20 @@ defmodule Module.Types do
end
end

defp skip_unused_clause?(info, type_index) do
case info do
# If an inferred clause returns an empty type, then the reverse arrow
# will never propagate its domain up, which may lead to the clause never
# being invoked.
{:infer, _, inferred} ->
{_args_types, return} = Enum.fetch!(inferred, type_index)
Descr.empty?(return)

_ ->
false
end
end

defp local_handler(_meta, fun_arity, stack, context, finder) do
case context.local_sigs do
%{^fun_arity => {kind, inferred, _mapping}} ->
Expand Down
28 changes: 18 additions & 10 deletions lib/elixir/test/elixir/module/types/integration_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -361,20 +361,22 @@ defmodule Module.Types.IntegrationTest do
assert_warnings(files, warnings)
end

test "unused generated private clauses" do
test "unused overridable private clauses" do
files = %{
"a.ex" => """
defmodule A do
use B
def public(x), do: private(List.to_tuple(x))
def public(x), do: private(x)
defp private(x), do: super(List.to_tuple(x))
end
""",
"b.ex" => """
defmodule B do
defmacro __using__(_) do
quote generated: true do
quote do
defp private({:ok, ok}), do: ok
defp private(:error), do: :error
defoverridable private: 1
end
end
end
Expand All @@ -384,22 +386,28 @@ defmodule Module.Types.IntegrationTest do
assert_no_warnings(files)
end

test "unused overridable private clauses" do
test "unused private clauses without warnings" do
files = %{
"a.ex" => """
defmodule A do
use B
def public(x), do: private(x)
defp private(x), do: super(List.to_tuple(x))
# Not all clauses are invoked, but do not warn since they are generated
def public1(x), do: generated(List.to_tuple(x))
# Avoid false positives caused by inference
def public2(x), do: (:ok = raising_private(x))
defp raising_private(true), do: :ok
defp raising_private(false), do: raise "oops"
end
""",
"b.ex" => """
defmodule B do
defmacro __using__(_) do
quote do
defp private({:ok, ok}), do: ok
defp private(:error), do: :error
defoverridable private: 1
quote generated: true do
defp generated({:ok, ok}), do: ok
defp generated(:error), do: :error
end
end
end
Expand Down

0 comments on commit 6a96204

Please sign in to comment.