Skip to content

Add :sort_maps to Inspect.Opts.custom_options #12404

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/elixir/lib/inspect.ex
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,12 @@ end

defimpl Inspect, for: Map do
def inspect(map, opts) do
list = Map.to_list(map)
list =
if Keyword.get(opts.custom_options, :sort_maps) do
map |> Map.to_list() |> :lists.sort()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I benched this with the new :maps.iterator(map, :ordered): sabiwara/elixir_benches@71055e9, but this implementation has the benefit of:

  • being faster or equivalent
  • using half as much memory
  • work on OTP < 26

So I went with this one instead.

else
Map.to_list(map)
end

fun =
if Inspect.List.keyword?(list) do
Expand Down
5 changes: 5 additions & 0 deletions lib/elixir/lib/inspect/algebra.ex
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ defmodule Inspect.Opts do
options. Useful when implementing the `Inspect` protocol for nested structs
to pass the custom options through.

It supports some pre-defined keys:
- `:sort_maps` (since v1.15.0) - if set to `true`, sorts key-value pairs in maps.
This can be helpful to make map inspection deterministic for testing,
especially since key order is random since OTP 26.

* `:inspect_fun` (since v1.9.0) - a function to build algebra documents.
Defaults to `Inspect.Opts.default_inspect_fun/0`.

Expand Down
33 changes: 22 additions & 11 deletions lib/elixir/test/elixir/inspect_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -417,14 +417,20 @@ defmodule Inspect.MapTest do
test "basic" do
assert inspect(%{1 => "b"}) == "%{1 => \"b\"}"

assert inspect(%{1 => "b", 2 => "c"}, pretty: true, width: 1) ==
assert inspect(%{1 => "b", 2 => "c"},
pretty: true,
width: 1,
custom_options: [sort_maps: true]
) ==
"%{\n 1 => \"b\",\n 2 => \"c\"\n}"
end

test "keyword" do
assert inspect(%{a: 1}) == "%{a: 1}"
assert inspect(%{a: 1, b: 2}) == "%{a: 1, b: 2}"
assert inspect(%{a: 1, b: 2, c: 3}) == "%{a: 1, b: 2, c: 3}"
assert inspect(%{a: 1, b: 2}, custom_options: [sort_maps: true]) == "%{a: 1, b: 2}"

assert inspect(%{a: 1, b: 2, c: 3}, custom_options: [sort_maps: true]) ==
"%{a: 1, b: 2, c: 3}"
end

test "with limit" do
Expand All @@ -446,12 +452,12 @@ defmodule Inspect.MapTest do
test "public modified struct" do
public = %Public{key: 1}

assert inspect(Map.put(public, :foo, :bar)) ==
assert inspect(Map.put(public, :foo, :bar), custom_options: [sort_maps: true]) ==
"%{__struct__: Inspect.MapTest.Public, foo: :bar, key: 1}"
end

test "private struct" do
assert inspect(%{__struct__: Private, key: 1}) ==
assert inspect(%{__struct__: Private, key: 1}, custom_options: [sort_maps: true]) ==
"%{__struct__: Inspect.MapTest.Private, key: 1}"
end

Expand Down Expand Up @@ -486,7 +492,7 @@ defmodule Inspect.MapTest do
%{__struct__: Inspect.MapTest.Failing, name: "Foo"}
'''

assert inspect(%Failing{name: "Foo"}) =~ message
assert inspect(%Failing{name: "Foo"}, custom_options: [sort_maps: true]) =~ message
end

test "safely inspect bad implementation disables colors" do
Expand All @@ -505,7 +511,10 @@ defmodule Inspect.MapTest do
%{__struct__: Inspect.MapTest.Failing, name: "Foo"}
'''

assert inspect(%Failing{name: "Foo"}, syntax_colors: [atom: [:green]]) =~ message
assert inspect(%Failing{name: "Foo"},
syntax_colors: [atom: [:green]],
custom_options: [sort_maps: true]
) =~ message
end

test "unsafely inspect bad implementation" do
Expand All @@ -524,7 +533,7 @@ defmodule Inspect.MapTest do
'''

try do
inspect(%Failing{name: "Foo"}, safe: false)
inspect(%Failing{name: "Foo"}, safe: false, custom_options: [sort_maps: true])
rescue
exception in Inspect.Error ->
assert Exception.message(exception) =~ exception_message
Expand Down Expand Up @@ -552,14 +561,16 @@ defmodule Inspect.MapTest do

while inspecting:

%{__struct__: Inspect.MapTest.Failing, name: "Foo"}
'''

try do
Enum.to_list(%Failing{name: "Foo"})
rescue
exception in Protocol.UndefinedError ->
assert Exception.message(exception) =~ exception_message
message = Exception.message(exception)
assert message =~ exception_message
assert message =~ "__struct__: Inspect.MapTest.Failing"
assert message =~ "name: \"Foo\""

assert [
{Enumerable, :impl_for!, 1, _} | _
Expand Down Expand Up @@ -617,7 +628,7 @@ defmodule Inspect.MapTest do
)
)

assert inspect(%Failing{name: "Foo"}) =~ message
assert inspect(%Failing{name: "Foo"}, custom_options: [sort_maps: true]) =~ message
assert inspected =~ message
end

Expand Down
2 changes: 1 addition & 1 deletion lib/elixir/test/elixir/map_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ defmodule MapTest do
assert Map.replace!(map, :b, 10) == %{c: 3, b: 10, a: 1}
assert Map.replace!(map, :a, 1) == map

assert_raise KeyError, "key :x not found in: %{a: 1, b: 2, c: 3}", fn ->
assert_raise KeyError, ~r/key :x not found in: %{.*a: 1.*}/, fn ->
Map.replace!(map, :x, 10)
end

Expand Down