Skip to content

Commit

Permalink
Fix non-existing atom exception when accidentally matching on map lit…
Browse files Browse the repository at this point in the history
…eral

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.
  • Loading branch information
s3cur3 committed Dec 9, 2024
1 parent 133289b commit 039204c
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 5 deletions.
14 changes: 9 additions & 5 deletions lib/machete/matchers/literal_map_matcher.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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)) ++
Expand All @@ -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
13 changes: 13 additions & 0 deletions test/machete/matchers/literal_map_matcher_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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)} ~> []
Expand Down
4 changes: 4 additions & 0 deletions test/machete/matchers/superset_matcher_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 039204c

Please sign in to comment.