From 039204ca11bd1a8f603bf313aeef56cba172e506 Mon Sep 17 00:00:00 2001 From: Tyler Young Date: Mon, 9 Dec 2024 11:03:46 -0600 Subject: [PATCH] Fix non-existing atom exception when accidentally matching on map literal This fixes a crash that looks like this: ``` ** (ArgumentError) errors were found at the given arguments: * 1st argument: not an already existing atom ``` ...which was previously occurring when you used the literal map matcher on a map with arbitrary string keys that don't have a corresponding atom defined. For me, this occurred when I forgot to wrap my map matcher in a `superset/1`. So, for instance, what I *meant* was: ```elixir assert %{"foo" => "bar", "baz" => "bop"} ~> superset(%{"foo" => "bar"}) ``` ...but what I accidentally wrote was just: ```elixir assert %{"foo" => "bar", "baz" => "bop"} ~> %{"foo" => "bar"} ``` Because the error was so generic, this was pretty difficult to debug. With this change applied, though, you now get a nice "Unexpected key" error. --- lib/machete/matchers/literal_map_matcher.ex | 14 +++++++++----- test/machete/matchers/literal_map_matcher_test.exs | 13 +++++++++++++ test/machete/matchers/superset_matcher_test.exs | 4 ++++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/lib/machete/matchers/literal_map_matcher.ex b/lib/machete/matchers/literal_map_matcher.ex index cce2e8b..cd8ef8b 100644 --- a/lib/machete/matchers/literal_map_matcher.ex +++ b/lib/machete/matchers/literal_map_matcher.ex @@ -31,9 +31,7 @@ defmodule Machete.LiteralMapMatcher do string_keys = extra_keys - |> Enum.filter( - &(is_binary(&1) && MapSet.member?(missing_keys, String.to_existing_atom(&1))) - ) + |> Enum.filter(&(is_binary(&1) && MapSet.member?(missing_keys, safe_existing_atom(&1)))) |> MapSet.new() extra_keys = @@ -43,8 +41,8 @@ defmodule Machete.LiteralMapMatcher do missing_keys = missing_keys - |> MapSet.difference(atom_keys |> Enum.map(&Kernel.to_string/1) |> MapSet.new()) - |> MapSet.difference(string_keys |> Enum.map(&String.to_existing_atom/1) |> MapSet.new()) + |> MapSet.difference(MapSet.new(atom_keys, &Kernel.to_string/1)) + |> MapSet.difference(MapSet.new(string_keys, &safe_existing_atom/1)) Enum.flat_map(extra_keys, &mismatch("Unexpected key", &1)) ++ Enum.flat_map(missing_keys, &mismatch("Missing key", &1)) ++ @@ -60,5 +58,11 @@ defmodule Machete.LiteralMapMatcher do _ -> [] end) end + + defp safe_existing_atom(str) do + String.to_existing_atom(str) + rescue + ArgumentError -> {:error, :machete_no_existing_atom} + end end end diff --git a/test/machete/matchers/literal_map_matcher_test.exs b/test/machete/matchers/literal_map_matcher_test.exs index 56ac554..3423e13 100644 --- a/test/machete/matchers/literal_map_matcher_test.exs +++ b/test/machete/matchers/literal_map_matcher_test.exs @@ -33,6 +33,19 @@ defmodule LiteralMapMatcherTest do assert 1 ~>> %{} ~> mismatch("Value is not a map") end + test "produces useful mismatches on non-existing atoms" do + assert %{"abcdefghijkl" => 1, "mnopqrstuvwxyz" => 2} + ~>> %{"abcdefghijkl" => 1} + ~> mismatch("Unexpected key", "mnopqrstuvwxyz") + + assert %{"abcdefghijkl" => 1, "mnopqrstuvwxyz" => 2, "zyxwvutsrq" => 3} + ~>> %{"abcdefghijkl" => 1} + ~> [ + %Machete.Mismatch{message: "Unexpected key", path: ["mnopqrstuvwxyz"]}, + %Machete.Mismatch{message: "Unexpected key", path: ["zyxwvutsrq"]} + ] + end + describe "nested matchers" do test "matches when nested matcher returns empty list" do assert %{a: 1} ~>> %{a: test_matcher(behaviour: :match_returning_list)} ~> [] diff --git a/test/machete/matchers/superset_matcher_test.exs b/test/machete/matchers/superset_matcher_test.exs index 5345100..bf060c7 100644 --- a/test/machete/matchers/superset_matcher_test.exs +++ b/test/machete/matchers/superset_matcher_test.exs @@ -17,4 +17,8 @@ defmodule SupersetMatcherTest do test "produces a useful mismatch for when value is not a map" do assert 1 ~>> superset(%{a: 1}) ~> mismatch("1 is not a map") end + + test "handles non-existing atoms" do + assert %{"abcdefghijkl" => 1, "mnopqrstuvwxyz" => 2} ~> superset(%{"abcdefghijkl" => 1}) + end end