Warns on redundant clauses from fn

josevalim · josevalim · commit aed240bf4071 · 2026-02-27T11:12:22.000+01:00
diff --git a/lib/elixir/lib/module/types/expr.ex b/lib/elixir/lib/module/types/expr.ex
@@ -325,7 +325,7 @@ defmodule Module.Types.Expr do
     else
       clauses
     end
-    |> of_redundant_clauses([case_type], expected, info, stack, context, none())
+    |> of_clauses([case_type], expected, info, stack, context, none())
     |> dynamic_unless_static(stack)
   end
 
@@ -336,10 +336,9 @@ defmodule Module.Types.Expr do
     domain = Enum.map(patterns, fn _ -> dynamic() end)
 
     {acc, context} =
-      of_clauses_fun(clauses, domain, @pending, nil, :fn, stack, context, [], fn
-        trees, body, context, acc ->
-          args = Pattern.of_domain(trees, stack, context)
-          add_inferred(acc, args, body)
+      of_clauses_fun(clauses, domain, @pending, :fn, stack, context, [], fn
+        args_types, body, acc ->
+          add_inferred(acc, args_types, body)
       end)
 
     {fun_from_inferred_clauses(acc), context}
@@ -353,7 +352,7 @@ defmodule Module.Types.Expr do
       if else_block do
         {type, context} = of_expr(body, @pending, body, stack, original)
         info = {:try_else, meta, body, type}
-        of_redundant_clauses(else_block, [type], expected, info, stack, context, none())
+        of_clauses(else_block, [type], expected, info, stack, context, none())
       else
         of_expr(body, expected, expr, stack, original)
       end
@@ -378,7 +377,7 @@ defmodule Module.Types.Expr do
 
         {:catch, clauses}, {acc, context} ->
           args = [@try_catch, dynamic()]
-          of_redundant_clauses(clauses, args, expected, :try_catch, stack, context, acc)
+          of_clauses(clauses, args, expected, :try_catch, stack, context, acc)
       end)
       |> dynamic_unless_static(stack)
 
@@ -399,7 +398,7 @@ defmodule Module.Types.Expr do
         acc_context
 
       {:do, clauses}, {acc, context} ->
-        of_redundant_clauses(clauses, [dynamic()], expected, :receive, stack, context, acc)
+        of_clauses(clauses, [dynamic()], expected, :receive, stack, context, acc)
 
       {:after, [{:->, meta, [[timeout], body]}] = after_expr}, {acc, context} ->
         {timeout_type, context} = of_expr(timeout, @timeout_type, after_expr, stack, context)
@@ -427,7 +426,7 @@ defmodule Module.Types.Expr do
       # TODO: We need to type check against dynamic() instead of using reduce_type
       # because this is recursive. We need to infer the block type first.
       args = [dynamic()]
-      of_redundant_clauses(block, args, expected, :for_reduce, stack, context, reduce_type)
+      of_clauses(block, args, expected, :for_reduce, stack, context, reduce_type)
     else
       # TODO: Use the collectable protocol for the output
       into = Keyword.get(opts, :into, [])
@@ -683,7 +682,7 @@ defmodule Module.Types.Expr do
 
   defp with_option({:else, clauses}, stack, context, _original) do
     {_, context} =
-      of_clauses(clauses, [dynamic()], @pending, nil, :with_else, stack, context, none())
+      of_clauses(clauses, [dynamic()], @pending, :with_else, stack, context, none())
 
     context
   end
@@ -717,29 +716,12 @@ defmodule Module.Types.Expr do
   defp dynamic_unless_static({_, _} = output, %{mode: :static}), do: output
   defp dynamic_unless_static({type, context}, %{mode: _}), do: {dynamic(type), context}
 
-  defp of_clauses(clauses, domain, expected, expr, info, stack, context, acc) do
-    fun = fn _trees, result, _context, acc -> union(result, acc) end
-    of_clauses_fun(clauses, domain, expected, expr, info, stack, context, acc, fun)
+  defp of_clauses(clauses, domain, expected, clause_info, stack, context, acc) do
+    fun = fn _args_types, result, acc -> union(result, acc) end
+    of_clauses_fun(clauses, domain, expected, clause_info, stack, context, acc, fun)
   end
 
-  defp of_clauses_fun(clauses, domain, expected, expr, info, stack, original, acc, fun) do
-    %{failed: failed?} = original
-
-    Enum.reduce(clauses, {acc, original}, fn {:->, meta, [head, body]}, {acc, context} ->
-      {failed?, context} = reset_failed(context, failed?)
-      {patterns, guards} = extract_head(head)
-
-      {trees, _precise?, context} =
-        Pattern.of_head(patterns, guards, domain, info, meta, stack, context)
-
-      {result, context} = of_expr(body, expected, expr || body, stack, context)
-
-      {fun.(trees, result, context, acc),
-       context |> set_failed(failed?) |> Of.reset_vars(original)}
-    end)
-  end
-
-  defp of_redundant_clauses(clauses, domain, expected, clause_info, stack, original, acc) do
+  defp of_clauses_fun(clauses, domain, expected, clause_info, stack, original, acc, fun) do
     %{failed: failed?} = original
 
     {result, _previous, context} =
@@ -766,24 +748,24 @@ defmodule Module.Types.Expr do
               {trees, precise?, context}
             end
 
+          args_types =
+            Enum.map(trees, fn {tree, _, _} ->
+              Pattern.of_pattern_tree(tree, stack, context)
+            end)
+
           {previous, context} =
             if context.failed do
               {previous, context}
             else
-              clause_type =
-                Enum.map(trees, fn {tree, _, _} ->
-                  tree
-                  |> Pattern.of_pattern_tree(stack, context)
-                  |> upper_bound()
-                end)
+              upper_types = Enum.map(args_types, &upper_bound/1)
 
               cond do
                 stack.mode != :infer and previous != [] and
-                    Pattern.args_subtype?(clause_type, previous) ->
+                    Pattern.args_subtype?(upper_types, previous) ->
                   {previous, Pattern.badpattern_warn(clause, info, stack, context)}
 
                 precise? ->
-                  {[clause_type | previous], context}
+                  {[upper_types | previous], context}
 
                 true ->
                   {previous, context}
@@ -792,7 +774,7 @@ defmodule Module.Types.Expr do
 
           {result, context} = of_expr(body, expected, body, stack, context)
 
-          {union(result, acc), previous,
+          {fun.(args_types, result, acc), previous,
            context |> set_failed(failed?) |> Of.reset_vars(original)}
       end)
 
diff --git a/lib/elixir/lib/module/types/pattern.ex b/lib/elixir/lib/module/types/pattern.ex
@@ -1375,9 +1375,9 @@ defmodule Module.Types.Pattern do
   # $ type tag = head_pattern() or match_pattern()
   #
   # $ typep head_pattern =
-  #     :with_else or :fn or :default or
+  #     :fn or :default or
   #       {{:case | :try_else, meta, expr, type}, [arg], [previous]} or
-  #       {:for_reduce | :receive | :try_catch, [arg], [previous]}
+  #       {:for_reduce | :receive | :try_catch | :with_else | :fn, [arg], [previous]}
   #
   # $ typep match_pattern =
   #     :with or :for or {:match, type}
@@ -1515,7 +1515,8 @@ defmodule Module.Types.Pattern do
     end
   end
 
-  defp badpattern({op, args, previous}, _, _) when op in [:receive, :try_catch, :for_reduce] do
+  defp badpattern({op, args, previous}, _, _)
+       when op in [:receive, :try_catch, :for_reduce, :with_else, :fn] do
     {args,
      """
      the following clause is redundant:
diff --git a/lib/elixir/test/elixir/module/types/expr_test.exs b/lib/elixir/test/elixir/module/types/expr_test.exs
@@ -187,7 +187,7 @@ defmodule Module.Types.ExprTest do
                [map],
                (fn
                   %{a: a} = data -> %{data | b: a}
-                  data -> data
+                  %{} = data -> data
                 end).(map)
              ) == dynamic()
 
@@ -202,6 +202,22 @@ defmodule Module.Types.ExprTest do
              ) == dynamic(atom([:ok, :error]))
     end
 
+    test "warns on redundant clauses" do
+      assert typewarn!(fn
+               x when is_binary(x) -> x
+               "foo" -> "bar"
+             end)
+             |> elem(1) == """
+             the following clause is redundant:
+
+                 "foo" ->
+
+             previous clauses have already matched on the following types:
+
+                 binary()
+             """
+    end
+
     test "bad function" do
       assert typeerror!([%x{}, a1, a2], x.(a1, a2)) == ~l"""
              expected a 2-arity function on function call:
@@ -1782,7 +1798,7 @@ defmodule Module.Types.ExprTest do
                atom([:non_empty_map, :maybe_empty_map])
     end
 
-    test "reports error from redundant clauses" do
+    test "warns on redundant clauses" do
       assert typeerror!(
                [x],
                case System.get_env(x) do
@@ -2044,7 +2060,7 @@ defmodule Module.Types.ExprTest do
              ) == union(atom([:ok]), dynamic(tuple([atom([:other]), negation(binary())])))
     end
 
-    test "errors on redundant clauses" do
+    test "warns on redundant clauses" do
       assert typewarn!(
                receive do
                  x when is_binary(x) -> x
@@ -2161,7 +2177,7 @@ defmodule Module.Types.ExprTest do
              ) == union(atom([:ok]), dynamic(tuple([atom([:other]), negation(binary())])))
     end
 
-    test "catch: errors on redundant clauses" do
+    test "catch: warns on redundant clauses" do
       assert typewarn!(
                try do
                  flunk("whatever")
@@ -2195,7 +2211,7 @@ defmodule Module.Types.ExprTest do
                union(atom([:ok, :unused]), dynamic(tuple([atom([:other]), negation(binary())])))
     end
 
-    test "else: errors on redundant clauses" do
+    test "else: warns on redundant clauses" do
       assert typewarn!(
                try do
                  Process.get(:x)
@@ -2583,7 +2599,7 @@ defmodule Module.Types.ExprTest do
              ) == dynamic(integer())
     end
 
-    test ":reduce errors on redundant clauses" do
+    test ":reduce warns on redundant clauses" do
       assert typewarn!(
                [list, x],
                for _ <- list, reduce: x do
@@ -2603,6 +2619,29 @@ defmodule Module.Types.ExprTest do
     end
   end
 
+  describe "with" do
+    test "warns on redundant clauses in else" do
+      assert typewarn!(
+               [x],
+               with false <- x do
+                 x
+               else
+                 x when is_binary(x) -> x
+                 "foo" -> "bar"
+               end
+             )
+             |> elem(1) == ~l"""
+             the following clause is redundant:
+
+                 "foo" ->
+
+             previous clauses have already matched on the following types:
+
+                 binary()
+             """
+    end
+  end
+
   describe "info" do
     test "__info__/1" do
       assert typecheck!(GenServer.__info__(:functions)) == list(tuple([atom(), integer()]))
