From ca065e493e7a62f22cb330659e75f8567fa6131f Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Mon, 7 May 2018 16:04:11 +0200 Subject: Forbids combining named and unnamed variables in `gitlab.pot` This would break with an argument error when interpolating using `sprintf` in ruby. --- spec/lib/gitlab/i18n/po_linter_spec.rb | 102 ++++++++++++++++++++------------- 1 file changed, 62 insertions(+), 40 deletions(-) (limited to 'spec/lib/gitlab/i18n') diff --git a/spec/lib/gitlab/i18n/po_linter_spec.rb b/spec/lib/gitlab/i18n/po_linter_spec.rb index 3a962ba7f22..9bbd51fe212 100644 --- a/spec/lib/gitlab/i18n/po_linter_spec.rb +++ b/spec/lib/gitlab/i18n/po_linter_spec.rb @@ -5,6 +5,25 @@ describe Gitlab::I18n::PoLinter do let(:linter) { described_class.new(po_path) } let(:po_path) { 'spec/fixtures/valid.po' } + def fake_translation(id:, translation:, plural_id: nil, plurals: []) + data = { msgid: id, msgid_plural: plural_id } + + if plural_id + [translation, *plurals].each_with_index do |plural, index| + allow(FastGettext::Translation).to receive(:n_).with(plural_id, index).and_return(plural) + data.merge!("msgstr[#{index}]" => plural) + end + else + allow(FastGettext::Translation).to receive(:_).with(id).and_return(translation) + data[:msgstr] = translation + end + + Gitlab::I18n::TranslationEntry.new( + data, + plurals.size + 1 + ) + end + describe '#errors' do it 'only calls validation once' do expect(linter).to receive(:validate_po).once.and_call_original @@ -155,9 +174,8 @@ describe Gitlab::I18n::PoLinter do describe '#validate_entries' do it 'keeps track of errors for entries' do - fake_invalid_entry = Gitlab::I18n::TranslationEntry.new( - { msgid: "Hello %{world}", msgstr: "Bonjour %{monde}" }, 2 - ) + fake_invalid_entry = fake_translation(id: "Hello %{world}", + translation: "Bonjour %{monde}") allow(linter).to receive(:translation_entries) { [fake_invalid_entry] } expect(linter).to receive(:validate_entry) @@ -177,6 +195,7 @@ describe Gitlab::I18n::PoLinter do expect(linter).to receive(:validate_newlines).with([], fake_entry) expect(linter).to receive(:validate_number_of_plurals).with([], fake_entry) expect(linter).to receive(:validate_unescaped_chars).with([], fake_entry) + expect(linter).to receive(:validate_translation).with([], fake_entry) linter.validate_entry(fake_entry) end @@ -201,13 +220,12 @@ describe Gitlab::I18n::PoLinter do end describe '#validate_variables' do - it 'validates both signular and plural in a pluralized string when the entry has a singular' do - pluralized_entry = Gitlab::I18n::TranslationEntry.new( - { msgid: 'Hello %{world}', - msgid_plural: 'Hello all %{world}', - 'msgstr[0]' => 'Bonjour %{world}', - 'msgstr[1]' => 'Bonjour tous %{world}' }, - 2 + it 'validates both singular and plural in a pluralized string when the entry has a singular' do + pluralized_entry = fake_translation( + id: 'Hello %{world}', + translation: 'Bonjour %{world}', + plural_id: 'Hello all %{world}', + plurals: ['Bonjour tous %{world}'] ) expect(linter).to receive(:validate_variables_in_message) @@ -221,11 +239,10 @@ describe Gitlab::I18n::PoLinter do end it 'only validates plural when there is no separate singular' do - pluralized_entry = Gitlab::I18n::TranslationEntry.new( - { msgid: 'Hello %{world}', - msgid_plural: 'Hello all %{world}', - 'msgstr[0]' => 'Bonjour %{world}' }, - 1 + pluralized_entry = fake_translation( + id: 'Hello %{world}', + translation: 'Bonjour %{world}', + plural_id: 'Hello all %{world}' ) expect(linter).to receive(:validate_variables_in_message) @@ -235,10 +252,7 @@ describe Gitlab::I18n::PoLinter do end it 'validates the message variables' do - entry = Gitlab::I18n::TranslationEntry.new( - { msgid: 'Hello', msgstr: 'Bonjour' }, - 2 - ) + entry = fake_translation(id: 'Hello', translation: 'Bonjour') expect(linter).to receive(:validate_variables_in_message) .with([], 'Hello', 'Bonjour') @@ -251,21 +265,34 @@ describe Gitlab::I18n::PoLinter do it 'detects when a variables are used incorrectly' do errors = [] - expected_errors = [' is missing: [%{hello}]', - ' is using unknown variables: [%{world}]', - 'is combining multiple unnamed variables'] + expected_errors = ['<%d hello %{world} %s> is missing: [%{hello}]', + '<%d hello %{world} %s> is using unknown variables: [%{world}]', + 'is combining multiple unnamed variables', + 'is combining named variables with unnamed variables'] - linter.validate_variables_in_message(errors, '%{hello} world %d', 'hello %{world} %d') + linter.validate_variables_in_message(errors, '%d %{hello} world %s', '%d hello %{world} %s') expect(errors).to include(*expected_errors) end + + it 'does not allow combining 1 `%d` unnamed variable with named variables' do + errors = [] + + linter.validate_variables_in_message(errors, + '%{type} detected %d vulnerability', + '%{type} detecteerde %d kwetsbaarheid') + + expect(errors).not_to be_empty + end end describe '#validate_translation' do + let(:entry) { fake_translation(id: 'Hello %{world}', translation: 'Bonjour %{world}') } + it 'succeeds with valid variables' do errors = [] - linter.validate_translation(errors, 'Hello %{world}', ['%{world}']) + linter.validate_translation(errors, entry) expect(errors).to be_empty end @@ -275,43 +302,38 @@ describe Gitlab::I18n::PoLinter do expect(FastGettext::Translation).to receive(:_) { raise 'broken' } - linter.validate_translation(errors, 'Hello', []) + linter.validate_translation(errors, entry) - expect(errors).to include('Failure translating to en with []: broken') + expect(errors).to include('Failure translating to en: broken') end it 'adds an error message when translating fails when translating with context' do + entry = fake_translation(id: 'Tests|Hello', translation: 'broken') errors = [] expect(FastGettext::Translation).to receive(:s_) { raise 'broken' } - linter.validate_translation(errors, 'Tests|Hello', []) + linter.validate_translation(errors, entry) - expect(errors).to include('Failure translating to en with []: broken') + expect(errors).to include('Failure translating to en: broken') end it "adds an error when trying to translate with incorrect variables when using unnamed variables" do + entry = fake_translation(id: 'Hello %s', translation: 'Hello %d') errors = [] - linter.validate_translation(errors, 'Hello %d', ['%s']) + linter.validate_translation(errors, entry) - expect(errors.first).to start_with("Failure translating to en with") + expect(errors.first).to start_with("Failure translating to en") end it "adds an error when trying to translate with named variables when unnamed variables are expected" do + entry = fake_translation(id: 'Hello %s', translation: 'Hello %{thing}') errors = [] - linter.validate_translation(errors, 'Hello %d', ['%{world}']) - - expect(errors.first).to start_with("Failure translating to en with") - end - - it 'adds an error when translated with incorrect variables using named variables' do - errors = [] - - linter.validate_translation(errors, 'Hello %{thing}', ['%d']) + linter.validate_translation(errors, entry) - expect(errors.first).to start_with("Failure translating to en with") + expect(errors.first).to start_with("Failure translating to en") end end -- cgit v1.2.3