diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b5a33d7a45e..7e7ce7f41f4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,7 +6,7 @@ on: - master pull_request: branches-ignore: - - 'tests-passed' + - "tests-passed" jobs: build: @@ -28,12 +28,12 @@ jobs: fail-fast: false matrix: - build_types: [ 'BACKEND', 'FRONTEND', 'LINT' ] - target: [ 'PLUGINS', 'CORE' ] - os: [ ubuntu-latest ] - ruby: [ '2.6' ] - postgres: [ '10' ] - redis: [ '4.x' ] + build_types: ["BACKEND", "FRONTEND", "LINT"] + target: ["PLUGINS", "CORE"] + os: [ubuntu-latest] + ruby: ["2.6"] + postgres: ["10"] + redis: ["4.x"] services: postgres: @@ -77,7 +77,7 @@ jobs: uses: actions/setup-ruby@v1 with: ruby-version: ${{ matrix.ruby }} - architecture: 'x64' + architecture: "x64" - name: Setup bundler run: | @@ -145,6 +145,14 @@ jobs: yarn prettier -v yarn prettier --list-different "app/assets/stylesheets/**/*.scss" "app/assets/javascripts/**/*.js" "app/assets/javascripts/**/*.es6" "test/javascripts/**/*.es6" "plugins/**/*.scss" "plugins/**/*.es6" + - name: Core English locale + if: env.BUILD_TYPE == 'LINT' && env.TARGET == 'CORE' + run: bundle exec ruby script/i18n_lint.rb "config/**/locales/{client,server}.en.yml" + + - name: Plugin English locale + if: env.BUILD_TYPE == 'LINT' && env.TARGET == 'PLUGINS' + run: bundle exec ruby script/i18n_lint.rb "plugins/**/locales/{client,server}.en.yml" + - name: Core RSpec if: env.BUILD_TYPE == 'BACKEND' && env.TARGET == 'CORE' run: | @@ -167,5 +175,5 @@ jobs: - name: Plugin QUnit # Tests core plugins in TARGET=CORE, and all plugins in TARGET=PLUGINS if: env.BUILD_TYPE == 'FRONTEND' - run: bundle exec rake plugin:qunit + run: bundle exec rake plugin:qunit['*','1200000'] timeout-minutes: 30 diff --git a/lefthook.yml b/lefthook.yml index f3645c3d1a4..65fe913ccaa 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -23,6 +23,9 @@ pre-commit: # database.yml is an erb file not a yaml file exclude: "database.yml" run: bundle exec yaml-lint {staged_files} + i18n-lint: + glob: "**/{client,server}.en.yml" + run: bundle exec ruby script/i18n_lint.rb {staged_files} commands: &commands bundle-install: @@ -58,8 +61,11 @@ lints: eslint-test: run: yarn eslint --ext .es6 test/javascripts eslint-plugins-assets: - run: yarn eslint --ext .es6 plugins/**/assets/javascripts + run: yarn eslint --global I18n --ext .es6 plugins/**/assets/javascripts eslint-plugins-test: - run: yarn eslint --ext .es6 plugins/**/test/javascripts + run: yarn eslint --global I18n --ext .es6 plugins/**/test/javascripts eslint-assets-tests: run: yarn eslint app/assets/javascripts test/javascripts + i18n-lint: + glob: "**/{client,server}.en.yml" + run: bundle exec ruby script/i18n_lint.rb {all_files} diff --git a/lib/tasks/docker.rake b/lib/tasks/docker.rake index 6733ae31a00..b0d2e6e2ec5 100644 --- a/lib/tasks/docker.rake +++ b/lib/tasks/docker.rake @@ -66,6 +66,7 @@ task 'docker:test' do if ENV["SINGLE_PLUGIN"] @good &&= run_or_fail("bundle exec rubocop --parallel plugins/#{ENV["SINGLE_PLUGIN"]}") + @good &&= run_or_fail("bundle exec ruby script/i18n_lint.rb plugins/#{ENV["SINGLE_PLUGIN"]}/config/locales/{client,server}.en.yml") @good &&= run_or_fail("yarn eslint --global I18n --ext .es6 plugins/#{ENV['SINGLE_PLUGIN']}") puts "Listing prettier offenses in #{ENV['SINGLE_PLUGIN']}:" @@ -78,6 +79,9 @@ task 'docker:test' do # TODO: remove --global I18n once plugins can be updated @good &&= run_or_fail("yarn eslint --global I18n --ext .es6 plugins") unless ENV["SKIP_PLUGINS"] + @good &&= run_or_fail('bundle exec ruby script/i18n_lint.rb "config/locales/{client,server}.en.yml"') unless ENV["SKIP_CORE"] + @good &&= run_or_fail('bundle exec ruby script/i18n_lint.rb "plugins/**/locales/{client,server}.en.yml"') unless ENV["SKIP_PLUGINS"] + unless ENV["SKIP_CORE"] puts "Listing prettier offenses in core:" @good &&= run_or_fail('yarn prettier --list-different "app/assets/stylesheets/**/*.scss" "app/assets/javascripts/**/*.es6" "test/javascripts/**/*.es6"') diff --git a/script/i18n_lint.rb b/script/i18n_lint.rb new file mode 100644 index 00000000000..298efbccaad --- /dev/null +++ b/script/i18n_lint.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +require 'colored2' +require 'psych' + +class I18nLinter + def initialize(filenames_or_patterns) + @filenames = filenames_or_patterns.map { |fp| Dir[fp] }.flatten + @errors = {} + end + + def run + has_errors = false + + @filenames.each do |filename| + validator = LocaleFileValidator.new(filename) + + if validator.has_errors? + validator.print_errors + has_errors = true + end + end + + exit 1 if has_errors + end +end + +class LocaleFileValidator + ERROR_MESSAGES = { + invalid_relative_links: "The following keys have relative links, but do not start with %{base_url} or %{base_path}:", + invalid_relative_image_sources: "The following keys have relative image sources, but do not start with %{base_url} or %{base_path}:", + invalid_interpolation_key_format: "The following keys use {{key}} instead of %{key} for interpolation keys:", + wrong_pluralization_keys: "Pluralized strings must have only the sub-keys 'one' and 'other'.\nThe following keys have missing or additional keys:", + invald_one_keys: "The following keys contain the number 1 instead of the interpolation key %{count}:" + } + + PLURALIZATION_KEYS = ['zero', 'one', 'two', 'few', 'many', 'other'] + ENGLISH_KEYS = ['one', 'other'] + + def initialize(filename) + @filename = filename + @errors = {} + end + + def has_errors? + yaml = Psych.safe_load(File.read(@filename), aliases: true) + yaml = yaml[yaml.keys.first] + + validate_pluralizations(yaml) + validate_content(yaml) + + @errors.any? { |_, value| value.any? } + end + + def print_errors + puts "", "Errors in #{@filename}".red + + @errors.each do |type, keys| + next if keys.empty? + + ERROR_MESSAGES[type].split("\n").each { |msg| puts " #{msg}" } + keys.each { |key| puts " * #{key}" } + end + end + + private + + def each_translation(hash, parent_key = '', &block) + hash.each do |key, value| + current_key = parent_key.empty? ? key : "#{parent_key}.#{key}" + + if Hash === value + each_translation(value, current_key, &block) + else + yield(current_key, value.to_s) + end + end + end + + def validate_content(yaml) + @errors[:invalid_relative_links] = [] + @errors[:invalid_relative_image_sources] = [] + @errors[:invalid_interpolation_key_format] = [] + + each_translation(yaml) do |key, value| + if value.match?(/href\s*=\s*["']\/[^\/]|\]\(\/[^\/]/i) + @errors[:invalid_relative_links] << key + end + + if value.match?(/src\s*=\s*["']\/[^\/]/i) + @errors[:invalid_relative_image_sources] << key + end + + if value.match?(/{{.+?}}/) && !key.end_with?("_MF") + @errors[:invalid_interpolation_key_format] << key + end + end + end + + def each_pluralization(hash, parent_key = '', &block) + hash.each do |key, value| + if Hash === value + current_key = parent_key.empty? ? key : "#{parent_key}.#{key}" + each_pluralization(value, current_key, &block) + elsif PLURALIZATION_KEYS.include? key + yield(parent_key, hash) + end + end + end + + def validate_pluralizations(yaml) + @errors[:wrong_pluralization_keys] = [] + @errors[:invald_one_keys] = [] + + each_pluralization(yaml) do |key, hash| + # ignore errors from some ActiveRecord messages + next if key.include?("messages.restrict_dependent_destroy") + + @errors[:wrong_pluralization_keys] << key if hash.keys.sort != ENGLISH_KEYS + + one_value = hash['one'] + if one_value && one_value.include?('1') && !one_value.match?(/%{count}|{{count}}/) + @errors[:invald_one_keys] << key + end + end + end +end + +I18nLinter.new(ARGV).run diff --git a/spec/integrity/i18n_spec.rb b/spec/integrity/i18n_spec.rb index 0d44d5ea5dd..49cabe87eb4 100644 --- a/spec/integrity/i18n_spec.rb +++ b/spec/integrity/i18n_spec.rb @@ -7,22 +7,6 @@ def extract_locale(path) path[/\.([^.]{2,})\.yml$/, 1] end -PLURALIZATION_KEYS ||= ['zero', 'one', 'two', 'few', 'many', 'other'] -ENGLISH_KEYS ||= ['one', 'other'] - -def find_pluralizations(hash, parent_key = '', pluralizations = Hash.new) - hash.each do |key, value| - if Hash === value - current_key = parent_key.blank? ? key : "#{parent_key}.#{key}" - find_pluralizations(value, current_key, pluralizations) - elsif PLURALIZATION_KEYS.include? key - pluralizations[parent_key] = hash - end - end - - pluralizations -end - def is_yaml_compatible?(english, translated) english.each do |k, v| if translated.has_key?(k) @@ -39,18 +23,6 @@ def is_yaml_compatible?(english, translated) true end -def each_translation(hash, parent_key = '', &block) - hash.each do |key, value| - current_key = parent_key.blank? ? key : "#{parent_key}.#{key}" - - if Hash === value - each_translation(value, current_key, &block) - else - yield(current_key, value.to_s) - end - end -end - describe "i18n integrity checks" do it 'has an i18n key for each Trust Levels' do @@ -107,71 +79,6 @@ describe "i18n integrity checks" do english_duplicates = DuplicateKeyFinder.new.find_duplicates(english_path) expect(english_duplicates).to be_empty end - - context "pluralizations" do - wrong_keys = [] - invald_one_keys = [] - - find_pluralizations(english_yaml).each do |key, hash| - next if key["messages.restrict_dependent_destroy"] - - wrong_keys << key if hash.keys.sort != ENGLISH_KEYS - - if one_value = hash['one'] - invald_one_keys << key if one_value.include?('1') && !one_value.match?(/%{count}|{{count}}/) - end - end - - it "has valid pluralizations keys" do - keys = wrong_keys.join("\n") - expect(wrong_keys).to be_empty, <<~MSG - Pluralized strings must have only the sub-keys 'one' and 'other'. - The following keys have missing or additional keys:\n\n#{keys} - MSG - end - - it "should use %{count} instead of 1 in 'one' keys" do - keys = invald_one_keys.join(".one\n") - expect(invald_one_keys).to be_empty, <<~MSG - The following keys contain the number 1 instead of the interpolation key %{count}:\n\n#{keys} - MSG - end - end - - context "valid translations" do - invalid_relative_links = {} - invalid_relative_image_sources = {} - invalid_interpolation_key_format = {} - - each_translation(english_yaml) do |key, value| - if value.match?(/href\s*=\s*["']\/[^\/]|\]\(\/[^\/]/i) - invalid_relative_links[key] = value - end - - if value.match?(/src\s*=\s*["']\/[^\/]/i) - invalid_relative_image_sources[key] = value - end - - if value.match?(/\{\{.+?}}/) - invalid_interpolation_key_format[key] = value - end - end - - it "uses %{base_url} or %{base_path} for relative links" do - keys = invalid_relative_links.keys.join("\n") - expect(invalid_relative_links).to be_empty, "The following keys have relative links, but do not start with %{base_url} or %{base_path}:\n\n#{keys}" - end - - it "uses %{base_url} or %{base_path} for relative image src" do - keys = invalid_relative_image_sources.keys.join("\n") - expect(invalid_relative_image_sources).to be_empty, "The following keys have relative image sources, but do not start with %{base_url} or %{base_path}:\n\n#{keys}" - end - - skip "uses the %{key} as interpolation key format" do - keys = invalid_interpolation_key_format.keys.join("\n") - expect(invalid_interpolation_key_format).to be_empty, "The following keys use {{key}} instead of %{key} for interpolation keys:\n\n#{keys}" - end - end end Dir[english_path.sub(".en.yml", ".*.yml")].each do |path|