Skip to content

Commit 4930cd4

Browse files
authored
Fix: put_in_map/set_map_key not setting keys properly (#348)
* Fix put_key_in_map not setting keys properly * Fix set_map_key not returning to correct map position consistently * Fix inserting into nested map in put_in_map * Fix put_in_map not setting nested keys properly * License comment * Use replace_code instead of Zipper.replace
1 parent bd2b582 commit 4930cd4

2 files changed

Lines changed: 179 additions & 82 deletions

File tree

lib/igniter/code/map.ex

Lines changed: 86 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -30,57 +30,59 @@ defmodule Igniter.Code.Map do
3030
end
3131

3232
defp do_put_in_map(zipper, [key | rest], value, updater) do
33-
cond do
34-
Common.node_matches_pattern?(zipper, {:%{}, _, []}) ->
35-
value = Common.use_aliases(value, zipper)
36-
37-
{:ok,
38-
Zipper.append_child(
39-
zipper,
40-
mappify([key | rest], value)
41-
)}
42-
43-
Common.node_matches_pattern?(zipper, {:%{}, _, _}) ->
44-
zipper
45-
|> Zipper.down()
46-
|> Igniter.Code.List.move_to_list_item(fn item ->
47-
if Igniter.Code.Tuple.tuple?(item) do
48-
case Igniter.Code.Tuple.tuple_elem(item, 0) do
49-
{:ok, first_elem} ->
50-
Common.nodes_equal?(first_elem, key)
51-
52-
:error ->
53-
false
33+
Common.within(zipper, fn zipper ->
34+
cond do
35+
Common.node_matches_pattern?(zipper, {:%{}, _, []}) ->
36+
value = Common.use_aliases(value, zipper)
37+
38+
{:ok,
39+
Common.replace_code(
40+
zipper,
41+
mappify([key | rest], value)
42+
)}
43+
44+
Common.node_matches_pattern?(zipper, {:%{}, _, _}) ->
45+
zipper
46+
|> Zipper.down()
47+
|> Common.move_right(fn item ->
48+
if Igniter.Code.Tuple.tuple?(item) do
49+
case Igniter.Code.Tuple.tuple_elem(item, 0) do
50+
{:ok, first_elem} ->
51+
Common.nodes_equal?(first_elem, key)
52+
53+
:error ->
54+
false
55+
end
5456
end
57+
end)
58+
|> case do
59+
:error ->
60+
value = Common.use_aliases(value, zipper)
61+
format = map_keys_format(zipper)
62+
value = mappify(rest, value)
63+
64+
{:ok,
65+
Zipper.append_child(
66+
zipper,
67+
{{:__block__, [format: format], [key]}, {:__block__, [], [value]}}
68+
)}
69+
70+
{:ok, zipper} ->
71+
zipper
72+
|> Igniter.Code.Tuple.tuple_elem(1)
73+
|> case do
74+
{:ok, zipper} ->
75+
do_put_in_map(zipper, rest, value, updater)
76+
77+
:error ->
78+
:error
79+
end
5580
end
56-
end)
57-
|> case do
58-
:error ->
59-
value = Common.use_aliases(value, zipper)
60-
format = map_keys_format(zipper)
61-
value = mappify(rest, value)
62-
63-
{:ok,
64-
Zipper.append_child(
65-
zipper,
66-
{{:__block__, [format: format], [key]}, {:__block__, [], [value]}}
67-
)}
68-
69-
{:ok, zipper} ->
70-
zipper
71-
|> Igniter.Code.Tuple.tuple_elem(1)
72-
|> case do
73-
{:ok, zipper} ->
74-
do_put_in_map(zipper, rest, value, updater)
75-
76-
:error ->
77-
:error
78-
end
79-
end
8081

81-
true ->
82-
:error
83-
end
82+
true ->
83+
:error
84+
end
85+
end)
8486
end
8587

8688
@doc "Puts a key into a map, calling `updater` on the zipper at the value if the key is already present"
@@ -92,48 +94,50 @@ defmodule Igniter.Code.Map do
9294
value = Common.use_aliases(value, zipper)
9395

9496
{:ok,
95-
Zipper.append_child(
97+
Common.replace_code(
9698
zipper,
9799
mappify([key], value)
98100
)}
99101

100102
Common.node_matches_pattern?(zipper, {:%{}, _, _}) ->
101-
zipper
102-
|> Zipper.down()
103-
|> Common.move_right(fn item ->
104-
if Igniter.Code.Tuple.tuple?(item) do
105-
case Igniter.Code.Tuple.tuple_elem(item, 0) do
106-
{:ok, first_elem} ->
107-
Common.nodes_equal?(first_elem, key)
108-
109-
:error ->
110-
false
103+
Common.within(zipper, fn zipper ->
104+
zipper
105+
|> Zipper.down()
106+
|> Common.move_right(fn item ->
107+
if Igniter.Code.Tuple.tuple?(item) do
108+
case Igniter.Code.Tuple.tuple_elem(item, 0) do
109+
{:ok, first_elem} ->
110+
Common.nodes_equal?(first_elem, key)
111+
112+
:error ->
113+
false
114+
end
111115
end
116+
end)
117+
|> case do
118+
:error ->
119+
value = Common.use_aliases(value, zipper)
120+
121+
format = map_keys_format(zipper)
122+
123+
{:ok,
124+
Zipper.append_child(
125+
zipper,
126+
{{:__block__, [format: format], [key]}, {:__block__, [], [value]}}
127+
)}
128+
129+
{:ok, zipper} ->
130+
zipper
131+
|> Igniter.Code.Tuple.tuple_elem(1)
132+
|> case do
133+
{:ok, zipper} ->
134+
{:ok, updater.(zipper)}
135+
136+
:error ->
137+
:error
138+
end
112139
end
113140
end)
114-
|> case do
115-
:error ->
116-
value = Common.use_aliases(value, zipper)
117-
118-
format = map_keys_format(zipper)
119-
120-
{:ok,
121-
Zipper.append_child(
122-
zipper,
123-
{{:__block__, [format: format], [key]}, {:__block__, [], [value]}}
124-
)}
125-
126-
{:ok, zipper} ->
127-
zipper
128-
|> Igniter.Code.Tuple.tuple_elem(1)
129-
|> case do
130-
{:ok, zipper} ->
131-
updater.(zipper)
132-
133-
:error ->
134-
:error
135-
end
136-
end
137141

138142
true ->
139143
:error

test/igniter/code/map_test.exs

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
# SPDX-FileCopyrightText: 2025 igniter contributors <https://github.com/ash-project/igniter/graphs.contributors>
2+
#
3+
# SPDX-License-Identifier: MIT
4+
5+
defmodule Igniter.Code.Map.Test do
6+
alias Igniter.Code
7+
alias Sourceror.Zipper
8+
9+
use ExUnit.Case
10+
11+
doctest Igniter.Code.Map
12+
13+
describe "set_map_key/4" do
14+
test "inserts value into empty map" do
15+
{:ok, zipper} =
16+
"%{}"
17+
|> Code.Common.parse_to_zipper!()
18+
|> Code.Map.set_map_key(:foo, :bar, fn _valuex -> flunk() end)
19+
20+
assert {:ok, %{foo: :bar}} == Code.Common.expand_literal(zipper)
21+
end
22+
23+
test "inserts value into map if not present" do
24+
{:ok, zipper} =
25+
"%{hello: :world}"
26+
|> Code.Common.parse_to_zipper!()
27+
|> Code.Map.set_map_key(:foo, :bar, &flunk/1)
28+
29+
assert {:ok, %{foo: :bar, hello: :world}} == Code.Common.expand_literal(zipper)
30+
end
31+
32+
test "replaces keys in map with the updater rather than the passed value" do
33+
{:ok, zipper} =
34+
"%{hello: :world}"
35+
|> Code.Common.parse_to_zipper!()
36+
|> Code.Map.set_map_key(:hello, :this_value_is_ignored, fn %Zipper{node: :world} = zipper ->
37+
Zipper.replace(zipper, :baz)
38+
end)
39+
40+
assert {:ok, %{hello: :baz}} == Code.Common.expand_literal(zipper)
41+
end
42+
end
43+
44+
describe "put_in_map/4" do
45+
test "inserts value into empty map" do
46+
{:ok, zipper} =
47+
"%{}"
48+
|> Code.Common.parse_to_zipper!()
49+
|> Code.Map.put_in_map([:foo], :bar, fn _valuex -> flunk() end)
50+
51+
assert {:ok, %{foo: :bar}} == Code.Common.expand_literal(zipper)
52+
end
53+
54+
test "inserts value into empty map at nested position" do
55+
{:ok, zipper} =
56+
"%{}"
57+
|> Code.Common.parse_to_zipper!()
58+
|> Code.Map.put_in_map([:alpha, :beta, :gamma], :abc, fn _valuex -> flunk() end)
59+
60+
assert {:ok, %{alpha: %{beta: %{gamma: :abc}}}} == Code.Common.expand_literal(zipper)
61+
end
62+
63+
test "inserts value into map if not present" do
64+
{:ok, zipper} =
65+
"%{hello: :world}"
66+
|> Code.Common.parse_to_zipper!()
67+
|> Code.Map.put_in_map([:foo], :bar, fn _valuex -> flunk() end)
68+
69+
assert {:ok, %{hello: :world, foo: :bar}} ==
70+
Code.Common.expand_literal(zipper)
71+
end
72+
73+
test "inserts nested value into map if not present" do
74+
{:ok, zipper} =
75+
"%{hello: :world}"
76+
|> Code.Common.parse_to_zipper!()
77+
|> Code.Map.put_in_map([:foo, :bar], :baz, fn _valuex -> flunk() end)
78+
79+
assert {:ok, %{hello: :world, foo: %{bar: :baz}}} ==
80+
Code.Common.expand_literal(zipper)
81+
end
82+
83+
test "inserts value into map if not present at nested position" do
84+
{:ok, zipper} =
85+
"%{alpha: %{beta: %{delta: :abd}}}"
86+
|> Code.Common.parse_to_zipper!()
87+
|> Code.Map.put_in_map([:alpha, :beta, :gamma], :abc, fn _valuex -> flunk() end)
88+
89+
assert {:ok, %{alpha: %{beta: %{delta: :abd, gamma: :abc}}}} ==
90+
Code.Common.expand_literal(zipper)
91+
end
92+
end
93+
end

0 commit comments

Comments
 (0)