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

Add Style/RedundantRegexpEscape cop #7908

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
### New features

* [#7895](https://github.com/rubocop-hq/rubocop/pull/7895): Include `.simplecov` file by default. ([@robotdana][])
* [#7908](https://github.com/rubocop-hq/rubocop/pull/7908): Add unnecessary `/`-escapes offence to `Style/RegexpLiteral`. ([@owst][])
Copy link
Collaborator

Choose a reason for hiding this comment

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

offenses is the spelling we use.


### Bug fixes

Expand Down
72 changes: 53 additions & 19 deletions lib/rubocop/cop/style/regexp_literal.rb
Expand Up @@ -3,7 +3,8 @@
module RuboCop
module Cop
module Style
# This cop enforces using // or %r around regular expressions.
# This cop enforces using // or %r around regular expressions, and
# prevents unnecessary /-escapes inside %r literals.
#
# @example EnforcedStyle: slashes (default)
# # bad
Expand Down Expand Up @@ -81,12 +82,23 @@ module Style
# @example AllowInnerSlashes: true
# # good
# x =~ /home\//
#
# @example
# # bad
# r = %r{foo\/bar}
#
# # good
# r = %r{foo/bar}
#
# # good
# r = %r/foo\/bar/
class RegexpLiteral < Cop
include ConfigurableEnforcedStyle
include RangeHelp

MSG_USE_SLASHES = 'Use `//` around regular expression.'
MSG_USE_PERCENT_R = 'Use `%r` around regular expression.'
MSG_UNNECESSARY_ESCAPE = 'Unnecessary `/`-escape inside `%r` literal'

def on_regexp(node)
if slash_literal?(node)
Expand All @@ -98,8 +110,17 @@ def on_regexp(node)

def autocorrect(node)
lambda do |corrector|
correct_delimiters(node, corrector)
correct_inner_slashes(node, corrector)
if !slash_literal?(node) && allowed_percent_r_literal?(node)
correct_inner_slashes(node, corrector, '\/', '/')
else
correct_delimiters(node, corrector)
correct_inner_slashes(
node,
corrector,
inner_slash_before_delimiter_correction(node),
inner_slash_after_delimiter_correction(node)
)
end
end
end

Expand All @@ -112,9 +133,13 @@ def check_slash_literal(node)
end

def check_percent_r_literal(node)
return if allowed_percent_r_literal?(node)

add_offense(node, message: MSG_USE_SLASHES)
if allowed_percent_r_literal?(node)
percent_r_unnecessary_escapes(node).each do |loc|
add_offense(node, location: loc, message: MSG_UNNECESSARY_ESCAPE)
end
else
add_offense(node, message: MSG_USE_SLASHES)
end
end

def allowed_slash_literal?(node)
Expand Down Expand Up @@ -146,6 +171,12 @@ def contains_slash?(node)
node_body(node).include?('/')
end

def percent_r_unnecessary_escapes(node)
return [] if percent_r_slash_delimiters?(node) || allow_inner_slashes?

inner_slash_ranges(node, '\/')
end

def allow_inner_slashes?
cop_config['AllowInnerSlashes']
end
Expand All @@ -159,6 +190,10 @@ def slash_literal?(node)
node.loc.begin.source == '/'
end

def percent_r_slash_delimiters?(node)
node.loc.begin.source == '%r/'
end

def preferred_delimiters
config.for_cop('Style/PercentLiteralDelimiters') \
['PreferredDelimiters']['%r'].split(//)
Expand All @@ -170,25 +205,24 @@ def correct_delimiters(node, corrector)
corrector.replace(node.loc.end, replacement.last)
end

def correct_inner_slashes(node, corrector)
def correct_inner_slashes(node, corrector, existing, replacement)
inner_slash_ranges(node, existing).map do |range|
corrector.replace(range, replacement)
end
end

def inner_slash_ranges(node, slash)
regexp_begin = node.loc.begin.end_pos

inner_slash_indices(node).each do |index|
inner_slash_indices(node, slash).map do |index|
start = regexp_begin + index

corrector.replace(
range_between(
start,
start + inner_slash_before_correction(node).length
),
inner_slash_after_correction(node)
)
range_between(start, start + slash.length)
end
end

def inner_slash_indices(node)
def inner_slash_indices(node, pattern)
text = node_body(node, include_begin_nodes: true)
pattern = inner_slash_before_correction(node)
index = -1
indices = []

Expand All @@ -199,11 +233,11 @@ def inner_slash_indices(node)
indices
end

def inner_slash_before_correction(node)
def inner_slash_before_delimiter_correction(node)
inner_slash_for(node.loc.begin.source)
end

def inner_slash_after_correction(node)
def inner_slash_after_delimiter_correction(node)
inner_slash_for(calculate_replacement(node).first)
end

Expand Down
13 changes: 12 additions & 1 deletion manual/cops_style.md
Expand Up @@ -5903,7 +5903,8 @@ Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChan
--- | --- | --- | --- | ---
Enabled | Yes | Yes | 0.9 | 0.30

This cop enforces using // or %r around regular expressions.
This cop enforces using // or %r around regular expressions, and
prevents unnecessary /-escapes inside %r literals.

### Examples

Expand Down Expand Up @@ -5994,6 +5995,16 @@ x =~ %r{home/}
# good
x =~ /home\//
```
```ruby
# bad
r = %r{foo\/bar}

# good
r = %r{foo/bar}

# good
r = %r/foo\/bar/
```

### Configurable attributes

Expand Down
73 changes: 73 additions & 0 deletions spec/rubocop/cop/style/regexp_literal_spec.rb
Expand Up @@ -263,6 +263,26 @@
RUBY
end

describe 'with unnecessarily-escaped slashes' do
it 'registers an offense and corrects' do
expect_offense(<<~'RUBY')
foo = %r{
https?:\/\/
^^ Unnecessary `/`-escape inside `%r` literal
^^ Unnecessary `/`-escape inside `%r` literal
example\.com
}x
RUBY

expect_correction(<<~'RUBY')
foo = %r{
https?://
example\.com
}x
RUBY
end
end

describe 'when configured to allow inner slashes' do
before { cop_config['AllowInnerSlashes'] = true }

Expand Down Expand Up @@ -378,6 +398,19 @@
end
end

describe 'a single-line %r regex with unnecessarily-escaped slashes' do
it 'registers an offense and corrects' do
expect_offense(<<~'RUBY')
foo = %r{home\/}
^^ Unnecessary `/`-escape inside `%r` literal
RUBY

expect_correction(<<~RUBY)
foo = %r{home/}
RUBY
end
end

describe 'a multi-line %r regex without slashes' do
it 'is accepted' do
expect_no_offenses(<<~RUBY)
Expand All @@ -399,6 +432,26 @@
RUBY
end
end

describe 'a multi-line %r regex with unnecessarily-escaped slashes' do
it 'registers an offense and corrects' do
expect_offense(<<~'RUBY')
foo = %r{
https?:\/\/
^^ Unnecessary `/`-escape inside `%r` literal
^^ Unnecessary `/`-escape inside `%r` literal
example\.com
}x
RUBY

expect_correction(<<~'RUBY')
foo = %r{
https?://
example\.com
}x
RUBY
end
end
end

context 'when EnforcedStyle is set to mixed' do
Expand Down Expand Up @@ -542,5 +595,25 @@
RUBY
end
end

describe 'a multi-line %r regex with unnecessarily-escaped slashes' do
it 'registers an offense and corrects' do
expect_offense(<<~'RUBY')
foo = %r{
https?:\/\/
^^ Unnecessary `/`-escape inside `%r` literal
^^ Unnecessary `/`-escape inside `%r` literal
example\.com
}x
RUBY

expect_correction(<<~'RUBY')
foo = %r{
https?://
example\.com
}x
RUBY
end
end
end
end