Merge branch 'bvl-i18n-validation' into 'master'

Updates to `.po` & `.pot` linting

Closes #46096 et #39292

See merge request gitlab-org/gitlab-ce!18779

4 files changed, 131 insertions, 53 deletions

diff --git a/lib/gitlab/i18n.rb b/lib/gitlab/i18n.rb
index 3772ef11c7f..343487bc361 100644
--- a/lib/gitlab/i18n.rb
+++ b/lib/gitlab/i18n.rb
@@ -21,7 +21,8 @@ module Gitlab
       'nl_NL' => 'Nederlands',
       'tr_TR' => 'Türkçe',
       'id_ID' => 'Bahasa Indonesia',
-      'fil_PH' => 'Filipino'
+      'fil_PH' => 'Filipino',
+      'pl_PL' => 'Polski'
     }.freeze
 
     def available_locales
diff --git a/lib/gitlab/i18n/metadata_entry.rb b/lib/gitlab/i18n/metadata_entry.rb
index 35d57459a3d..36fc1bcdcb7 100644
--- a/lib/gitlab/i18n/metadata_entry.rb
+++ b/lib/gitlab/i18n/metadata_entry.rb
@@ -3,16 +3,25 @@ module Gitlab
     class MetadataEntry
       attr_reader :entry_data
 
+      # Avoid testing too many plurals if `nplurals` was incorrectly set.
+      # Based on info on https://www.gnu.org/software/gettext/manual/html_node/Plural-forms.html
+      # which mentions special cases for numbers ending in 2 digits
+      MAX_FORMS_TO_TEST = 101
+
       def initialize(entry_data)
         @entry_data = entry_data
       end
 
-      def expected_plurals
+      def expected_forms
         return nil unless plural_information
 
         plural_information['nplurals'].to_i
       end
 
+      def forms_to_test
+        @forms_to_test ||= [expected_forms, MAX_FORMS_TO_TEST].compact.min
+      end
+
       private
 
       def plural_information
diff --git a/lib/gitlab/i18n/po_linter.rb b/lib/gitlab/i18n/po_linter.rb
index 7d3ff8c7f58..d8e7269a2c2 100644
--- a/lib/gitlab/i18n/po_linter.rb
+++ b/lib/gitlab/i18n/po_linter.rb
@@ -1,6 +1,8 @@
 module Gitlab
   module I18n
     class PoLinter
+      include Gitlab::Utils::StrongMemoize
+
       attr_reader :po_path, :translation_entries, :metadata_entry, :locale
 
       VARIABLE_REGEX = /%{\w*}|%[a-z]/.freeze
@@ -34,7 +36,7 @@ module Gitlab
         end
 
         @translation_entries = entries.map do |entry_data|
-          Gitlab::I18n::TranslationEntry.new(entry_data, metadata_entry.expected_plurals)
+          Gitlab::I18n::TranslationEntry.new(entry_data, metadata_entry.expected_forms)
         end
 
         nil
@@ -48,7 +50,7 @@ module Gitlab
 
         translation_entries.each do |entry|
           errors_for_entry = validate_entry(entry)
-          errors[join_message(entry.msgid)] = errors_for_entry if errors_for_entry.any?
+          errors[entry.msgid] = errors_for_entry if errors_for_entry.any?
         end
 
         errors
@@ -62,6 +64,7 @@ module Gitlab
         validate_newlines(errors, entry)
         validate_number_of_plurals(errors, entry)
         validate_unescaped_chars(errors, entry)
+        validate_translation(errors, entry)
 
         errors
       end
@@ -81,35 +84,39 @@ module Gitlab
       end
 
       def validate_number_of_plurals(errors, entry)
-        return unless metadata_entry&.expected_plurals
+        return unless metadata_entry&.expected_forms
         return unless entry.translated?
 
-        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)}"
+        if entry.has_plural? && entry.all_translations.size != metadata_entry.expected_forms
+          errors << "should have #{metadata_entry.expected_forms} "\
+                    "#{'translations'.pluralize(metadata_entry.expected_forms)}"
         end
       end
 
       def validate_newlines(errors, entry)
-        if entry.msgid_contains_newlines?
+        if entry.msgid_has_multiple_lines?
           errors << 'is defined over multiple lines, this breaks some tooling.'
         end
 
-        if entry.plural_id_contains_newlines?
+        if entry.plural_id_has_multiple_lines?
           errors << 'plural is defined over multiple lines, this breaks some tooling.'
         end
 
-        if entry.translations_contain_newlines?
+        if entry.translations_have_multiple_lines?
           errors << 'has translations defined over multiple lines, this breaks some tooling.'
         end
       end
 
       def validate_variables(errors, entry)
         if entry.has_singular_translation?
+          validate_variables_in_message(errors, entry.msgid, entry.msgid)
+
           validate_variables_in_message(errors, entry.msgid, entry.singular_translation)
         end
 
         if entry.has_plural?
+          validate_variables_in_message(errors, entry.plural_id, entry.plural_id)
+
           entry.plural_translations.each do |translation|
             validate_variables_in_message(errors, entry.plural_id, translation)
           end
@@ -117,41 +124,98 @@ module Gitlab
       end
 
       def validate_variables_in_message(errors, message_id, message_translation)
-        message_id = join_message(message_id)
         required_variables = message_id.scan(VARIABLE_REGEX)
 
         validate_unnamed_variables(errors, required_variables)
-        validate_translation(errors, message_id, required_variables)
         validate_variable_usage(errors, message_translation, required_variables)
       end
 
-      def validate_translation(errors, message_id, used_variables)
+      def validate_translation(errors, entry)
+        Gitlab::I18n.with_locale(locale) do
+          if entry.has_plural?
+            translate_plural(entry)
+          else
+            translate_singular(entry)
+          end
+        end
+
+      # `sprintf` could raise an `ArgumentError` when invalid passing something
+      # other than a Hash when using named variables
+      #
+      # `sprintf` could raise `TypeError` when passing a wrong type when using
+      # unnamed variables
+      #
+      # FastGettext::Translation could raise `RuntimeError` (raised as a string),
+      # or as subclassess `NoTextDomainConfigured` & `InvalidFormat`
+      #
+      # `FastGettext::Translation` could raise `ArgumentError` as subclassess
+      # `InvalidEncoding`, `IllegalSequence` & `InvalidCharacter`
+      rescue ArgumentError, TypeError, RuntimeError => e
+        errors << "Failure translating to #{locale}: #{e.message}"
+      end
+
+      def translate_singular(entry)
+        used_variables = entry.msgid.scan(VARIABLE_REGEX)
         variables = fill_in_variables(used_variables)
 
-        begin
-          Gitlab::I18n.with_locale(locale) do
-            translated = if message_id.include?('|')
-                           FastGettext::Translation.s_(message_id)
-                         else
-                           FastGettext::Translation._(message_id)
-                         end
+        translation = if entry.msgid.include?('|')
+                        FastGettext::Translation.s_(entry.msgid)
+                      else
+                        FastGettext::Translation._(entry.msgid)
+                      end
 
-            translated % variables
+        translation % variables if used_variables.any?
+      end
+
+      def translate_plural(entry)
+        used_variables = entry.plural_id.scan(VARIABLE_REGEX)
+        variables = fill_in_variables(used_variables)
+
+        numbers_covering_all_plurals.map do |number|
+          translation = FastGettext::Translation.n_(entry.msgid, entry.plural_id, number)
+
+          translation % variables if used_variables.any?
+        end
+      end
+
+      def numbers_covering_all_plurals
+        @numbers_covering_all_plurals ||= calculate_numbers_covering_all_plurals
+      end
+
+      def calculate_numbers_covering_all_plurals
+        required_numbers = []
+        discovered_indexes = []
+        counter = 0
+
+        while discovered_indexes.size < metadata_entry.forms_to_test && counter < Gitlab::I18n::MetadataEntry::MAX_FORMS_TO_TEST
+          index_for_count = index_for_pluralization(counter)
+
+          unless discovered_indexes.include?(index_for_count)
+            discovered_indexes << index_for_count
+            required_numbers << counter
           end
 
-        # `sprintf` could raise an `ArgumentError` when invalid passing something
-        # other than a Hash when using named variables
-        #
-        # `sprintf` could raise `TypeError` when passing a wrong type when using
-        # unnamed variables
-        #
-        # FastGettext::Translation could raise `RuntimeError` (raised as a string),
-        # or as subclassess `NoTextDomainConfigured` & `InvalidFormat`
-        #
-        # `FastGettext::Translation` could raise `ArgumentError` as subclassess
-        # `InvalidEncoding`, `IllegalSequence` & `InvalidCharacter`
-        rescue ArgumentError, TypeError, RuntimeError => e
-          errors << "Failure translating to #{locale} with #{variables}: #{e.message}"
+          counter += 1
+        end
+
+        required_numbers
+      end
+
+      def index_for_pluralization(counter)
+        # This calls the C function that defines the pluralization rule, it can
+        # return a boolean (`false` represents 0, `true` represents 1) or an integer
+        # that specifies the plural form to be used for the given number
+        pluralization_result = Gitlab::I18n.with_locale(locale) do
+          FastGettext.pluralisation_rule.call(counter)
+        end
+
+        case pluralization_result
+        when false
+          0
+        when true
+          1
+        else
+          pluralization_result
         end
       end
 
@@ -172,14 +236,18 @@ module Gitlab
       end
 
       def validate_unnamed_variables(errors, variables)
-        if  variables.size > 1 && variables.any? { |variable_name| unnamed_variable?(variable_name) }
+        unnamed_variables, named_variables = variables.partition { |name| unnamed_variable?(name) }
+
+        if unnamed_variables.any? && named_variables.any?
+          errors << 'is combining named variables with unnamed variables'
+        end
+
+        if unnamed_variables.size > 1
           errors << 'is combining multiple unnamed variables'
         end
       end
 
       def validate_variable_usage(errors, translation, required_variables)
-        translation = join_message(translation)
-
         # We don't need to validate when the message is empty.
         # In this case we fall back to the default, which has all the the
         # required variables.
@@ -205,10 +273,6 @@ module Gitlab
       def validate_flags(errors, entry)
         errors << "is marked #{entry.flag}" if entry.flag
       end
-
-      def join_message(message)
-        Array(message).join
-      end
     end
   end
 end
diff --git a/lib/gitlab/i18n/translation_entry.rb b/lib/gitlab/i18n/translation_entry.rb
index e6c95afca7e..54adb98f42d 100644
--- a/lib/gitlab/i18n/translation_entry.rb
+++ b/lib/gitlab/i18n/translation_entry.rb
@@ -11,11 +11,11 @@ module Gitlab
       end
 
       def msgid
-        entry_data[:msgid]
+        @msgid ||= Array(entry_data[:msgid]).join
       end
 
       def plural_id
-        entry_data[:msgid_plural]
+        @plural_id ||= Array(entry_data[:msgid_plural]).join
       end
 
       def has_plural?
@@ -23,12 +23,11 @@ module Gitlab
       end
 
       def singular_translation
-        all_translations.first if has_singular_translation?
+        all_translations.first.to_s if has_singular_translation?
       end
 
       def all_translations
-        @all_translations ||= entry_data.fetch_values(*translation_keys)
-                                .reject(&:empty?)
+        @all_translations ||= translation_entries.map { |translation| Array(translation).join }
       end
 
       def translated?
@@ -54,16 +53,16 @@ module Gitlab
         nplurals > 1 || !has_plural?
       end
 
-      def msgid_contains_newlines?
-        msgid.is_a?(Array)
+      def msgid_has_multiple_lines?
+        entry_data[:msgid].is_a?(Array)
       end
 
-      def plural_id_contains_newlines?
-        plural_id.is_a?(Array)
+      def plural_id_has_multiple_lines?
+        entry_data[:msgid_plural].is_a?(Array)
       end
 
-      def translations_contain_newlines?
-        all_translations.any? { |translation| translation.is_a?(Array) }
+      def translations_have_multiple_lines?
+        translation_entries.any? { |translation| translation.is_a?(Array) }
       end
 
       def msgid_contains_unescaped_chars?
@@ -84,6 +83,11 @@ module Gitlab
 
       private
 
+      def translation_entries
+        @translation_entries ||= entry_data.fetch_values(*translation_keys)
+                                   .reject(&:empty?)
+      end
+
       def translation_keys
         @translation_keys ||= entry_data.keys.select { |key| key.to_s =~ /\Amsgstr(\[\d+\])?\z/ }
       end