Merge pull request rails#41845 from jhawthorn/template_suggestions

Add DidYouMean suggestions for missing templates

Author: jhawthorn

actionview/lib/action_view/template/error.rb

@@ -27,11 +27,16 @@ def message
   end
 
   class MissingTemplate < ActionViewError #:nodoc:
-    attr_reader :path
+    attr_reader :path, :paths, :prefixes
 
     def initialize(paths, path, prefixes, partial, details, *)
+      if partial && path.present?
+        path = path.sub(%r{([^/]+)$}, "_\\1")
+      end
+
       @path = path
-      prefixes = Array(prefixes)
+      @paths = paths
+      @prefixes = Array(prefixes)
       template_type = if partial
         "partial"
       elsif /layouts/i.match?(path)
@@ -40,15 +45,103 @@ def initialize(paths, path, prefixes, partial, details, *)
         "template"
       end
 
-      if partial && path.present?
-        path = path.sub(%r{([^/]+)$}, "_\\1")
-      end
-      searched_paths = prefixes.map { |prefix| [prefix, path].join("/") }
+      searched_paths = @prefixes.map { |prefix| [prefix, path].join("/") }
 
-      out  = "Missing #{template_type} #{searched_paths.join(", ")} with #{details.inspect}. Searched in:\n"
+      out  = "Missing #{template_type} #{searched_paths.join(", ")} with #{details.inspect}.\n\nSearched in:\n"
       out += paths.compact.map { |p| "  * #{p.to_s.inspect}\n" }.join
       super out
     end
+
+    class Correction
+      Result = Struct.new(:path, :score)
+
+      class Results
+        def initialize(size)
+          @size = size
+          @results = []
+        end
+
+        def to_a
+          @results.map(&:path)
+        end
+
+        def should_record?(score)
+          if @results.size < @size
+            true
+          else
+            score < @results.last.score
+          end
+        end
+
+        def add(path, score)
+          if should_record?(score)
+            @results << Result.new(path, score)
+            @results.sort_by!(&:score)
+            @results.pop if @results.size > @size
+          end
+        end
+      end
+
+      def initialize(error)
+        @error = error
+      end
+
+      # Apps may have thousands of candidate templates so we attempt to
+      # generate the suggestions as efficiently as possible.
+      # First we split templates into prefixes and basenames, so that those can
+      # be matched separately.
+      def corrections
+        path = @error.path
+        prefixes = @error.prefixes
+        candidates = @error.paths.flat_map(&:template_paths_for_suggestions).uniq
+
+        # Group by possible prefixes
+        files_by_dir = candidates.group_by do |x|
+          File.dirname(x)
+        end.transform_values do |files|
+          files.map do |file|
+            # Remove template extensions
+            File.basename(file).split(".", 2)[0]
+          end.uniq
+        end
+
+        # No suggestions if there's an exact match, but wrong details
+        if prefixes.any? { |prefix| files_by_dir[prefix]&.include?(path) }
+          return []
+        end
+
+        cached_distance = Hash.new do |h, args|
+          h[args] = -DidYouMean::Jaro.distance(*args)
+        end
+
+        results = Results.new(6)
+
+        files_by_dir.keys.index_with do |dirname|
+          prefixes.map do |prefix|
+            cached_distance[[prefix, dirname]]
+          end.min
+        end.sort_by(&:last).each do |dirname, dirweight|
+          # If our directory's score makes it impossible to find a better match
+          # we can prune this search branch.
+          next unless results.should_record?(dirweight - 1.0)
+
+          files = files_by_dir[dirname]
+
+          files.each do |file|
+            fileweight = cached_distance[[path, file]]
+            score = dirweight + fileweight
+
+            results.add(File.join(dirname, file), score)
+          end
+        end
+
+        results.to_a
+      end
+    end
+
+    if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error)
+      DidYouMean.correct_error(self, Correction)
+    end
   end
 
   class Template

actionview/lib/action_view/template/resolver.rb

@@ -160,6 +160,11 @@ def find_all_with_query(query) # :nodoc:
       @cache.cache_query(query) { find_template_paths(File.join(@path, query)) }
     end
 
+    def template_paths_for_suggestions # :nodoc:
+      # Not implemented by default
+      []
+    end
+
   private
     def _find_all(name, prefix, partial, details, key, locals)
       find_templates(name, prefix, partial, details, locals)
@@ -337,6 +342,10 @@ def eql?(resolver)
       self.class.equal?(resolver.class) && to_path == resolver.to_path
     end
     alias :== :eql?
+
+    def template_paths_for_suggestions # :nodoc:
+      Dir.glob("**/*", base: path)
+    end
   end
 
   # An Optimized resolver for Rails' most common case.

actionview/test/template/lookup_context_test.rb

@@ -208,21 +208,21 @@ def setup
     e = assert_raise ActionView::MissingTemplate do
       @lookup_context.find("foo", %w(parent child))
     end
-    assert_match %r{Missing template parent/foo, child/foo with .* Searched in:\n  \* "/Path/to/views"\n}, e.message
+    assert_match %r{Missing template parent/foo, child/foo with .*\n\nSearched in:\n  \* "/Path/to/views"\n}, e.message
   end
 
   test "if no partial was found we get a helpful error message including the inheritance chain" do
     e = assert_raise ActionView::MissingTemplate do
       @lookup_context.find("foo", %w(parent child), true)
     end
-    assert_match %r{Missing partial parent/_foo, child/_foo with .* Searched in:\n  \* "/Path/to/views"\n}, e.message
+    assert_match %r{Missing partial parent/_foo, child/_foo with .*\n\nSearched in:\n  \* "/Path/to/views"\n}, e.message
   end
 
   test "if a single prefix is passed as a string and the lookup fails, MissingTemplate accepts it" do
     e = assert_raise ActionView::MissingTemplate do
       details = { handlers: [], formats: [], variants: [], locale: [] }
       @lookup_context.view_paths.find("foo", "parent", true, details)
     end
-    assert_match %r{Missing partial parent/_foo with .* Searched in:\n  \* "/Path/to/views"\n}, e.message
+    assert_match %r{Missing partial parent/_foo with .*\n\nSearched in:\n  \* "/Path/to/views"\n}, e.message
   end
 end

actionview/test/template/render_test.rb

@@ -630,6 +630,18 @@ def test_render_partial_with_layout_raises_descriptive_error
     assert_match "Missing partial /_true with", e.message
   end
 
+  if defined?(DidYouMean) && DidYouMean.respond_to?(:correct_error)
+    def test_render_partial_provides_spellcheck
+      e = assert_raises(ActionView::MissingTemplate) { @view.render(partial: "test/partail") }
+      assert_match %r{Did you mean\?  test/_partial\n *test/_partialhtml}, e.message
+    end
+  end
+
+  def test_render_partial_wrong_details_no_spellcheck
+    e = assert_raises(ActionView::MissingTemplate) { @view.render(partial: "test/partial_with_only_html_version", formats: [:xml]) }
+    assert_no_match %r{Did you mean\?}, e.message
+  end
+
   def test_render_with_nested_layout
     assert_equal %(<title>title</title>\n\n<div id="column">column</div>\n<div id="content">content</div>\n),
       @view.render(template: "test/nested_layout", layout: "layouts/yield")