Skip to content

Commit

Permalink
Implement spot and don't use keep_script_lines in Ruby 3.2
Browse files Browse the repository at this point in the history
We want to use error highlight with eval'd code, specifically ERB
templates.

Previously we could only get the information we needed by setting
`keep_script_lines` to true. In Ruby 3.2 and error_highlight we added
the ability to get this information without setting `keep_script_lines`.

This change implements that new behavior for Rails.

I removed the script line changes to support this in 3.1 because it is
not in any released version.

Ruby change: ruby/ruby#6593
Erorr highlight change: ruby/error_highlight#26

Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
  • Loading branch information
eileencodes and tenderlove committed Jan 13, 2023
1 parent 717d53e commit d9dd1c5
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 28 deletions.
Expand Up @@ -228,7 +228,12 @@ def initialize(location, template)
end

def spot(exc)
location = super
if RubyVM::AbstractSyntaxTree.respond_to?(:node_id_for_backtrace_location)
location = @template.spot(__getobj__)
else
location = super
end

if location
@template.translate_location(__getobj__, location)
end
Expand Down
21 changes: 20 additions & 1 deletion actionview/lib/action_view/template.rb
Expand Up @@ -154,11 +154,19 @@ def locals
end
end

def spot(location) # :nodoc:
ast = RubyVM::AbstractSyntaxTree.parse(compiled_source, keep_script_lines: true)
node_id = RubyVM::AbstractSyntaxTree.node_id_for_backtrace_location(location)
node = find_node_by_id(ast, node_id)

ErrorHighlight.spot(node)
end

# Translate an error location returned by ErrorHighlight to the correct
# source location inside the template.
def translate_location(backtrace_location, spot)
if handler.respond_to?(:translate_location)
handler.translate_location(spot, backtrace_location, source)
handler.translate_location(spot, backtrace_location, encode!)
else
spot
end
Expand Down Expand Up @@ -303,6 +311,17 @@ def method_name # :nodoc:
end

private
def find_node_by_id(node, node_id)
return node if node.node_id == node_id

node.children.grep(node.class).each do |child|
found = find_node_by_id(child, node_id)
return found if found
end

false
end

# Compile a template. This method ensures a template is compiled
# just once and removes the source after it is compiled.
def compile!(view)
Expand Down
34 changes: 12 additions & 22 deletions actionview/test/template/render_test.rb
Expand Up @@ -208,30 +208,20 @@ def test_render_outside_path
end
end

# rubocop:disable Minitest/SkipEnsure
def test_render_runtime_error
skip unless RubyVM.respond_to?(:keep_script_lines)

# We need to enable this setting so that when we eval the template
# the compiled source is kept around.
setting = RubyVM.keep_script_lines
RubyVM.keep_script_lines = true

ex = assert_raises(ActionView::Template::Error) {
@view.render(template: "test/runtime_error")
}
erb_btl = ex.backtrace_locations.first

# Get the spot information from ErrorHighlight
spot = erb_btl.spot(ex.cause)
translated_spot = ex.template.translate_location(erb_btl, spot)
assert_equal 6, translated_spot[:first_column]
ensure
if RubyVM.respond_to?(:keep_script_lines)
RubyVM.keep_script_lines = setting
if RUBY_VERSION >= "3.2"
def test_render_runtime_error
ex = assert_raises(ActionView::Template::Error) {
@view.render(template: "test/runtime_error")
}
erb_btl = ex.backtrace_locations.first

# Get the spot information from ErrorHighlight
translating_frame = ActionDispatch::ExceptionWrapper::SourceMapLocation.new(erb_btl, ex.template)
translated_spot = translating_frame.spot(ex.cause)

assert_equal 6, translated_spot[:first_column]
end
end
# rubocop:enable Minitest/SkipEnsure

def test_render_partial
assert_equal "only partial", @view.render(partial: "test/partial_only")
Expand Down
4 changes: 0 additions & 4 deletions railties/lib/rails/application/configuration.rb
Expand Up @@ -34,10 +34,6 @@ def initialize(*)
@filter_redirect = []
@helpers_paths = []
if Rails.env.development?
if defined?(RubyVM) && RubyVM.respond_to?(:keep_script_lines=)
RubyVM.keep_script_lines = true
end

@hosts = ActionDispatch::HostAuthorization::ALLOWED_HOSTS_IN_DEVELOPMENT +
ENV["RAILS_DEVELOPMENT_HOSTS"].to_s.split(",").map(&:strip)
else
Expand Down

0 comments on commit d9dd1c5

Please sign in to comment.