Validate the number of plurals in an entry

2 files changed, 49 insertions, 7 deletions

diff --git a/lib/gitlab/i18n/po_entry.rb b/lib/gitlab/i18n/po_entry.rb
index aabb477bbea..f2a4bfbd1cd 100644
--- a/lib/gitlab/i18n/po_entry.rb
+++ b/lib/gitlab/i18n/po_entry.rb
@@ -28,11 +28,16 @@ module Gitlab
       end
 
       def all_translations
-        @all_translations ||= entry_data.fetch_values(*translation_keys)
+        @all_translations ||= entry_data.fetch_values(*translation_keys).reject(&:empty?)
+      end
+
+      def translated?
+        all_translations.any?
       end
 
       def plural_translations
         return [] unless plural?
+        return [] unless translated?
 
         # The singular translation is used if there's only translation. This is
         # the case for languages without plurals.
@@ -45,8 +50,34 @@ module Gitlab
         entry_data[:flag]
       end
 
+      def expected_plurals
+        return nil unless metadata?
+        return nil unless plural_information
+
+        nplurals = plural_information['nplurals'].to_i
+        if nplurals > 0
+          nplurals
+        end
+      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
+      end
+
       private
 
+      def plural_information
+        return nil unless metadata?
+        return @plural_information if defined?(@plural_information)
+
+        if plural_line = entry_data[:msgstr].detect { |metadata_line| metadata_line.starts_with?('Plural-Forms: ') }
+          @plural_information = Hash[plural_line.scan(/(\w+)=([^;\n]+)/)]
+        end
+      end
+
       def plural_translation_keys
         @plural_translation_keys ||= translation_keys.select do |key|
           plural_index = key.scan(/\d+/).first.to_i
diff --git a/lib/gitlab/i18n/po_linter.rb b/lib/gitlab/i18n/po_linter.rb
index 2f6965a19aa..e7c92be1383 100644
--- a/lib/gitlab/i18n/po_linter.rb
+++ b/lib/gitlab/i18n/po_linter.rb
@@ -3,7 +3,7 @@ require 'simple_po_parser'
 module Gitlab
   module I18n
     class PoLinter
-      attr_reader :po_path, :entries, :locale
+      attr_reader :po_path, :entries, :metadata, :locale
 
       VARIABLE_REGEX = /%{\w*}|%[a-z]/.freeze
 
@@ -26,6 +26,7 @@ module Gitlab
 
       def parse_po
         @entries = SimplePoParser.parse(po_path).map { |data| Gitlab::I18n::PoEntry.new(data) }
+        @metadata = @entries.detect { |entry| entry.metadata? }
         nil
       rescue SimplePoParser::ParserError => e
         @entries = []
@@ -51,24 +52,34 @@ module Gitlab
         validate_flags(errors, entry)
         validate_variables(errors, entry)
         validate_newlines(errors, entry)
+        validate_number_of_plurals(errors, entry)
 
         errors
       end
 
-      def validate_newlines(errors, entry)
-        message_id = join_message(entry.msgid)
+      def validate_number_of_plurals(errors, entry)
+        return unless metadata&.expected_plurals
+        return unless entry.translated?
+
+        if entry.plural? && entry.all_translations.size != metadata.expected_plurals
+          errors << "should have #{metadata.expected_plurals} #{'translations'.pluralize(metadata.expected_plurals)}"
+        end
+      end
 
+      def validate_newlines(errors, entry)
         if entry.msgid.is_a?(Array)
-          errors << "<#{message_id}> is defined over multiple lines, this breaks some tooling."
+          errors << "is defined over multiple lines, this breaks some tooling."
         end
 
         if entry.all_translations.any? { |translation| translation.is_a?(Array) }
-          errors << "<#{message_id}> has translations defined over multiple lines, this breaks some tooling."
+          errors << "has translations defined over multiple lines, this breaks some tooling."
         end
       end
 
       def validate_variables(errors, entry)
-        validate_variables_in_message(errors, entry.msgid, entry.singular_translation)
+        if entry.has_singular?
+          validate_variables_in_message(errors, entry.msgid, entry.singular_translation)
+        end
 
         if entry.plural?
           entry.plural_translations.each do |translation|