Replace dependency tracker with ripper based tracker

Instead of using the ERBTracker, we can use RipperTracker which is extracted from https://github.com/jhawthorn/actionview_precompiler.

Using a parser finds dependencies that would otherwise be difficult to find with the regular expressions. It should also theoretically work with other template systems since it operates on the compiled template instead of the contents of the file.

Co-authored-by: John Hawthorn <john@hawthorn.email>

actionview/lib/action_view/dependency_tracker.rb

@@ -2,9 +2,15 @@
 
 require "concurrent/map"
 require "action_view/path_set"
+require "action_view/render_parser"
 
 module ActionView
   class DependencyTracker # :nodoc:
+    extend ActiveSupport::Autoload
+
+    autoload :ERBTracker
+    autoload :RipperTracker
+
     @trackers = Concurrent::Map.new
 
     def self.find_dependencies(name, template, view_paths = nil)
@@ -29,155 +35,6 @@ module ActionView
       @trackers.delete(handler)
     end
 
-    class ERBTracker # :nodoc:
+    register_tracker :erb, RipperTracker
-      EXPLICIT_DEPENDENCY = /# Template Dependency: (\S+)/
-
-      # A valid ruby identifier - suitable for class, method and specially variable names
-      IDENTIFIER = /
-        [[:alpha:]_] # at least one uppercase letter, lowercase letter or underscore
-        [[:word:]]*  # followed by optional letters, numbers or underscores
-      /x
-
-      # Any kind of variable name. e.g. @instance, @@class, $global or local.
-      # Possibly following a method call chain
-      VARIABLE_OR_METHOD_CHAIN = /
-        (?:\$|@{1,2})?            # optional global, instance or class variable indicator
-        (?:#{IDENTIFIER}\.)*      # followed by an optional chain of zero-argument method calls
-        (?<dynamic>#{IDENTIFIER}) # and a final valid identifier, captured as DYNAMIC
-      /x
-
-      # A simple string literal. e.g. "School's out!"
-      STRING = /
-        (?<quote>['"]) # an opening quote
-        (?<static>.*?) # with anything inside, captured as STATIC
-        \k<quote>      # and a matching closing quote
-      /x
-
-      # Part of any hash containing the :partial key
-      PARTIAL_HASH_KEY = /
-        (?:\bpartial:|:partial\s*=>) # partial key in either old or new style hash syntax
-        \s*                          # followed by optional spaces
-      /x
-
-      # Part of any hash containing the :layout key
-      LAYOUT_HASH_KEY = /
-        (?:\blayout:|:layout\s*=>)   # layout key in either old or new style hash syntax
-        \s*                          # followed by optional spaces
-      /x
-
-      # Matches:
-      #   partial: "comments/comment", collection: @all_comments => "comments/comment"
-      #   (object: @single_comment, partial: "comments/comment") => "comments/comment"
-      #
-      #   "comments/comments"
-      #   'comments/comments'
-      #   ('comments/comments')
-      #
-      #   (@topic)         => "topics/topic"
-      #    topics          => "topics/topic"
-      #   (message.topics) => "topics/topic"
-      RENDER_ARGUMENTS = /\A
-        (?:\s*\(?\s*)                                  # optional opening paren surrounded by spaces
-        (?:.*?#{PARTIAL_HASH_KEY}|#{LAYOUT_HASH_KEY})? # optional hash, up to the partial or layout key declaration
-        (?:#{STRING}|#{VARIABLE_OR_METHOD_CHAIN})      # finally, the dependency name of interest
-      /xm
-
-      LAYOUT_DEPENDENCY = /\A
-        (?:\s*\(?\s*)                                  # optional opening paren surrounded by spaces
-        (?:.*?#{LAYOUT_HASH_KEY})                      # check if the line has layout key declaration
-        (?:#{STRING}|#{VARIABLE_OR_METHOD_CHAIN})      # finally, the dependency name of interest
-      /xm
-
-      def self.supports_view_paths? # :nodoc:
-        true
-      end
-
-      def self.call(name, template, view_paths = nil)
-        new(name, template, view_paths).dependencies
-      end
-
-      def initialize(name, template, view_paths = nil)
-        @name, @template, @view_paths = name, template, view_paths
-      end
-
-      def dependencies
-        render_dependencies + explicit_dependencies
-      end
-
-      attr_reader :name, :template
-      private :name, :template
-
-      private
-        def source
-          template.source
-        end
-
-        def directory
-          name.split("/")[0..-2].join("/")
-        end
-
-        def render_dependencies
-          render_dependencies = []
-          render_calls = source.split(/\brender\b/).drop(1)
-
-          render_calls.each do |arguments|
-            add_dependencies(render_dependencies, arguments, LAYOUT_DEPENDENCY)
-            add_dependencies(render_dependencies, arguments, RENDER_ARGUMENTS)
-          end
-
-          render_dependencies.uniq
-        end
-
-        def add_dependencies(render_dependencies, arguments, pattern)
-          arguments.scan(pattern) do
-            match = Regexp.last_match
-            add_dynamic_dependency(render_dependencies, match[:dynamic])
-            add_static_dependency(render_dependencies, match[:static], match[:quote])
-          end
-        end
-
-        def add_dynamic_dependency(dependencies, dependency)
-          if dependency
-            dependencies << "#{dependency.pluralize}/#{dependency.singularize}"
-          end
-        end
-
-        def add_static_dependency(dependencies, dependency, quote_type)
-          if quote_type == '"'
-            # Ignore if there is interpolation
-            return if dependency.include?('#{')
-          end
-
-          if dependency
-            if dependency.include?("/")
-              dependencies << dependency
-            else
-              dependencies << "#{directory}/#{dependency}"
-            end
-          end
-        end
-
-        def resolve_directories(wildcard_dependencies)
-          return [] unless @view_paths
-          return [] if wildcard_dependencies.empty?
-
-          # Remove trailing "/*"
-          prefixes = wildcard_dependencies.map { |query| query[0..-3] }
-
-          @view_paths.flat_map(&:all_template_paths).uniq.filter_map { |path|
-            path.to_s if prefixes.include?(path.prefix)
-          }.sort
-        end
-
-        def explicit_dependencies
-          dependencies = source.scan(EXPLICIT_DEPENDENCY).flatten.uniq
-
-          wildcards, explicits = dependencies.partition { |dependency| dependency.end_with?("/*") }
-
-          (explicits + resolve_directories(wildcards)).uniq
-        end
-    end
-
-    register_tracker :erb, ERBTracker
   end
 end

actionview/lib/action_view/dependency_tracker/erb_tracker.rb

@@ -0,0 +1,154 @@
+# frozen_string_literal: true
+
+module ActionView
+  class DependencyTracker # :nodoc:
+    class ERBTracker # :nodoc:
+      EXPLICIT_DEPENDENCY = /# Template Dependency: (\S+)/
+
+      # A valid ruby identifier - suitable for class, method and specially variable names
+      IDENTIFIER = /
+        [[:alpha:]_] # at least one uppercase letter, lowercase letter or underscore
+        [[:word:]]*  # followed by optional letters, numbers or underscores
+      /x
+
+      # Any kind of variable name. e.g. @instance, @@class, $global or local.
+      # Possibly following a method call chain
+      VARIABLE_OR_METHOD_CHAIN = /
+        (?:\$|@{1,2})?            # optional global, instance or class variable indicator
+        (?:#{IDENTIFIER}\.)*      # followed by an optional chain of zero-argument method calls
+        (?<dynamic>#{IDENTIFIER}) # and a final valid identifier, captured as DYNAMIC
+      /x
+
+      # A simple string literal. e.g. "School's out!"
+      STRING = /
+        (?<quote>['"]) # an opening quote
+        (?<static>.*?) # with anything inside, captured as STATIC
+        \k<quote>      # and a matching closing quote
+      /x
+
+      # Part of any hash containing the :partial key
+      PARTIAL_HASH_KEY = /
+        (?:\bpartial:|:partial\s*=>) # partial key in either old or new style hash syntax
+        \s*                          # followed by optional spaces
+      /x
+
+      # Part of any hash containing the :layout key
+      LAYOUT_HASH_KEY = /
+        (?:\blayout:|:layout\s*=>)   # layout key in either old or new style hash syntax
+        \s*                          # followed by optional spaces
+      /x
+
+      # Matches:
+      #   partial: "comments/comment", collection: @all_comments => "comments/comment"
+      #   (object: @single_comment, partial: "comments/comment") => "comments/comment"
+      #
+      #   "comments/comments"
+      #   'comments/comments'
+      #   ('comments/comments')
+      #
+      #   (@topic)         => "topics/topic"
+      #    topics          => "topics/topic"
+      #   (message.topics) => "topics/topic"
+      RENDER_ARGUMENTS = /\A
+        (?:\s*\(?\s*)                                  # optional opening paren surrounded by spaces
+        (?:.*?#{PARTIAL_HASH_KEY}|#{LAYOUT_HASH_KEY})? # optional hash, up to the partial or layout key declaration
+        (?:#{STRING}|#{VARIABLE_OR_METHOD_CHAIN})      # finally, the dependency name of interest
+      /xm
+
+      LAYOUT_DEPENDENCY = /\A
+        (?:\s*\(?\s*)                                  # optional opening paren surrounded by spaces
+        (?:.*?#{LAYOUT_HASH_KEY})                      # check if the line has layout key declaration
+        (?:#{STRING}|#{VARIABLE_OR_METHOD_CHAIN})      # finally, the dependency name of interest
+      /xm
+
+      def self.supports_view_paths? # :nodoc:
+        true
+      end
+
+      def self.call(name, template, view_paths = nil)
+        new(name, template, view_paths).dependencies
+      end
+
+      def initialize(name, template, view_paths = nil)
+        @name, @template, @view_paths = name, template, view_paths
+      end
+
+      def dependencies
+        render_dependencies + explicit_dependencies
+      end
+
+      attr_reader :name, :template
+      private :name, :template
+
+      private
+        def source
+          template.source
+        end
+
+        def directory
+          name.split("/")[0..-2].join("/")
+        end
+
+        def render_dependencies
+          render_dependencies = []
+          render_calls = source.split(/\brender\b/).drop(1)
+
+          render_calls.each do |arguments|
+            add_dependencies(render_dependencies, arguments, LAYOUT_DEPENDENCY)
+            add_dependencies(render_dependencies, arguments, RENDER_ARGUMENTS)
+          end
+
+          render_dependencies.uniq
+        end
+
+        def add_dependencies(render_dependencies, arguments, pattern)
+          arguments.scan(pattern) do
+            match = Regexp.last_match
+            add_dynamic_dependency(render_dependencies, match[:dynamic])
+            add_static_dependency(render_dependencies, match[:static], match[:quote])
+          end
+        end
+
+        def add_dynamic_dependency(dependencies, dependency)
+          if dependency
+            dependencies << "#{dependency.pluralize}/#{dependency.singularize}"
+          end
+        end
+
+        def add_static_dependency(dependencies, dependency, quote_type)
+          if quote_type == '"'
+            # Ignore if there is interpolation
+            return if dependency.include?('#{')
+          end
+
+          if dependency
+            if dependency.include?("/")
+              dependencies << dependency
+            else
+              dependencies << "#{directory}/#{dependency}"
+            end
+          end
+        end
+
+        def resolve_directories(wildcard_dependencies)
+          return [] unless @view_paths
+          return [] if wildcard_dependencies.empty?
+
+          # Remove trailing "/*"
+          prefixes = wildcard_dependencies.map { |query| query[0..-3] }
+
+          @view_paths.flat_map(&:all_template_paths).uniq.filter_map { |path|
+            path.to_s if prefixes.include?(path.prefix)
+          }.sort
+        end
+
+        def explicit_dependencies
+          dependencies = source.scan(EXPLICIT_DEPENDENCY).flatten.uniq
+
+          wildcards, explicits = dependencies.partition { |dependency| dependency.end_with?("/*") }
+
+          (explicits + resolve_directories(wildcards)).uniq
+        end
+    end
+  end
+end

actionview/lib/action_view/dependency_tracker/ripper_tracker.rb

@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+module ActionView
+  class DependencyTracker # :nodoc:
+    class RipperTracker # :nodoc:
+      EXPLICIT_DEPENDENCY = /# Template Dependency: (\S+)/
+
+      def self.call(name, template, view_paths = nil)
+        new(name, template, view_paths).dependencies
+      end
+
+      def dependencies
+        render_dependencies + explicit_dependencies
+      end
+
+      def self.supports_view_paths? # :nodoc:
+        true
+      end
+
+      def initialize(name, template, view_paths = nil)
+        @name, @template, @view_paths = name, template, view_paths
+      end
+
+      private
+        attr_reader :template, :name, :view_paths
+
+        def render_dependencies
+          return [] unless template.source.include?("render")
+
+          compiled_source = template.handler.call(template, template.source)
+
+          RenderParser.new(@name, compiled_source).render_calls.filter_map do |render_call|
+            next if render_call.end_with?("/_")
+            render_call.gsub(%r|/_|, "/")
+          end
+        end
+
+        def explicit_dependencies
+          dependencies = template.source.scan(EXPLICIT_DEPENDENCY).flatten.uniq
+
+          wildcards, explicits = dependencies.partition { |dependency| dependency.end_with?("/*") }
+
+          (explicits + resolve_directories(wildcards)).uniq
+        end
+
+        def resolve_directories(wildcard_dependencies)
+          return [] unless view_paths
+          return [] if wildcard_dependencies.empty?
+
+          # Remove trailing "/*"
+          prefixes = wildcard_dependencies.map { |query| query[0..-3] }
+
+          view_paths.flat_map(&:all_template_paths).uniq.filter_map { |path|
+            path.to_s if prefixes.include?(path.prefix)
+          }.sort
+        end
+    end
+  end
+end

actionview/lib/action_view/render_parser.rb

@@ -0,0 +1,188 @@
+# frozen_string_literal: true
+
+require "action_view/ripper_ast_parser"
+
+module ActionView
+  class RenderParser # :nodoc:
+    def initialize(name, code)
+      @name = name
+      @code = code
+      @parser = RipperASTParser
+    end
+
+    def render_calls
+      render_nodes = @parser.parse_render_nodes(@code)
+
+      render_nodes.map do |method, nodes|
+        nodes.map { |n| send(:parse_render, n) }
+      end.flatten.compact
+    end
+
+    private
+      def directory
+        File.dirname(@name)
+      end
+
+      def resolve_path_directory(path)
+        if path.include?("/")
+          path
+        else
+          "#{directory}/#{path}"
+        end
+      end
+
+      # Convert
+      #   render("foo", ...)
+      # into either
+      #   render(template: "foo", ...)
+      # or
+      #   render(partial: "foo", ...)
+      def normalize_args(string, options_hash)
+        if options_hash
+          { partial: string, locals: options_hash }
+        else
+          { partial: string }
+        end
+      end
+
+      def parse_render(node)
+        node = node.argument_nodes
+
+        if (node.length == 1 || node.length == 2) && !node[0].hash?
+          if node.length == 1
+            options = normalize_args(node[0], nil)
+          elsif node.length == 2
+            options = normalize_args(node[0], node[1])
+          end
+
+          return nil unless options
+
+          parse_render_from_options(options)
+        elsif node.length == 1 && node[0].hash?
+          options = parse_hash_to_symbols(node[0])
+
+          return nil unless options
+
+          parse_render_from_options(options)
+        else
+          nil
+        end
+      end
+
+      def parse_hash(node)
+        node.hash? && node.to_hash
+      end
+
+      def parse_hash_to_symbols(node)
+        hash = parse_hash(node)
+
+        return unless hash
+
+        hash.transform_keys do |key_node|
+          key = parse_sym(key_node)
+
+          return unless key
+
+          key
+        end
+      end
+
+      ALL_KNOWN_KEYS = [:partial, :template, :layout, :formats, :locals, :object, :collection, :as, :status, :content_type, :location, :spacer_template]
+
+      RENDER_TYPE_KEYS =
+        [:partial, :template, :layout]
+
+      def parse_render_from_options(options_hash)
+        renders = []
+        keys = options_hash.keys
+
+        if (keys & RENDER_TYPE_KEYS).size < 1
+          # Must have at least one of render keys
+          return nil
+        end
+
+        if (keys - ALL_KNOWN_KEYS).any?
+          # de-opt in case of unknown option
+          return nil
+        end
+
+        render_type = (keys & RENDER_TYPE_KEYS)[0]
+
+        node = options_hash[render_type]
+
+        if node.string?
+          template = resolve_path_directory(node.to_string)
+        else
+          if node.variable_reference?
+            dependency = node.variable_name.sub(/\A(?:\$|@{1,2})/, "")
+          elsif node.vcall?
+            dependency = node.variable_name
+          elsif node.call?
+            dependency = node.call_method_name
+          else
+            return
+          end
+
+          object_template = true
+          template = "#{dependency.pluralize}/#{dependency.singularize}"
+        end
+
+        return unless template
+
+        if spacer_template = render_template_with_spacer?(options_hash)
+          virtual_path = partial_to_virtual_path(:partial, spacer_template)
+          renders << virtual_path
+        end
+
+        if options_hash.key?(:object) || options_hash.key?(:collection) || object_template
+          return nil if options_hash.key?(:object) && options_hash.key?(:collection)
+          return nil unless options_hash.key?(:partial)
+        end
+
+        virtual_path = partial_to_virtual_path(render_type, template)
+        renders << virtual_path
+
+        # Support for rendering multiple templates (i.e. a partial with a layout)
+        if layout_template = render_template_with_layout?(render_type, options_hash)
+          virtual_path = partial_to_virtual_path(:layout, layout_template)
+
+          renders << virtual_path
+        end
+
+        renders
+      end
+
+      def parse_str(node)
+        node.string? && node.to_string
+      end
+
+      def parse_sym(node)
+        node.symbol? && node.to_symbol
+      end
+
+      private
+        def render_template_with_layout?(render_type, options_hash)
+          if render_type != :layout && options_hash.key?(:layout)
+            parse_str(options_hash[:layout])
+          end
+        end
+
+        def render_template_with_spacer?(options_hash)
+          if options_hash.key?(:spacer_template)
+            parse_str(options_hash[:spacer_template])
+          end
+        end
+
+        def partial_to_virtual_path(render_type, partial_path)
+          if render_type == :partial || render_type == :layout
+            partial_path.gsub(%r{(/|^)([^/]*)\z}, '\1_\2')
+          else
+            partial_path
+          end
+        end
+
+        def layout_to_virtual_path(layout_path)
+          "layouts/#{layout_path}"
+        end
+  end
+end

actionview/lib/action_view/ripper_ast_parser.rb

@@ -0,0 +1,198 @@
+# frozen_string_literal: true
+
+require "ripper"
+
+module ActionView
+  class RenderParser
+    module RipperASTParser # :nodoc:
+      class Node < ::Array # :nodoc:
+        attr_reader :type
+
+        def initialize(type, arr, opts = {})
+          @type = type
+          super(arr)
+        end
+
+        def children
+          to_a
+        end
+
+        def inspect
+          typeinfo = type && type != :list ? ":" + type.to_s + ", " : ""
+          "s(" + typeinfo + map(&:inspect).join(", ") + ")"
+        end
+
+        def fcall?
+          type == :command || type == :fcall
+        end
+
+        def fcall_named?(name)
+          fcall? &&
+            self[0].type == :@ident &&
+            self[0][0] == name
+        end
+
+        def argument_nodes
+          raise unless fcall?
+          return [] if self[1].nil?
+          if self[1].last == false || self[1].last.type == :vcall
+            self[1][0...-1]
+          else
+            self[1][0..-1]
+          end
+        end
+
+        def string?
+          type == :string_literal
+        end
+
+        def variable_reference?
+          type == :var_ref
+        end
+
+        def vcall?
+          type == :vcall
+        end
+
+        def call?
+          type == :call
+        end
+
+        def variable_name
+          self[0][0]
+        end
+
+        def call_method_name
+          self.last.first
+        end
+
+        def to_string
+          raise unless string?
+          self[0][0][0]
+        end
+
+        def hash?
+          type == :bare_assoc_hash || type == :hash
+        end
+
+        def to_hash
+          if type == :bare_assoc_hash
+            hash_from_body(self[0])
+          elsif type == :hash && self[0] == nil
+            {}
+          elsif type == :hash && self[0].type == :assoclist_from_args
+            hash_from_body(self[0][0])
+          end
+        end
+
+        def hash_from_body(body)
+          body.map do |hash_node|
+            return nil if hash_node.type != :assoc_new
+
+            [hash_node[0], hash_node[1]]
+          end.to_h
+        end
+
+        def symbol?
+          type == :@label || type == :symbol_literal
+        end
+
+        def to_symbol
+          if type == :@label && self[0] =~ /\A(.+):\z/
+            $1.to_sym
+          elsif type == :symbol_literal && self[0].type == :symbol && self[0][0].type == :@ident
+            self[0][0][0].to_sym
+          else
+            raise "not a symbol?: #{self.inspect}"
+          end
+        end
+      end
+
+      class NodeParser < ::Ripper # :nodoc:
+        PARSER_EVENTS.each do |event|
+          arity = PARSER_EVENT_TABLE[event]
+          if arity == 0 && event.to_s.end_with?("_new")
+            module_eval(<<-eof, __FILE__, __LINE__ + 1)
+            def on_#{event}(*args)
+              Node.new(:list, args, lineno: lineno(), column: column())
+            end
+            eof
+          elsif event.to_s.match?(/_add(_.+)?\z/)
+            module_eval(<<-eof, __FILE__, __LINE__ + 1)
+            begin; undef on_#{event}; rescue NameError; end
+            def on_#{event}(list, item)
+              list.push(item)
+              list
+            end
+            eof
+          else
+            module_eval(<<-eof, __FILE__, __LINE__ + 1)
+            begin; undef on_#{event}; rescue NameError; end
+            def on_#{event}(*args)
+              Node.new(:#{event}, args, lineno: lineno(), column: column())
+            end
+            eof
+          end
+        end
+
+        SCANNER_EVENTS.each do |event|
+          module_eval(<<-End, __FILE__, __LINE__ + 1)
+          def on_#{event}(tok)
+            Node.new(:@#{event}, [tok], lineno: lineno(), column: column())
+          end
+          End
+        end
+      end
+
+      class RenderCallExtractor < NodeParser # :nodoc:
+        attr_reader :render_calls
+
+        METHODS_TO_PARSE = %w(render render_to_string)
+
+        def initialize(*args)
+          super
+
+          @render_calls = []
+        end
+
+        private
+          def on_fcall(name, *args)
+            on_render_call(super)
+          end
+
+          def on_command(name, *args)
+            on_render_call(super)
+          end
+
+          def on_render_call(node)
+            METHODS_TO_PARSE.each do |method|
+              if node.fcall_named?(method)
+                @render_calls << [method, node]
+                return node
+              end
+            end
+            node
+          end
+
+          def on_arg_paren(content)
+            content
+          end
+
+          def on_paren(content)
+            content
+          end
+      end
+
+      extend self
+
+      def parse_render_nodes(code)
+        parser = RenderCallExtractor.new(code)
+        parser.parse
+
+        parser.render_calls.group_by(&:first).collect do |method, nodes|
+          [ method.to_sym, nodes.collect { |v| v[1] } ]
+        end.to_h
+      end
+    end
+  end
+end

actionview/test/fixtures/digestor/messages/show.html.erb

@@ -6,8 +6,8 @@
 
 <%= render @message.history.events %>
 
-<%# render "something_missing"   %>
+<%= render "something_missing"   %>
-<%# render "something_missing_1" %>
+<%= render "something_missing_1" %>
 
 <%
   # Template Dependency: messages/form

actionview/test/template/dependency_tracker_test.rb

@@ -14,6 +14,13 @@ class FakeTemplate
 
   def initialize(source, handler = Neckbeard)
     @source, @handler = source, handler
+    if handler == :erb
+      @handler = ActionView::Template::Handlers::ERB.new
+    end
+  end
+
+  def type
+    ["text/html"]
   end
 end
 
@@ -48,69 +55,69 @@ class DependencyTrackerTest < ActionView::TestCase
   end
 end
 
-class ERBTrackerTest < Minitest::Test
+class RipperTrackerTest < Minitest::Test
   def make_tracker(name, template)
-    ActionView::DependencyTracker::ERBTracker.new(name, template)
+    ActionView::DependencyTracker::RipperTracker.new(name, template)
   end
 
   def test_dependency_of_erb_template_with_number_in_filename
-    template = FakeTemplate.new("<%# render 'messages/message123' %>", :erb)
+    template = FakeTemplate.new("<%= render 'messages/message123' %>", :erb)
     tracker = make_tracker("messages/_message123", template)
 
     assert_equal ["messages/message123"], tracker.dependencies
   end
 
   def test_dependency_of_template_partial_with_layout
-    template = FakeTemplate.new("<%# render partial: 'messages/show', layout: 'messages/layout' %>", :erb)
+    template = FakeTemplate.new("<%= render partial: 'messages/show', layout: 'messages/layout' %>", :erb)
     tracker = make_tracker("multiple/_dependencies", template)
 
-    assert_equal ["messages/layout", "messages/show"], tracker.dependencies
+    assert_equal ["messages/show", "messages/layout"], tracker.dependencies
   end
 
   def test_dependency_of_template_layout_standalone
-    template = FakeTemplate.new("<%# render layout: 'messages/layout' do %>", :erb)
+    template = FakeTemplate.new("<%= render layout: 'messages/layout' do %>", :erb)
     tracker = make_tracker("messages/layout", template)
 
     assert_equal ["messages/layout"], tracker.dependencies
   end
 
   def test_finds_dependency_in_correct_directory
-    template = FakeTemplate.new("<%# render(message.topic) %>", :erb)
+    template = FakeTemplate.new("<%= render(message.topic) %>", :erb)
     tracker = make_tracker("messages/_message", template)
 
     assert_equal ["topics/topic"], tracker.dependencies
   end
 
   def test_finds_dependency_in_correct_directory_with_underscore
-    template = FakeTemplate.new("<%# render(message_type.messages) %>", :erb)
+    template = FakeTemplate.new("<%= render(message_type.messages) %>", :erb)
     tracker = make_tracker("message_types/_message_type", template)
 
     assert_equal ["messages/message"], tracker.dependencies
   end
 
   def test_dependency_of_erb_template_with_no_spaces_after_render
-    template = FakeTemplate.new("<%# render'messages/message' %>", :erb)
+    template = FakeTemplate.new("<%= render'messages/message' %>", :erb)
     tracker = make_tracker("messages/_message", template)
 
     assert_equal ["messages/message"], tracker.dependencies
   end
 
   def test_finds_no_dependency_when_render_begins_the_name_of_an_identifier
-    template = FakeTemplate.new("<%# rendering 'it useless' %>", :erb)
+    template = FakeTemplate.new("<%= rendering 'it useless' %>", :erb)
     tracker = make_tracker("resources/_resource", template)
 
     assert_equal [], tracker.dependencies
   end
 
   def test_finds_no_dependency_when_render_ends_the_name_of_another_method
-    template = FakeTemplate.new("<%# surrender 'to reason' %>", :erb)
+    template = FakeTemplate.new("<%= surrender 'to reason' %>", :erb)
     tracker = make_tracker("resources/_resource", template)
 
     assert_equal [], tracker.dependencies
   end
 
   def test_finds_dependency_on_multiline_render_calls
-    template = FakeTemplate.new("<%#
+    template = FakeTemplate.new("<%=
       render :object => @all_posts,
              :partial => 'posts' %>", :erb)
 
@@ -121,9 +128,9 @@ class ERBTrackerTest < Minitest::Test
 
   def test_finds_multiple_unrelated_odd_dependencies
     template = FakeTemplate.new("
-      <%# render('shared/header', title: 'Title') %>
+      <%= render('shared/header', title: 'Title') %>
       <h2>Section title</h2>
-      <%# render@section %>
+      <%= render@section %>
     ", :erb)
 
     tracker = make_tracker("multiple/_dependencies", template)
@@ -133,9 +140,9 @@ class ERBTrackerTest < Minitest::Test
 
   def test_finds_dependencies_for_all_kinds_of_identifiers
     template = FakeTemplate.new("
-      <%# render $globals %>
+      <%= render $globals %>
-      <%# render @instance_variables %>
+      <%= render @instance_variables %>
-      <%# render @@class_variables %>
+      <%= render @@class_variables %>
     ", :erb)
 
     tracker = make_tracker("identifiers/_all", template)
@@ -148,14 +155,14 @@ class ERBTrackerTest < Minitest::Test
   end
 
   def test_finds_dependencies_on_method_chains
-    template = FakeTemplate.new("<%# render @parent.child.grandchildren %>", :erb)
+    template = FakeTemplate.new("<%= render @parent.child.grandchildren %>", :erb)
     tracker = make_tracker("method/_chains", template)
 
     assert_equal ["grandchildren/grandchild"], tracker.dependencies
   end
 
   def test_finds_dependencies_with_special_characters
-    template = FakeTemplate.new("<%# render @pokémon, partial: 'ピカチュウ' %>", :erb)
+    template = FakeTemplate.new("<%= render partial: 'ピカチュウ', object: @pokémon %>", :erb)
     tracker = make_tracker("special/_characters", template)
 
     assert_equal ["special/ピカチュウ"], tracker.dependencies
@@ -163,8 +170,8 @@ class ERBTrackerTest < Minitest::Test
 
   def test_finds_dependencies_with_quotes_within
     template = FakeTemplate.new(%{
-      <%# render "single/quote's" %>
+      <%= render "single/quote's" %>
-      <%# render 'double/quote"s' %>
+      <%= render 'double/quote"s' %>
     }, :erb)
 
     tracker = make_tracker("quotes/_single_and_double", template)
@@ -195,11 +202,47 @@ class ERBTrackerTest < Minitest::Test
 
   def test_dependencies_with_interpolation
     template = FakeTemplate.new(%q{
-      <%# render "double/#{quote}" %>
+      <%= render "double/#{quote}" %>
-      <%# render 'single/#{quote}' %>
+      <%= render 'single/#{quote}' %>
     }, :erb)
     tracker = make_tracker("interpolation/_string", template)
 
     assert_equal ["single/\#{quote}"], tracker.dependencies
   end
+
+  def test_dependencies_skip_commented_out_renders
+    template = FakeTemplate.new(%{
+      <%# render "double/#{quote}" %>
+    }, :erb)
+    tracker = make_tracker("interpolation/_string", template)
+
+    assert_equal [], tracker.dependencies
+  end
+
+  def test_dependencies_skip_unknown_options
+    template = FakeTemplate.new(%{
+      <%= render partial: "unknown_render_call", unknown_render_option: "yes" %>
+    }, :erb)
+    tracker = make_tracker("interpolation/_string", template)
+
+    assert_equal [], tracker.dependencies
+  end
+
+  def test_dependencies_finds_spacer_templates
+    template = FakeTemplate.new(%{
+      <%= render partial: "messages/message", collection: books, spacer_template: "messages/message_spacer" %>
+    }, :erb)
+    tracker = make_tracker("messages/show", template)
+
+    assert_equal ["messages/message_spacer", "messages/message"], tracker.dependencies
+  end
+
+  def test_dependencies_skip_commented_out_renders
+    template = FakeTemplate.new(%{
+      <%# render "messages/legacy_message" %>
+    }, :erb)
+    tracker = make_tracker("messages/show", template)
+
+    assert_equal [], tracker.dependencies
+  end
 end