Take `nplurals` into account when validating translations.

4 files changed, 44 insertions, 55 deletions

diff --git a/lib/gitlab/i18n/metadata_entry.rb b/lib/gitlab/i18n/metadata_entry.rb
index 3f9cbae62c8..35d57459a3d 100644
--- a/lib/gitlab/i18n/metadata_entry.rb
+++ b/lib/gitlab/i18n/metadata_entry.rb
@@ -1,13 +1,16 @@
 module Gitlab
   module I18n
-    class MetadataEntry < PoEntry
+    class MetadataEntry
+      attr_reader :entry_data
+
+      def initialize(entry_data)
+        @entry_data = entry_data
+      end
+
       def expected_plurals
         return nil unless plural_information
 
-        nplurals = plural_information['nplurals'].to_i
-        if nplurals > 0
-          nplurals
-        end
+        plural_information['nplurals'].to_i
       end
 
       private
diff --git a/lib/gitlab/i18n/po_entry.rb b/lib/gitlab/i18n/po_entry.rb
deleted file mode 100644
index 014043cfdd6..00000000000
--- a/lib/gitlab/i18n/po_entry.rb
+++ /dev/null
@@ -1,19 +0,0 @@
-module Gitlab
-  module I18n
-    class PoEntry
-      def self.build(entry_data)
-        if entry_data[:msgid].empty?
-          MetadataEntry.new(entry_data)
-        else
-          TranslationEntry.new(entry_data)
-        end
-      end
-
-      attr_reader :entry_data
-
-      def initialize(entry_data)
-        @entry_data = entry_data
-      end
-    end
-  end
-end
diff --git a/lib/gitlab/i18n/po_linter.rb b/lib/gitlab/i18n/po_linter.rb
index f5ffc6669e4..c3b1fe582af 100644
--- a/lib/gitlab/i18n/po_linter.rb
+++ b/lib/gitlab/i18n/po_linter.rb
@@ -25,12 +25,19 @@ module Gitlab
       end
 
       def parse_po
-        entries = SimplePoParser.parse(po_path).map { |data| Gitlab::I18n::PoEntry.build(data) }
+        entries = SimplePoParser.parse(po_path)
 
         # The first entry is the metadata entry if there is one.
         # This is an entry when empty `msgid`
-        @metadata_entry = entries.shift if entries.first.is_a?(Gitlab::I18n::MetadataEntry)
-        @translation_entries = entries
+        if entries.first[:msgid].empty?
+          @metadata_entry = Gitlab::I18n::MetadataEntry.new(entries.shift)
+        else
+          return 'Missing metadata entry.'
+        end
+
+        @translation_entries = entries.map do |entry_data|
+          Gitlab::I18n::TranslationEntry.new(entry_data, metadata_entry.expected_plurals)
+        end
 
         nil
       rescue SimplePoParser::ParserError => e
@@ -64,7 +71,7 @@ module Gitlab
         return unless metadata_entry&.expected_plurals
         return unless entry.translated?
 
-        if entry.plural? && entry.all_translations.size != metadata_entry.expected_plurals
+        if entry.has_plural? && entry.all_translations.size != metadata_entry.expected_plurals
           errors << "should have #{metadata_entry.expected_plurals} "\
                     "#{'translations'.pluralize(metadata_entry.expected_plurals)}"
         end
@@ -85,11 +92,11 @@ module Gitlab
       end
 
       def validate_variables(errors, entry)
-        if entry.has_singular?
+        if entry.has_singular_translation?
           validate_variables_in_message(errors, entry.msgid, entry.singular_translation)
         end
 
-        if entry.plural?
+        if entry.has_plural?
           entry.plural_translations.each do |translation|
             validate_variables_in_message(errors, entry.plural_id, translation)
           end
@@ -161,8 +168,8 @@ module Gitlab
         translation = join_message(translation)
 
         # We don't need to validate when the message is empty.
-        # Translations could fallback to the default, or we could be validating a
-        # language that does not have plurals.
+        # In this case we fall back to the default, which has all the the
+        # required variables.
         return if translation.empty?
 
         found_variables = translation.scan(VARIABLE_REGEX)
diff --git a/lib/gitlab/i18n/translation_entry.rb b/lib/gitlab/i18n/translation_entry.rb
index 4fe8f569f9c..75d5aa0cfe1 100644
--- a/lib/gitlab/i18n/translation_entry.rb
+++ b/lib/gitlab/i18n/translation_entry.rb
@@ -1,6 +1,13 @@
 module Gitlab
   module I18n
-    class TranslationEntry < PoEntry
+    class TranslationEntry
+      attr_reader :nplurals, :entry_data
+
+      def initialize(entry_data, nplurals)
+        @entry_data = entry_data
+        @nplurals = nplurals
+      end
+
       def msgid
         entry_data[:msgid]
       end
@@ -9,16 +16,17 @@ module Gitlab
         entry_data[:msgid_plural]
       end
 
-      def plural?
+      def has_plural?
         plural_id.present?
       end
 
       def singular_translation
-        plural? ? entry_data['msgstr[0]'] : entry_data[:msgstr]
+        all_translations.first if has_singular_translation?
       end
 
       def all_translations
-        @all_translations ||= entry_data.fetch_values(*translation_keys).reject(&:empty?)
+        @all_translations ||= entry_data.fetch_values(*translation_keys)
+                                .reject(&:empty?)
       end
 
       def translated?
@@ -26,25 +34,22 @@ module Gitlab
       end
 
       def plural_translations
-        return [] unless plural?
+        return [] unless has_plural?
         return [] unless translated?
 
-        # The singular translation is used if there's only translation. This is
-        # the case for languages without plurals.
-        return all_translations if all_translations.size == 1
-
-        entry_data.fetch_values(*plural_translation_keys)
+        @plural_translations ||= if has_singular_translation?
+                                   all_translations.drop(1)
+                                 else
+                                   all_translations
+                                 end
       end
 
       def flag
         entry_data[:flag]
       end
 
-      # When a translation is a plural, but only has 1 translation, we could be
-      # talking about a language in which plural and singular is the same thing.
-      # In which case we always translate as a plural.
-      def has_singular?
-        !plural? || all_translations.size > 1
+      def has_singular_translation?
+        nplurals > 1 || !has_plural?
       end
 
       def msgid_contains_newlines?
@@ -61,15 +66,8 @@ module Gitlab
 
       private
 
-      def plural_translation_keys
-        @plural_translation_keys ||= translation_keys.select do |key|
-          plural_index = key.scan(/\d+/).first.to_i
-          plural_index > 0
-        end
-      end
-
       def translation_keys
-        @translation_keys ||= if plural?
+        @translation_keys ||= if has_plural?
                                 entry_data.keys.select { |key| key =~ /msgstr\[\d+\]/ }
                               else
                                 [:msgstr]