From c6d969949ef98f1b4aebf38ca7f3ed1e59791d48 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Fri, 25 Aug 2017 15:23:51 +0200 Subject: Validate the number of plurals in an entry --- lib/gitlab/i18n/po_entry.rb | 33 ++++++++++++++++- lib/gitlab/i18n/po_linter.rb | 23 +++++++++--- spec/fixtures/missing_plurals.po | 22 +++++++++++ spec/fixtures/no_plurals.po | 24 ------------ spec/lib/gitlab/i18n/po_entry_spec.rb | 67 ++++++++++++++++++++++++++++++++++ spec/lib/gitlab/i18n/po_linter_spec.rb | 45 ++++++++++++++++++++--- 6 files changed, 177 insertions(+), 37 deletions(-) create mode 100644 spec/fixtures/missing_plurals.po delete mode 100644 spec/fixtures/no_plurals.po 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| diff --git a/spec/fixtures/missing_plurals.po b/spec/fixtures/missing_plurals.po new file mode 100644 index 00000000000..09ca0c82718 --- /dev/null +++ b/spec/fixtures/missing_plurals.po @@ -0,0 +1,22 @@ +# Spanish translations for gitlab package. +# Copyright (C) 2017 THE PACKAGE'S COPYRIGHT HOLDER +# This file is distributed under the same license as the gitlab package. +# FIRST AUTHOR , 2017. +# +msgid "" +msgstr "" +"Project-Id-Version: gitlab 1.0.0\n" +"Report-Msgid-Bugs-To: \n" +"PO-Revision-Date: 2017-07-13 12:10-0500\n" +"Language-Team: Spanish\n" +"Language: es\n" +"MIME-Version: 1.0\n" +"Content-Type: text/plain; charset=UTF-8\n" +"Content-Transfer-Encoding: 8bit\n" +"Plural-Forms: nplurals=2; plural=n != 1;\n" +"Last-Translator: Bob Van Landuyt \n" +"X-Generator: Poedit 2.0.2\n" + +msgid "%d commit" +msgid_plural "%d commits" +msgstr[0] "%d cambio" diff --git a/spec/fixtures/no_plurals.po b/spec/fixtures/no_plurals.po deleted file mode 100644 index 1bfc4ecbb04..00000000000 --- a/spec/fixtures/no_plurals.po +++ /dev/null @@ -1,24 +0,0 @@ -# Arthur Charron , 2017. #zanata -# Huang Tao , 2017. #zanata -# Kohei Ota , 2017. #zanata -# Taisuke Inoue , 2017. #zanata -# Takuya Noguchi , 2017. #zanata -# YANO Tethurou , 2017. #zanata -msgid "" -msgstr "" -"Project-Id-Version: gitlab 1.0.0\n" -"Report-Msgid-Bugs-To: \n" -"MIME-Version: 1.0\n" -"Content-Type: text/plain; charset=UTF-8\n" -"Content-Transfer-Encoding: 8bit\n" -"PO-Revision-Date: 2017-08-06 11:23-0400\n" -"Last-Translator: Taisuke Inoue \n" -"Language-Team: Japanese \"Language-Team: Russian (https://translate.zanata.org/" -"project/view/GitLab)\n" -"Language: ja\n" -"X-Generator: Zanata 3.9.6\n" -"Plural-Forms: nplurals=1; plural=0\n" - -msgid "%d commit" -msgid_plural "%d commits" -msgstr[0] "%d個のコミット" diff --git a/spec/lib/gitlab/i18n/po_entry_spec.rb b/spec/lib/gitlab/i18n/po_entry_spec.rb index 0317caedbff..e671f3c17a1 100644 --- a/spec/lib/gitlab/i18n/po_entry_spec.rb +++ b/spec/lib/gitlab/i18n/po_entry_spec.rb @@ -68,4 +68,71 @@ describe Gitlab::I18n::PoEntry do expect(entry.plural_translations).to eq(['Bonjour mondes', 'Bonjour tous les mondes']) end end + + describe '#expected_plurals' do + it 'returns nil when the entry is an actual translation' do + data = { msgid: 'Hello world', msgstr: 'Bonjour monde' } + entry = described_class.new(data) + + expect(entry.expected_plurals).to be_nil + end + + it 'returns the number of plurals' do + data = { + msgid: "", + msgstr: [ + "", + "Project-Id-Version: gitlab 1.0.0\\n", + "Report-Msgid-Bugs-To: \\n", + "PO-Revision-Date: 2017-07-13 12:10-0500\\n", + "Language-Team: Spanish\\n", + "Language: es\\n", + "MIME-Version: 1.0\\n", + "Content-Type: text/plain; charset=UTF-8\\n", + "Content-Transfer-Encoding: 8bit\\n", + "Plural-Forms: nplurals=2; plural=n != 1;\\n", + "Last-Translator: Bob Van Landuyt \\n", + "X-Generator: Poedit 2.0.2\\n" + ] + } + entry = described_class.new(data) + + expect(entry.expected_plurals).to eq(2) + end + end + + describe '#has_singular?' do + it 'has a singular when the translation is not pluralized' do + data = { + msgid: 'hello world', + msgstr: 'hello' + } + entry = described_class.new(data) + + expect(entry).to have_singular + end + + it 'has a singular when plural and singular are separately defined' do + data = { + msgid: 'hello world', + msgid_plural: 'hello worlds', + "msgstr[0]" => 'hello world', + "msgstr[1]" => 'hello worlds' + } + entry = described_class.new(data) + + expect(entry).to have_singular + end + + it 'does not have a separate singular if the plural string only has one translation' do + data = { + msgid: 'hello world', + msgid_plural: 'hello worlds', + "msgstr[0]" => 'hello worlds' + } + entry = described_class.new(data) + + expect(entry).not_to have_singular + end + end end diff --git a/spec/lib/gitlab/i18n/po_linter_spec.rb b/spec/lib/gitlab/i18n/po_linter_spec.rb index 2e155511076..307ea8b2640 100644 --- a/spec/lib/gitlab/i18n/po_linter_spec.rb +++ b/spec/lib/gitlab/i18n/po_linter_spec.rb @@ -28,21 +28,21 @@ describe Gitlab::I18n::PoLinter do it 'has an error for a normal string' do message_id = "You are going to remove %{group_name}.\\nRemoved groups CANNOT be restored!\\nAre you ABSOLUTELY sure?" - expected_message = "<#{message_id}> is defined over multiple lines, this breaks some tooling." + expected_message = "is defined over multiple lines, this breaks some tooling." expect(errors[message_id]).to include(expected_message) end it 'has an error when a translation is defined over multiple lines' do message_id = "You are going to remove %{group_name}.\\nRemoved groups CANNOT be restored!\\nAre you ABSOLUTELY sure?" - expected_message = "<#{message_id}> has translations defined over multiple lines, this breaks some tooling." + expected_message = "has translations defined over multiple lines, this breaks some tooling." expect(errors[message_id]).to include(expected_message) end it 'raises an error when a plural translation is defined over multiple lines' do message_id = 'With plural' - expected_message = "<#{message_id}> has translations defined over multiple lines, this breaks some tooling." + expected_message = "has translations defined over multiple lines, this breaks some tooling." expect(errors[message_id]).to include(expected_message) end @@ -81,10 +81,10 @@ describe Gitlab::I18n::PoLinter do end context 'with missing plurals' do - let(:po_path) { 'spec/fixtures/no_plurals.po' } + let(:po_path) { 'spec/fixtures/missing_plurals.po' } it 'has no errors' do - is_expected.to be_empty + is_expected.not_to be_empty end end @@ -151,13 +151,33 @@ describe Gitlab::I18n::PoLinter do expect(linter).to receive(:validate_flags).with([], fake_entry) expect(linter).to receive(:validate_variables).with([], fake_entry) expect(linter).to receive(:validate_newlines).with([], fake_entry) + expect(linter).to receive(:validate_number_of_plurals).with([], fake_entry) linter.validate_entry(fake_entry) end end + describe '#validate_number_of_plurals' do + it 'validates when there are an incorrect number of translations' do + fake_metadata = double + allow(fake_metadata).to receive(:expected_plurals).and_return(2) + allow(linter).to receive(:metadata).and_return(fake_metadata) + + fake_entry = Gitlab::I18n::PoEntry.new( + msgid: 'the singular', + msgid_plural: 'the plural', + 'msgstr[0]' => 'the singular' + ) + errors = [] + + linter.validate_number_of_plurals(errors, fake_entry) + + expect(errors).to include('should have 2 translations') + end + end + describe '#validate_variables' do - it 'validates both signular and plural in a pluralized string' do + it 'validates both signular and plural in a pluralized string when the entry has a singular' do pluralized_entry = Gitlab::I18n::PoEntry.new({ msgid: 'Hello %{world}', msgid_plural: 'Hello all %{world}', @@ -173,6 +193,19 @@ describe Gitlab::I18n::PoLinter do linter.validate_variables([], pluralized_entry) end + it 'only validates plural when there is no separate singular' do + pluralized_entry = Gitlab::I18n::PoEntry.new({ + msgid: 'Hello %{world}', + msgid_plural: 'Hello all %{world}', + 'msgstr[0]' => 'Bonjour %{world}' + }) + + expect(linter).to receive(:validate_variables_in_message) + .with([], 'Hello all %{world}', 'Bonjour %{world}') + + linter.validate_variables([], pluralized_entry) + end + it 'validates the message variables' do entry = Gitlab::I18n::PoEntry.new({ msgid: 'Hello', msgstr: 'Bonjour' }) -- cgit v1.2.3