Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle comments and blank interpolations in regexp parsed_tree #8803

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')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

s.add_runtime_dependency('rexml')
s.add_runtime_dependency('rubocop-ast', '>= 0.5.0')
s.add_runtime_dependency('rubocop-ast', '>= 0.6.0')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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