diff --git a/CHANGELOG.md b/CHANGELOG.md index 18121f1b60e..509cfb51a3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/rubocop/cop/style/regexp_literal.rb b/lib/rubocop/cop/style/regexp_literal.rb index 51256ec4745..7761a141749 100644 --- a/lib/rubocop/cop/style/regexp_literal.rb +++ b/lib/rubocop/cop/style/regexp_literal.rb @@ -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 @@ -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) @@ -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 @@ -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) @@ -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 @@ -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(//) @@ -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 = [] @@ -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 diff --git a/manual/cops_style.md b/manual/cops_style.md index dacc165e830..f31d9502407 100644 --- a/manual/cops_style.md +++ b/manual/cops_style.md @@ -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 @@ -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 diff --git a/spec/rubocop/cop/style/regexp_literal_spec.rb b/spec/rubocop/cop/style/regexp_literal_spec.rb index 5a4bdf51244..929f3d788eb 100644 --- a/spec/rubocop/cop/style/regexp_literal_spec.rb +++ b/spec/rubocop/cop/style/regexp_literal_spec.rb @@ -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 } @@ -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) @@ -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 @@ -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