Consider keys may be overridden in a map

Author: josevalim

lib/elixir/lib/module/types/expr.ex

@@ -118,14 +118,32 @@ defmodule Module.Types.Expr do
   def of_expr({:%{}, meta, [{:|, _, [map, args]}]} = expr, stack, context) do
     {map_type, context} = of_expr(map, stack, context)
 
-    Of.permutate_map(args, stack, context, &of_expr/3, fn _closed?, pairs ->
-      # TODO: If closed? is false, we need to open up the map
-      Enum.reduce(pairs, map_type, fn {key, type}, acc ->
-        case map_put(acc, key, type) do
-          descr when is_descr(descr) -> descr
-          :badmap -> throw({:badmap, map_type, expr, context})
-        end
-      end)
+    Of.permutate_map(args, stack, context, &of_expr/3, fn fallback, keys, pairs ->
+      # If there is no fallback (i.e. it is closed), we can update the existing map,
+      # otherwise we only assert the existing keys.
+      keys = if fallback == none(), do: keys, else: Enum.map(pairs, &elem(&1, 0)) ++ keys
+
+      # Assert the keys exist
+      fallback =
+        Enum.reduce(keys, fallback, fn key, acc ->
+          case map_fetch(map_type, key) do
+            {_, value_type} -> union(value_type, acc)
+            :badkey -> throw({:badkey, map_type, key, expr, context})
+            :badmap -> throw({:badmap, map_type, expr, context})
+          end
+        end)
+
+      if fallback == none() do
+        Enum.reduce(pairs, map_type, fn {key, type}, acc ->
+          case map_put(acc, key, type) do
+            descr when is_descr(descr) -> descr
+            :badmap -> throw({:badmap, map_type, expr, context})
+          end
+        end)
+      else
+        # TODO: Use the fallback type to actually indicate if open or closed.
+        open_map(pairs)
+      end
     end)
   catch
     error -> {error_type(), error(__MODULE__, error, meta, stack, context)}
@@ -547,6 +565,27 @@ defmodule Module.Types.Expr do
     }
   end
 
+  def format_diagnostic({:badkey, type, key, expr, context}) do
+    traces = collect_traces(expr, context)
+
+    %{
+      details: %{typing_traces: traces},
+      message:
+        IO.iodata_to_binary([
+          """
+          expected a map with key #{inspect(key)} in map update syntax:
+
+              #{expr_to_string(expr) |> indent(4)}
+
+          but got type:
+
+              #{to_quoted_string(type) |> indent(4)}
+          """,
+          format_traces(traces)
+        ])
+    }
+  end
+
   def format_diagnostic({:badbinary, type, expr, context}) do
     traces = collect_traces(expr, context)
 

lib/elixir/lib/module/types/of.ex

@@ -108,58 +108,72 @@ defmodule Module.Types.Of do
   Builds a closed map.
   """
   def closed_map(pairs, stack, context, of_fun) do
-    permutate_map(pairs, stack, context, of_fun, fn closed?, pairs ->
-      if closed?, do: closed_map(pairs), else: open_map(pairs)
+    permutate_map(pairs, stack, context, of_fun, fn fallback, _keys, pairs ->
+      # TODO: Use the fallback type to actually indicate if open or closed.
+      if fallback == none(), do: closed_map(pairs), else: open_map(pairs)
     end)
   end
 
   @doc """
   Builds permutation of maps according to the given keys.
   """
   def permutate_map(pairs, stack, context, of_fun, of_map) do
-    {closed?, single, multiple, context} =
-      Enum.reduce(pairs, {true, [], [], context}, fn
-        {key, value}, {closed?, single, multiple, context} ->
-          {keys, context} = of_finite_key_type(key, stack, context, of_fun)
+    {dynamic?, fallback, single, multiple, assert, context} =
+      Enum.reduce(pairs, {false, none(), [], [], [], context}, fn
+        {key, value}, {dynamic?, fallback, single, multiple, assert, context} ->
+          {dynamic_key?, keys, context} = of_finite_key_type(key, stack, context, of_fun)
           {value_type, context} = of_fun.(value, stack, context)
+          dynamic? = dynamic? or dynamic_key? or gradual?(value_type)
 
           case keys do
             :none ->
-              {false, single, multiple, context}
+              fallback = union(fallback, value_type)
+
+              {fallback, assert} =
+                Enum.reduce(single, {fallback, assert}, fn {key, type}, {fallback, assert} ->
+                  {union(fallback, type), [key | assert]}
+                end)
+
+              {fallback, assert} =
+                Enum.reduce(multiple, {fallback, assert}, fn {keys, type}, {fallback, assert} ->
+                  {union(fallback, type), keys ++ assert}
+                end)
+
+              {dynamic?, fallback, [], [], assert, context}
 
             [key] when multiple == [] ->
-              {closed?, [{key, value_type} | single], multiple, context}
+              {dynamic?, fallback, [{key, value_type} | single], multiple, assert, context}
 
             keys ->
-              {closed?, single, [{keys, value_type} | multiple], context}
+              {dynamic?, fallback, single, [{keys, value_type} | multiple], assert, context}
           end
       end)
 
     map =
       case Enum.reverse(multiple) do
         [] ->
-          of_map.(closed?, Enum.reverse(single))
+          of_map.(fallback, Enum.uniq(assert), Enum.reverse(single))
 
         [{keys, type} | tail] ->
           for key <- keys, t <- cartesian_map(tail) do
-            of_map.(closed?, Enum.reverse(single, [{key, type} | t]))
+            of_map.(fallback, Enum.uniq(assert), Enum.reverse(single, [{key, type} | t]))
           end
           |> Enum.reduce(&union/2)
       end
 
-    {map, context}
+    if dynamic?, do: {dynamic(map), context}, else: {map, context}
   end
 
   defp of_finite_key_type(key, _stack, context, _of_fun) when is_atom(key) do
-    {[key], context}
+    {false, [key], context}
   end
 
   defp of_finite_key_type(key, stack, context, of_fun) do
     {key_type, context} = of_fun.(key, stack, context)
 
     case atom_fetch(key_type) do
-      {:finite, list} -> {list, context}
-      _ -> {:none, context}
+      {:finite, list} -> {gradual?(key_type), list, context}
+      _ -> {gradual?(key_type), :none, context}
     end
   end
 

lib/elixir/test/elixir/module/types/expr_test.exs

@@ -443,10 +443,8 @@ defmodule Module.Types.ExprTest do
   end
 
   describe "maps/structs" do
-    test "creating maps" do
+    test "creating closed maps" do
       assert typecheck!(%{foo: :bar}) == closed_map(foo: atom([:bar]))
-      assert typecheck!(%{123 => 456}) == open_map()
-      assert typecheck!(%{123 => 456, foo: :bar}) == open_map(foo: atom([:bar]))
       assert typecheck!([x], %{key: x}) == dynamic(closed_map(key: term()))
 
       assert typecheck!(
@@ -457,6 +455,14 @@ defmodule Module.Types.ExprTest do
              ) == closed_map(foo: atom([:second]))
     end
 
+    test "creating open maps" do
+      assert typecheck!(%{123 => 456}) == open_map()
+      # Since key cannot override :foo, we preserve it
+      assert typecheck!([key], %{key => 456, foo: :bar}) == dynamic(open_map(foo: atom([:bar])))
+      # Since key can override :foo, we do not preserve it
+      assert typecheck!([key], %{:foo => :bar, key => :baz}) == dynamic(open_map())
+    end
+
     test "creating structs" do
       assert typecheck!(%Point{}) ==
                closed_map(
@@ -475,7 +481,7 @@ defmodule Module.Types.ExprTest do
                )
     end
 
-    test "updating maps" do
+    test "updating to closed maps" do
       assert typecheck!([x], %{x | x: :zero}) ==
                dynamic(open_map(x: atom([:zero])))
 
@@ -499,6 +505,64 @@ defmodule Module.Types.ExprTest do
              """
     end
 
+    test "updating to open maps" do
+      assert typecheck!(
+               [key],
+               (
+                 x = %{foo: :bar}
+                 %{x | key => :baz}
+               )
+             ) == dynamic(open_map())
+
+      # Since key cannot override :foo, we preserve it
+      assert typecheck!(
+               [key],
+               (
+                 x = %{foo: :bar}
+                 %{x | key => :baz, foo: :bat}
+               )
+             ) == dynamic(open_map(foo: atom([:bat])))
+
+      # Since key can override :foo, we do not preserve it
+      assert typecheck!(
+               [key],
+               (
+                 x = %{foo: :bar}
+                 %{x | :foo => :baz, key => :bat}
+               )
+             ) == dynamic(open_map())
+
+      # The goal of this test is to verufy we assert keys,
+      # even if they may be overridden later.
+      assert typeerror!(
+               [key],
+               (
+                 x = %{key: :value}
+                 %{x | :foo => :baz, key => :bat}
+               )
+             ) == ~l"""
+             expected a map with key :foo in map update syntax:
+
+                 %{x | :foo => :baz, key => :bat}
+
+             but got type:
+
+                 %{key: :value}
+
+             where "key" was given the type:
+
+                 # type: dynamic()
+                 # from: types_test.ex:538
+                 key
+
+             where "x" was given the type:
+
+                 # type: %{key: :value}
+                 # from: types_test.ex:540
+                 x = %{key: :value}
+             """
+    end
+
     test "updating structs" do
       assert typecheck!([x], %Point{x | x: :zero}) ==
                dynamic(open_map(__struct__: atom([Point]), x: atom([:zero])))