fix: properly avoid adding duplicate children to application tree

lib/igniter.ex

@@ -1344,7 +1344,26 @@ defmodule Igniter do
     end
   end
 
-  defp changed?(source) do
+  @doc "Returns true if any of the files specified in `paths` have changed."
+  @spec changed?(t(), String.t() | list(String.t())) :: boolean()
+  def changed?(%Igniter{} = igniter, paths) do
+    paths = List.wrap(paths)
+
+    igniter.rewrite
+    |> Rewrite.sources()
+    |> Enum.filter(&(&1.path in paths))
+    |> Enum.any?(&changed?/1)
+  end
+
+  @doc "Returns true if the igniter or source provided has changed"
+  @spec changed?(t() | Rewrite.Source.t()) :: boolean()
+  def changed?(%Igniter{} = igniter) do
+    igniter.rewrite
+    |> Rewrite.sources()
+    |> Enum.any?(&changed?/1)
+  end
+
+  def changed?(%Rewrite.Source{} = source) do
     diff = Rewrite.Source.diff(source) |> IO.iodata_to_binary()
 
     String.trim(diff) != ""

lib/igniter/code/common.ex

@@ -747,7 +747,7 @@ defmodule Igniter.Code.Common do
   def nodes_equal?(v, v), do: true
 
   def nodes_equal?(l, r) do
-    equal_vals?(l, r) || equal_modules?(l, r)
+    equal_vals?(l, r)
   end
 
   defp equal_vals?({:__block__, _, [value]}, value) do
@@ -766,7 +766,7 @@ defmodule Igniter.Code.Common do
             nodes_equal?(left, right)
 
           _ ->
-            false
+            equal_modules?(left, right)
         end
 
       extendable_block?(right) ->
@@ -775,7 +775,7 @@ defmodule Igniter.Code.Common do
             nodes_equal?(left, right)
 
           _ ->
-            false
+            equal_modules?(left, right)
         end
 
       is_list(left) and is_list(right) ->
@@ -783,7 +783,7 @@ defmodule Igniter.Code.Common do
           Enum.all?(Enum.zip(left, right), fn {l, r} -> nodes_equal?(l, r) end)
 
       true ->
-        false
+        equal_modules?(left, right)
     end
   end
 

lib/igniter/project/application.ex

@@ -178,20 +178,16 @@ defmodule Igniter.Project.Application do
            {:ok, zipper} <- Igniter.Code.Function.move_to_nth_argument(zipper, 1) do
         if Igniter.Code.List.find_list_item_index(zipper, fn item ->
              if Igniter.Code.Tuple.tuple?(item) do
-               with {:ok, zipper} <- Igniter.Code.Tuple.tuple_elem(zipper, 0),
-                    zipper <- Igniter.Code.Common.expand_alias(zipper),
-                    module when is_atom(module) <- alias_to_mod(zipper.node) do
-                 module == to_supervise_module
+               with {:ok, item} <- Igniter.Code.Tuple.tuple_elem(item, 0),
+                    item <- Igniter.Code.Common.expand_alias(item) do
+                 Igniter.Code.Common.nodes_equal?(item, to_supervise_module)
                else
                  _ -> false
                end
              else
-               with zipper <- Igniter.Code.Common.expand_alias(zipper),
-                    module when is_atom(module) <- zipper.node do
-                 module == to_supervise_module
-               else
-                 _ -> false
-               end
+               item
+               |> Igniter.Code.Common.expand_alias()
+               |> Igniter.Code.Common.nodes_equal?(to_supervise_module)
              end
            end) do
           {:ok, zipper}

lib/igniter/test.ex

@@ -143,6 +143,32 @@ defmodule Igniter.Test do
     end
   end
 
+  defmacro assert_unchanged(igniter, path_or_paths) do
+    quote bind_quoted: [igniter: igniter, path_or_paths: path_or_paths] do
+      for path <- List.wrap(path_or_paths) do
+        refute Igniter.changed?(igniter, path), """
+        Expected #{inspect(path)} to be unchanged, but it was changed.
+
+        Diff:
+
+        #{igniter.rewrite.sources |> Map.take([path]) |> Igniter.diff()}
+        """
+      end
+    end
+  end
+
+  defmacro assert_unchanged(igniter) do
+    quote bind_quoted: [igniter: igniter] do
+      refute Igniter.changed?(igniter), """
+      Expected there to be no changes, but there were changes.
+
+      Diff:
+
+      #{Rewrite.sources(igniter.rewrite) |> Igniter.diff()}
+      """
+    end
+  end
+
   @doc false
   def sanitize_diff(diff, actual \\ nil) do
     diff

test/igniter/project/application_test.exs

@@ -28,6 +28,30 @@ defmodule Igniter.Project.ApplicationTest do
       """)
     end
 
+    test "doesnt add a module if its already supervised" do
+      test_project()
+      |> Igniter.Project.Application.add_new_child(Foo)
+      |> apply_igniter!()
+      |> Igniter.Project.Application.add_new_child(Foo)
+      |> assert_unchanged()
+    end
+
+    test "doesnt add a module if its already supervised as a tuple" do
+      test_project()
+      |> Igniter.Project.Application.add_new_child({Foo, a: 1})
+      |> apply_igniter!()
+      |> Igniter.Project.Application.add_new_child(Foo)
+      |> assert_unchanged()
+    end
+
+    test "doesnt add a module if its already supervised as an atom and we're adding a tuple" do
+      test_project()
+      |> Igniter.Project.Application.add_new_child(Foo)
+      |> apply_igniter!()
+      |> Igniter.Project.Application.add_new_child({Foo, a: 1})
+      |> assert_unchanged()
+    end
+
     test "supports taking options as the second argument" do
       test_project()
       |> Igniter.Project.Application.add_new_child({Foo, a: :b})