Have specific type violation messages per construct instead of generic ones

Author: josevalim

lib/elixir/lib/module/types/of.ex

@@ -389,22 +389,25 @@ defmodule Module.Types.Of do
     type = specifier_type(kind, right)
     expr = {:<<>>, meta, args}
 
-    {_type, context} =
+    {actual, context} =
       case kind do
         :match ->
           Module.Types.Pattern.of_match_var(left, type, expr, stack, context)
 
         :guard ->
-          {actual, context} = Module.Types.Pattern.of_guard(left, type, expr, stack, context)
-          compatible(actual, type, expr, stack, context)
+          Module.Types.Pattern.of_guard(left, type, expr, stack, context)
 
         :expr ->
           left = annotate_interpolation(left, right)
-          {actual, context} = Module.Types.Expr.of_expr(left, type, expr, stack, context)
-          compatible(actual, type, expr, stack, context)
+          Module.Types.Expr.of_expr(left, type, expr, stack, context)
       end
 
-    specifier_size(kind, right, stack, context)
+    if compatible?(actual, type) do
+      specifier_size(kind, right, stack, context)
+    else
+      error = {:badbinary, kind, meta, expr, type, actual, context}
+      error(error, meta, stack, context)
+    end
   end
 
   defp annotate_interpolation(
@@ -441,21 +444,28 @@ defmodule Module.Types.Of do
   defp specifier_size(:expr, {:size, _, [arg]} = expr, stack, context)
        when not is_integer(arg) do
     {actual, context} = Module.Types.Expr.of_expr(arg, integer(), expr, stack, context)
-    {_, context} = compatible(actual, integer(), expr, stack, context)
-    context
+    compatible_size(actual, expr, stack, context)
   end
 
   defp specifier_size(_pattern_or_guard, {:size, _, [arg]} = expr, stack, context)
        when not is_integer(arg) do
     {actual, context} = Module.Types.Pattern.of_guard(arg, integer(), expr, stack, context)
-    {_, context} = compatible(actual, integer(), expr, stack, context)
-    context
+    compatible_size(actual, expr, stack, context)
   end
 
   defp specifier_size(_kind, _specifier, _stack, context) do
     context
   end
 
+  defp compatible_size(actual, expr, stack, context) do
+    if compatible?(actual, integer()) do
+      context
+    else
+      error = {:badsize, expr, actual, context}
+      error(error, elem(expr, 1), stack, context)
+    end
+  end
+
   ## Modules
 
   @doc """
@@ -474,39 +484,12 @@ defmodule Module.Types.Of do
     end
   end
 
-  ## Warning helpers
-
-  @doc """
-  Checks if two types are compatible and emit an incompatible error if not.
-  """
-  # TODO: Consider getting rid of this and emitting precise errors instead.
-  def compatible(actual, expected, expr, stack, context) do
-    if compatible?(actual, expected) do
-      {actual, context}
-    else
-      {error_type(), incompatible_error(expr, expected, actual, stack, context)}
-    end
-  end
-
-  @doc """
-  Emits incompatible types warning for the given expression.
-
-  This is a generic warning for when the expected/actual types
-  themselves may come from several different circumstances.
-  """
-  def incompatible_error(expr, expected_type, actual_type, stack, context) do
-    meta = get_meta(expr) || stack.meta
-    hints = if meta[:inferred_bitstring_spec], do: [:inferred_bitstring_spec], else: []
-    warning = {:incompatible, expr, expected_type, actual_type, hints, context}
-    error(warning, meta, stack, context)
-  end
+  ## Warning
 
   defp error(warning, meta, stack, context) do
     error(__MODULE__, warning, meta, stack, context)
   end
 
-  ## Warning formatting
-
   def format_diagnostic({:refine_head_var, old_type, new_type, var, context}) do
     traces = collect_traces(var, context)
 
@@ -524,15 +507,17 @@ defmodule Module.Types.Of do
     }
   end
 
-  def format_diagnostic({:incompatible, expr, expected_type, actual_type, hints, context}) do
+  def format_diagnostic({:badbinary, kind, meta, expr, expected_type, actual_type, context}) do
+    type = if kind == :match, do: "matching", else: "construction"
+    hints = if meta[:inferred_bitstring_spec], do: [:inferred_bitstring_spec], else: []
     traces = collect_traces(expr, context)
 
     %{
       details: %{typing_traces: traces},
       message:
         IO.iodata_to_binary([
           """
-          incompatible types in expression:
+          incompatible types in binary #{type}:
 
               #{expr_to_string(expr) |> indent(4)}
 
@@ -550,6 +535,27 @@ defmodule Module.Types.Of do
     }
   end
 
+  def format_diagnostic({:badsize, expr, type, context}) do
+    traces = collect_traces(expr, context)
+
+    %{
+      details: %{typing_traces: traces},
+      message:
+        IO.iodata_to_binary([
+          """
+          expected an integer in binary size:
+
+              #{expr_to_string(expr) |> indent(4)}
+
+          got type:
+
+              #{to_quoted_string(type) |> indent(4)}
+          """,
+          format_traces(traces)
+        ])
+    }
+  end
+
   def format_diagnostic({:badmodule, expr, type, fun, arity, hints, context}) do
     traces = collect_traces(expr, context)
 

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

@@ -364,12 +364,6 @@ defmodule Module.Types.Pattern do
   Given those values are actually checked at compile-time,
   except for the variables, that's the only scenario we need to handle.
   """
-  # TODO: Perhaps merge this with guards
-  def of_match_var({:^, _, [var]}, expected, expr, stack, context) do
-    {type, context} = Of.refine_body_var(var, expected, expr, stack, context)
-    Of.compatible(type, expected, expr, stack, context)
-  end
-
   def of_match_var({:_, _, _}, expected, _expr, _stack, context) do
     {expected, context}
   end
@@ -379,8 +373,8 @@ defmodule Module.Types.Pattern do
     {type, context}
   end
 
-  def of_match_var(_ast, expected, _expr, _stack, context) do
-    {expected, context}
+  def of_match_var(ast, expected, expr, stack, context) do
+    of_guard(ast, expected, expr, stack, context)
   end
 
   ## Patterns
@@ -491,13 +485,19 @@ defmodule Module.Types.Pattern do
 
   # %^var{...} and %_{...}
   defp of_pattern(
-         {:%, _meta, [var, {:%{}, _meta2, args}]} = expr,
+         {:%, meta, [var, {:%{}, _meta2, args}]} = expr,
          path,
          stack,
          context
        ) do
     {refined, context} = of_match_var(var, atom(), expr, stack, context)
-    of_open_map(args, [__struct__: refined], [], path, stack, context)
+
+    if compatible?(refined, atom()) do
+      of_open_map(args, [__struct__: refined], [], path, stack, context)
+    else
+      error = {:badstruct, refined, expr, context}
+      {error_type(), error(__MODULE__, error, meta, stack, context)}
+    end
   end
 
   # %{...}
@@ -706,8 +706,7 @@ defmodule Module.Types.Pattern do
   # ^var
   def of_guard({:^, _meta, [var]}, expected, expr, stack, context) do
     # This is used by binary size, which behaves as a mixture of match and guard
-    {type, context} = Of.refine_body_var(var, expected, expr, stack, context)
-    Of.compatible(type, expected, expr, stack, context)
+    Of.refine_body_var(var, expected, expr, stack, context)
   end
 
   # {...}
@@ -745,8 +744,8 @@ defmodule Module.Types.Pattern do
   end
 
   # var
-  def of_guard(var, expected, expr, stack, context) when is_var(var) do
-    Of.compatible(Of.var(var, context), expected, expr, stack, context)
+  def of_guard(var, _expected, _expr, _stack, context) when is_var(var) do
+    {Of.var(var, context), context}
   end
 
   ## Helpers
@@ -763,6 +762,27 @@ defmodule Module.Types.Pattern do
     {pattern_info, %{context | pattern_info: nil}}
   end
 
+  def format_diagnostic({:badstruct, type, expr, context}) do
+    traces = collect_traces(expr, context)
+
+    %{
+      details: %{typing_traces: traces},
+      message:
+        IO.iodata_to_binary([
+          """
+          expected an atom as struct name:
+
+              #{expr_to_string(expr) |> indent(4)}
+
+          got type:
+
+              #{to_quoted_string(type) |> indent(4)}
+          """,
+          format_traces(traces)
+        ])
+    }
+  end
+
   # $ type tag = head_pattern() or match_pattern()
   #
   # $ typep head_pattern =

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

@@ -415,7 +415,7 @@ defmodule Module.Types.ExprTest do
     test "warnings" do
       assert typeerror!([<<x::binary-size(2)>>], <<x::float>>) ==
                ~l"""
-               incompatible types in expression:
+               incompatible types in binary construction:
 
                    <<x::float>>
 
@@ -436,7 +436,7 @@ defmodule Module.Types.ExprTest do
 
       assert typeerror!([<<x::binary>>], <<x>>) ==
                ~l"""
-               incompatible types in expression:
+               incompatible types in binary construction:
 
                    <<x>>
 
@@ -459,7 +459,7 @@ defmodule Module.Types.ExprTest do
 
       assert typeerror!([<<x>>], <<x::binary>>) ==
                ~l"""
-               incompatible types in expression:
+               incompatible types in binary construction:
 
                    <<x::binary>>
 
@@ -488,18 +488,14 @@ defmodule Module.Types.ExprTest do
     test "size error" do
       assert typeerror!([<<x::binary>>, y], <<y::size(x)>>) ==
                ~l"""
-               incompatible types in expression:
+               expected an integer in binary size:
 
                    size(x)
 
                got type:
 
                    binary()
 
-               but expected type:
-
-                   integer()
-
                where "x" was given the type:
 
                    # type: binary()

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

@@ -149,18 +149,14 @@ defmodule Module.Types.PatternTest do
 
       assert typeerror!([m = 123], %^m{} = %Point{}) ==
                ~l"""
-               incompatible types in expression:
+               expected an atom as struct name:
 
                    %^m{}
 
                got type:
 
                    integer()
 
-               but expected type:
-
-                   atom()
-
                where "m" was given the type:
 
                    # type: integer()
@@ -314,18 +310,14 @@ defmodule Module.Types.PatternTest do
     test "size error" do
       assert typeerror!([<<x::float, _::size(x)>>], :ok) ==
                ~l"""
-               incompatible types in expression:
+               expected an integer in binary size:
 
                    size(x)
 
                got type:
 
                    float()
 
-               but expected type:
-
-                   integer()
-
                where "x" was given the type:
 
                    # type: float()