Skip to content

Commit

Permalink
Add offence for unnecessary /-escapes in Style/RegexpLiteral
Browse files Browse the repository at this point in the history
  • Loading branch information
owst committed Apr 25, 2020
1 parent db23f5e commit cc1b1ce
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 20 deletions.
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][])

### 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

0 comments on commit cc1b1ce

Please sign in to comment.