Warn user if conflicting plural messages are defined (#400) (#401)

elixir-gettext · Nov 13, 2024 · 528e9cb · 528e9cb
1 parent b41445e
commit 528e9cb
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 4 deletions.
diff --git 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,12 +77,45 @@ defmodule Gettext.ExtractorAgent do
     end)
   end
 
-  defp merge_messages(message_1, message_2) do
+  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
+    # Make sure message choice is deterministic
+    [message_1, message_2] =
+      Enum.sort_by([message_1, message_2], &IO.iodata_to_binary(&1.msgid_plural))
+
+    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)}'.
+      References: #{dump_references(message_1.references ++ message_2.references)}\
+      """)
+    end
+
+    merge_messages_after_checks(message_1, message_2)
+  end
+
+  defp merge_messages(message_1, message_2), do: merge_messages_after_checks(message_1, message_2)
+
+  defp merge_messages_after_checks(message_1, message_2) do
     message_1
     |> Map.put(:references, message_1.references ++ message_2.references)
     |> Map.put(
       :extracted_comments,
       Enum.uniq(message_1.extracted_comments ++ message_2.extracted_comments)
     )
   end
+
+  defp dump_references(references) do
+    references
+    |> List.flatten()
+    |> Enum.map(fn
+      {file, line} -> [file, ":", Integer.to_string(line)]
+      file -> file
+    end)
+    |> Enum.intersperse(", ")
+  end
 end
diff --git 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,38 @@ 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'.
+             References: foo.ex:9, foo.ex:10
+             """
+  after
+    Extractor.disable()
+  end
+
   defp write_file(path, contents) do
     path |> Path.dirname() |> File.mkdir_p!()
     File.write!(path, contents)