Skip to content

Commit aee1747

Browse files
josevalimjosevalim
committed
Type check Map.pop!/2, Map.pop/2, and Map.pop/3
1 parent fabbf5e commit aee1747

File tree

2 files changed

+225
-27
lines changed

2 files changed

+225
-27
lines changed

‎lib/elixir/lib/module/types/apply.ex‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,9 @@ defmodule Module.Types.Apply do
245245

246246
## Map
247247
{Map, :from_struct, [{[open_map()], open_map(__struct__: not_set())}]},
248+
{Map, :pop, [{[open_map(), term()], tuple([term(), open_map()])}]},
249+
{Map, :pop, [{[open_map(), term(), term()], tuple([term(), open_map()])}]},
250+
{Map, :pop!, [{[open_map(), term()], tuple([term(), open_map()])}]},
248251
{:maps, :from_keys, [{[list(term()), term()], open_map()}]},
249252
{:maps, :find,
250253
[{[term(), open_map()], tuple([atom([:ok]), term()]) |> union(atom([:error]))}]},
@@ -429,6 +432,7 @@ defmodule Module.Types.Apply do
429432
end
430433

431434
@struct_key atom([:__struct__])
435+
@nil_atom atom([nil])
432436

433437
defp remote_apply(Map, :from_struct, _info, [map] = args_types, stack) do
434438
case map_update(map, @struct_key, not_set(), false, true) do
@@ -438,6 +442,34 @@ defmodule Module.Types.Apply do
438442
end
439443
end
440444

445+
defp remote_apply(Map, :pop, _info, args_types, stack) do
446+
[map, key, default] =
447+
case args_types do
448+
[map, key] -> [map, key, @nil_atom]
449+
_ -> args_types
450+
end
451+
452+
case map_update(map, key, not_set(), true, false) do
453+
{value, descr, _errors} ->
454+
value = union(value, default)
455+
{:ok, return(tuple([value, descr]), args_types, stack)}
456+
457+
:badmap ->
458+
{:error, badremote(Map, :pop, length(args_types))}
459+
460+
{:error, _errors} ->
461+
{:error, {:badkeydomain, map, key, tuple([default, map])}}
462+
end
463+
end
464+
465+
defp remote_apply(Map, :pop!, _info, [map, key] = args_types, stack) do
466+
case map_update(map, key, not_set(), true, false) do
467+
{value, descr, _errors} -> {:ok, return(tuple([value, descr]), args_types, stack)}
468+
:badmap -> {:error, badremote(Map, :pop!, 2)}
469+
{:error, _errors} -> {:error, {:badkeydomain, map, key, nil}}
470+
end
471+
end
472+
441473
defp remote_apply(:maps, :find, _info, [key, map] = args_types, stack) do
442474
case map_get(map, key) do
443475
{_, value} ->
@@ -485,18 +517,8 @@ defmodule Module.Types.Apply do
485517

486518
defp remote_apply(:maps, :take, _info, [key, map] = args_types, stack) do
487519
case map_update(map, key, not_set(), true, false) do
488-
# We could suggest to use :maps.delete if the key always exists
489-
# but :maps.take/2 means calling Erlang directly, so we are fine.
490-
{value, descr, errors} ->
491-
result = tuple([value, descr])
492-
493-
result =
494-
if errors == [] and not gradual?(map) do
495-
result
496-
else
497-
union(result, atom([:error]))
498-
end
499-
520+
{value, descr, _errors} ->
521+
result = union(tuple([value, descr]), atom([:error]))
500522
{:ok, return(result, args_types, stack)}
501523

502524
:badmap ->

‎lib/elixir/test/elixir/module/types/map_test.exs‎

Lines changed: 191 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,16 @@ defmodule Module.Types.MapTest do
2424

2525
describe ":maps.take/2" do
2626
test "checking" do
27-
assert typecheck!(:maps.take(:key, %{key: 123})) |> equal?(tuple([integer(), empty_map()]))
27+
assert typecheck!(:maps.take(:key, %{key: 123})) ==
28+
tuple([integer(), empty_map()]) |> union(atom([:error]))
2829

29-
assert typecheck!([x], :maps.take(:key, x))
30-
|> equal?(
30+
assert typecheck!([x], :maps.take(:key, x)) ==
3131
union(
3232
dynamic(tuple([term(), open_map(key: not_set())])),
3333
atom([:error])
3434
)
35-
)
3635

37-
assert typecheck!([condition?, x], :maps.take(if(condition?, do: :foo, else: :bar), x))
38-
|> equal?(
36+
assert typecheck!([condition?, x], :maps.take(if(condition?, do: :foo, else: :bar), x)) ==
3937
union(
4038
dynamic(
4139
tuple([
@@ -48,15 +46,12 @@ defmodule Module.Types.MapTest do
4846
),
4947
atom([:error])
5048
)
51-
)
5249

53-
assert typecheck!([x], :maps.take(123, x))
54-
|> equal?(
50+
assert typecheck!([x], :maps.take(123, x)) ==
5551
union(
5652
dynamic(tuple([term(), open_map()])),
5753
atom([:error])
5854
)
59-
)
6055
end
6156

6257
test "inference" do
@@ -144,17 +139,17 @@ defmodule Module.Types.MapTest do
144139
assert typecheck!(Map.fetch(%{key: 123}, :key)) ==
145140
tuple([atom([:ok]), integer()]) |> union(atom([:error]))
146141

147-
assert typecheck!([x], Map.fetch(x, :key))
148-
|> equal?(dynamic(tuple([atom([:ok]), term()])) |> union(atom([:error])))
142+
assert typecheck!([x], Map.fetch(x, :key)) ==
143+
dynamic(tuple([atom([:ok]), term()])) |> union(atom([:error]))
149144

150145
# If one of them succeeds, we are still fine!
151146
assert typecheck!(
152147
[condition?],
153148
Map.fetch(%{foo: 123}, if(condition?, do: :foo, else: :bar))
154149
) == tuple([atom([:ok]), integer()]) |> union(atom([:error]))
155150

156-
assert typecheck!([x], Map.fetch(x, 123))
157-
|> equal?(dynamic(tuple([atom([:ok]), term()])) |> union(atom([:error])))
151+
assert typecheck!([x], Map.fetch(x, 123)) ==
152+
dynamic(tuple([atom([:ok]), term()])) |> union(atom([:error]))
158153
end
159154

160155
test "inference" do
@@ -199,7 +194,7 @@ defmodule Module.Types.MapTest do
199194
Map.fetch!(%{foo: 123}, if(condition?, do: :foo, else: :bar))
200195
) == integer()
201196

202-
assert typecheck!([x], Map.fetch!(x, 123)) |> equal?(dynamic())
197+
assert typecheck!([x], Map.fetch!(x, 123)) == dynamic()
203198
end
204199

205200
test "inference" do
@@ -534,6 +529,187 @@ defmodule Module.Types.MapTest do
534529
end
535530
end
536531

532+
describe "Map.pop/2" do
533+
test "checking" do
534+
assert typecheck!(Map.pop(%{key: 123}, :key)) ==
535+
tuple([union(integer(), atom([nil])), empty_map()])
536+
537+
assert typecheck!([x], Map.pop(x, :key)) ==
538+
dynamic(tuple([term(), open_map(key: not_set())]))
539+
540+
assert typecheck!([condition?, x], Map.pop(x, if(condition?, do: :foo, else: :bar))) ==
541+
dynamic(
542+
tuple([
543+
term(),
544+
union(
545+
open_map(foo: not_set()),
546+
open_map(bar: not_set())
547+
)
548+
])
549+
)
550+
551+
assert typecheck!(
552+
[x],
553+
(
554+
x = %{String.to_integer(x) => :before}
555+
Map.pop(x, 123)
556+
)
557+
) ==
558+
tuple([
559+
atom([:before, nil]),
560+
closed_map([{domain_key(:integer), atom([:before])}])
561+
])
562+
end
563+
564+
test "inference" do
565+
assert typecheck!(
566+
[x],
567+
(
568+
_ = Map.pop(x, :key)
569+
x
570+
)
571+
) == dynamic(open_map())
572+
end
573+
574+
test "errors" do
575+
assert typeerror!([x = []], Map.pop(x, :foo)) =~
576+
"incompatible types given to Map.pop/2"
577+
578+
assert typeerror!(Map.pop(%{}, :key)) =~ """
579+
incompatible types given to Map.pop/2:
580+
581+
Map.pop(%{}, :key)
582+
583+
the map:
584+
585+
empty_map()
586+
587+
does not have all required keys:
588+
589+
:key
590+
591+
"""
592+
end
593+
end
594+
595+
describe "Map.pop/3" do
596+
test "checking" do
597+
assert typecheck!(Map.pop(%{key: 123}, :key, :error)) ==
598+
tuple([union(integer(), atom([:error])), empty_map()])
599+
600+
assert typecheck!([x], Map.pop(x, :key, :error)) ==
601+
dynamic(tuple([term(), open_map(key: not_set())]))
602+
603+
assert typecheck!([condition?, x], Map.pop(x, if(condition?, do: :foo, else: :bar), :error)) ==
604+
dynamic(
605+
tuple([
606+
term(),
607+
union(
608+
open_map(foo: not_set()),
609+
open_map(bar: not_set())
610+
)
611+
])
612+
)
613+
614+
assert typecheck!(
615+
[x],
616+
(
617+
x = %{String.to_integer(x) => :before}
618+
Map.pop(x, 123, :after)
619+
)
620+
) ==
621+
tuple([
622+
atom([:before, :after]),
623+
closed_map([{domain_key(:integer), atom([:before])}])
624+
])
625+
end
626+
627+
test "inference" do
628+
assert typecheck!(
629+
[x],
630+
(
631+
_ = Map.pop(x, :key, :error)
632+
x
633+
)
634+
) == dynamic(open_map())
635+
end
636+
637+
test "errors" do
638+
assert typeerror!([x = []], Map.pop(x, :foo, :error)) =~
639+
"incompatible types given to Map.pop/3"
640+
641+
assert typeerror!(Map.pop(%{}, :key, :error)) =~ """
642+
incompatible types given to Map.pop/3:
643+
644+
Map.pop(%{}, :key, :error)
645+
646+
the map:
647+
648+
empty_map()
649+
650+
does not have all required keys:
651+
652+
:key
653+
654+
"""
655+
end
656+
end
657+
658+
describe "Map.pop!/2" do
659+
test "checking" do
660+
assert typecheck!(Map.pop!(%{key: 123}, :key)) ==
661+
tuple([integer(), empty_map()])
662+
663+
assert typecheck!([x], Map.pop!(x, :key)) ==
664+
dynamic(tuple([term(), open_map(key: not_set())]))
665+
666+
assert typecheck!([condition?, x], Map.pop!(x, if(condition?, do: :foo, else: :bar))) ==
667+
dynamic(
668+
tuple([
669+
term(),
670+
union(
671+
open_map(foo: not_set()),
672+
open_map(bar: not_set())
673+
)
674+
])
675+
)
676+
677+
assert typecheck!([x], Map.pop!(x, 123)) ==
678+
dynamic(tuple([term(), open_map()]))
679+
end
680+
681+
test "inference" do
682+
assert typecheck!(
683+
[x],
684+
(
685+
_ = Map.pop!(x, :key)
686+
x
687+
)
688+
) == dynamic(open_map())
689+
end
690+
691+
test "errors" do
692+
assert typeerror!([x = []], Map.pop!(x, :foo)) =~
693+
"incompatible types given to Map.pop!/2"
694+
695+
assert typeerror!(Map.pop!(%{}, :key)) =~ """
696+
incompatible types given to Map.pop!/2:
697+
698+
Map.pop!(%{}, :key)
699+
700+
the map:
701+
702+
empty_map()
703+
704+
does not have all required keys:
705+
706+
:key
707+
708+
therefore this function will always raise
709+
"""
710+
end
711+
end
712+
537713
describe "Map.values/1" do
538714
test "checking" do
539715
assert typecheck!([x = %{}], Map.values(x)) == dynamic(list(term()))

0 commit comments

Comments
 (0)