Improve protocol violation warnings

Author: josevalim

lib/elixir/lib/module/types/apply.ex

@@ -979,7 +979,7 @@ defmodule Module.Types.Apply do
     {mod, fun, arity, converter} = mfac
     meta = elem(expr, 1)
 
-    {banner, hints, traces} =
+    {banner, traces} =
       case Keyword.get(meta, :type_check) do
         :interpolation ->
           {_, _, [arg]} = expr
@@ -990,7 +990,7 @@ defmodule Module.Types.Apply do
                #{expr_to_string(arg) |> indent(4)}
 
            it has type:
-           """, [:interpolation], collect_traces(expr, context)}
+           """, collect_traces(expr, context)}
 
         :generator ->
           {:<-, _, [_, arg]} = expr
@@ -1001,7 +1001,7 @@ defmodule Module.Types.Apply do
                #{expr_to_string(expr) |> indent(4)}
 
            it has type:
-           """, [:generator], collect_traces(arg, context)}
+           """, collect_traces(arg, context)}
 
         :into ->
           {"""
@@ -1010,7 +1010,7 @@ defmodule Module.Types.Apply do
                into: #{expr_to_string(expr) |> indent(4)}
 
            it has type:
-           """, [:into], collect_traces(expr, context)}
+           """, collect_traces(expr, context)}
 
         _ ->
           mfa_or_fa = if mod, do: Exception.format_mfa(mod, fun, arity), else: "#{fun}/#{arity}"
@@ -1021,40 +1021,68 @@ defmodule Module.Types.Apply do
                #{expr_to_string(expr) |> indent(4)}
 
            given types:
-           """, [], collect_traces(expr, context)}
+           """, collect_traces(expr, context)}
       end
 
-    explanation =
+    {explanation, impls} =
       cond do
         reason = empty_arg_reason(converter.(args_types)) ->
-          reason
+          {reason, ""}
 
         Code.ensure_loaded?(mod) and
             Keyword.has_key?(mod.module_info(:attributes), :__protocol__) ->
           if function_exported?(mod, :__protocol__, 1) and
                mod.__protocol__(:impls) == {:consolidated, []} do
-            """
-            but the protocol was not yet implemented for any type and therefore will always fail. \
-            This error typically happens within libraries that define protocols and will disappear as \
-            soon as there is one implementation. If you expect the protocol to be implemented later on, \
-            you can define an implementation specific for development/test.
-            """
+            {"""
+             but the #{inspect(mod)} protocol was not yet implemented \
+             for any type and therefore will always fail.
+
+             This warning will disappear once you define a implementation. \
+             If the protocol is part of a library, you may define a dummy \
+             implementation for development/test.
+             """, ""}
           else
-            # Protocol errors can be very verbose, so we collapse structs
-            """
-            but expected a type that implements the #{inspect(mod)} protocol. You either passed the wrong \
-            value or you forgot to implement the protocol.
-
-            The #{inspect(mod)} protocol is implemented for the following types:
-            #{clauses_args_to_quoted_string(clauses, converter, collapse_structs: true)}
-            """
+            fix =
+              case mod do
+                String.Chars ->
+                  """
+                  You either passed the wrong value or you must:
+
+                  1. convert the given value to a string explicitly
+                     (use inspect/1 if you want to convert any data structure to a string)
+                  2. implement the String.Chars protocol
+                  """
+
+                Enumerable ->
+                  """
+                  You either passed the wrong value or you must:
+
+                  1. convert the given value to an Enumerable explicitly
+                  2. implement the Enumerable protocol
+                  """
+
+                _ ->
+                  """
+                  You either passed the wrong value or you forgot to implement the protocol.
+                  """
+              end
+
+            {"""
+             but expected a type that implements the #{inspect(mod)} protocol.
+             #{fix}\
+             """,
+             """
+
+             #{hint()} the #{inspect(mod)} protocol is implemented for the following types:
+             #{clauses_args_to_quoted_string(clauses, converter, collapse_structs: true)}
+             """}
           end
 
         true ->
-          """
-          but expected one of:
-          #{clauses_args_to_quoted_string(clauses, converter, [])}
-          """
+          {"""
+           but expected one of:
+           #{clauses_args_to_quoted_string(clauses, converter, [])}
+           """, ""}
       end
 
     %{
@@ -1069,7 +1097,7 @@ defmodule Module.Types.Apply do
           """,
           explanation,
           format_traces(traces),
-          format_hints(hints)
+          impls
         ])
     }
   end

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

@@ -91,29 +91,6 @@ defmodule Module.Types.Helpers do
         "var.fun()" (with parentheses) means "var" is an atom()
         """
 
-      :interpolation ->
-        """
-
-        #{hint()} string interpolation uses the String.Chars protocol to \
-        convert a data structure into a string. Either convert the data type into a \
-        string upfront or implement the protocol accordingly
-        """
-
-      :generator ->
-        """
-
-        #{hint()} for-comprehensions use the Enumerable protocol to traverse \
-        data structures. Either convert the data type into a list (or another Enumerable) \
-        or implement the protocol accordingly
-        """
-
-      :into ->
-        """
-
-        #{hint()} the :into option in for-comprehensions use the Collectable protocol to \
-        build its result. Either pass a valid data type or implement the protocol accordingly
-        """
-
       :anonymous_rescue ->
         """
 

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

@@ -449,10 +449,9 @@ defmodule Module.Types.IntegrationTest do
 
                 -:hello-
 
-            but the protocol was not yet implemented for any type and therefore will always fail. \
-        This error typically happens within libraries that define protocols and will disappear as \
-        soon as there is one implementation. If you expect the protocol to be implemented later on, \
-        you can define an implementation specific for development/test.
+            but the NoImplProtocol protocol was not yet implemented for any type and therefore will always fail.
+
+            This warning will disappear once you define a implementation. If the protocol is part of a library, you may define a dummy implementation for development/test.
 
             typing violation found at:
             │
@@ -479,53 +478,54 @@ defmodule Module.Types.IntegrationTest do
 
       warnings = [
         """
-            warning: incompatible value given to string interpolation:
+            warning: incompatible types given to String.Chars.to_string/1:
 
-                data
+                to_string(data)
 
-            it has type:
+            given types:
 
                 -dynamic(%Range{})-
 
-            but expected a type that implements the String.Chars protocol. You either passed the wrong value or you forgot to implement the protocol.
+            but expected a type that implements the String.Chars protocol.
+            You either passed the wrong value or you must:
 
-            The String.Chars protocol is implemented for the following types:
-
-                dynamic(
-                  %Date{} or %DateTime{} or %NaiveDateTime{} or %Time{} or %URI{} or %Version{} or
-                    %Version.Requirement{}
-                ) or atom() or binary() or empty_list() or float() or integer() or non_empty_list(term(), term())
+            1. convert the given value to a string explicitly
+               (use inspect/1 if you want to convert any data structure to a string)
+            2. implement the String.Chars protocol
 
             where "data" was given the type:
 
                 # type: dynamic(%Range{})
-                # from: a.ex:3:24
+                # from: a.ex:2:24
                 _.._//_ = data
 
-            hint: string interpolation uses the String.Chars protocol to convert a data structure into a string. Either convert the data type into a string upfront or implement the protocol accordingly
+            hint: the String.Chars protocol is implemented for the following types:
+
+                dynamic(
+                  %Date{} or %DateTime{} or %NaiveDateTime{} or %Time{} or %URI{} or %Version{} or
+                    %Version.Requirement{}
+                ) or atom() or binary() or empty_list() or float() or integer() or non_empty_list(term(), term())
         """,
         """
-            warning: incompatible types given to String.Chars.to_string/1:
+            warning: incompatible value given to string interpolation:
 
-                to_string(data)
+                data
 
-            given types:
+            it has type:
 
                 -dynamic(%Range{})-
 
-            but expected a type that implements the String.Chars protocol. You either passed the wrong value or you forgot to implement the protocol.
+            but expected a type that implements the String.Chars protocol.
+            You either passed the wrong value or you must:
 
-            The String.Chars protocol is implemented for the following types:
-
-                dynamic(
-                  %Date{} or %DateTime{} or %NaiveDateTime{} or %Time{} or %URI{} or %Version{} or
-                    %Version.Requirement{}
-                ) or atom() or binary() or empty_list() or float() or integer() or non_empty_list(term(), term())
+            1. convert the given value to a string explicitly
+               (use inspect/1 if you want to convert any data structure to a string)
+            2. implement the String.Chars protocol
 
             where "data" was given the type:
 
                 # type: dynamic(%Range{})
-                # from: a.ex:2:24
+                # from: a.ex:3:24
                 _.._//_ = data
         """
       ]
@@ -555,29 +555,41 @@ defmodule Module.Types.IntegrationTest do
 
                 -dynamic(%Date{})-
 
-            but expected a type that implements the Enumerable protocol. You either passed the wrong value or you forgot to implement the protocol.
+            but expected a type that implements the Enumerable protocol.
+            You either passed the wrong value or you must:
 
-            The Enumerable protocol is implemented for the following types:
-
-                dynamic(
-                  %Date.Range{} or %File.Stream{} or %GenEvent.Stream{} or %HashDict{} or %HashSet{} or
-                    %IO.Stream{} or %MapSet{} or %Range{} or %Stream{}
-                ) or empty_list() or fun() or non_empty_list(term(), term()) or non_struct_map()
+            1. convert the given value to an Enumerable explicitly
+            2. implement the Enumerable protocol
 
             where "date" was given the type:
 
                 # type: dynamic(%Date{})
                 # from: a.ex:2:24
                 %Date{} = date
 
-            hint: for-comprehensions use the Enumerable protocol to traverse data structures. Either convert the data type into a list (or another Enumerable) or implement the protocol accordingly
+            hint: the Enumerable protocol is implemented for the following types:
+
+                dynamic(
+                  %Date.Range{} or %File.Stream{} or %GenEvent.Stream{} or %HashDict{} or %HashSet{} or
+                    %IO.Stream{} or %MapSet{} or %Range{} or %Stream{}
+                ) or empty_list() or fun() or non_empty_list(term(), term()) or non_struct_map()
         """,
         """
             warning: incompatible value given to :into option in for-comprehension:
 
                 into: Date.utc_today()
 
             it has type:
+
+                -dynamic(%Date{})-
+
+            but expected a type that implements the Collectable protocol.
+            You either passed the wrong value or you forgot to implement the protocol.
+
+            hint: the Collectable protocol is implemented for the following types:
+
+                dynamic(%File.Stream{} or %HashDict{} or %HashSet{} or %IO.Stream{} or %MapSet{}) or binary() or
+                  empty_list() or non_empty_list(term(), term()) or non_struct_map()
         """,
         """
             warning: incompatible value given to :into option in for-comprehension:
@@ -588,14 +600,13 @@ defmodule Module.Types.IntegrationTest do
 
                 -integer()-
 
-            but expected a type that implements the Collectable protocol. You either passed the wrong value or you forgot to implement the protocol.
+            but expected a type that implements the Collectable protocol.
+            You either passed the wrong value or you forgot to implement the protocol.
 
-            The Collectable protocol is implemented for the following types:
+            hint: the Collectable protocol is implemented for the following types:
 
                 dynamic(%File.Stream{} or %HashDict{} or %HashSet{} or %IO.Stream{} or %MapSet{}) or binary() or
                   empty_list() or non_empty_list(term(), term()) or non_struct_map()
-
-            hint: the :into option in for-comprehensions use the Collectable protocol to build its result. Either pass a valid data type or implement the protocol accordingly
         """
       ]