Compute redundant clauses for try-catch and try-else

Author: josevalim

lib/elixir/lib/module/types/descr.ex

@@ -254,9 +254,6 @@ defmodule Module.Types.Descr do
   * For gradual tuple types: processes both dynamic and static components separately,
     then combines them.
 
-  The list of arguments can be flattened into a broad domain by calling:
-
-      |> Enum.zip_with(fn types -> Enum.reduce(types, &union/2) end)
   """
   def domain_to_args(descr) do
     case :maps.take(:dynamic, descr) do
@@ -269,19 +266,23 @@ defmodule Module.Types.Descr do
     end
   end
 
+  @doc """
+  Converts the domain to a flatten list of arguments.
+  """
+  def domain_to_flat_args(domain, arity_or_args) do
+    case domain_to_args(domain) do
+      [] when is_integer(arity_or_args) -> List.duplicate(none(), arity_or_args)
+      [] when is_list(arity_or_args) -> Enum.map(arity_or_args, fn _ -> none() end)
+      args -> Enum.zip_with(args, fn types -> Enum.reduce(types, &union/2) end)
+    end
+  end
+
   defp unwrap_domain_tuple(%{tuple: bdd} = descr, transform) when map_size(descr) == 1 do
     tuple_bdd_to_dnf(bdd) |> Enum.map(transform)
   end
 
   defp unwrap_domain_tuple(descr, _transform) when descr == %{}, do: []
 
-  defp domain_to_flat_args(domain, arity) do
-    case domain_to_args(domain) do
-      [] -> List.duplicate(none(), arity)
-      args -> Enum.zip_with(args, fn types -> Enum.reduce(types, &union/2) end)
-    end
-  end
-
   ## Optional
 
   # `not_set()` is a special base type that represents a not_set field in a map.

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

@@ -308,7 +308,7 @@ defmodule Module.Types.Expr do
 
   def of_expr({:case, meta, [case_expr, [{:do, clauses}]]}, expected, _expr, stack, context) do
     {case_type, context} = of_expr(case_expr, @pending, case_expr, stack, context)
-    info = {:case, meta, case_expr}
+    info = {:case, meta, case_expr, case_type}
 
     added_meta =
       if Macro.quoted_literal?(case_expr) do
@@ -325,7 +325,7 @@ defmodule Module.Types.Expr do
     else
       clauses
     end
-    |> of_redundant_clauses(case_type, expected, info, stack, context, none())
+    |> of_redundant_clauses([case_type], expected, info, stack, context, none())
     |> dynamic_unless_static(stack)
   end
 
@@ -345,15 +345,15 @@ defmodule Module.Types.Expr do
     {fun_from_inferred_clauses(acc), context}
   end
 
-  def of_expr({:try, _meta, [[do: body] ++ blocks]}, expected, expr, stack, original) do
+  def of_expr({:try, meta, [[do: body] ++ blocks]}, expected, expr, stack, original) do
     {after_block, blocks} = Keyword.pop(blocks, :after)
     {else_block, blocks} = Keyword.pop(blocks, :else)
 
     {type, context} =
       if else_block do
         {type, context} = of_expr(body, @pending, body, stack, original)
-        info = {:try_else, type}
-        of_clauses(else_block, [type], expected, expr, info, stack, context, none())
+        info = {:try_else, meta, body, type}
+        of_redundant_clauses(else_block, [type], expected, info, stack, context, none())
       else
         of_expr(body, expected, expr, stack, original)
       end
@@ -378,7 +378,7 @@ defmodule Module.Types.Expr do
 
         {:catch, clauses}, {acc, context} ->
           args = [@try_catch, dynamic()]
-          of_clauses(clauses, args, expected, expr, :try_catch, stack, context, acc)
+          of_redundant_clauses(clauses, args, expected, :try_catch, stack, context, acc)
       end)
       |> dynamic_unless_static(stack)
 
@@ -399,7 +399,7 @@ defmodule Module.Types.Expr do
         acc_context
 
       {:do, clauses}, {acc, context} ->
-        of_redundant_clauses(clauses, dynamic(), expected, :receive, stack, context, acc)
+        of_redundant_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)
@@ -742,23 +742,22 @@ defmodule Module.Types.Expr do
     %{failed: failed?} = original
 
     {result, _previous, context} =
-      Enum.reduce(clauses, {acc, none(), original}, fn
-        {:->, meta, [[clause] = head, body]}, {acc, previous, context} ->
+      Enum.reduce(clauses, {acc, [], original}, fn
+        {:->, meta, [head, body]} = clause, {acc, previous, context} ->
           {failed?, context} = reset_failed(context, failed?)
           {patterns, guards} = extract_head(head)
-
-          info = {clause_info, [domain], [previous]}
+          info = {clause_info, head, previous}
 
           {trees, precise?, context} =
-            Pattern.of_head(patterns, guards, [domain], [previous], info, meta, stack, context)
+            Pattern.of_head(patterns, guards, domain, previous, info, meta, stack, context)
 
           # It failed, let's try to detect if it was due a previous or current clause.
           # The current clause will be easier to understand, so we prefer that
-          {[{clause_tree, _, _}], precise?, context} =
-            if context.failed and previous != none() do
-              info = {clause_info, [domain], [none()]}
+          {trees, precise?, context} =
+            if context.failed and previous != [] do
+              info = {clause_info, head, []}
 
-              case Pattern.of_head(patterns, guards, [domain], info, meta, stack, context) do
+              case Pattern.of_head(patterns, guards, domain, info, meta, stack, context) do
                 {_, _, %{failed: true}} = result -> result
                 _ -> {trees, precise?, context}
               end
@@ -770,15 +769,21 @@ defmodule Module.Types.Expr do
             if context.failed do
               {previous, context}
             else
-              clause_type = Pattern.of_pattern_tree(clause_tree, stack, context) |> upper_bound()
+              clause_type =
+                Enum.map(trees, fn {tree, _, _} ->
+                  tree
+                  |> Pattern.of_pattern_tree(stack, context)
+                  |> upper_bound()
+                end)
 
               cond do
-                stack.mode != :infer and previous != none() and subtype?(clause_type, previous) ->
+                stack.mode != :infer and previous != [] and
+                    Pattern.args_subtype?(clause_type, previous) ->
                   stack = %{stack | meta: meta}
-                  {previous, Pattern.badpattern_error(clause, 0, info, stack, context)}
+                  {previous, Pattern.badpattern_error(clause, nil, info, stack, context)}
 
                 precise? ->
-                  {union(previous, clause_type), context}
+                  {[clause_type | previous], context}
 
                 true ->
                   {previous, context}

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

@@ -8,6 +8,22 @@ defmodule Module.Types.Pattern do
   alias Module.Types.{Apply, Of}
   import Module.Types.{Helpers, Descr}
 
+  @doc """
+  Defines if two list of arguments are subtypes of each other.
+  """
+  def args_subtype?([], []),
+    do: true
+
+  def args_subtype?([type], previous),
+    do: subtype?(type, Enum.reduce(previous, none(), &union(&2, hd(&1))))
+
+  def args_subtype?(args, previous) do
+    subtype?(
+      args_to_domain(args),
+      Enum.reduce(previous, none(), &union(&2, args_to_domain(&1)))
+    )
+  end
+
   @doc """
   Refine the dependencies of variables represented by version.
   """
@@ -127,7 +143,7 @@ defmodule Module.Types.Pattern do
   4. Then we propagate all dependencies to refine variables
 
   """
-  def of_head(patterns, guards, expected, previous \\ nil, tag, meta, stack, context) do
+  def of_head(patterns, guards, expected, previous \\ [], tag, meta, stack, context) do
     %{vars: vars} = context
     stack = %{stack | meta: meta}
 
@@ -216,7 +232,7 @@ defmodule Module.Types.Pattern do
 
     with {:ok, types} <- of_pattern_intersect(args, 0, [], pattern_info, tag, stack, context),
          {[type], changed, context} <-
-           of_pattern_refine(types, nil, pattern_info, tag, stack, context) do
+           of_pattern_refine(types, [], pattern_info, tag, stack, context) do
       {type, of_changed(changed, stack, context)}
     else
       {:error, context} -> {expected, context}
@@ -237,7 +253,7 @@ defmodule Module.Types.Pattern do
 
     with {:ok, types} <- of_pattern_intersect(args, 0, [], pattern_info, tag, stack, context),
          {_types, changed, context} <-
-           of_pattern_refine(types, nil, pattern_info, tag, stack, context) do
+           of_pattern_refine(types, [], pattern_info, tag, stack, context) do
       {_precise?, context} = of_guards(guards, changed, vars, stack, context)
       context
     else
@@ -265,13 +281,17 @@ defmodule Module.Types.Pattern do
 
   defp of_pattern_refine(types, previous, pattern_info, tag, stack, context) do
     types =
-      case previous do
-        nil ->
+      case types do
+        _ when previous == [] ->
           types
 
-        [previous] ->
-          [type] = types
-          [difference(type, previous)]
+        [type] ->
+          [Enum.reduce(previous, type, &difference(&2, hd(&1)))]
+
+        [_ | _] ->
+          previous
+          |> Enum.reduce(args_to_domain(types), &difference(&2, args_to_domain(&1)))
+          |> domain_to_flat_args(types)
       end
 
     try do
@@ -1346,15 +1366,17 @@ defmodule Module.Types.Pattern do
   # $ type tag = head_pattern() or match_pattern()
   #
   # $ typep head_pattern =
-  #     :for_reduce or :with_else or :receive or :try_catch or :fn or :default or
-  #       {:try_else, type} or {{:case, meta, expr}, [domain], [previous_type]}
+  #     :for_reduce or :with_else or :fn or :default or
+  #       {{:case | :try_else, meta, expr, type}, [arg], [previous]} or
+  #       {: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)
     traces = collect_traces(to_trace, context)
 
@@ -1370,24 +1392,9 @@ defmodule Module.Types.Pattern do
     }
   end
 
-  defp badpattern({:try_else, type}, pattern, _) do
-    {pattern,
-     """
-     the following clause will never match:
-
-         #{expr_to_string(pattern) |> indent(4)}
-
-     it attempts to match on the result of the try do-block which has incompatible type:
-
-         #{to_quoted_string(type) |> indent(4)}
-     """}
-  end
-
-  defp badpattern({{:case, meta, expr}, [type], [previous_type]}, pattern, _) do
+  defp badpattern({{op, meta, expr, type}, args, previous}, _, _) when op in [:case, :try_else] do
     cond do
       meta[:type_check] == :expr ->
-        error_type = if previous_type == none(), do: type, else: previous_type
-
         {expr,
          """
          the following conditional expression:
@@ -1396,15 +1403,15 @@ defmodule Module.Types.Pattern do
 
          will always evaluate to:
 
-             #{to_quoted_string(error_type) |> indent(4)}
+             #{to_quoted_string(type) |> indent(4)}
          """}
 
-      previous_type == none() ->
-        {pattern,
+      previous == [] ->
+        {args,
          """
          the following clause will never match:
 
-             #{expr_to_string(pattern) |> indent(4)} ->
+             #{args_to_string(args) |> indent(4)} ->
 
          because it attempts to match on the result of:
 
@@ -1415,46 +1422,46 @@ defmodule Module.Types.Pattern do
              #{to_quoted_string(type) |> indent(4)}
          """}
 
-      subtype?(type, previous_type) ->
-        {pattern,
+      args_subtype?([type], previous) ->
+        {args,
          """
          the following clause cannot match because the previous clauses already matched all possible values:
 
-             #{expr_to_string(pattern) |> indent(4)} ->
+             #{args_to_string(args) |> indent(4)} ->
 
          it attempts to match on the result of:
 
              #{expr_to_string(expr) |> indent(4)}
 
-         and the following types have already been matched:
+         which has the already matched type:
 
-             #{to_quoted_string(previous_type) |> indent(4)}
+             #{to_quoted_string(type) |> indent(4)}
          """}
 
       true ->
-        {pattern,
+        {args,
          """
          the following clause is redundant:
 
-             #{expr_to_string(pattern) |> indent(4)} ->
+             #{args_to_string(args) |> indent(4)} ->
 
-         the following types are expected (and have already been matched):
+         previous clauses have already matched on the following types:
 
-             #{to_quoted_string(previous_type) |> indent(4)}
+             #{previous_to_string(previous)}
          """}
     end
   end
 
-  defp badpattern({:receive, [_type], [previous_type]}, pattern, _) do
-    {pattern,
+  defp badpattern({op, args, previous}, _, _) when op in [:receive, :try_catch] do
+    {args,
      """
      the following clause is redundant:
 
-         #{expr_to_string(pattern) |> indent(4)} ->
+         #{args_to_string(args) |> indent(4)} ->
 
      previous clauses have already matched on the following types:
 
-         #{to_quoted_string(previous_type) |> indent(4)}
+         #{previous_to_string(previous)}
      """}
   end
 
@@ -1504,4 +1511,18 @@ defmodule Module.Types.Pattern do
          #{expr_to_string(pattern_or_expr) |> indent(4)}
      """}
   end
+
+  defp args_to_string(args) do
+    args
+    |> Enum.map_join(", ", &expr_to_string/1)
+    |> indent(4)
+  end
+
+  defp previous_to_string(previous) do
+    Enum.map_join(previous, "\n    ", fn types ->
+      types
+      |> Enum.map_join(", ", &to_quoted_string/1)
+      |> indent(4)
+    end)
+  end
 end