Move struct handling from unify to AST (#10320)

We also improve error messages to list all
available fields and introduce Module.Types.Of
module to hold shared functionality between Expr
and Pattern.

Author: josevalim

lib/elixir/lib/module/types.ex

@@ -251,16 +251,19 @@ defmodule Module.Types do
 
   def format_warning({:unable_unify, left, right, {location, expr, traces}}) do
     cond do
-      map_type?(left) and map_type?(right) and match?({:ok, _}, missing_field(left, right)) ->
-        {:ok, atom} = missing_field(left, right)
+      map_type?(left) and map_type?(right) and match?({:ok, _, _}, missing_field(left, right)) ->
+        {:ok, atom, known_atoms} = missing_field(left, right)
 
         # Drop the last trace which is the expression map.foo
         traces = Enum.drop(traces, 1)
-        {traces, hints} = format_traces(traces, false)
+        {traces, hints} = format_traces(traces, true)
 
         [
           "undefined field \"#{atom}\" ",
           format_expr(expr, location),
+          "expected one of the following fields: ",
+          Enum.map_join(Enum.sort(known_atoms), ", ", & &1),
+          "\n\n",
           traces,
           format_message_hints(hints),
           "Conflict found at"
@@ -290,26 +293,27 @@ defmodule Module.Types do
          {:map, [{:required, {:atom, atom} = type, _}, {:optional, :dynamic, :dynamic}]},
          {:map, fields}
        ) do
-    if List.keymember?(fields, type, 1) do
-      :error
-    else
-      {:ok, atom}
-    end
+    matched_missing_field(fields, type, atom)
   end
 
   defp missing_field(
          {:map, fields},
          {:map, [{:required, {:atom, atom} = type, _}, {:optional, :dynamic, :dynamic}]}
        ) do
+    matched_missing_field(fields, type, atom)
+  end
+
+  defp missing_field(_, _), do: :error
+
+  defp matched_missing_field(fields, type, atom) do
     if List.keymember?(fields, type, 1) do
       :error
     else
-      {:ok, atom}
+      known_atoms = for {_, {:atom, atom}, _} <- fields, do: atom
+      {:ok, atom, known_atoms}
     end
   end
 
-  defp missing_field(_, _), do: :error
-
   defp format_traces([], _simplify?) do
     {[], []}
   end

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

@@ -1,7 +1,7 @@
 defmodule Module.Types.Expr do
   @moduledoc false
 
-  alias Module.Types.{Remote, Pattern}
+  alias Module.Types.{Of, Pattern}
   import Module.Types.{Helpers, Infer}
 
   def of_expr(expr, %{context: stack_context} = stack, context) when stack_context != :expr do
@@ -35,7 +35,7 @@ defmodule Module.Types.Expr do
 
   # <<...>>>
   def of_expr({:<<>>, _meta, args}, stack, context) do
-    result = of_binary(args, stack, context, &of_expr/3)
+    result = Of.binary(args, stack, context, &of_expr/3)
 
     case result do
       {:ok, context} -> {:ok, :binary, context}
@@ -142,41 +142,47 @@ defmodule Module.Types.Expr do
   def of_expr({:%{}, _, [{:|, _, [map, args]}]} = expr, stack, context) do
     stack = push_expr_stack(expr, stack)
 
-    additional_pairs = [{:optional, :dynamic, :dynamic}]
-    map_update(map, args, additional_pairs, stack, context)
+    with {:ok, map_type, context} <- of_expr(map, stack, context),
+         {:ok, {:map, arg_pairs}, context} <- Of.closed_map(args, stack, context, &of_expr/3),
+         dynamic_value_pairs =
+           Enum.map(arg_pairs, fn {:required, key, _value} -> {:required, key, :dynamic} end),
+         args_type = {:map, dynamic_value_pairs ++ [{:optional, :dynamic, :dynamic}]},
+         {:ok, type, context} <- unify(args_type, map_type, stack, context) do
+      # Retrieve map type and overwrite with the new value types from the map update
+      {:map, pairs} = resolve_var(type, context)
+
+      updated_pairs =
+        Enum.reduce(arg_pairs, pairs, fn {:required, key, value}, pairs ->
+          List.keyreplace(pairs, key, 1, {:required, key, value})
+        end)
+
+      {:ok, {:map, updated_pairs}, context}
+    end
   end
 
   # %Struct{map | ...}
-  def of_expr({:%, meta, [module, {:%{}, _, [{:|, _, [map, args]}]}]} = expr, stack, context) do
-    context = Remote.check(module, :__struct__, 0, meta, context)
+  def of_expr({:%, meta, [module, {:%{}, _, [{:|, _, [_, _]}]} = update]} = expr, stack, context) do
     stack = push_expr_stack(expr, stack)
 
-    additional_pairs = [{:required, {:atom, :__struct__}, {:atom, module}}]
-    map_update(map, args, additional_pairs, stack, context)
+    with {:ok, struct, context} <- Of.struct(module, meta, context),
+         {:ok, update, context} <- of_expr(update, stack, context) do
+      unify(update, struct, stack, context)
+    end
   end
 
   # %{...}
   def of_expr({:%{}, _meta, args} = expr, stack, context) do
     stack = push_expr_stack(expr, stack)
-
-    case of_pairs(args, stack, context) do
-      {:ok, pairs, context} -> {:ok, {:map, pairs_to_unions(pairs, context)}, context}
-      {:error, reason} -> {:error, reason}
-    end
+    Of.closed_map(args, stack, context, &of_expr/3)
   end
 
   # %Struct{...}
   def of_expr({:%, meta1, [module, {:%{}, _meta2, args}]} = expr, stack, context) do
-    context = Remote.check(module, :__struct__, 0, meta1, context)
     stack = push_expr_stack(expr, stack)
 
-    case of_pairs(args, stack, context) do
-      {:ok, pairs, context} ->
-        pairs = [{:required, {:atom, :__struct__}, {:atom, module}} | pairs]
-        {:ok, {:map, pairs}, context}
-
-      {:error, reason} ->
-        {:error, reason}
+    with {:ok, struct, context} <- Of.struct(module, meta1, context),
+         {:ok, map, context} <- Of.open_map(args, stack, context, &of_expr/3) do
+      unify(map, struct, stack, context)
     end
   end
 
@@ -327,7 +333,7 @@ defmodule Module.Types.Expr do
 
   # expr.fun(arg)
   def of_expr({{:., meta1, [expr1, fun]}, _meta2, args} = expr2, stack, context) do
-    context = Remote.check(expr1, fun, length(args), meta1, context)
+    context = Of.remote(expr1, fun, length(args), meta1, context)
     stack = push_expr_stack(expr2, stack)
 
     with {:ok, _expr_type, context} <- of_expr(expr1, stack, context),
@@ -342,7 +348,7 @@ defmodule Module.Types.Expr do
   # &Foo.bar/1
   def of_expr({:&, meta, [{:/, _, [{{:., _, [module, fun]}, _, []}, arity]}]}, _stack, context)
       when is_atom(module) and is_atom(fun) do
-    context = Remote.check(module, fun, arity, meta, context)
+    context = Of.remote(module, fun, arity, meta, context)
     {:ok, :dynamic, context}
   end
 
@@ -363,51 +369,6 @@ defmodule Module.Types.Expr do
     end
   end
 
-  defp of_pairs(pairs, stack, context) do
-    map_reduce_ok(pairs, context, fn {key, value}, context ->
-      with {:ok, key_type, context} <- of_expr(key, stack, context),
-           {:ok, value_type, context} <- of_expr(value, stack, context),
-           do: {:ok, {:required, key_type, value_type}, context}
-    end)
-  end
-
-  defp pairs_to_unions(pairs, context) do
-    # We are currently creating overlapping key types
-
-    Enum.reduce(pairs, [], fn {kind_left, key, value_left}, pairs ->
-      case List.keyfind(pairs, key, 1) do
-        {kind_right, ^key, value_right} ->
-          kind = unify_kinds(kind_left, kind_right)
-          value = to_union([value_left, value_right], context)
-          List.keystore(pairs, key, 1, {kind, key, value})
-
-        nil ->
-          [{kind_left, key, value_left} | pairs]
-      end
-    end)
-  end
-
-  defp map_update(map, args, additional_pairs, stack, context) do
-    with {:ok, map_type, context} <- of_expr(map, stack, context),
-         {:ok, arg_pairs, context} <- of_pairs(args, stack, context),
-         arg_pairs = pairs_to_unions(arg_pairs, context),
-         # Change value types to dynamic to reuse map unification for map updates
-         dynamic_value_pairs =
-           Enum.map(arg_pairs, fn {:required, key, _value} -> {:required, key, :dynamic} end),
-         args_type = {:map, additional_pairs ++ dynamic_value_pairs},
-         {:ok, type, context} <- unify(args_type, map_type, stack, context) do
-      # Retrieve map type and overwrite with the new value types from the map update
-      {:map, pairs} = resolve_var(type, context)
-
-      updated_pairs =
-        Enum.reduce(arg_pairs, pairs, fn {:required, key, value}, pairs ->
-          List.keyreplace(pairs, key, 1, {:required, key, value})
-        end)
-
-      {:ok, {:map, updated_pairs}, context}
-    end
-  end
-
   defp for_clause({:<-, _, [left, expr]}, stack, context) do
     {pattern, guards} = extract_head([left])
 

lib/elixir/lib/module/types/helpers.ex

@@ -1,11 +1,7 @@
 defmodule Module.Types.Helpers do
+  # AST and enumeration helpers.
   @moduledoc false
 
-  alias Module.Types.Infer
-
-  @prefix quote(do: ...)
-  @suffix quote(do: ...)
-
   @doc """
   Guard function to check if an AST node is a variable.
   """
@@ -157,103 +153,6 @@ defmodule Module.Types.Helpers do
     end
   end
 
-  @doc """
-  Handles binaries.
-
-  In the stack, we add nodes such as <<expr>>, <<..., expr>>, etc,
-  based on the position of the expression within the binary.
-  """
-  def of_binary([], _stack, context, _fun) do
-    {:ok, context}
-  end
-
-  def of_binary([head], stack, context, fun) do
-    head_stack = push_expr_stack({:<<>>, get_meta(head), [head]}, stack)
-    of_binary_segment(head, head_stack, context, fun)
-  end
-
-  def of_binary([head | tail], stack, context, fun) do
-    head_stack = push_expr_stack({:<<>>, get_meta(head), [head, @suffix]}, stack)
-
-    case of_binary_segment(head, head_stack, context, fun) do
-      {:ok, context} -> of_binary_many(tail, stack, context, fun)
-      {:error, reason} -> {:error, reason}
-    end
-  end
-
-  defp of_binary_many([last], stack, context, fun) do
-    last_stack = push_expr_stack({:<<>>, get_meta(last), [@prefix, last]}, stack)
-    of_binary_segment(last, last_stack, context, fun)
-  end
-
-  defp of_binary_many([head | tail], stack, context, fun) do
-    head_stack = push_expr_stack({:<<>>, get_meta(head), [@prefix, head, @suffix]}, stack)
-
-    case of_binary_segment(head, head_stack, context, fun) do
-      {:ok, context} -> of_binary_many(tail, stack, context, fun)
-      {:error, reason} -> {:error, reason}
-    end
-  end
-
-  defp of_binary_segment({:"::", _meta, [expr, specifiers]}, stack, context, fun) do
-    expected_type =
-      collect_binary_specifier(specifiers, &binary_type(stack.context, &1)) || :integer
-
-    utf? = collect_binary_specifier(specifiers, &utf_type?/1)
-    float? = collect_binary_specifier(specifiers, &float_type?/1)
-
-    # Special case utf and float specifiers because they can be two types as literals
-    # but only a specific type as a variable in a pattern
-    cond do
-      stack.context == :pattern and utf? and is_binary(expr) ->
-        {:ok, context}
-
-      stack.context == :pattern and float? and is_integer(expr) ->
-        {:ok, context}
-
-      true ->
-        with {:ok, type, context} <- fun.(expr, stack, context),
-             {:ok, _type, context} <- Infer.unify(type, expected_type, stack, context),
-             do: {:ok, context}
-    end
-  end
-
-  # TODO: Remove this clause once we properly handle comprehensions
-  defp of_binary_segment({:<-, _, _}, _stack, context, _fun) do
-    {:ok, context}
-  end
-
-  # Collect binary type specifiers,
-  # from `<<pattern::integer-size(10)>>` collect `integer`
-  defp collect_binary_specifier({:-, _meta, [left, right]}, fun) do
-    collect_binary_specifier(left, fun) || collect_binary_specifier(right, fun)
-  end
-
-  defp collect_binary_specifier(other, fun) do
-    fun.(other)
-  end
-
-  defp binary_type(:expr, {:float, _, _}), do: :number
-  defp binary_type(:expr, {:utf8, _, _}), do: {:union, [:integer, :binary]}
-  defp binary_type(:expr, {:utf16, _, _}), do: {:union, [:integer, :binary]}
-  defp binary_type(:expr, {:utf32, _, _}), do: {:union, [:integer, :binary]}
-  defp binary_type(:pattern, {:utf8, _, _}), do: :integer
-  defp binary_type(:pattern, {:utf16, _, _}), do: :integer
-  defp binary_type(:pattern, {:utf32, _, _}), do: :integer
-  defp binary_type(:pattern, {:float, _, _}), do: :float
-  defp binary_type(_context, {:integer, _, _}), do: :integer
-  defp binary_type(_context, {:bits, _, _}), do: :binary
-  defp binary_type(_context, {:bitstring, _, _}), do: :binary
-  defp binary_type(_context, {:bytes, _, _}), do: :binary
-  defp binary_type(_context, {:binary, _, _}), do: :binary
-  defp binary_type(_context, _specifier), do: nil
-
-  defp utf_type?({specifier, _, _}), do: specifier in [:utf8, :utf16, :utf32]
-  defp utf_type?(_), do: false
-
-  defp float_type?({:float, _, _}), do: true
-  defp float_type?(_), do: false
-
   # TODO: Remove this and let multiple when be treated as multiple clauses,
   #       meaning they will be intersection types
   def guards_to_or([]) do