From 1c6dbfc0d1705a29f835b1f19f2e1253a67b724c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Chr=C3=A1stek?= <mchrastek-ext@gitlab.com>
Date: Wed, 20 Nov 2024 13:57:50 +0000
Subject: [PATCH] Add namespace validation to PO linter
---
lib/gitlab/i18n/po_linter.rb | 8 ++
lib/gitlab/i18n/translation_entry.rb | 9 ++
spec/fixtures/namespaces.po | 25 ++++
spec/fixtures/valid.po | 6 +
spec/lib/gitlab/i18n/po_linter_spec.rb | 19 +++
.../lib/gitlab/i18n/translation_entry_spec.rb | 134 ++++++++++++++++++
6 files changed, 201 insertions(+)
create mode 100644 spec/fixtures/namespaces.po
diff --git a/lib/gitlab/i18n/po_linter.rb b/lib/gitlab/i18n/po_linter.rb
index 0c7da1d19525f..b85fd991a94d9 100644
--- a/lib/gitlab/i18n/po_linter.rb
+++ b/lib/gitlab/i18n/po_linter.rb
@@ -75,10 +75,18 @@ def validate_entry(entry)
validate_unescaped_chars(errors, entry)
validate_html(errors, entry)
validate_translation(errors, entry)
+ validate_namespace(errors, entry)
errors
end
+ def validate_namespace(errors, entry)
+ if entry.translations_contain_namespace?
+ errors << 'contains a namespace, remove it from the translation. For more information see ' \
+ 'https://docs.gitlab.com/ee/development/i18n/translation.html#namespaced-strings'
+ end
+ end
+
def validate_unescaped_chars(errors, entry)
if entry.msgid_contains_unescaped_chars?
errors << 'contains unescaped `%`, escape it using `%%`'
diff --git a/lib/gitlab/i18n/translation_entry.rb b/lib/gitlab/i18n/translation_entry.rb
index 6f4451d42eeaf..22ec6e8dbe80a 100644
--- a/lib/gitlab/i18n/translation_entry.rb
+++ b/lib/gitlab/i18n/translation_entry.rb
@@ -5,6 +5,7 @@ module I18n
class TranslationEntry
PERCENT_REGEX = /(?:^|[^%])%(?!{\w*}|[a-z%])/
ANGLE_BRACKET_REGEX = /[<>]/
+ NAMESPACE_REGEX = /^((?u)\w+|\s)*\|/
attr_reader :nplurals, :entry_data
@@ -68,6 +69,14 @@ def translations_have_multiple_lines?
translation_entries.any?(Array)
end
+ def translations_contain_namespace?
+ all_translations.any? { |translation| contains_namespace?(translation) }
+ end
+
+ def contains_namespace?(string)
+ string =~ NAMESPACE_REGEX
+ end
+
def msgid_contains_unescaped_chars?
contains_unescaped_chars?(msgid)
end
diff --git a/spec/fixtures/namespaces.po b/spec/fixtures/namespaces.po
new file mode 100644
index 0000000000000..962bb4f0752c3
--- /dev/null
+++ b/spec/fixtures/namespaces.po
@@ -0,0 +1,25 @@
+# SOME DESCRIPTIVE TITLE.
+# Copyright (C) YEAR THE PACKAGE'S COPYRIGHT HOLDER
+# This file is distributed under the same license as the gitlab package.
+# FIRST AUTHOR <EMAIL@ADDRESS>, YEAR.
+#
+#, fuzzy
+msgid ""
+msgstr ""
+"Project-Id-Version: gitlab 1.0.0\n"
+"Report-Msgid-Bugs-To: \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"
+"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n"
+
+msgid "404|Not found"
+msgstr "404|No encontrado"
+
+msgid "CommitHistory|1 commit"
+msgid_plural "CommitHistory|%d commits"
+msgstr[0] "CommitHistory|1 cambio"
+msgstr[1] "CommitHistory|%d cambios"
diff --git a/spec/fixtures/valid.po b/spec/fixtures/valid.po
index 3a833e0f65661..2b4a914be38e7 100644
--- a/spec/fixtures/valid.po
+++ b/spec/fixtures/valid.po
@@ -1131,3 +1131,9 @@ msgid "CycleAnalytics|%{stageName}"
msgid_plural "CycleAnalytics|%d stages selected"
msgstr[0] "%{stageName}"
msgstr[1] "%d stages selected"
+
+msgid "Example: (feature|hotfix)\\/.*"
+msgstr "Ejemplo: (feature|hotfix)\\/.*"
+
+msgid "Example: (jar|exe)$"
+msgstr "Ejemplo: (jar|exe)$"
diff --git a/spec/lib/gitlab/i18n/po_linter_spec.rb b/spec/lib/gitlab/i18n/po_linter_spec.rb
index 3f0eb66886eac..ed29c7c4e37ae 100644
--- a/spec/lib/gitlab/i18n/po_linter_spec.rb
+++ b/spec/lib/gitlab/i18n/po_linter_spec.rb
@@ -47,6 +47,24 @@ def fake_translation(msgid:, translation:, plural_id: nil, plurals: [])
end
end
+ context 'for a translations with namespaces' do
+ let(:po_path) { 'spec/fixtures/namespaces.po' }
+
+ it 'has an error for translation with namespace' do
+ message_id = "404|Not found"
+ expected_message = "contains a namespace, remove it from the translation. For more information see https://docs.gitlab.com/ee/development/i18n/translation.html#namespaced-strings"
+
+ expect(errors[message_id]).to include(expected_message)
+ end
+
+ it 'has an error for plural translation with namespace' do
+ message_id = "CommitHistory|1 commit"
+ expected_message = "contains a namespace, remove it from the translation. For more information see https://docs.gitlab.com/ee/development/i18n/translation.html#namespaced-strings"
+
+ expect(errors[message_id]).to include(expected_message)
+ end
+ end
+
context 'for a translations with newlines' do
let(:po_path) { 'spec/fixtures/newlines.po' }
@@ -214,6 +232,7 @@ def fake_translation(msgid:, translation:, plural_id: nil, plurals: [])
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)
+ expect(linter).to receive(:validate_namespace).with([], fake_entry)
expect(linter).to receive(:validate_html).with([], fake_entry)
linter.validate_entry(fake_entry)
diff --git a/spec/lib/gitlab/i18n/translation_entry_spec.rb b/spec/lib/gitlab/i18n/translation_entry_spec.rb
index 3531314cf9cd4..ddb72335ef916 100644
--- a/spec/lib/gitlab/i18n/translation_entry_spec.rb
+++ b/spec/lib/gitlab/i18n/translation_entry_spec.rb
@@ -133,6 +133,140 @@
end
end
+ describe '#translations_contain_namespace' do
+ it 'is true when the msgstr contains namespace' do
+ data = {
+ msgid: '404|Not found',
+ msgstr: '404|No encontrado'
+ }
+ entry = described_class.new(entry_data: data, nplurals: 2)
+
+ expect(entry.translations_contain_namespace?).to be_truthy
+ end
+
+ it 'is true when one plural translation contains namespace' do
+ data = {
+ msgid: 'Test|hello world',
+ msgid_plural: 'Test|hello worlds',
+ "msgstr[0]" => 'Test|hello world',
+ "msgstr[1]" => 'hello worlds'
+ }
+ entry = described_class.new(entry_data: data, nplurals: 2)
+
+ expect(entry.translations_contain_namespace?).to be_truthy
+ end
+
+ it 'is true when all plural translation contains namespace' do
+ data = {
+ msgid: 'Test|hello world',
+ msgid_plural: 'Test|hello worlds',
+ "msgstr[0]" => 'Test|hello world',
+ "msgstr[1]" => 'Test|hello worlds'
+ }
+ entry = described_class.new(entry_data: data, nplurals: 2)
+
+ expect(entry.translations_contain_namespace?).to be_truthy
+ end
+
+ it "is false when the msgstr doesn't contain namespace" do
+ data = {
+ msgid: '404|Not found',
+ msgstr: 'No encontrado'
+ }
+ entry = described_class.new(entry_data: data, nplurals: 2)
+
+ expect(entry.translations_contain_namespace?).to be_falsy
+ end
+
+ it "is false when the msgstr contains namespace but source is from list of false positives" do
+ data = {
+ msgid: 'Example: (jar|exe)$',
+ msgstr: 'Ejemplo: (jar|exe)$'
+ }
+ entry = described_class.new(entry_data: data, nplurals: 2)
+
+ data_two = {
+ msgid: 'Example: (feature|hotfix)\\\\/.*',
+ msgstr: 'Ejemplo: (feature|hotfix)\\\\/.*'
+ }
+ entry_two = described_class.new(entry_data: data_two, nplurals: 2)
+
+ expect(entry_two.translations_contain_namespace?).to be_falsy
+ expect(entry.translations_contain_namespace?).to be_falsy
+ end
+
+ it 'is false when no plural translation contains namespace' do
+ data = {
+ msgid: 'Test|hello world',
+ msgid_plural: 'Test|hello worlds',
+ "msgstr[0]" => 'hello world',
+ "msgstr[1]" => 'hello worlds'
+ }
+ entry = described_class.new(entry_data: data, nplurals: 2)
+
+ expect(entry.translations_contain_namespace?).to be_falsy
+ end
+ end
+
+ describe '#contains_namespace' do
+ let(:data) { { msgid: '' } }
+ let(:entry) { described_class.new(entry_data: data, nplurals: 2) }
+
+ it 'is true when the string contains a namespace' do
+ string = '404|Not found'
+
+ expect(entry.contains_namespace?(string)).to be_truthy
+ end
+
+ it 'is true when the string contains a namespace with leading spaces' do
+ string = ' 404|Not found'
+
+ expect(entry.contains_namespace?(string)).to be_truthy
+ end
+
+ it 'is true when the string contains a namespace with trailing spaces' do
+ string = '404 |Not found'
+
+ expect(entry.contains_namespace?(string)).to be_truthy
+ end
+
+ it 'is true when the string contains a namespace with leading and trailing spaces' do
+ string = ' 404 |Not found'
+
+ expect(entry.contains_namespace?(string)).to be_truthy
+ end
+
+ it 'is true when the string contains a namespace with a space' do
+ string = '40 4|Not found'
+
+ expect(entry.contains_namespace?(string)).to be_truthy
+ end
+
+ it 'is true when the string contains a namespace with unicode characters' do
+ string = 'ZobrazenÃÄŒasu|System'
+
+ expect(entry.contains_namespace?(string)).to be_truthy
+ end
+
+ it 'is true when the string contains a namespace with unicode characters and a space' do
+ string = 'Zobrazenà Času|System'
+
+ expect(entry.contains_namespace?(string)).to be_truthy
+ end
+
+ it 'is false when the string contains a pipe, but not a namespace' do
+ string = 'Example: (jar|exe)$'
+
+ expect(entry.contains_namespace?(string)).to be_falsy
+ end
+
+ it 'is false when the string does not contain a namespace' do
+ string = 'Not found'
+
+ expect(entry.contains_namespace?(string)).to be_falsy
+ end
+ end
+
describe '#contains_unescaped_chars' do
let(:data) { { msgid: '' } }
let(:entry) { described_class.new(entry_data: data, nplurals: 2) }
--
GitLab