From 1d7d0363fab81481ca764301b8ae6542cc5c06df Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Wed, 20 Nov 2024 10:58:17 -0500 Subject: [PATCH 1/9] Make protocol errors pretty A common complaint I here from people is that error messages in Elixir are not pretty. Generally, this seems to be only the case when there are large structs in codebases that render in one line and are hard to read. This PR changes the behavior to pretty inspect the structs in the case that a protocol is not implemented for some value. The downside here is that the logging will no longer be in one line, but I think this is a fair tradeoff for legibility --- lib/elixir/lib/exception.ex | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index f3e02edbd55..9a4313a496d 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -346,7 +346,7 @@ defmodule Exception do defp rewrite_arg(arg) do Macro.prewalk(arg, fn {:%{}, meta, [__struct__: Range, first: first, last: last, step: step]} -> - {:..//, meta, [first, last, step]} + {:"..//", meta, [first, last, step]} other -> other @@ -1997,11 +1997,11 @@ defmodule Protocol.UndefinedError do @impl true def message(%{protocol: protocol, value: value, description: description}) do - "protocol #{inspect(protocol)} not implemented for #{inspect(value)} of type " <> + "protocol #{inspect(protocol)} not implemented for #{inspect(value, pretty: true)} of type " <> value_type(value) <> maybe_description(description) <> maybe_available(protocol) end - defp value_type(%{__struct__: struct}), do: "#{inspect(struct)} (a struct)" + defp value_type(%{__struct__: struct}), do: "#{inspect(struct, pretty: true)} (a struct)" defp value_type(value) when is_atom(value), do: "Atom" defp value_type(value) when is_bitstring(value), do: "BitString" defp value_type(value) when is_float(value), do: "Float" From 886ba535a27808150b8c1fe125dc578f8c38dc9f Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Wed, 20 Nov 2024 11:02:11 -0500 Subject: [PATCH 2/9] Update lib/elixir/lib/exception.ex --- lib/elixir/lib/exception.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index 9a4313a496d..7685d0a33a7 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -346,7 +346,7 @@ defmodule Exception do defp rewrite_arg(arg) do Macro.prewalk(arg, fn {:%{}, meta, [__struct__: Range, first: first, last: last, step: step]} -> - {:"..//", meta, [first, last, step]} + {:..//, meta, [first, last, step]} other -> other From 07afa2cd7068827893edcc86a445018712e9ee31 Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Wed, 20 Nov 2024 11:59:38 -0500 Subject: [PATCH 3/9] Update lib/elixir/lib/exception.ex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Valim --- lib/elixir/lib/exception.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index 7685d0a33a7..9a26edc27e9 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -2001,7 +2001,7 @@ defmodule Protocol.UndefinedError do value_type(value) <> maybe_description(description) <> maybe_available(protocol) end - defp value_type(%{__struct__: struct}), do: "#{inspect(struct, pretty: true)} (a struct)" + defp value_type(%{__struct__: struct}), do: "#{inspect(struct)} (a struct)" defp value_type(value) when is_atom(value), do: "Atom" defp value_type(value) when is_bitstring(value), do: "BitString" defp value_type(value) when is_float(value), do: "Float" From 89e8d88968c0f749d9ae65e3273df2e33fbfad6b Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Wed, 20 Nov 2024 11:59:49 -0500 Subject: [PATCH 4/9] Update lib/elixir/lib/exception.ex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Valim --- lib/elixir/lib/exception.ex | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index 9a26edc27e9..ed904f565ab 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1997,8 +1997,9 @@ defmodule Protocol.UndefinedError do @impl true def message(%{protocol: protocol, value: value, description: description}) do - "protocol #{inspect(protocol)} not implemented for #{inspect(value, pretty: true)} of type " <> - value_type(value) <> maybe_description(description) <> maybe_available(protocol) + "protocol #{inspect(protocol)} not implemented type " <> value_type(value) <> + maybe_description(description) <> maybe_available(protocol) <> + "\n\nGot value: #{inspect(value, pretty: true)}" end defp value_type(%{__struct__: struct}), do: "#{inspect(struct)} (a struct)" From c211861d6070985050b451931aafcee4f3002ffe Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Wed, 20 Nov 2024 17:27:03 -0500 Subject: [PATCH 5/9] Update exception.ex Co-authored-by: Jean Klingler --- lib/elixir/lib/exception.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index ed904f565ab..5422ed427e6 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1997,7 +1997,7 @@ defmodule Protocol.UndefinedError do @impl true def message(%{protocol: protocol, value: value, description: description}) do - "protocol #{inspect(protocol)} not implemented type " <> value_type(value) <> + "protocol #{inspect(protocol)} not implemented for type " <> value_type(value) <> maybe_description(description) <> maybe_available(protocol) <> "\n\nGot value: #{inspect(value, pretty: true)}" end From 1fed20357c38b9efae49e79d5be4bdb1f6eb3bc5 Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Wed, 20 Nov 2024 18:11:10 -0500 Subject: [PATCH 6/9] fix formatter --- lib/elixir/lib/exception.ex | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index 5422ed427e6..7c162ccdb7c 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1997,8 +1997,10 @@ defmodule Protocol.UndefinedError do @impl true def message(%{protocol: protocol, value: value, description: description}) do - "protocol #{inspect(protocol)} not implemented for type " <> value_type(value) <> - maybe_description(description) <> maybe_available(protocol) <> + "protocol #{inspect(protocol)} not implemented for type " <> + value_type(value) <> + maybe_description(description) <> + maybe_available(protocol) <> "\n\nGot value: #{inspect(value, pretty: true)}" end From 11d99442239ac7bc4808f47856d9255955d15fba Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Wed, 20 Nov 2024 20:48:53 -0500 Subject: [PATCH 7/9] fix the tests --- lib/elixir/lib/exception.ex | 2 +- lib/elixir/test/elixir/inspect_test.exs | 18 +++++++++++++++--- .../elixir/protocol/consolidation_test.exs | 5 +++-- lib/elixir/test/elixir/protocol_test.exs | 2 +- lib/elixir/test/elixir/string/chars_test.exs | 18 +++++++++++------- 5 files changed, 31 insertions(+), 14 deletions(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index 7c162ccdb7c..c450d93b3a0 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -2001,7 +2001,7 @@ defmodule Protocol.UndefinedError do value_type(value) <> maybe_description(description) <> maybe_available(protocol) <> - "\n\nGot value: #{inspect(value, pretty: true)}" + "\n\nGot value:\n\n#{inspect(value, pretty: true)}" end defp value_type(%{__struct__: struct}), do: "#{inspect(struct)} (a struct)" diff --git a/lib/elixir/test/elixir/inspect_test.exs b/lib/elixir/test/elixir/inspect_test.exs index 9cd13caecc0..a5e40cbbc3b 100644 --- a/lib/elixir/test/elixir/inspect_test.exs +++ b/lib/elixir/test/elixir/inspect_test.exs @@ -526,7 +526,11 @@ defmodule Inspect.MapTest do # Inspect.Error is raised here when we tried to print the error message # called by another exception (Protocol.UndefinedError in this case) exception_message = ~s''' - protocol Enumerable not implemented for #Inspect.Error< + protocol Enumerable not implemented for type Inspect.MapTest.Failing (a struct) + + Got value: + + #Inspect.Error< got ArgumentError with message: """ @@ -926,7 +930,11 @@ defmodule Inspect.CustomProtocolTest do got Protocol.UndefinedError with message: """ - protocol Inspect.CustomProtocolTest.CustomInspect not implemented for %Inspect.CustomProtocolTest.MissingImplementation{} of type Inspect.CustomProtocolTest.MissingImplementation (a struct) + protocol Inspect.CustomProtocolTest.CustomInspect not implemented for type Inspect.CustomProtocolTest.MissingImplementation (a struct) + + Got value: + + %Inspect.CustomProtocolTest.MissingImplementation{} """ while inspecting: @@ -953,7 +961,11 @@ defmodule Inspect.CustomProtocolTest do got Protocol.UndefinedError with message: """ - protocol Inspect.CustomProtocolTest.CustomInspect not implemented for %Inspect.CustomProtocolTest.MissingImplementation{} of type Inspect.CustomProtocolTest.MissingImplementation (a struct) + protocol Inspect.CustomProtocolTest.CustomInspect not implemented for type Inspect.CustomProtocolTest.MissingImplementation (a struct) + + Got value: + + %Inspect.CustomProtocolTest.MissingImplementation{} """ while inspecting: diff --git a/lib/elixir/test/elixir/protocol/consolidation_test.exs b/lib/elixir/test/elixir/protocol/consolidation_test.exs index 43ce2380c54..d9f01335cc9 100644 --- a/lib/elixir/test/elixir/protocol/consolidation_test.exs +++ b/lib/elixir/test/elixir/protocol/consolidation_test.exs @@ -182,8 +182,9 @@ defmodule Protocol.ConsolidationTest do test "protocol not implemented" do message = - "protocol Protocol.ConsolidationTest.Sample not implemented for :foo of type Atom. " <> - "This protocol is implemented for the following type(s): Protocol.ConsolidationTest.ImplStruct" + "protocol Protocol.ConsolidationTest.Sample not implemented for type Atom. " <> + "This protocol is implemented for the following type(s): Protocol.ConsolidationTest.ImplStruct" <> + "\n\nGot value:\n\n:foo" assert_raise Protocol.UndefinedError, message, fn -> sample = String.to_atom("Elixir.Protocol.ConsolidationTest.Sample") diff --git a/lib/elixir/test/elixir/protocol_test.exs b/lib/elixir/test/elixir/protocol_test.exs index 72184400e10..e0f846325f0 100644 --- a/lib/elixir/test/elixir/protocol_test.exs +++ b/lib/elixir/test/elixir/protocol_test.exs @@ -113,7 +113,7 @@ defmodule ProtocolTest do end test "protocol not implemented" do - message = "protocol ProtocolTest.Sample not implemented for :foo of type Atom" + message = "protocol ProtocolTest.Sample not implemented for type Atom\n\nGot value:\n\n:foo" assert_raise Protocol.UndefinedError, message, fn -> sample = String.to_atom("Elixir.ProtocolTest.Sample") diff --git a/lib/elixir/test/elixir/string/chars_test.exs b/lib/elixir/test/elixir/string/chars_test.exs index 934425f0292..eb5342d20da 100644 --- a/lib/elixir/test/elixir/string/chars_test.exs +++ b/lib/elixir/test/elixir/string/chars_test.exs @@ -105,7 +105,7 @@ defmodule String.Chars.ErrorsTest do test "bitstring" do message = - "protocol String.Chars not implemented for <<0, 1::size(4)>> of type BitString, cannot convert a bitstring to a string" + "protocol String.Chars not implemented for type BitString, cannot convert a bitstring to a string\n\nGot value:\n\n<<0, 1::size(4)>>" assert_raise Protocol.UndefinedError, message, fn -> to_string(<<1::size(12)-integer-signed>>) @@ -113,7 +113,7 @@ defmodule String.Chars.ErrorsTest do end test "tuple" do - message = "protocol String.Chars not implemented for {1, 2, 3} of type Tuple" + message = "protocol String.Chars not implemented for type Tuple\n\nGot value:\n\n{1, 2, 3}" assert_raise Protocol.UndefinedError, message, fn -> to_string({1, 2, 3}) @@ -121,7 +121,7 @@ defmodule String.Chars.ErrorsTest do end test "PID" do - message = ~r"^protocol String\.Chars not implemented for #PID<.+?> of type PID$" + message = ~r"^protocol String\.Chars not implemented for type PID\n\nGot value:\n\n#PID<.+?>$" assert_raise Protocol.UndefinedError, message, fn -> to_string(self()) @@ -129,7 +129,8 @@ defmodule String.Chars.ErrorsTest do end test "ref" do - message = ~r"^protocol String\.Chars not implemented for #Reference<.+?> of type Reference$" + message = + ~r"^protocol String\.Chars not implemented for type Reference\n\nGot value:\n\n#Reference<.+?>$" assert_raise Protocol.UndefinedError, message, fn -> to_string(make_ref()) == "" @@ -137,7 +138,8 @@ defmodule String.Chars.ErrorsTest do end test "function" do - message = ~r"^protocol String\.Chars not implemented for #Function<.+?> of type Function$" + message = + ~r"^protocol String\.Chars not implemented for type Function\n\nGot value:\n\n#Function<.+?>$" assert_raise Protocol.UndefinedError, message, fn -> to_string(fn -> nil end) @@ -146,7 +148,9 @@ defmodule String.Chars.ErrorsTest do test "port" do [port | _] = Port.list() - message = ~r"^protocol String\.Chars not implemented for #Port<.+?> of type Port$" + + message = + ~r"^protocol String\.Chars not implemented for type Port\n\nGot value:\n\n#Port<.+?>$" assert_raise Protocol.UndefinedError, message, fn -> to_string(port) @@ -155,7 +159,7 @@ defmodule String.Chars.ErrorsTest do test "user-defined struct" do message = - "protocol String\.Chars not implemented for %String.Chars.ErrorsTest.Foo{foo: \"bar\"} of type String.Chars.ErrorsTest.Foo (a struct)" + "protocol String\.Chars not implemented for type String.Chars.ErrorsTest.Foo (a struct)\n\nGot value:\n\n%String.Chars.ErrorsTest.Foo{foo: \"bar\"}" assert_raise Protocol.UndefinedError, message, fn -> to_string(%Foo{}) From a6cc84c6c06748a76de82d4b9071e7f8a409e15f Mon Sep 17 00:00:00 2001 From: Dave Lucia Date: Wed, 20 Nov 2024 21:30:05 -0500 Subject: [PATCH 8/9] indent the text --- lib/elixir/lib/exception.ex | 13 ++++++++- lib/elixir/test/elixir/inspect_test.exs | 18 ++++++------- .../elixir/protocol/consolidation_test.exs | 2 +- lib/elixir/test/elixir/protocol_test.exs | 9 ++++++- lib/elixir/test/elixir/string/chars_test.exs | 27 ++++++++++++++----- 5 files changed, 50 insertions(+), 19 deletions(-) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index c450d93b3a0..986211c8fb1 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -1997,11 +1997,22 @@ defmodule Protocol.UndefinedError do @impl true def message(%{protocol: protocol, value: value, description: description}) do + inspected = + value + |> inspect(pretty: true) + |> String.replace(~r/^(?=.+)/m, " ") + "protocol #{inspect(protocol)} not implemented for type " <> value_type(value) <> maybe_description(description) <> maybe_available(protocol) <> - "\n\nGot value:\n\n#{inspect(value, pretty: true)}" + """ + + + Got value: + + #{inspected} + """ end defp value_type(%{__struct__: struct}), do: "#{inspect(struct)} (a struct)" diff --git a/lib/elixir/test/elixir/inspect_test.exs b/lib/elixir/test/elixir/inspect_test.exs index a5e40cbbc3b..d7f2ffca477 100644 --- a/lib/elixir/test/elixir/inspect_test.exs +++ b/lib/elixir/test/elixir/inspect_test.exs @@ -530,16 +530,16 @@ defmodule Inspect.MapTest do Got value: - #Inspect.Error< - got ArgumentError with message: + #Inspect.Error< + got ArgumentError with message: - """ - errors were found at the given arguments: + """ + errors were found at the given arguments: - * 1st argument: not an atom - """ + * 1st argument: not an atom + """ - while inspecting: + while inspecting: ''' @@ -934,7 +934,7 @@ defmodule Inspect.CustomProtocolTest do Got value: - %Inspect.CustomProtocolTest.MissingImplementation{} + %Inspect.CustomProtocolTest.MissingImplementation{} """ while inspecting: @@ -965,7 +965,7 @@ defmodule Inspect.CustomProtocolTest do Got value: - %Inspect.CustomProtocolTest.MissingImplementation{} + %Inspect.CustomProtocolTest.MissingImplementation{} """ while inspecting: diff --git a/lib/elixir/test/elixir/protocol/consolidation_test.exs b/lib/elixir/test/elixir/protocol/consolidation_test.exs index d9f01335cc9..d8adc72dd97 100644 --- a/lib/elixir/test/elixir/protocol/consolidation_test.exs +++ b/lib/elixir/test/elixir/protocol/consolidation_test.exs @@ -184,7 +184,7 @@ defmodule Protocol.ConsolidationTest do message = "protocol Protocol.ConsolidationTest.Sample not implemented for type Atom. " <> "This protocol is implemented for the following type(s): Protocol.ConsolidationTest.ImplStruct" <> - "\n\nGot value:\n\n:foo" + "\n\nGot value:\n\n :foo\n" assert_raise Protocol.UndefinedError, message, fn -> sample = String.to_atom("Elixir.Protocol.ConsolidationTest.Sample") diff --git a/lib/elixir/test/elixir/protocol_test.exs b/lib/elixir/test/elixir/protocol_test.exs index e0f846325f0..041937f44f1 100644 --- a/lib/elixir/test/elixir/protocol_test.exs +++ b/lib/elixir/test/elixir/protocol_test.exs @@ -113,7 +113,14 @@ defmodule ProtocolTest do end test "protocol not implemented" do - message = "protocol ProtocolTest.Sample not implemented for type Atom\n\nGot value:\n\n:foo" + message = + """ + protocol ProtocolTest.Sample not implemented for type Atom + + Got value: + + :foo + """ assert_raise Protocol.UndefinedError, message, fn -> sample = String.to_atom("Elixir.ProtocolTest.Sample") diff --git a/lib/elixir/test/elixir/string/chars_test.exs b/lib/elixir/test/elixir/string/chars_test.exs index eb5342d20da..dd3cf61a02e 100644 --- a/lib/elixir/test/elixir/string/chars_test.exs +++ b/lib/elixir/test/elixir/string/chars_test.exs @@ -105,7 +105,13 @@ defmodule String.Chars.ErrorsTest do test "bitstring" do message = - "protocol String.Chars not implemented for type BitString, cannot convert a bitstring to a string\n\nGot value:\n\n<<0, 1::size(4)>>" + """ + protocol String.Chars not implemented for type BitString, cannot convert a bitstring to a string + + Got value: + + <<0, 1::size(4)>> + """ assert_raise Protocol.UndefinedError, message, fn -> to_string(<<1::size(12)-integer-signed>>) @@ -113,7 +119,13 @@ defmodule String.Chars.ErrorsTest do end test "tuple" do - message = "protocol String.Chars not implemented for type Tuple\n\nGot value:\n\n{1, 2, 3}" + message = """ + protocol String.Chars not implemented for type Tuple + + Got value: + + {1, 2, 3} + """ assert_raise Protocol.UndefinedError, message, fn -> to_string({1, 2, 3}) @@ -121,7 +133,8 @@ defmodule String.Chars.ErrorsTest do end test "PID" do - message = ~r"^protocol String\.Chars not implemented for type PID\n\nGot value:\n\n#PID<.+?>$" + message = + ~r"^protocol String\.Chars not implemented for type PID\n\nGot value:\n\n #PID<.+?>$" assert_raise Protocol.UndefinedError, message, fn -> to_string(self()) @@ -130,7 +143,7 @@ defmodule String.Chars.ErrorsTest do test "ref" do message = - ~r"^protocol String\.Chars not implemented for type Reference\n\nGot value:\n\n#Reference<.+?>$" + ~r"^protocol String\.Chars not implemented for type Reference\n\nGot value:\n\n #Reference<.+?>$" assert_raise Protocol.UndefinedError, message, fn -> to_string(make_ref()) == "" @@ -139,7 +152,7 @@ defmodule String.Chars.ErrorsTest do test "function" do message = - ~r"^protocol String\.Chars not implemented for type Function\n\nGot value:\n\n#Function<.+?>$" + ~r"^protocol String\.Chars not implemented for type Function\n\nGot value:\n\n #Function<.+?>$" assert_raise Protocol.UndefinedError, message, fn -> to_string(fn -> nil end) @@ -150,7 +163,7 @@ defmodule String.Chars.ErrorsTest do [port | _] = Port.list() message = - ~r"^protocol String\.Chars not implemented for type Port\n\nGot value:\n\n#Port<.+?>$" + ~r"^protocol String\.Chars not implemented for type Port\n\nGot value:\n\n #Port<.+?>$" assert_raise Protocol.UndefinedError, message, fn -> to_string(port) @@ -159,7 +172,7 @@ defmodule String.Chars.ErrorsTest do test "user-defined struct" do message = - "protocol String\.Chars not implemented for type String.Chars.ErrorsTest.Foo (a struct)\n\nGot value:\n\n%String.Chars.ErrorsTest.Foo{foo: \"bar\"}" + "protocol String\.Chars not implemented for type String.Chars.ErrorsTest.Foo (a struct)\n\nGot value:\n\n %String.Chars.ErrorsTest.Foo{foo: \"bar\"}\n" assert_raise Protocol.UndefinedError, message, fn -> to_string(%Foo{}) From 08ed97ec0d28c9ed09e799b8fbe6614aa4178033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 21 Nov 2024 09:58:01 +0100 Subject: [PATCH 9/9] Update lib/elixir/lib/exception.ex --- lib/elixir/lib/exception.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex index 986211c8fb1..63ec7a2407d 100644 --- a/lib/elixir/lib/exception.ex +++ b/lib/elixir/lib/exception.ex @@ -2000,6 +2000,7 @@ defmodule Protocol.UndefinedError do inspected = value |> inspect(pretty: true) + # Indent only lines with contents on them |> String.replace(~r/^(?=.+)/m, " ") "protocol #{inspect(protocol)} not implemented for type " <>