From 70a341083e420b53ceac730bea95c05c378174bb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonatan=20M=C3=A4nnchen?= <jonatan@maennchen.ch>
Date: Tue, 12 Nov 2024 20:57:33 +0000
Subject: [PATCH] Warn user if conflicting plural messages are defined (#400)

---
 lib/gettext/extractor_agent.ex  | 23 ++++++++++++++++++-
 test/gettext/extractor_test.exs | 40 ++++++++++++++++++++++++++++++---
 2 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/lib/gettext/extractor_agent.ex b/lib/gettext/extractor_agent.ex
index 62cdf890..f20422c6 100644
--- a/lib/gettext/extractor_agent.ex
+++ b/lib/gettext/extractor_agent.ex
@@ -3,6 +3,8 @@ defmodule Gettext.ExtractorAgent do
 
   use Agent
 
+  require Logger
+
   alias Expo.Message
 
   @name __MODULE__
@@ -75,7 +77,26 @@ defmodule Gettext.ExtractorAgent do
     end)
   end
 
-  defp merge_messages(message_1, message_2) do
+  defp merge_messages(message_1, message_2)
+
+  defp merge_messages(%Message.Singular{} = message_1, %Message.Plural{} = message_2) do
+    # Flipping the arguments to make sure that the pluaral message (more information) is used as the base message
+    merge_messages(message_2, message_1)
+  end
+
+  defp merge_messages(%Message.Plural{} = message_1, %Message.Plural{} = message_2) do
+    if IO.iodata_to_binary(message_1.msgid_plural) != IO.iodata_to_binary(message_2.msgid_plural) do
+      Logger.warning(
+        "Plural message for '#{IO.iodata_to_binary(message_1.msgid)}' is not matching. Using '#{IO.iodata_to_binary(message_2.msgid_plural)}' instead of '#{IO.iodata_to_binary(message_1.msgid_plural)}'."
+      )
+    end
+
+    _merge_messages(message_1, message_2)
+  end
+
+  defp merge_messages(message_1, message_2), do: _merge_messages(message_1, message_2)
+
+  defp _merge_messages(message_1, message_2) do
     message_1
     |> Map.put(:references, message_1.references ++ message_2.references)
     |> Map.put(
diff --git a/test/gettext/extractor_test.exs b/test/gettext/extractor_test.exs
index 6285a465..0ecd66b7 100644
--- a/test/gettext/extractor_test.exs
+++ b/test/gettext/extractor_test.exs
@@ -1,6 +1,8 @@
 defmodule Gettext.ExtractorTest do
   use ExUnit.Case
 
+  import ExUnit.CaptureLog
+
   alias Expo.Message
   alias Expo.Messages
   alias Gettext.Extractor
@@ -322,6 +324,8 @@ defmodule Gettext.ExtractorTest do
         Gettext.Macros.gettext_comment("repeated comment")
         Gettext.Macros.gettext_with_backend(Gettext.ExtractorTest.MyGettext, "foo")
         Gettext.Macros.dngettext_with_backend(Gettext.ExtractorTest.MyGettext, "errors", "one error", "%{count} errors", 2)
+        Gettext.Macros.dngettext_with_backend(Gettext.ExtractorTest.MyGettext, "errors", "one error", "%{count} errors", 2)
+        Gettext.Macros.dgettext_with_backend(Gettext.ExtractorTest.MyGettext, "errors", "one error")
         Gettext.Macros.gettext_comment("one more comment")
         Gettext.Macros.gettext_comment("repeated comment")
         Gettext.Macros.gettext_comment("repeated comment")
@@ -345,12 +349,12 @@ defmodule Gettext.ExtractorTest do
        #. repeated comment
        #. one more comment
        #: foo.ex:16
-       #: foo.ex:21
+       #: foo.ex:23
        #, elixir-autogen, elixir-format
        msgid "foo"
        msgstr ""
 
-       #: foo.ex:23
+       #: foo.ex:25
        #, elixir-autogen, elixir-format
        msgctxt "test"
        msgid "context based message"
@@ -362,6 +366,8 @@ defmodule Gettext.ExtractorTest do
        msgstr ""
 
        #: foo.ex:17
+       #: foo.ex:18
+       #: foo.ex:19
        #, elixir-autogen, elixir-format
        msgid "one error"
        msgid_plural "%{count} errors"
@@ -373,7 +379,7 @@ defmodule Gettext.ExtractorTest do
        msgid ""
        msgstr ""
 
-       #: foo.ex:22
+       #: foo.ex:24
        #, elixir-autogen, elixir-format
        msgid "hi"
        msgstr ""
@@ -435,6 +441,34 @@ defmodule Gettext.ExtractorTest do
     Extractor.disable()
   end
 
+  test "warns on conflicting plural messages" do
+    refute Extractor.extracting?()
+    Extractor.enable()
+    assert Extractor.extracting?()
+
+    code = """
+    defmodule Gettext.ExtractorTest.ConflictingPlural.Gettext do
+      use Gettext.Backend, otp_app: :test_conflicting_plural
+    end
+
+    defmodule Gettext.ExtractorTest.ConflictingPlural.Foo do
+      require Gettext.Macros
+
+      def bar do
+        Gettext.Macros.dngettext_with_backend(Gettext.ExtractorTest.ConflictingPlural.Gettext, "errors", "one error", "%{count} errors", 2)
+        Gettext.Macros.dngettext_with_backend(Gettext.ExtractorTest.ConflictingPlural.Gettext, "errors", "one error", "multiple errors", 2)
+      end
+    end
+    """
+
+    assert capture_log(fn ->
+             Code.compile_string(code, Path.join(File.cwd!(), "foo.ex"))
+           end) =~
+             "Plural message for 'one error' is not matching. Using 'multiple errors' instead of '%{count} errors'."
+  after
+    Extractor.disable()
+  end
+
   defp write_file(path, contents) do
     path |> Path.dirname() |> File.mkdir_p!()
     File.write!(path, contents)