kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Forbids combining named and unnamed variables in gitlab.pot

Browse source 
This would break with an argument error when interpolating using
`sprintf` in ruby.
This commit is contained in:
Bob Van Landuyt 2018-05-07 16:04:11 +02:00
parent 03b8893775
commit ca065e493e
3 changed files with 140 additions and 81 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

99
lib/gitlab/i18n/po_linter.rb
View file

@ -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
@ -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
@ -117,41 +120,73 @@ 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
end
translation % variables if used_variables.any?
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}"
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 ||= Array.new(metadata_entry.expected_plurals) do |index|
number_for_pluralization(index)
end
end
def number_for_pluralization(counter)
pluralization_result = FastGettext.pluralisation_rule.call(counter)
if pluralization_result.is_a?(TrueClass) || pluralization_result.is_a?(FalseClass)
counter
else
pluralization_result
end
end
@ -172,14 +207,16 @@ module Gitlab
end
def validate_unnamed_variables(errors, variables)
if variables.size > 1 && variables.any? { |variable_name| unnamed_variable?(variable_name) }
if variables.any? { |name| unnamed_variable?(name) } && variables.any? { |name| !unnamed_variable?(name) }
errors << 'is combining named variables with unnamed variables'
end
if variables.select { |variable_name| unnamed_variable?(variable_name) }.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 +242,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

20
lib/gitlab/i18n/translation_entry.rb
View file

@ -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?
@ -55,15 +54,15 @@ module Gitlab
end
def msgid_contains_newlines?
msgid.is_a?(Array)
entry_data[:msgid].is_a?(Array)
end
def plural_id_contains_newlines?
plural_id.is_a?(Array)
entry_data[:msgid_plural].is_a?(Array)
end
def translations_contain_newlines?
all_translations.any? { |translation| translation.is_a?(Array) }
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

102
spec/lib/gitlab/i18n/po_linter_spec.rb
View file

@ -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 = ['<hello %{world} %d> is missing: [%{hello}]',
'<hello %{world} %d> 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}'])
linter.validate_translation(errors, entry)
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'])
expect(errors.first).to start_with("Failure translating to en with")
expect(errors.first).to start_with("Failure translating to en")
end
end