Use error_highlight gem to locate the columns where an error was raised

This change incorporates to Rails a feature called error_highlight that
has been available since Ruby 3.1. This allow Rails' error report screen
to display the fine-grained location where an error occurred (not only a
line number but also beginning and end column numbers of the code
fragment).

For ErrorHighlight, see https://bugs.ruby-lang.org/issues/17930 in
detail.

To use error_highlight, ExceptionWrapper now prefers
`Exception#backtrace_locations` (since Ruby 2.1), which returns an array
of `Thread::Backtrace::Location`s, instead of `Exception#backtrace`.
This is because error_highlight requires `Thread::Backtrace::Location`
to locate the column where an error was raised.

Co-Authored-By: John Hawthorn <john@hawthorn.email>
Co-Authored-By: Jean Boussier <jean.boussier@gmail.com>

Gemfile

@@ -176,3 +176,10 @@ end
 
 gem "tzinfo-data", platforms: [:mingw, :mswin, :x64_mingw, :jruby]
 gem "wdm", ">= 0.1.0", platforms: [:mingw, :mswin, :x64_mingw, :mswin64]
+
+# The error_highlight gem only works on CRuby 3.1 or later.
+# Also, Rails depends on a new API available since error_highlight 0.4.0.
+# (Note that Ruby 3.1 bundles error_highlight 0.3.0.)
+if RUBY_VERSION >= "3.1"
+  gem "error_highlight", ">= 0.4.0", platforms: [:ruby]
+end

Gemfile.lock

@@ -205,6 +205,7 @@ GEM
       http_parser.rb (>= 0.6.0)
     em-socksify (0.3.2)
       eventmachine (>= 1.0.0.beta.4)
+    error_highlight (0.4.0)
     erubi (1.11.0)
     et-orbi (1.2.6)
       tzinfo
@@ -572,6 +573,7 @@ DEPENDENCIES
   debug (>= 1.1.0)
   delayed_job
   delayed_job_active_record
+  error_highlight (>= 0.4.0)
   google-cloud-storage (~> 1.11)
   image_processing (~> 1.2)
   importmap-rails

actionpack/lib/action_dispatch/middleware/debug_exceptions.rb

@@ -121,6 +121,7 @@ module ActionDispatch
           trace_to_show: wrapper.trace_to_show,
           routes_inspector: routes_inspector(wrapper.exception),
           source_extracts: wrapper.source_extracts,
+          error_highlight_available: wrapper.error_highlight_available?,
           line_number: wrapper.line_number,
           file: wrapper.file
         )

actionpack/lib/action_dispatch/middleware/exception_wrapper.rb

@@ -124,15 +124,18 @@ module ActionDispatch
 
     def source_extracts
       backtrace.map do |trace|
-        file, line_number = extract_file_and_line_number(trace)
-
-        {
-          code: source_fragment(file, line_number),
-          line_number: line_number
-        }
+        extract_source(trace)
       end
     end
 
+    def error_highlight_available?
+      # ErrorHighlight.spot with backtrace_location keyword is available since error_highlight 0.4.0
+      unless defined?(@@error_highlight_available)
+        @@error_highlight_available = defined?(ErrorHighlight) && Gem::Version.new(ErrorHighlight::VERSION) >= Gem::Version.new("0.4.0")
+      end
+      @@error_highlight_available
+    end
+
     def trace_to_show
       if traces["Application Trace"].empty? && rescue_template != "routing_error"
         "Full Trace"
@@ -147,7 +150,17 @@ module ActionDispatch
 
     private
       def backtrace
-        Array(@exception.backtrace)
+        backtrace_locations = @exception.backtrace_locations
+        backtrace = @exception.backtrace
+
+        if backtrace_locations && backtrace_locations.size == backtrace.size
+          # Prefer #backtrace_locations as it looks consistent with #backtrace
+          Array(backtrace_locations)
+        else
+          # Conservatively fallback to #backtrace as they are inconsistent;
+          # probably #set_backtrace is used somewhere?
+          Array(backtrace)
+        end
       end
 
       def causes_for(exception)
@@ -168,19 +181,55 @@ module ActionDispatch
         end
       end
 
+      def extract_source(trace)
+        if error_highlight_available?
+          spot = ErrorHighlight.spot(@exception, backtrace_location: trace)
+          if spot
+            line = spot[:first_lineno]
+            code = extract_source_fragment_lines(spot[:script_lines], line)
+
+            if line == spot[:last_lineno]
+              code[line] = [
+                code[line][0, spot[:first_column]],
+                code[line][spot[:first_column]...spot[:last_column]],
+                code[line][spot[:last_column]..-1],
+              ]
+            end
+
+            return {
+              code: code,
+              line_number: line
+            }
+          end
+        end
+
+        file, line_number = extract_file_and_line_number(trace)
+
+        {
+          code: source_fragment(file, line_number),
+          line_number: line_number
+        }
+      end
+
+      def extract_source_fragment_lines(source_lines, line)
+        start = [line - 3, 0].max
+        lines = source_lines.drop(start).take(6)
+        Hash[*(start + 1..(lines.count + start)).zip(lines).flatten]
+      end
+
       def source_fragment(path, line)
         return unless Rails.respond_to?(:root) && Rails.root
         full_path = Rails.root.join(path)
         if File.exist?(full_path)
           File.open(full_path, "r") do |file|
-            start = [line - 3, 0].max
-            lines = file.each_line.drop(start).take(6)
-            Hash[*(start + 1..(lines.count + start)).zip(lines).flatten]
+            extract_source_fragment_lines(file.each_line, line)
           end
         end
       end
 
       def extract_file_and_line_number(trace)
+        return [trace.path, trace.lineno] if Thread::Backtrace::Location === trace
+
         # Split by the first colon followed by some digits, which works for both
         # Windows and Unix path styles.
         file, line = trace.match(/^(.+?):(\d+).*$/, &:captures) || trace

actionpack/lib/action_dispatch/middleware/templates/rescues/_source.html.erb

@@ -18,12 +18,19 @@
             </td>
 <td width="100%">
 <pre>
-<% source_extract[:code].each do |line, source| -%><div class="line<%= " active" if line == source_extract[:line_number] -%>"><%= source -%></div><% end -%>
+<% source_extract[:code].each do |line, source| -%>
+<div class="line<%= " active" if line == source_extract[:line_number] -%>"><% if source.is_a?(Array) -%><%= source[0] -%><span class="error_highlight"><%= source[1] -%></span><%= source[2] -%>
+<% else -%>
+<%= source -%>
+<% end -%></div><% end -%>
 </pre>
 </td>
           </tr>
         </table>
       </div>
+      <%- if defined?(ErrorHighlight) && !error_highlight_available -%>
+        <p class="error_highlight_tip">Tip: You may want to add <code>gem 'error_highlight', '&gt;= 0.4.0'</code> into your Gemfile, which will display the fine-grained error location.</p>
+      <%- end -%>
     </div>
   <% end %>
 <% end %>

actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb

@@ -11,7 +11,7 @@
   <%= render "rescues/message_and_suggestions", exception: @exception %>
   <%= render "rescues/actions", exception: @exception, request: @request %>
 
-  <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx, error_index: 0 %>
+  <%= render "rescues/source", source_extracts: @source_extracts, error_highlight_available: @error_highlight_available, show_source_idx: @show_source_idx, error_index: 0 %>
   <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show, error_index: 0 %>
 
   <% if @exception.cause %>
@@ -26,7 +26,7 @@
     </div>
 
     <div id="<%= wrapper.exception.object_id %>" class="hidden">
-      <%= render "rescues/source", source_extracts: wrapper.source_extracts, show_source_idx: wrapper.source_to_show_id, error_index: index %>
+      <%= render "rescues/source", source_extracts: wrapper.source_extracts, error_highlight_available: wrapper.error_highlight_available?, show_source_idx: wrapper.source_to_show_id, error_index: index %>
       <%= render "rescues/trace", traces: wrapper.traces, trace_to_show: wrapper.trace_to_show, error_index: index %>
     </div>
   <% end %>

actionpack/lib/action_dispatch/middleware/templates/rescues/invalid_statement.html.erb

@@ -18,7 +18,7 @@
     <% end %>
   </h2>
 
-  <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx %>
+  <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx, error_highlight_available: nil %>
   <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show %>
   <%= render template: "rescues/_request_and_response" %>
 </main>

actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb

@@ -148,6 +148,18 @@
       background-color: #FCC;
     }
 
+    .error_highlight {
+      display: inline-block;
+      background-color: #FF9;
+      text-decoration: #F00 wavy underline;
+    }
+
+    .error_highlight_tip {
+      color: #666;
+      padding: 2px 2px;
+      font-size: 10px;
+    }
+
     .button_to {
       display: inline-block;
       margin-top: 0.75em;

actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb

@@ -5,7 +5,7 @@
 <main role="main" id="container">
   <h2><%= h @exception.message %></h2>
 
-  <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx %>
+  <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx, error_highlight_available: nil %>
   <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show %>
   <%= render template: "rescues/_request_and_response" %>
 </main>

actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb

@@ -11,7 +11,7 @@
   </p>
   <pre><code><%= h @exception.message %></code></pre>
 
-  <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx %>
+  <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx, error_highlight_available: nil %>
 
   <p><%= @exception.sub_template_message %></p>
 

actionpack/test/dispatch/exception_wrapper_test.rb

@@ -53,6 +53,25 @@ module ActionDispatch
       end
     end
 
+    if defined?(ErrorHighlight) && Gem::Version.new(ErrorHighlight::VERSION) >= Gem::Version.new("0.4.0")
+      test "#source_extracts works with error_highlight" do
+        lineno = __LINE__
+        begin
+          1.time
+        rescue NameError => exc
+        end
+
+        wrapper = ExceptionWrapper.new(nil, exc)
+
+        code = {}
+        File.foreach(__FILE__).to_a.drop(lineno - 1).take(6).each_with_index do |line, i|
+          code[lineno + i] = line
+        end
+        code[lineno + 2] = ["          1", ".time", "\n"]
+        assert_equal({ code: code, line_number: lineno + 2 }, wrapper.source_extracts.first)
+      end
+    end
+
     test "#application_trace returns traces only from the application" do
       exception = TestError.new(caller.prepend("lib/file.rb:42:in `index'"))
       wrapper = ExceptionWrapper.new(@cleaner, exception)

activesupport/lib/active_support/backtrace_cleaner.rb

@@ -106,7 +106,7 @@ module ActiveSupport
 
       def filter_backtrace(backtrace)
         @filters.each do |f|
-          backtrace = backtrace.map { |line| f.call(line) }
+          backtrace = backtrace.map { |line| f.call(line.to_s) }
         end
 
         backtrace
@@ -114,7 +114,7 @@ module ActiveSupport
 
       def silence(backtrace)
         @silencers.each do |s|
-          backtrace = backtrace.reject { |line| s.call(line) }
+          backtrace = backtrace.reject { |line| s.call(line.to_s) }
         end
 
         backtrace
@@ -123,7 +123,7 @@ module ActiveSupport
       def noise(backtrace)
         backtrace.select do |line|
           @silencers.any? do |s|
-            s.call(line)
+            s.call(line.to_s)
           end
         end
       end

railties/lib/rails/generators/rails/app/templates/Gemfile.tt

@@ -56,6 +56,10 @@ group :development do
 <%- end -%>
   # Speed up commands on slow machines / big apps [https://github.com/rails/spring]
   # gem "spring"
+
+<%- if RUBY_VERSION >= "3.1" -%>
+  gem "error_highlight", ">= 0.4.0", platforms: [:ruby]
+<%- end -%>
 end
 
 <%- if depends_on_system_test? -%>