Skip to content

Commit

Permalink
Handle comments and blank interpolations in regexp parsed_tree (#8803)
Browse files Browse the repository at this point in the history
* Handle comments and blank interpolations in regexp parsed_tree
  • Loading branch information
owst committed Sep 30, 2020
1 parent 5302e52 commit 751edc7
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@

### Changes

* [#8803](https://github.com/rubocop-hq/rubocop/pull/8803): **(Breaking)** `RegexpNode#parsed_tree` now processes regexps including interpolation (by blanking the interpolation before parsing, rather than skipping). ([@owst][])
* [#8646](https://github.com/rubocop-hq/rubocop/issues/8646): Faster find of all files in `TargetFinder` class which improves initial startup speed. ([@tleish][])
* [#8102](https://github.com/rubocop-hq/rubocop/issues/8102): Consider class length instead of block length for `Struct.new`. ([@tejasbubane][])

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/lint/mixed_regexp_capture_types.rb
Expand Up @@ -25,6 +25,7 @@ class MixedRegexpCaptureTypes < Base
'in a Regexp literal.'

def on_regexp(node)
return if node.interpolation?
return if node.each_capture(named: false).none?
return if node.each_capture(named: true).none?

Expand Down
24 changes: 20 additions & 4 deletions lib/rubocop/ext/regexp_node.rb
Expand Up @@ -17,11 +17,9 @@ class << self

# @return [Regexp::Expression::Root, nil]
def parsed_tree
return if interpolation?

str = content
str = with_interpolations_blanked
Ext::RegexpNode.parsed_cache[str] ||= begin
Regexp::Parser.parse(str)
Regexp::Parser.parse(str, options: options)
rescue StandardError
nil
end
Expand All @@ -40,6 +38,24 @@ def each_capture(named: ANY)
self
end

private

def with_interpolations_blanked
# Ignore the trailing regopt node
children[0...-1].map do |child|
source = child.source

# We don't want to consider the contents of interpolations as part of the pattern source,
# but need to preserve their width, to allow offsets to correctly line up with the
# original source: spaces have no effect, and preserve width.
if child.begin_type?
' ' * source.length
else
source
end
end.join
end

AST::RegexpNode.include self
end
end
Expand Down
4 changes: 2 additions & 2 deletions rubocop.gemspec
Expand Up @@ -36,9 +36,9 @@ Gem::Specification.new do |s|
s.add_runtime_dependency('parallel', '~> 1.10')
s.add_runtime_dependency('parser', '>= 2.7.1.5')
s.add_runtime_dependency('rainbow', '>= 2.2.2', '< 4.0')
s.add_runtime_dependency('regexp_parser', '>= 1.7')
s.add_runtime_dependency('regexp_parser', '>= 1.8')
s.add_runtime_dependency('rexml')
s.add_runtime_dependency('rubocop-ast', '>= 0.5.0')
s.add_runtime_dependency('rubocop-ast', '>= 0.6.0')
s.add_runtime_dependency('ruby-progressbar', '~> 1.7')
s.add_runtime_dependency('unicode-display_width', '>= 1.4.0', '< 2.0')

Expand Down
65 changes: 65 additions & 0 deletions spec/rubocop/ext/regexp_node_spec.rb
Expand Up @@ -32,4 +32,69 @@
it { is_expected.to match [named] }
end
end

describe '#parsed_tree' do
let(:source) { '/foo#{bar}baz/' }

context 'with an extended mode regexp with comment' do
let(:source) { '/42 # the answer/x' }

it 'returns the expected tree' do
tree = node.parsed_tree

expect(tree.is_a?(Regexp::Expression::Root)).to eq(true)
expect(tree.map(&:token)).to eq(%i[literal whitespace comment])
end
end

context 'with a regexp containing interpolation' do
it 'returns the expected blanked tree' do
tree = node.parsed_tree

expect(tree.is_a?(Regexp::Expression::Root)).to eq(true)
expect(tree.to_s).to eq('foo baz')
end
end

context 'with a regexp containing a multi-line interpolation' do
let(:source) do
<<~'REGEXP'
/
foo
#{
bar(
42
)
}
baz
/
REGEXP
end

it 'returns the expected blanked tree' do
tree = node.parsed_tree

expect(tree.is_a?(Regexp::Expression::Root)).to eq(true)
expect(tree.to_s.split("\n")).to eq(
[
'',
' foo',
' ' * 32,
' baz'
]
)
end
end

context 'with a regexp not containing interpolation' do
let(:source) { '/foobaz/' }

it 'returns the expected tree' do
tree = node.parsed_tree

expect(tree.is_a?(Regexp::Expression::Root)).to eq(true)
expect(tree.to_s).to eq('foobaz')
end
end
end
end

0 comments on commit 751edc7

Please sign in to comment.