Raise errors on correct line with annotations enabled

joelhawksley · tenderlove · joelhawksley · commit ab8ea48111a5 · 2020-04-14T15:26:26.000-06:00
Co-authored-by: Aaron Patterson &lt;tenderlove@github.com&gt;
diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb
@@ -334,8 +334,10 @@ def #{method_name}(local_assigns, output_buffer)
           raise WrongEncodingError.new(source, Encoding.default_internal)
         end
 
+        start_line = @handler.respond_to?(:start_line) ? @handler.start_line(self) : 0
+
         begin
-          mod.module_eval(source, identifier, 0)
+          mod.module_eval(source, identifier, start_line)
         rescue SyntaxError
           # Account for when code in the template is not syntactically valid; e.g. if we're using
           # ERB and the user writes <%= foo( %>, attempting to call a helper `foo` and interpolate
diff --git a/actionview/lib/action_view/template/handlers/erb.rb b/actionview/lib/action_view/template/handlers/erb.rb
@@ -40,6 +40,15 @@ def handles_encoding?
           true
         end
 
+        # Line number to pass to #module_eval
+        #
+        # If we're annotating the template, we need to offset the starting
+        # line number passed to #module_eval so that errors in the template
+        # will be raised on the correct line.
+        def start_line(template)
+          annotate?(template) ? -1 : 0
+        end
+
         def call(template, source)
           # First, convert to BINARY, so in case the encoding is
           # wrong, we can still find an encoding tag
@@ -60,8 +69,7 @@ def call(template, source)
             trim: (self.class.erb_trim_mode == "-")
           }
 
-          # Annotate output with template file names, if we're rendering HTML
-          if ActionView::Base.annotate_template_file_names && template.format == :html
+          if annotate?(template)
             options[:preamble] = "@output_buffer.safe_append='<!-- BEGIN #{template.short_identifier} -->\n';"
             options[:postamble] = "@output_buffer.safe_append='<!-- END #{template.short_identifier} -->\n';@output_buffer.to_s"
           end
@@ -70,6 +78,10 @@ def call(template, source)
         end
 
       private
+        def annotate?(template)
+          ActionView::Base.annotate_template_file_names && template.format == :html
+        end
+
         def valid_encoding(string, encoding)
           # If a magic encoding comment was found, tag the
           # String with this encoding. This is for a case
diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb
@@ -1482,4 +1482,18 @@ def test_template_annotations_do_not_render_for_non_html_format
   ensure
     ActionView::Base.annotate_template_file_names = false
   end
+
+  def test_line_offset_with_annotations_enabled
+    ActionView::Base.annotate_template_file_names = true
+
+    exc = assert_raises ActionView::Template::Error do
+      get :render_line_offset
+    end
+    line = exc.backtrace.first
+    assert(line =~ %r{:(\d+):})
+    assert_equal "1", $1,
+      "The line offset is wrong, perhaps the wrong exception has been raised, exception was: #{exc.inspect}"
+  ensure
+    ActionView::Base.annotate_template_file_names = false
+  end
 end
