i18n-tasks icon indicating copy to clipboard operation
i18n-tasks copied to clipboard

New mode detecting incorrect translations

Open deivid-rodriguez opened this issue 8 years ago • 6 comments

I just added localeapp to my app, and it detected some errors that i18n-tasks missed. In particular, it detected some keys, that although used and translated in every language, were wrong because they incorrectly included a placeholder in some languages.

Would it be possible to detect this by adding another command, say i18n-tasks incorrect?

Thanks for your work!

deivid-rodriguez avatar Mar 09 '16 13:03 deivid-rodriguez

Do you mean incorrect %{interpolations}? It would be possible to detect this, but first the AST parser would need to be implemented for view files (currently it only supports .rb files, and for view files we simply do regexp matching), in order to be able to extract the passed arguments.

glebm avatar Mar 10 '16 11:03 glebm

Yes, that's exactly what I mean!

deivid-rodriguez avatar Mar 10 '16 11:03 deivid-rodriguez

@glebm is there are a way to check interpolations to be used in source translations from master. For example, if en locale (used as a master) has %{var1} %{var2} and es has %{var1} %{des2} should notice such case, because es don't have %{var2}, but instead has %{des2}. Is there are possibility at the moment to find such cases?

dmitry avatar May 02 '16 13:05 dmitry

i18n-tasks doesn't provide this at the moment, but this is a useful feature to have and I would merge a PR implementing it.

glebm avatar May 02 '16 13:05 glebm

:+1: on this request.

I've implemented something similar in RSpec:

  let(:i18n) { I18n::Tasks::BaseTask.new }

  def i18n_args(string)
    string.scan(/%\{\w+\}/)
  end

  it 'does not have missing interpolation arguments' do
    errors = []
    en_keys = i18n.data['en'].keys
    other_locales = i18n.locales - ['en']

    other_locales.each do |locale|
      diff_keys = i18n.data[locale].keys.each do |key, node|
        en_node = en_keys.detect { |en_key, _| en_key == key }
        next unless en_node
        en_value = en_node[1].value
        translated_args = i18n_args(node.value).sort
        en_args = i18n_args(en_value).sort
        missing_some = (translated_args - en_args).length > 0

        if missing_some
          errors <<
            "#{locale}: #{key} is missing args: #{(translated_args - en_args)}"
        end
      end
    end

    expect(errors).to be_empty,
      "Found i18n keys that are missing translations: #{errors.join("\n")}"
  end

Sorry for not PR'ing, but I don't have quite enough time to grok the codebase enough to understand how to add this properly. Hopefully this is useful to someone though!

ajb avatar Jul 08 '16 22:07 ajb

I have implemented a check for missing placeholders some times ago, but since it gives some false positives (like detecting count as missing placeholder, when it was passed for pluralization; as well as some parsing false-positives when there are nested calls), it was never published. Anyway, here is the code, if someone wants to make it part of i18n-tasks, welcome

class TranslationsScanner < I18n::Tasks::Scanners::PatternScanner
  def default_pattern
    # capture the first argument and count argument if present
    /#{super}
    \s*,\s*([^\)]+ )? (?# capture arguments )
    /x
  end

  def plurals
    result.select { |entry| extract_variables(entry.occurrences).include?('count') }.map(&:key)
  end

  def variables
    vars = {}
    result.each { |entry| vars[entry.key] = extract_variables(entry.occurrences) }
    vars.reject { |_key, value| value.empty? }
  end

  protected

  def result
    @result ||= keys
  end

  def extract_variables(occurrences)
    occurrences
      .map(&:variables)
      .flatten
      .reject { |key| %w(scope locale).include?(key) }
  end

  # Given
  # @param [MatchData] match
  # @param [String] path
  # @return [String] full absolute key name with count resolved if any
  def match_to_key(match, path, location)
    class << location
      attr_accessor :variables
    end
    key = super
    variables = match[1].split(/,(?=\s*[a-z]+:)|,(?=\s*:[a-z]+\s=>)/).map do |variable|
      parts = variable.split(':')
      return nil if parts.size < 2 # hash is passed instead of individual variables
      parts.first.strip
    end
    location.variables = variables.reject(&:nil?)
    return key if variables.any?
  end
end
require_relative 'translations_scanner'

class TranslationsTasks < ::I18n::Tasks::BaseTask
  def check_pluralization(opts = {})
    translations(opts).inject(empty_forest) do |forest, locale|
      forest.merge! missing_pluralizations(data[locale]).set_root_key!(locale)
    end
  end

  def check_variable_usage(opts = {})
    translations(opts).inject(empty_forest) do |forest, locale|
      forest.merge! missing_variables(data[locale]).set_root_key!(locale)
    end
  end

  private

  def scanner
    @scanner ||= begin
      search_config = (config[:search] || {}).with_indifferent_access
      TranslationsScanner.new config: search_config.symbolize_keys
    end
  end

  def translations(opts = {})
    locales = opts[:locales].presence || self.locales
    locales - [base_locale]
  end

  def missing_pluralizations(data)
    ok = {}

    data.nodes do |node|
      next unless pluralized?(node)

      if node.children?
        missing = []
        missing << 'one' if node.get(:one).nil?
        missing << 'other' if node.get(:other).nil?
        accept_node(node, ok, 'Missing keys: ' + missing.join(',')) if missing.any?
      else
        accept_node(node, ok, 'Missing pluralization structure')
      end
    end

    data.select_nodes do |node|
      ok[node]
    end
  end

  def accept_node(node, list, message)
    node.value = message
    node.children = nil # only leafs are displayed in the tree
    node.walk_to_root do |p|
      break if list[p]
      list[p] = true
    end
  end

  def missing_variables(data)
    data.select_keys do |key, node|
      next unless scanner.variables.include?(key)

      missing = []
      scanner.variables[key].each do |variable|
        missing << variable unless node.value.include?("%{#{variable}}")
      end
      node.value = "Missing #{missing.join(',')} variable(s)" if missing.any?
      missing.any?
    end
  end

  def pluralized?(node)
    key = node.full_key(root: false)
    return false unless scanner.plurals.include?(key)

    data[base_locale].nodes do |base_node|
      return true if base_node.full_key(root: false) == key && base_node.children?
    end
    false
  end
end
require_relative 'translations_tasks'

module TranslationsCommands
  include ::I18n::Tasks::Command::Collection
  cmd :validate_translation,
      desc: 'Checks wheter all variables are used and plurals have all required keys',
      args: [:locales, :out_format]

  def validate_translation(opts = {})
    check_pluralization(opts)
    check_variable_usage(opts)
  end

  private

  def check_pluralization(opts = {})
    task = TranslationsTasks.new
    forest = task.check_pluralization(opts)
    print_forest forest, opts
    :exit_1 unless forest.empty?
  end

  def check_variable_usage(opts = {})
    task = TranslationsTasks.new
    forest = task.check_variable_usage(opts)
    print_forest forest, opts
    :exit_1 unless forest.empty?
  end
end

Darhazer avatar Oct 05 '17 12:10 Darhazer