Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Handle syntax error in Ripper #2104

Merged
merged 1 commit into from
Oct 31, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/rspec/core/formatters/snippet_extractor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def expression_lines
end

source.lines[(line_range.begin - 1)..(line_range.end - 1)]
rescue NoExpressionAtLineError
rescue SyntaxError, NoExpressionAtLineError
[self.class.extract_line_at(source.path, beginning_line_number)]
end

Expand Down
1 change: 1 addition & 0 deletions lib/rspec/core/source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def ast
@ast ||= begin
require 'ripper'
sexp = Ripper.sexp(source)
raise SyntaxError unless sexp
Node.new(sexp)
end
end
Expand Down
22 changes: 22 additions & 0 deletions spec/rspec/core/formatters/snippet_extractor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,28 @@ def another_expression(*)
end
end

context 'when Ripper cannot parse the source (which can happen on JRuby -- see jruby/jruby#2427)', :isolated_directory do
let(:file_path) { 'invalid_source.rb' }

let(:line_number) { 1 }

let(:source) { <<-EOS.gsub(/^ +\|/, '') }
|expect("some string").to include(
| "some", "string"
|]
EOS

before do
File.open(file_path, 'w') { |file| file.write(source) }
end

it 'returns the line by falling back to the simple single line extraction' do
expect(expression_lines).to eq([
'expect("some string").to include('
])
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This spec tests a situation that, as far as I can tell, can't happen in real life: a spec file with invalid syntax could never get run by RSpec and therefore could never be the source of a failure. I understand the need to write this kind of test for the JRuby thing but nothing in the test mentions the JRuby bug, so future me will be confused about why this behavior is necessary. WDYT about adding an explanatory note to the context?

context 'when Ripper cannot parse the source (which can happen on JRuby -- see jruby/jruby#2427)', :isolated_directory do
  # ...
end

This makes the need for the test much more explicit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


context 'when max line count is given' do
let(:max_line_count) do
2
Expand Down