Implement redundant clauses for reduce and convert them to warnings

Author: josevalim

lib/elixir/lib/module/types/expr.ex

@@ -426,7 +426,8 @@ defmodule Module.Types.Expr do
       {reduce_type, context} = of_expr(reduce, expected, expr, stack, context)
       # 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.
-      of_clauses(block, [dynamic()], expected, expr, :for_reduce, stack, context, reduce_type)
+      args = [dynamic()]
+      of_redundant_clauses(block, args, expected, :for_reduce, stack, context, reduce_type)
     else
       # TODO: Use the collectable protocol for the output
       into = Keyword.get(opts, :into, [])
@@ -779,8 +780,7 @@ defmodule Module.Types.Expr do
               cond do
                 stack.mode != :infer and previous != [] and
                     Pattern.args_subtype?(clause_type, previous) ->
-                  stack = %{stack | meta: meta}
-                  {previous, Pattern.badpattern_error(clause, nil, info, stack, context)}
+                  {previous, Pattern.badpattern_warn(clause, info, stack, context)}
 
                 precise? ->
                   {[clause_type | previous], context}

lib/elixir/lib/module/types/pattern.ex

@@ -366,6 +366,15 @@ defmodule Module.Types.Pattern do
     error(__MODULE__, error, meta, stack, context)
   end
 
+  @doc """
+  Marks a badpattern warnings.
+  """
+  def badpattern_warn(expr, tag, stack, context) do
+    meta = error_meta(expr, stack)
+    error = {:badpattern, meta, expr, nil, tag, context}
+    warn(__MODULE__, error, meta, stack, context)
+  end
+
   defp error_meta(expr, stack) do
     if meta = get_meta(expr) do
       meta ++ Keyword.take(stack.meta, [:generated, :line, :type_check])
@@ -1366,15 +1375,12 @@ defmodule Module.Types.Pattern do
   # $ type tag = head_pattern() or match_pattern()
   #
   # $ typep head_pattern =
-  #     :for_reduce or :with_else or :fn or :default or
+  #     :with_else or :fn or :default or
   #       {{:case | :try_else, meta, expr, type}, [arg], [previous]} or
-  #       {:receive | :try_catch, [arg], [previous]}
+  #       {:for_reduce | :receive | :try_catch, [arg], [previous]}
   #
   # $ typep match_pattern =
   #     :with or :for or {:match, type}
-  #
-  # The match pattern ones have the whole expression instead
-  # of a single pattern.
   def format_diagnostic({:badpattern, meta, pattern_or_expr, index, tag, context}) do
     # TODO: stop passing pattern_or_expr as argument
     {to_trace, message} = badpattern(tag, pattern_or_expr, index)
@@ -1509,7 +1515,7 @@ defmodule Module.Types.Pattern do
     end
   end
 
-  defp badpattern({op, args, previous}, _, _) when op in [:receive, :try_catch] do
+  defp badpattern({op, args, previous}, _, _) when op in [:receive, :try_catch, :for_reduce] do
     {args,
      """
      the following clause is redundant:

lib/elixir/test/elixir/module/types/expr_test.exs

@@ -1804,13 +1804,14 @@ defmodule Module.Types.ExprTest do
                  dynamic(nil or binary())
              """
 
-      assert typeerror!(
+      assert typewarn!(
                [x],
                case String.to_atom(x) do
                  :ok -> 1
                  :ok -> 2
                end
-             ) == ~l"""
+             )
+             |> elem(1) == ~l"""
              the following clause is redundant:
 
                  :ok ->
@@ -1820,13 +1821,14 @@ defmodule Module.Types.ExprTest do
                  :ok
              """
 
-      assert typeerror!(
+      assert typewarn!(
                [a, b],
                case {a, b} do
                  {x, y} when is_integer(x) and is_integer(y) -> 1
                  {x, y} when is_integer(x) and is_integer(y) -> 2
                end
-             ) =~ ~l"""
+             )
+             |> elem(1) =~ ~l"""
              the following clause is redundant:
 
                  {x, y} when is_integer(x) and is_integer(y) ->
@@ -2043,12 +2045,13 @@ defmodule Module.Types.ExprTest do
     end
 
     test "errors on redundant clauses" do
-      assert typeerror!(
+      assert typewarn!(
                receive do
                  x when is_binary(x) -> x
                  "foo" -> "bar"
                end
-             ) == """
+             )
+             |> elem(1) == """
              the following clause is redundant:
 
                  "foo" ->
@@ -2159,14 +2162,15 @@ defmodule Module.Types.ExprTest do
     end
 
     test "catch: errors on redundant clauses" do
-      assert typeerror!(
+      assert typewarn!(
                try do
                  flunk("whatever")
                catch
                  x when is_binary(x) -> x
                  "foo" -> "bar"
                end
-             ) == """
+             )
+             |> elem(1) == """
              the following clause is redundant:
 
                  :throw, "foo" ->
@@ -2192,7 +2196,7 @@ defmodule Module.Types.ExprTest do
     end
 
     test "else: errors on redundant clauses" do
-      assert typeerror!(
+      assert typewarn!(
                try do
                  Process.get(:x)
                rescue
@@ -2201,7 +2205,8 @@ defmodule Module.Types.ExprTest do
                  x when is_binary(x) -> x
                  "foo" -> "bar"
                end
-             ) == """
+             )
+             |> elem(1) == """
              the following clause is redundant:
 
                  "foo" ->
@@ -2577,6 +2582,25 @@ defmodule Module.Types.ExprTest do
                )
              ) == dynamic(integer())
     end
+
+    test ":reduce errors on redundant clauses" do
+      assert typewarn!(
+               [list, x],
+               for _ <- list, reduce: x do
+                 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
   end
 
   describe "info" do