Use a plain map for Ecto.Multi names to avoid Dialyzer opaqueness violations

lib/ecto/multi.ex

@@ -124,7 +124,7 @@ defmodule Ecto.Multi do
   alias __MODULE__
   alias Ecto.Changeset
 
-  defstruct operations: [], names: MapSet.new()
+  defstruct operations: [], names: %{}
 
   @typedoc """
   Map of changes made so far during the current transaction. For any Multi
@@ -147,7 +147,10 @@ defmodule Ecto.Multi do
            | {:insert_all, schema_or_source, [map | Keyword.t()], Keyword.t()}
   @typep operations :: [{name, operation}]
 
-  @typep names :: MapSet.t()
+  # A plain map is used instead of MapSet, as MapSet's internal :sets
+  # representation is opaque and Dialyzer reports opaqueness violations
+  # when the struct default above gets inlined into callers.
+  @typep names :: %{optional(name) => true}
 
   @typedoc """
   Name of an operation in the Multi. Can be any term, as long as it is unique
@@ -219,9 +222,9 @@ defmodule Ecto.Multi do
     %{names: lhs_names, operations: lhs_ops} = lhs
     %{names: rhs_names, operations: rhs_ops} = rhs
 
-    case MapSet.intersection(lhs_names, rhs_names) |> MapSet.to_list() do
+    case common_names(lhs_names, rhs_names) do
       [] ->
-        %Multi{names: MapSet.union(lhs_names, rhs_names), operations: joiner.(lhs_ops, rhs_ops)}
+        %Multi{names: Map.merge(lhs_names, rhs_names), operations: joiner.(lhs_ops, rhs_ops)}
 
       common ->
         raise ArgumentError, """
@@ -746,10 +749,10 @@ defmodule Ecto.Multi do
   defp add_operation(%Multi{} = multi, name, operation) do
     %{operations: operations, names: names} = multi
 
-    if MapSet.member?(names, name) do
+    if Map.has_key?(names, name) do
       raise "#{Kernel.inspect(name)} is already a member of the Ecto.Multi: \n#{Kernel.inspect(multi)}"
     else
-      %{multi | operations: [{name, operation} | operations], names: MapSet.put(names, name)}
+      %{multi | operations: [{name, operation} | operations], names: Map.put(names, name, true)}
     end
   end
 
@@ -942,18 +945,22 @@ defmodule Ecto.Multi do
   defp apply_run_fun(fun, repo, acc), do: apply(fun, [repo, acc])
 
   defp merge_results(changes, new_changes, names) do
-    new_names = new_changes |> Map.keys() |> MapSet.new()
+    new_names = Map.new(new_changes, fn {name, _} -> {name, true} end)
 
-    case MapSet.intersection(names, new_names) |> MapSet.to_list() do
+    case common_names(names, new_names) do
       [] ->
-        {Map.merge(changes, new_changes), MapSet.union(names, new_names)}
+        {Map.merge(changes, new_changes), Map.merge(names, new_names)}
 
       common ->
         raise "cannot merge Multi; the following operations were found in " <>
                 "both Ecto.Multi: #{Kernel.inspect(common)}"
     end
   end
 
+  defp common_names(lhs_names, rhs_names) do
+    for {name, true} <- lhs_names, Map.has_key?(rhs_names, name), do: name
+  end
+
   defp operation_fun({:update_all, queryable_fun, updates}, opts) do
     fn repo, changes ->
       {:ok, repo.update_all(queryable_fun.(changes), updates, opts)}

test/ecto/multi_test.exs

@@ -31,7 +31,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.insert(:comment, changeset)
 
-    assert multi.names == MapSet.new([:comment])
+    assert multi.names == %{:comment => true}
     assert multi.operations == [{:comment, {:changeset, %{changeset | action: :insert}, []}}]
   end
 
@@ -43,7 +43,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.insert(:comment, struct)
 
-    assert multi.names == MapSet.new([:comment])
+    assert multi.names == %{:comment => true}
     assert multi.operations == [{:comment, {:changeset, %{changeset | action: :insert}, []}}]
   end
 
@@ -55,7 +55,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.insert(:fun, fun)
 
-    assert multi.names == MapSet.new([:fun])
+    assert multi.names == %{:fun => true}
     assert [{:fun, {:run, _fun}}] = multi.operations
   end
 
@@ -66,7 +66,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.update(:comment, changeset)
 
-    assert multi.names == MapSet.new([:comment])
+    assert multi.names == %{:comment => true}
     assert multi.operations == [{:comment, {:changeset, %{changeset | action: :update}, []}}]
   end
 
@@ -78,7 +78,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.update(:fun, fun)
 
-    assert multi.names == MapSet.new([:fun])
+    assert multi.names == %{:fun => true}
     assert [{:fun, {:run, _fun}}] = multi.operations
   end
 
@@ -89,7 +89,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.insert_or_update(:comment, changeset)
 
-    assert multi.names == MapSet.new([:comment])
+    assert multi.names == %{:comment => true}
     assert multi.operations == [{:comment, {:changeset, %{changeset | action: :insert}, []}}]
   end
 
@@ -98,7 +98,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.inspect()
 
-    assert multi.names == MapSet.new([])
+    assert multi.names == %{}
     assert multi.operations == [{:inspect, {:inspect, []}}]
   end
 
@@ -110,7 +110,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.insert_or_update(:comment, changeset)
 
-    assert multi.names == MapSet.new([:comment])
+    assert multi.names == %{:comment => true}
     assert multi.operations == [{:comment, {:changeset, %{changeset | action: :update}, []}}]
   end
 
@@ -122,7 +122,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.insert_or_update(:fun, fun)
 
-    assert multi.names == MapSet.new([:fun])
+    assert multi.names == %{:fun => true}
     assert [{:fun, {:run, _fun}}] = multi.operations
   end
 
@@ -133,7 +133,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.delete(:comment, changeset)
 
-    assert multi.names == MapSet.new([:comment])
+    assert multi.names == %{:comment => true}
     assert multi.operations == [{:comment, {:changeset, %{changeset | action: :delete}, []}}]
   end
 
@@ -145,7 +145,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.delete(:comment, struct)
 
-    assert multi.names == MapSet.new([:comment])
+    assert multi.names == %{:comment => true}
     assert multi.operations == [{:comment, {:changeset, %{changeset | action: :delete}, []}}]
   end
 
@@ -157,7 +157,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.delete(:fun, fun)
 
-    assert multi.names == MapSet.new([:fun])
+    assert multi.names == %{:fun => true}
     assert [{:fun, {:run, _fun}}] = multi.operations
   end
 
@@ -166,7 +166,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.one(:comment, Comment)
 
-    assert multi.names == MapSet.new([:comment])
+    assert multi.names == %{:comment => true}
     assert [{:comment, {:run, _fun}}] = multi.operations
   end
 
@@ -177,7 +177,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.one(:comment, fun)
 
-    assert multi.names == MapSet.new([:comment])
+    assert multi.names == %{:comment => true}
     assert [{:comment, {:run, _fun}}] = multi.operations
   end
 
@@ -186,7 +186,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.all(:comments, Comment)
 
-    assert multi.names == MapSet.new([:comments])
+    assert multi.names == %{:comments => true}
     assert [{:comments, {:run, _fun}}] = multi.operations
   end
 
@@ -197,7 +197,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.all(:comments, fun)
 
-    assert multi.names == MapSet.new([:comments])
+    assert multi.names == %{:comments => true}
     assert [{:comments, {:run, _fun}}] = multi.operations
   end
 
@@ -206,7 +206,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.exists?(:comment, Comment)
 
-    assert multi.names == MapSet.new([:comment])
+    assert multi.names == %{:comment => true}
     assert [{:comment, {:run, _fun}}] = multi.operations
   end
 
@@ -217,7 +217,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.exists?(:comment, fun)
 
-    assert multi.names == MapSet.new([:comment])
+    assert multi.names == %{:comment => true}
     assert [{:comment, {:run, _fun}}] = multi.operations
   end
 
@@ -226,7 +226,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.error(:oops, :value)
 
-    assert multi.names == MapSet.new([:oops])
+    assert multi.names == %{:oops => true}
     assert multi.operations == [{:oops, {:error, :value}}]
   end
 
@@ -237,7 +237,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.run(:fun, fun)
 
-    assert multi.names == MapSet.new([:fun])
+    assert multi.names == %{:fun => true}
     assert multi.operations == [{:fun, {:run, fun}}]
   end
 
@@ -248,7 +248,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.run({:fun, 3}, fun)
 
-    assert multi.names == MapSet.new([{:fun, 3}])
+    assert multi.names == %{{:fun, 3} => true}
     assert multi.operations == [{{:fun, 3}, {:run, fun}}]
   end
 
@@ -259,7 +259,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.run(~c"myFunction", fun)
 
-    assert multi.names == MapSet.new([~c"myFunction"])
+    assert multi.names == %{~c"myFunction" => true}
     assert multi.operations == [{~c"myFunction", {:run, fun}}]
   end
 
@@ -268,7 +268,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.run(:fun, __MODULE__, :run_ok, [])
 
-    assert multi.names == MapSet.new([:fun])
+    assert multi.names == %{:fun => true}
     assert multi.operations == [{:fun, {:run, {__MODULE__, :run_ok, []}}}]
   end
 
@@ -277,7 +277,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.insert_all(:comments, Comment, [[x: 2]])
 
-    assert multi.names == MapSet.new([:comments])
+    assert multi.names == %{:comments => true}
     assert [{:comments, {:insert_all, Comment, [[x: 2]], []}}] = multi.operations
   end
 
@@ -288,7 +288,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.insert_all(:fun, Comment, fun_entries)
 
-    assert multi.names == MapSet.new([:fun])
+    assert multi.names == %{:fun => true}
     assert [{:fun, {:run, _fun}}] = multi.operations
 
     assert {:ok, changes} = TestRepo.transact(multi)
@@ -302,7 +302,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.update_all(:comments, Comment, set: [x: 2])
 
-    assert multi.names == MapSet.new([:comments])
+    assert multi.names == %{:comments => true}
     assert [{:comments, {:update_all, query, updates, []}}] = multi.operations
     assert updates == [set: [x: 2]]
     assert query == Ecto.Queryable.to_query(Comment)
@@ -315,7 +315,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.update_all(:fun, fun_queryable, [])
 
-    assert multi.names == MapSet.new([:fun])
+    assert multi.names == %{:fun => true}
     assert [{:fun, {:run, _fun}}] = multi.operations
 
     assert {:ok, changes} = TestRepo.transact(multi)
@@ -329,7 +329,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.delete_all(:comments, Comment)
 
-    assert multi.names == MapSet.new([:comments])
+    assert multi.names == %{:comments => true}
     assert [{:comments, {:delete_all, query, []}}] = multi.operations
     assert query == Ecto.Queryable.to_query(Comment)
   end
@@ -341,7 +341,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.delete_all(:fun, fun)
 
-    assert multi.names == MapSet.new([:fun])
+    assert multi.names == %{:fun => true}
     assert [{:fun, {:run, _fun}}] = multi.operations
 
     assert {:ok, changes} = TestRepo.transact(multi)
@@ -357,12 +357,12 @@ defmodule Ecto.MultiTest do
 
     merged = Multi.append(lhs, rhs)
     operations = Keyword.keys(merged.operations)
-    assert merged.names == MapSet.new([:one, :two, :three, :four])
+    assert merged.names == %{:one => true, :two => true, :three => true, :four => true}
     assert operations == [:four, :three, :two, :one]
 
     merged = Multi.prepend(lhs, rhs)
     operations = Keyword.keys(merged.operations)
-    assert merged.names == MapSet.new([:one, :two, :three, :four])
+    assert merged.names == %{:one => true, :two => true, :three => true, :four => true}
     assert operations == [:two, :one, :four, :three]
   end
 
@@ -419,7 +419,7 @@ defmodule Ecto.MultiTest do
       Multi.new()
       |> Multi.put(name, value)
 
-    assert multi.names == MapSet.new([name])
+    assert multi.names == %{name => true}
     assert multi.operations == [{name, {:put, value}}]
   end