From a63c3879c8c4a7fa810e1af54e635b56c615fcfa Mon Sep 17 00:00:00 2001
From: Gleb Mazovetskiy <glex.spb@gmail.com>
Date: Thu, 28 Jan 2016 02:28:16 +0000
Subject: [PATCH] Evaluate granular references information #150

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.
---
 Gemfile                                    |  2 +-
 lib/i18n/tasks/command/commands/usages.rb  |  2 +-
 lib/i18n/tasks/data.rb                     |  2 +-
 lib/i18n/tasks/data/tree/node.rb           |  4 ++
 lib/i18n/tasks/data/tree/siblings.rb       | 28 +++++++--
 lib/i18n/tasks/missing_keys.rb             |  2 +-
 lib/i18n/tasks/references.rb               | 71 ++++++++++++----------
 lib/i18n/tasks/scanners/pattern_scanner.rb |  2 +-
 lib/i18n/tasks/unused_keys.rb              |  5 +-
 lib/i18n/tasks/used_keys.rb                | 34 ++++++-----
 spec/fixtures/app/views/index.html.slim    |  4 ++
 spec/i18n_tasks_spec.rb                    | 11 +++-
 spec/references_spec.rb                    | 59 ++++++++++--------
 13 files changed, 137 insertions(+), 89 deletions(-)

diff --git a/Gemfile b/Gemfile
index a2808820..12ba587a 100644
--- a/Gemfile
+++ b/Gemfile
@@ -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
diff --git a/lib/i18n/tasks/command/commands/usages.rb b/lib/i18n/tasks/command/commands/usages.rb
index 82937029..4a79195b 100644
--- a/lib/i18n/tasks/command/commands/usages.rb
+++ b/lib/i18n/tasks/command/commands/usages.rb
@@ -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,
diff --git a/lib/i18n/tasks/data.rb b/lib/i18n/tasks/data.rb
index e4a2237e..6d767643 100644
--- a/lib/i18n/tasks/data.rb
+++ b/lib/i18n/tasks/data.rb
@@ -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
diff --git a/lib/i18n/tasks/data/tree/node.rb b/lib/i18n/tasks/data/tree/node.rb
index ed1e1e2d..578e3ecf 100644
--- a/lib/i18n/tasks/data/tree/node.rb
+++ b/lib/i18n/tasks/data/tree/node.rb
@@ -74,6 +74,10 @@ def data?
       @data.present?
     end
 
+    def reference?
+      value.is_a?(Symbol)
+    end
+
     def get(key)
       children.get(key)
     end
diff --git a/lib/i18n/tasks/data/tree/siblings.rb b/lib/i18n/tasks/data/tree/siblings.rb
index 531b9a76..92599009 100644
--- a/lib/i18n/tasks/data/tree/siblings.rb
+++ b/lib/i18n/tasks/data/tree/siblings.rb
@@ -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
 
@@ -132,10 +131,23 @@ 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
@@ -143,7 +155,8 @@ def set_root_key!(new_key, data = nil)
       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
@@ -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
 
diff --git a/lib/i18n/tasks/missing_keys.rb b/lib/i18n/tasks/missing_keys.rb
index 1f3d4a61..970299c2 100644
--- a/lib/i18n/tasks/missing_keys.rb
+++ b/lib/i18n/tasks/missing_keys.rb
@@ -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
 
diff --git a/lib/i18n/tasks/references.rb b/lib/i18n/tasks/references.rb
index 668b0e5e..c1e82256 100644
--- a/lib/i18n/tasks/references.rb
+++ b/lib/i18n/tasks/references.rb
@@ -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.
@@ -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
             })
diff --git a/lib/i18n/tasks/scanners/pattern_scanner.rb b/lib/i18n/tasks/scanners/pattern_scanner.rb
index 38db029c..162ba7c8 100644
--- a/lib/i18n/tasks/scanners/pattern_scanner.rb
+++ b/lib/i18n/tasks/scanners/pattern_scanner.rb
@@ -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)
diff --git a/lib/i18n/tasks/unused_keys.rb b/lib/i18n/tasks/unused_keys.rb
index 4f07c30b..335c4fe6 100644
--- a/lib/i18n/tasks/unused_keys.rb
+++ b/lib/i18n/tasks/unused_keys.rb
@@ -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
diff --git a/lib/i18n/tasks/used_keys.rb b/lib/i18n/tasks/used_keys.rb
index 09487d5a..e095dd52 100644
--- a/lib/i18n/tasks/used_keys.rb
+++ b/lib/i18n/tasks/used_keys.rb
@@ -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 }
@@ -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
@@ -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)
@@ -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)}"
@@ -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
diff --git a/spec/fixtures/app/views/index.html.slim b/spec/fixtures/app/views/index.html.slim
index 7588179f..13442782 100644
--- a/spec/fixtures/app/views/index.html.slim
+++ b/spec/fixtures/app/views/index.html.slim
@@ -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'
diff --git a/spec/i18n_tasks_spec.rb b/spec/i18n_tasks_spec.rb
index 85c560c3..1f308b8f 100644
--- a/spec/i18n_tasks_spec.rb
+++ b/spec/i18n_tasks_spec.rb
@@ -70,6 +70,7 @@
         magic_comment
         default_arg
         .not_relative
+        missing_target.a
       )
     }
     let (:expected_missing_keys_diff) {
@@ -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
@@ -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'] = {}
diff --git a/spec/references_spec.rb b/spec/references_spec.rb
index c741cada..ceb92aa2 100644
--- a/spec/references_spec.rb
+++ b/spec/references_spec.rb
@@ -4,52 +4,57 @@
 RSpec.describe 'Reference keys' do
   let(:task) { ::I18n::Tasks::BaseTask.new }
 
-  describe '#resolve_references' do
+  describe '#process_references' do
     it 'resolves plain references' do
-      result = task.resolve_references(
-          build_tree('en' => {
+      result = task.process_references(
+          build_tree(
               'reference'       => nil,
               'not-a-reference' => nil
-          }),
-          build_tree('en' => {
+          ),
+          build_tree(
               'reference' => :resolved
-          }))
-      expect(result).to(eq %w(reference resolved))
+          ))
+      expect(result.map(&:to_hash)).to(
+          eq [{'reference' => nil},
+              {'resolved' => nil},
+              {'reference' => nil}])
     end
 
     it 'resolves nested references' do
-      result = task.resolve_references(
-          build_tree('en' => {
+      result = task.process_references(
+          build_tree(
               'reference'       => {'a' => nil, 'b' => {'c' => nil}},
               'not-a-reference' => nil
-          }),
-          build_tree('en' => {
+          ),
+          build_tree(
               'reference' => :resolved
-          }))
-      expect(result).to(eq %w(reference resolved.a resolved.b.c))
+          ))
+      expect(result.map(&:to_hash)).to(
+          eq [{'reference' => {'a' => nil, 'b' => {'c' => nil}}},
+              {'resolved' => {'a' => nil, 'b' => {'c' => nil}}},
+              {'reference' => nil}])
     end
 
     it 'resolves nested references with nested keys' do
-      result = task.resolve_references(
-          build_tree('en' => {
+      result = task.process_references(
+          build_tree(
               'nested'          => {'reference' => {'a' => nil, 'b' => {'c' => nil}}},
               'not-a-reference' => nil
-          }),
-          build_tree('en' => {
+          ),
+          build_tree(
               'nested' => {'reference' => :resolved}
-          }))
-      expect(result).to(eq %w(nested.reference resolved.a resolved.b.c))
+          ))
+      expect(result.map(&:to_hash)).to(
+          eq [{'nested' => {'reference' => {'a' => nil, 'b' => {'c' => nil}}}},
+              {'resolved' => {'a' => nil, 'b' => {'c' => nil}}},
+              {'nested' => {'reference' => nil}}])
     end
 
     it 'returns empty array when nothing to resolve' do
-      result = task.resolve_references(
-          build_tree('en' => {
-                         'not-a-reference' => nil
-                     }),
-          build_tree('en' => {
-                         'reference' => :resolved
-                     }))
-      expect(result).to(eq [])
+      result = task.process_references(
+          build_tree('not-a-reference' => nil),
+          build_tree('reference' => :resolved))
+      expect(result.map(&:to_hash)).to(eq [{}, {}, {}])
     end
   end
 end