Extract multiline failure expression by parsing Ruby source

Author: yujinakayama

lib/rspec/core/formatters/exception_presenter.rb

@@ -1,4 +1,6 @@
 # encoding: utf-8
+RSpec::Support.require_rspec_support "encoded_string"
+
 module RSpec
   module Core
     module Formatters
@@ -138,11 +140,19 @@ def exception_class_name(exception=@exception)
         end
 
         def failure_lines
-          @failure_lines ||= [failure_slash_error_line] + exception_lines + extra_failure_lines
+          @failure_lines ||= failure_slash_error_lines + exception_lines + extra_failure_lines
         end
 
-        def failure_slash_error_line
-          "Failure/Error: #{read_failed_line.strip}"
+        def failure_slash_error_lines
+          lines = read_failed_lines
+          if lines.count == 1
+            lines[0] = "Failure/Error: #{lines[0].strip}"
+          else
+            least_indentation = lines.map { |line| line[/^[ \t]*/] }.min
+            lines = lines.map { |line| line.sub(/^#{least_indentation}/, '  ') }
+            lines.unshift('Failure/Error:')
+          end
+          lines
         end
 
         def exception_lines
@@ -175,22 +185,21 @@ def add_shared_group_lines(lines, colorizer)
           lines
         end
 
-        def read_failed_line
+        def read_failed_lines
           matching_line = find_failed_line
           unless matching_line
-            return "Unable to find matching line from backtrace"
+            return ["Unable to find matching line from backtrace"]
           end
 
           file_path, line_number = matching_line.match(/(.+?):(\d+)(|:\d+)/)[1..2]
-
-          if File.exist?(file_path)
-            File.readlines(file_path)[line_number.to_i - 1] ||
-              "Unable to find matching line in #{file_path}"
-          else
-            "Unable to find #{file_path} to read failed line"
-          end
+          RSpec::Support.require_rspec_core "formatters/snippet_extractor"
+          SnippetExtractor.extract_expression_lines_at(file_path, line_number.to_i)
+        rescue SnippetExtractor::NoSuchFileError
+          ["Unable to find #{file_path} to read failed line"]
+        rescue SnippetExtractor::NoSuchLineError
+          ["Unable to find matching line in #{file_path}"]
         rescue SecurityError
-          "Unable to read failed line"
+          ["Unable to read failed line"]
         end
 
         def find_failed_line

lib/rspec/core/formatters/snippet_extractor.rb

@@ -0,0 +1,129 @@
+RSpec::Support.require_rspec_core "source"
+
+module RSpec
+  module Core
+    module Formatters
+      # @private
+      class SnippetExtractor
+        NoSuchFileError = Class.new(StandardError)
+        NoSuchLineError = Class.new(StandardError)
+
+        def self.extract_line_at(file_path, line_number)
+          source = source_from_file(file_path)
+          line = source.lines[line_number - 1]
+          raise NoSuchLineError unless line
+          line
+        end
+
+        def self.source_from_file(path)
+          raise NoSuchFileError unless File.exist?(path)
+          RSpec.world.source_cache.source_from_file(path)
+        end
+
+        if RSpec::Support::RubyFeatures.ripper_supported?
+          NoExpressionAtLineError = Class.new(StandardError)
+
+          PAREN_TOKEN_TYPE_PAIRS = {
+            :on_lbracket    => :on_rbracket,
+            :on_lparen      => :on_rparen,
+            :on_lbrace      => :on_rbrace,
+            :on_heredoc_beg => :on_heredoc_end
+          }
+
+          attr_reader :source, :beginning_line_number
+
+          def self.extract_expression_lines_at(file_path, beginning_line_number)
+            source = source_from_file(file_path)
+            new(source, beginning_line_number).expression_lines
+          end
+
+          def initialize(source, beginning_line_number)
+            @source = source
+            @beginning_line_number = beginning_line_number
+          end
+
+          def expression_lines
+            line_range = line_range_of_expression
+            source.lines[(line_range.begin - 1)..(line_range.end - 1)]
+          rescue NoExpressionAtLineError
+            [self.class.extract_line_at(source.path, beginning_line_number)]
+          end
+
+          private
+
+          def line_range_of_expression
+            @line_range_of_expression ||= begin
+              line_range = line_range_of_location_nodes_in_expression
+              initial_unclosed_parens = unclosed_paren_tokens_in_line_range(line_range)
+              unclosed_parens = initial_unclosed_parens
+
+              until (initial_unclosed_parens & unclosed_parens).empty?
+                line_range = (line_range.begin)..(line_range.end + 1)
+                unclosed_parens = unclosed_paren_tokens_in_line_range(line_range)
+              end
+
+              line_range
+            end
+          end
+
+          def unclosed_paren_tokens_in_line_range(line_range)
+            tokens = FlatMap.flat_map(line_range) do |line_number|
+              source.tokens_by_line_number[line_number]
+            end
+
+            tokens.each_with_object([]) do |token, unclosed_tokens|
+              if PAREN_TOKEN_TYPE_PAIRS.keys.include?(token.type)
+                unclosed_tokens << token
+              else
+                index = unclosed_tokens.rindex do |unclosed_token|
+                  PAREN_TOKEN_TYPE_PAIRS[unclosed_token.type] == token.type
+                end
+                unclosed_tokens.delete_at(index) if index
+              end
+            end
+          end
+
+          def line_range_of_location_nodes_in_expression
+            line_numbers = expression_node.each_with_object(Set.new) do |node, set|
+              set << node.location.line if node.location
+            end
+
+            line_numbers.min..line_numbers.max
+          end
+
+          def expression_node
+            raise NoExpressionAtLineError if location_nodes_at_beginning_line.empty?
+
+            @expression_node ||= begin
+              common_ancestor_nodes = location_nodes_at_beginning_line.map do |node|
+                node.each_ancestor.to_a
+              end.reduce(:&)
+
+              common_ancestor_nodes.find { |node| expression_outmost_node?(node) }
+            end
+          end
+
+          def expression_outmost_node?(node)
+            return true unless node.parent
+            return false if node.type.to_s.start_with?('@')
+            ![node, node.parent].all? do |n|
+              # See `Ripper::PARSER_EVENTS` for the complete list of sexp types.
+              type = n.type.to_s
+              type.end_with?('call') || type.start_with?('method_add_')
+            end
+          end
+
+          def location_nodes_at_beginning_line
+            source.nodes_by_line_number[beginning_line_number]
+          end
+        else
+          # :nocov:
+          def self.extract_expression_lines_at(file_path, beginning_line_number)
+            [extract_line_at(file_path, beginning_line_number)]
+          end
+          # :nocov:
+        end
+      end
+    end
+  end
+end

lib/rspec/core/notifications.rb

@@ -1,7 +1,6 @@
 RSpec::Support.require_rspec_core "formatters/exception_presenter"
 RSpec::Support.require_rspec_core "formatters/helpers"
 RSpec::Support.require_rspec_core "shell_escape"
-RSpec::Support.require_rspec_support "encoded_string"
 
 module RSpec::Core
   # Notifications are value objects passed to formatters to provide them

lib/rspec/core/source.rb

@@ -55,6 +55,17 @@ def tokens_by_line_number
       def inspect
         "#<#{self.class} #{path}>"
       end
+
+      # @private
+      class Cache
+        def initialize
+          @sources_by_path = {}
+        end
+
+        def source_from_file(path)
+          @sources_by_path[path] ||= Source.from_file(path)
+        end
+      end
     end
   end
 end

lib/rspec/core/world.rb

@@ -107,6 +107,14 @@ def reporter
         @configuration.reporter
       end
 
+      # @private
+      def source_cache
+        @source_cache ||= begin
+          RSpec::Support.require_rspec_core "source"
+          Source::Cache.new
+        end
+      end
+
       # @api private
       #
       # Notify reporter of filters.

spec/rspec/core/formatters/exception_presenter_spec.rb

@@ -62,7 +62,7 @@ module RSpec::Core
         EOS
       end
 
-      it 'passes the indentation on to the `:detail_formatter` lambda so it can align things' do
+      it 'aligns lines' do
         detail_formatter = Proc.new { "Some Detail" }
 
         the_presenter = Formatters::ExceptionPresenter.new(exception, example, :indentation => 4,
@@ -169,9 +169,27 @@ module RSpec::Core
         EOS
       end
     end
-    describe "#read_failed_line" do
-      def read_failed_line
-        presenter.send(:read_failed_line)
+    describe "#read_failed_lines" do
+      def read_failed_lines
+        presenter.send(:read_failed_lines)
+      end
+
+      context 'when the failed expression spans multiple lines', :if => RSpec::Support::RubyFeatures.ripper_supported? do
+        let(:exception) do
+          begin
+            expect('RSpec').to start_with('R').
+                           and end_with('z')
+          rescue RSpec::Expectations::ExpectationNotMetError => exception
+            exception
+          end
+        end
+
+        it 'returns all the lines' do
+          expect(read_failed_lines).to eq([
+            "            expect('RSpec').to start_with('R').",
+            "                           and end_with('z')"
+          ])
+        end
       end
 
       context "when backtrace is a heterogeneous language stack trace" do
@@ -185,16 +203,21 @@ def read_failed_line
         end
 
         it "is handled gracefully" do
-          expect { read_failed_line }.not_to raise_error
+          expect { read_failed_lines }.not_to raise_error
         end
       end
 
       context "when backtrace will generate a security error" do
         let(:exception) { instance_double(Exception, :backtrace => [ "#{__FILE__}:#{__LINE__}"]) }
 
+        before do
+          # We lazily require SnippetExtractor but requiring it in $SAFE 3 environment raises error.
+          RSpec::Support.require_rspec_core "formatters/snippet_extractor"
+        end
+
         it "is handled gracefully" do
           with_safe_set_to_level_that_triggers_security_errors do
-            expect { read_failed_line }.not_to raise_error
+            expect { read_failed_lines }.not_to raise_error
           end
         end
       end
@@ -203,7 +226,7 @@ def read_failed_line
         let(:exception) { instance_double(Exception, :backtrace => [ "#{__FILE__}:10000000"]) }
 
         it "reports the filename and that it was unable to find the matching line" do
-          expect(read_failed_line).to include("Unable to find matching line")
+          expect(read_failed_lines.first).to include("Unable to find matching line")
         end
       end
 
@@ -213,7 +236,7 @@ def read_failed_line
 
         it "reports the filename and that it was unable to find the matching line" do
           example.metadata[:absolute_file_path] = file
-          expect(read_failed_line).to include("Unable to find #{file} to read failed line")
+          expect(read_failed_lines.first).to include("Unable to find #{file} to read failed line")
         end
       end
 
@@ -223,7 +246,7 @@ def read_failed_line
         let(:exception) { instance_double(Exception, :backtrace => ["#{relative_file}:#{line}"]) }
 
         it 'still finds the backtrace line' do
-          expect(read_failed_line).to include("line = __LINE__")
+          expect(read_failed_lines.first).to include("line = __LINE__")
         end
       end
 
@@ -243,7 +266,7 @@ def read_failed_line
         let(:exception) { instance_double(Exception, :backtrace => [ "#{__FILE__}:#{__LINE__}"]) }
 
         it "doesn't hang when file exists" do
-          expect(read_failed_line.strip).to eql(
+          expect(read_failed_lines.first.strip).to eql(
             %Q[let(:exception) { instance_double(Exception, :backtrace => [ "\#{__FILE__}:\#{__LINE__}"]) }])
         end
       end