Move detailed information of an entry into a separate class

lib/gitlab/i18n/po_entry.rb

+module Gitlab
+  module I18n
+    class PoEntry
+      attr_reader :entry_data
+
+      def initialize(entry_data)
+        @entry_data = entry_data
+      end
+
+      def msgid
+        entry_data[:msgid]
+      end
+
+      def metadata?
+        msgid.empty?
+      end
+
+      def plural_id
+        entry_data[:msgid_plural]
+      end
+
+      def plural?
+        plural_id.present?
+      end
+
+      def singular_translation
+        plural? ? entry_data['msgstr[0]'] : entry_data[:msgstr]
+      end
+
+      def all_translations
+        @all_translations ||= entry_data.fetch_values(*translation_keys)
+      end
+
+      def plural_translations
+        return [] unless plural?
+
+        # 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)
+      end
+
+      def flag
+        entry_data[:flag]
+      end
+
+      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?
+                                entry_data.keys.select { |key| key =~ /msgstr\[\d+\]/ }
+                              else
+                                [:msgstr]
+                              end
+      end
+    end
+  end
+end

lib/gitlab/i18n/po_linter.rb

@@ -25,7 +25,7 @@ module Gitlab
       end
 
       def parse_po
-        @entries = SimplePoParser.parse(po_path)
+        @entries = SimplePoParser.parse(po_path).map { |data| Gitlab::I18n::PoEntry.new(data) }
         nil
       rescue SimplePoParser::ParserError => e
         @entries = []
@@ -36,11 +36,10 @@ module Gitlab
         errors = {}
 
         entries.each do |entry|
-          # Skip validation of metadata
-          next if entry[:msgid].empty?
+          next if entry.metadata?
 
           errors_for_entry = validate_entry(entry)
-          errors[join_message(entry[:msgid])] = errors_for_entry if errors_for_entry.any?
+          errors[join_message(entry.msgid)] = errors_for_entry if errors_for_entry.any?
         end
 
         errors
@@ -57,27 +56,24 @@ module Gitlab
       end
 
       def validate_newlines(errors, entry)
-        message_id = join_message(entry[:msgid])
+        message_id = join_message(entry.msgid)
 
-        if entry[:msgid].is_a?(Array)
+        if entry.msgid.is_a?(Array)
           errors << "<#{message_id}> is defined over multiple lines, this breaks some tooling."
         end
 
-        if translations_in_entry(entry).any? { |translation| translation.is_a?(Array) }
+        if entry.all_translations.any? { |translation| translation.is_a?(Array) }
           errors << "<#{message_id}> has translations defined over multiple lines, this breaks some tooling."
         end
       end
 
       def validate_variables(errors, entry)
-        if entry[:msgid_plural].present?
-          validate_variables_in_message(errors, entry[:msgid], entry['msgstr[0]'])
+        validate_variables_in_message(errors, entry.msgid, entry.singular_translation)
 
-          # Validate all plurals
-          entry.keys.select { |key_name| key_name =~ /msgstr\[[1-9]\]/ }.each do |plural_key|
-            validate_variables_in_message(errors, entry[:msgid_plural], entry[plural_key])
+        if entry.plural?
+          entry.plural_translations.each do |translation|
+            validate_variables_in_message(errors, entry.plural_id, translation)
           end
-        else
-          validate_variables_in_message(errors, entry[:msgid], entry[:msgstr])
         end
       end
 
@@ -168,26 +164,12 @@ module Gitlab
       end
 
       def validate_flags(errors, entry)
-        if flag = entry[:flag]
-          errors << "is marked #{flag}"
-        end
+        errors << "is marked #{entry.flag}" if entry.flag
       end
 
       def join_message(message)
         Array(message).join
       end
-
-      def translations_in_entry(entry)
-        if entry[:msgid_plural].present?
-          entry.fetch_values(*plural_translation_keys_in_entry(entry))
-        else
-          [entry[:msgstr]]
-        end
-      end
-
-      def plural_translation_keys_in_entry(entry)
-        entry.keys.select { |key| key =~ /msgstr\[\d*\]/ }
-      end
     end
   end
 end

spec/lib/gitlab/i18n/po_entry_spec.rb

+require 'spec_helper'
+
+describe Gitlab::I18n::PoEntry do
+  describe '#singular_translation' do
+    it 'returns the normal `msgstr` for translations without plural' do
+      data = { msgid: 'Hello world', msgstr: 'Bonjour monde' }
+      entry = described_class.new(data)
+
+      expect(entry.singular_translation).to eq('Bonjour monde')
+    end
+
+    it 'returns the first string for entries with plurals' do
+      data = {
+        msgid: 'Hello world',
+        msgid_plural: 'Hello worlds',
+        'msgstr[0]' => 'Bonjour monde',
+        'msgstr[1]' => 'Bonjour mondes'
+      }
+      entry = described_class.new(data)
+
+      expect(entry.singular_translation).to eq('Bonjour monde')
+    end
+  end
+
+  describe '#all_translations' do
+    it 'returns all translations for singular translations' do
+      data = { msgid: 'Hello world', msgstr: 'Bonjour monde' }
+      entry = described_class.new(data)
+
+      expect(entry.all_translations).to eq(['Bonjour monde'])
+    end
+
+    it 'returns all translations when including plural translations' do
+      data = {
+        msgid: 'Hello world',
+        msgid_plural: 'Hello worlds',
+        'msgstr[0]' => 'Bonjour monde',
+        'msgstr[1]' => 'Bonjour mondes'
+      }
+      entry = described_class.new(data)
+
+      expect(entry.all_translations).to eq(['Bonjour monde', 'Bonjour mondes'])
+    end
+  end
+
+  describe '#plural_translations' do
+    it 'returns all translations if there is only one plural' do
+      data = {
+        msgid: 'Hello world',
+        msgid_plural: 'Hello worlds',
+        'msgstr[0]' => 'Bonjour monde'
+      }
+      entry = described_class.new(data)
+
+      expect(entry.plural_translations).to eq(['Bonjour monde'])
+    end
+
+    it 'returns all translations except for the first one if there are multiple' do
+      data = {
+        msgid: 'Hello world',
+        msgid_plural: 'Hello worlds',
+        'msgstr[0]' => 'Bonjour monde',
+        'msgstr[1]' => 'Bonjour mondes',
+        'msgstr[2]' => 'Bonjour tous les mondes'
+      }
+      entry = described_class.new(data)
+
+      expect(entry.plural_translations).to eq(['Bonjour mondes', 'Bonjour tous les mondes'])
+    end
+  end
+end

spec/lib/gitlab/i18n/po_linter_spec.rb

@@ -127,13 +127,13 @@ describe Gitlab::I18n::PoLinter do
 
   describe '#validate_entries' do
     it 'skips entries without a `msgid`' do
-      allow(linter).to receive(:entries) { [{ msgid: "" }] }
+      allow(linter).to receive(:entries) { [Gitlab::I18n::PoEntry.new({ msgid: "" })] }
 
       expect(linter.validate_entries).to be_empty
     end
 
     it 'keeps track of errors for entries' do
-      fake_invalid_entry = { msgid: "Hello %{world}", msgstr: "Bonjour %{monde}" }
+      fake_invalid_entry = Gitlab::I18n::PoEntry.new({ msgid: "Hello %{world}", msgstr: "Bonjour %{monde}" })
       allow(linter).to receive(:entries) { [fake_invalid_entry] }
 
       expect(linter).to receive(:validate_entry)
@@ -158,12 +158,12 @@ describe Gitlab::I18n::PoLinter do
 
   describe '#validate_variables' do
     it 'validates both signular and plural in a pluralized string' do
-      pluralized_entry = {
+      pluralized_entry = Gitlab::I18n::PoEntry.new({
         msgid: 'Hello %{world}',
         msgid_plural: 'Hello all %{world}',
         'msgstr[0]' => 'Bonjour %{world}',
         'msgstr[1]' => 'Bonjour tous %{world}'
-      }
+      })
 
       expect(linter).to receive(:validate_variables_in_message)
                           .with([], 'Hello %{world}', 'Bonjour %{world}')
@@ -174,7 +174,7 @@ describe Gitlab::I18n::PoLinter do
     end
 
     it 'validates the message variables' do
-      entry = { msgid: 'Hello', msgstr: 'Bonjour' }
+      entry = Gitlab::I18n::PoEntry.new({ msgid: 'Hello', msgstr: 'Bonjour' })
 
       expect(linter).to receive(:validate_variables_in_message)
                           .with([], 'Hello', 'Bonjour')