Merge pull request #38950 from joelhawksley/annotations-error-line

Raise template errors on correct line with annotations enabled

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

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
@@ -55,16 +64,24 @@ def call(template, source)
           # Always make sure we return a String in the default_internal
           erb.encode!
 
-          self.class.erb_implementation.new(
-            erb,
+          options = {
             escape: (self.class.escape_ignore_list.include? template.type),
-            trim: (self.class.erb_trim_mode == "-"),
-            format: template.format,
-            short_identifier: template.short_identifier
-          ).src
+            trim: (self.class.erb_trim_mode == "-")
+          }
+
+          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
+
+          self.class.erb_implementation.new(erb, options).src
         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

actionview/lib/action_view/template/handlers/erb/erubi.rb

@@ -14,14 +14,8 @@ def initialize(input, properties = {})
             # Dup properties so that we don't modify argument
             properties = Hash[properties]
 
-            # Annotate output with template file names, if we're rendering HTML
-            if ActionView::Base.annotate_template_file_names && properties[:format] == :html
-              properties[:preamble]   = "@output_buffer.safe_append='<!-- BEGIN #{properties[:short_identifier]} -->\n';"
-              properties[:postamble]  = "@output_buffer.safe_append='<!-- END #{properties[:short_identifier]} -->\n';@output_buffer.to_s"
-            else
-              properties[:preamble]   = ""
-              properties[:postamble]  = "@output_buffer.to_s"
-            end
+            properties[:preamble]   ||= ""
+            properties[:postamble]  ||= "@output_buffer.to_s"
 
             properties[:bufvar]     = "@output_buffer"
             properties[:escapefunc] = ""

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