Skip to content

Commit

Permalink
Evaluate granular references information #150
Browse files Browse the repository at this point in the history
In theory, references should be somewhat supported for `find`, `missing`,
and `unused` at this point.

Missing support for `add-missing` and `translate-missing`, finding keys
by their resolved name via `find`, and nice reporting.
  • Loading branch information
glebm committed Jan 28, 2016
1 parent 28003a2 commit a63c387
Show file tree
Hide file tree
Showing 13 changed files with 137 additions and 89 deletions.
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ gemspec

unless ENV['TRAVIS']
group :development do
gem 'byebug', platforms: [:mri_21, :mri_22, :mswin, :x64_mingw_21, :x64_mingw_22], require: false
gem 'byebug', platforms: [:mri_21, :mri_22, :mri_23, :mswin, :x64_mingw_21, :x64_mingw_22], require: false
gem 'rubinius-debugger', platform: :rbx, require: false
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/i18n/tasks/command/commands/usages.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module Usages

def find(opt = {})
opt[:filter] ||= opt.delete(:pattern) || opt[:arguments].try(:first)
print_forest i18n.used_tree(strict: opt[:strict], key_filter: opt[:filter].presence), opt, :used_keys
print_forest i18n.used_tree(strict: opt[:strict], key_filter: opt[:filter].presence, include_raw_references: true), opt, :used_keys
end

cmd :unused,
Expand Down
2 changes: 1 addition & 1 deletion lib/i18n/tasks/data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module Data
# @see I18n::Tasks::Data::FileSystem
def data
@data ||= begin
data_config = (config[:data] || {}).deep_symbolize_keys
data_config = (config[:data] || {}).deep_symbolize_keys
data_config.merge!(base_locale: base_locale, locales: config[:locales])
adapter_class = data_config[:adapter].presence || data_config[:class].presence || DATA_DEFAULTS[:adapter]
adapter_class = adapter_class.to_s
Expand Down
4 changes: 4 additions & 0 deletions lib/i18n/tasks/data/tree/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ def data?
@data.present?
end

def reference?
value.is_a?(Symbol)
end

def get(key)
children.get(key)
end
Expand Down
28 changes: 22 additions & 6 deletions lib/i18n/tasks/data/tree/siblings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,12 @@ def append(nodes)
derive.append!(nodes)
end

def merge!(nodes)
# @param leaves_merge_guard [Proc] invoked when a leaf is merged with another leaf
def merge!(nodes, leaves_merge_guard: nil)
nodes = Siblings.from_nested_hash(nodes) if nodes.is_a?(Hash)
nodes.each do |node|
merge_node! node
merge_node! node, leaves_merge_guard: leaves_merge_guard
end
@list = key_to_node.values
dirty!
self
end

Expand All @@ -132,18 +131,32 @@ def subtract_keys(keys)
remove_nodes_collapsing_emptied_ancestors to_remove
end

def subtract_keys!(keys)
to_remove = Set.new
keys.each do |full_key|
node = get full_key
to_remove << node if node
end
remove_nodes_collapsing_emptied_ancestors! to_remove
end

def subtract_by_key(other)
subtract_keys other.key_names(root: true)
end

def subtract_by_key!(other)
subtract_keys! other.key_names(root: true)
end

def set_root_key!(new_key, data = nil)
return self if empty?
rename_key first.key, new_key
leaves { |node| node.data.merge! data } if data
self
end

def merge_node!(node)
# @param leaves_merge_guard [Proc] invoked when a leaf is merged with another leaf
def merge_node!(node, leaves_merge_guard: nil)
if key_to_node.key?(node.key)
our = key_to_node[node.key]
return if our == node
Expand All @@ -156,9 +169,12 @@ def merge_node!(node)
warn_add_children_to_leaf our
our.children = node.children
end
elsif leaves_merge_guard
leaves_merge_guard.call(our, node)
end
else
key_to_node[node.key] = node.derive(parent: parent)
@list << (key_to_node[node.key] = node.derive(parent: parent))
dirty!
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/i18n/tasks/missing_keys.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def equal_values_tree(locale, compare_to = base_locale)
base = data[compare_to].first.children
data[locale].select_keys(root: false) { |key, node|
other_node = base[key]
other_node && node.value == other_node.value && !ignore_key?(key, :eq_base, locale)
other_node && !node.reference? && node.value == other_node.value && !ignore_key?(key, :eq_base, locale)
}.set_root_key!(locale, type: :eq_base)
end

Expand Down
71 changes: 38 additions & 33 deletions lib/i18n/tasks/references.rb
Original file line number Diff line number Diff line change
@@ -1,43 +1,48 @@
# frozen_string_literal: true
module I18n::Tasks
module References
# Given a tree of key usages, return all the reference keys in the tree in their resolved form.
# @param usages [Data::Tree::Siblings]
# @param references [Data::Tree::Siblings]
# @return [Array<String>] a list of all references and their resolutions.
def resolve_references(usages, references)
usages.each.flat_map do |node|
references.key_to_node.flat_map do |ref_key_part, ref_node|
if node.key == ref_key_part
if ref_node.leaf?
[ref_node.full_key(root: false)] +
if node.leaf?
[ref_node.value.to_s]
# Given a raw usage tree and a tree of reference keys in the data, return 3 trees:
# 1. Raw references -- a subset of the usages tree with keys that are reference key usages.
# 2. Resolved references -- all the used references in their fully resolved form.
# 3. Reference keys -- all the used reference keys.
def process_references(usages, data_references = merge_reference_trees(data_forest.select_keys { |_, node| node.reference? }))
fail ArgumentError.new('usages must be a Data::Tree::Instance') unless usages.is_a?(Data::Tree::Siblings)
fail ArgumentError.new('all_references must be a Data::Tree::Instance') unless data_references.is_a?(Data::Tree::Siblings)
raw_refs = empty_forest
resolved_refs = empty_forest
refs = empty_forest
data_references.key_to_node.each do |ref_key_part, ref_node|
usages.each do |usage_node|
next unless usage_node.key == ref_key_part
if ref_node.leaf?
unless refs.key_to_node.key?(ref_node.key)
refs.merge_node!(Data::Tree::Node.new(key: ref_node.key, data: usage_node.data))
end
resolved_refs.merge!(
Data::Tree::Siblings.from_key_names([ref_node.value.to_s]) { |_, resolved_node|
raw_refs.merge_node!(usage_node)
if usage_node.leaf?
resolved_node.data.merge!(usage_node.data)
else
node.children.flat_map { |child|
collect_referenced_keys(child, [ref_node.value.to_s])
}
resolved_node.children = usage_node.children
end
else
resolve_references(node.children, ref_node.children)
end
}.tap { |new_resolved_refs|
refs.key_to_node[ref_node.key].data.tap { |ref_data|
ref_data[:occurrences] ||= []
new_resolved_refs.leaves { |leaf| ref_data[:occurrences].concat(leaf.data[:occurrences] || []) }
ref_data[:occurrences].sort_by!(&:path)
ref_data[:occurrences].uniq!
}
})
else
[]
child_raw_refs, child_resolved_refs, child_refs = process_references(usage_node.children, ref_node.children)
raw_refs.merge_node! Data::Tree::Node.new(key: ref_node.key, children: child_raw_refs) unless child_raw_refs.empty?
resolved_refs.merge! child_resolved_refs
refs.merge_node! Data::Tree::Node.new(key: ref_node.key, children: child_refs) unless child_refs.empty?
end
end
end
end

# Given a node, return the keys of all the leaves up to the given node prefixed with the given prefix.
# @param node [Data::Tree::Node]
# @param prefix [Array<String>]
# @return Array<String> full keys
def collect_referenced_keys(node, prefix)
if node.leaf?
(prefix + [node.key]) * '.'
else
node.children.flat_map { |child| collect_referenced_keys(child, prefix + [node.key]) }
end
[raw_refs, resolved_refs, refs]
end

# Given a forest of references, merge trees into one tree, ensuring there are no conflicting references.
Expand All @@ -46,14 +51,14 @@ def collect_referenced_keys(node, prefix)
def merge_reference_trees(roots)
roots.inject(empty_forest) do |forest, root|
root.keys { |full_key, node|
::I18n::Tasks::Logging.log_warn(
log_warn(
"Self-referencing node: #{node.full_key.inspect} is #{node.value.inspect} in #{node.data[:locale]}"
) if full_key == node.value.to_s
}
forest.merge!(
root.children,
leaves_merge_guard: -> (node, other) {
::I18n::Tasks::Logging.log_warn(
log_warn(
"Conflicting references: #{node.full_key.inspect} is #{node.value.inspect} in #{node.data[:locale]}, but #{other.value.inspect} in #{other.data[:locale]}"
) if node.value != other.value
})
Expand Down
2 changes: 1 addition & 1 deletion lib/i18n/tasks/scanners/pattern_scanner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def initialize(**args)
protected

# Extract i18n keys from file based on the pattern which must capture the key literal.
# @return [Array<[key, Results::KeyOccurrence]>] each occurrence found in the file
# @return [Array<[key, Results::Occurrence]>] each occurrence found in the file
def scan_file(path)
keys = []
text = read_file(path)
Expand Down
5 changes: 4 additions & 1 deletion lib/i18n/tasks/unused_keys.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@ def unused_keys(locales: nil, strict: nil)
# @param [String] locale
# @param [Boolean] strict if true, do not match dynamic keys
def unused_tree(locale: base_locale, strict: nil)
used_key_names = used_tree(strict: true).keys.reject {|_key, node|
node.data[:type] == :used_reference_key_raw
}.map(&:first)
collapse_plural_nodes! data[locale].select_keys { |key, _node|
!ignore_key?(key, :unused) &&
(strict || !used_in_expr?(key)) &&
!used_key?(depluralize_key(key, locale))
!used_key_names.include?(depluralize_key(key, locale))
}
end
end
Expand Down
34 changes: 19 additions & 15 deletions lib/i18n/tasks/used_keys.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,20 @@ module UsedKeys
# @param key_filter [String] only return keys matching this pattern.
# @param strict [Boolean] if true, dynamic keys are excluded (e.g. `t("category.#{ category.key }")`)
# @return [Data::Tree::Siblings]
def used_tree(key_filter: nil, strict: nil)
keys = ((@used_tree ||= {})[strict?(strict)] ||= scanner(strict: strict).keys.freeze)
def used_tree(key_filter: nil, strict: nil, include_raw_references: false)
src_tree = used_in_source_tree(key_filter: key_filter, strict: strict)

raw_refs, resolved_refs, used_refs = process_references(src_tree['used'].children)
src_tree.tap do |result|
tree = result['used'].children
tree.subtract_by_key!(raw_refs) unless include_raw_references
tree.merge!(used_refs).merge!(resolved_refs)
end
end

def used_in_source_tree(key_filter: nil, strict: nil)
keys = ((@keys_used_in_source_tree ||= {})[strict?(strict)] ||=
scanner(strict: strict).keys.freeze)
if key_filter
key_filter_re = compile_key_pattern(key_filter)
keys = keys.reject { |k| k.key !~ key_filter_re }
Expand All @@ -44,6 +56,7 @@ def used_tree(key_filter: nil, strict: nil)
).to_siblings
end


def scanner(strict: nil)
(@scanner ||= {})[strict?(strict)] ||= begin
shared_options = search_config.dup
Expand Down Expand Up @@ -103,15 +116,6 @@ def caching_file_reader
@caching_file_reader ||= Scanners::Files::CachingFileReader.new
end

def used_key_names(strict: nil)
(@used_key_names ||= {})[strict?(strict)] ||= used_tree(strict: strict).key_names
end

# whether the key is used in the source
def used_key?(key)
used_key_names(strict: true).include?(key)
end

# @return whether the key is potentially used in a code expression such as `t("category.#{ category_key }")`
def used_in_expr?(key)
!!(key =~ expr_key_re)
Expand All @@ -131,7 +135,7 @@ def expr_key_re(replacement: ':'.freeze)
@expr_key_re ||= begin
# disallow patterns with no keys
ignore_pattern_re = /\A[\.#{replacement}]*\z/
patterns = used_key_names(strict: false).select { |k|
patterns = used_in_source_tree(strict: false).key_names.select { |k|
k.end_with?('.'.freeze) || k =~ /\#{/.freeze
}.map { |k|
pattern = "#{replace_key_exp(k, replacement)}#{replacement if k.end_with?('.'.freeze)}"
Expand All @@ -148,9 +152,9 @@ def expr_key_re(replacement: ':'.freeze)
# @return [String]
def replace_key_exp(key, replacement)
scanner = StringScanner.new(key)
braces = []
result = []
while (match_until = scanner.scan_until(/(?:#?\{|})/.freeze) )
braces = []
result = []
while (match_until = scanner.scan_until(/(?:#?\{|})/.freeze))
if scanner.matched == '#{'.freeze
braces << scanner.matched
result << match_until[0..-3] if braces.length == 1
Expand Down
4 changes: 4 additions & 0 deletions spec/fixtures/app/views/index.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,7 @@ p #{t :"missing_symbol.key_two"} #{t :'missing_symbol.key_three'}
= page_title

p = Spree.t 'not_a_key'

= t 'reference-ok-plain'
= t 'reference-ok-nested.a'
= t 'reference-missing-target.a'
11 changes: 9 additions & 2 deletions spec/i18n_tasks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
magic_comment
default_arg
.not_relative
missing_target.a
)
}
let (:expected_missing_keys_diff) {
Expand Down Expand Up @@ -99,7 +100,7 @@
end

let(:expected_unused_keys) do
%w(unused.a unused.numeric unused.plural).map do |k|
%w(unused.a unused.numeric unused.plural reference-unused reference-unused-target).map do |k|
%w(en es).map { |l| "#{l}.#{k}" }
end.reduce(:+)
end
Expand Down Expand Up @@ -294,7 +295,13 @@
'very' => {'scoped' => {'x' => v}},
'used' => {'a' => v},
'latin_extra' => {'çüéö' => v},
'not_a_comment' => v
'not_a_comment' => v,
'reference-ok-plain' => :'resolved_reference_target.a',
'reference-ok-nested' => :resolved_reference_target,
'reference-unused' => :'resolved_reference_target.a',
'reference-unused-target' => :'unused.a',
'reference-missing-target' => :missing_target,
'resolved_reference_target' => {'a' => v}
}.tap { |r|
if BENCH_KEYS > 0
gen = r['bench'] = {}
Expand Down
Loading

0 comments on commit a63c387

Please sign in to comment.