From 59053960a9d738bd4547af38c7e95d8d79fdfd45 Mon Sep 17 00:00:00 2001 From: SAKATA Sinji Date: Tue, 27 Oct 2020 18:51:37 +0900 Subject: [PATCH] [Fix #8948] Fix autocorrection for Style/RedundantRegexpCharacterClass with %r This commit uses `RegexpNode::RegexpParser#body` (#8960) instead of `Parser::Source::Range#adjust` in Style/RedundantRegexpCharacterClass. When regular expression are written by %r literal, RedundantRegexpCharacterClass made incorrect loc as following. ``` %r{abc[\d]+} ^^^^^^ ``` This bug are caused by calling`node.loc.begin.adjust` to get range of character class with %r regexp. `Parser::Source::Range#adjust` adjust based on `Parser::Source::Range#begin_pos` but it with %r literal doesn't indicate start of regular expression source, point to location of '%'. So, RedundantRegexpCharacterClass makes incorrect loc if %r literal are used in code. ``` r = %r{abc} ^ # `node.loc.begin.begin_pos` ``` ``` r = %r{abs} ^ # `node.loc.begin.end_pos` ``` --- CHANGELOG.md | 1 + docs/modules/ROOT/pages/cops_style.adoc | 6 ++ .../style/redundant_regexp_character_class.rb | 10 ++- .../redundant_regexp_character_class_spec.rb | 68 +++++++++++++++++++ 4 files changed, 82 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bfe1eea1b2..7d94a53ce04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ * [#8917](https://github.com/rubocop-hq/rubocop/issues/8917): Fix rubocop comment directives handling of cops with multiple levels in department name. ([@fatkodima][]) * [#8918](https://github.com/rubocop-hq/rubocop/issues/8918): Fix a false positives for `Bundler/DuplicatedGem` when a gem conditionally duplicated within `if-elsif` or `case-when` statements. ([@fatkodima][]) * [#8933](https://github.com/rubocop-hq/rubocop/pull/8933): Fix an error for `Layout/EmptyLinesAroundAccessModifier` when the first line is a comment. ([@matthieugendreau][]) +* [#8954](https://github.com/rubocop-hq/rubocop/pull/8954): Fix autocorrection for Style/RedundantRegexpCharacterClass with %r. ([@ysakasin][]) ### Changes diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index 047668cbec6..c306191002e 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -8230,6 +8230,12 @@ r = /[\s]/ # good r = /\s/ +# bad +r = %r{/[b]} + +# good +r = %r{/b} + # good r = /[ab]/ ---- diff --git a/lib/rubocop/cop/style/redundant_regexp_character_class.rb b/lib/rubocop/cop/style/redundant_regexp_character_class.rb index c18076cea74..05ca06bcf09 100644 --- a/lib/rubocop/cop/style/redundant_regexp_character_class.rb +++ b/lib/rubocop/cop/style/redundant_regexp_character_class.rb @@ -19,6 +19,12 @@ module Style # # good # r = /\s/ # + # # bad + # r = %r{/[b]} + # + # # good + # r = %r{/b} + # # # good # r = /[ab]/ class RedundantRegexpCharacterClass < Base @@ -48,9 +54,7 @@ def each_redundant_character_class(node) each_single_element_character_class(node) do |char_class| next unless redundant_single_element_character_class?(node, char_class) - begin_pos = 1 + char_class.ts - end_pos = begin_pos + char_class.expressions.first.text.length + 1 - yield node.loc.begin.adjust(begin_pos: begin_pos, end_pos: end_pos) + yield char_class.loc.body end end diff --git a/spec/rubocop/cop/style/redundant_regexp_character_class_spec.rb b/spec/rubocop/cop/style/redundant_regexp_character_class_spec.rb index 18ffd68c95f..efd52178f44 100644 --- a/spec/rubocop/cop/style/redundant_regexp_character_class_spec.rb +++ b/spec/rubocop/cop/style/redundant_regexp_character_class_spec.rb @@ -69,6 +69,74 @@ end end + context 'with %r{} regexp' do + context 'with a character class containing a single character' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY) + foo = %r{[a]} + ^^^ Redundant single-element character class, `[a]` can be replaced with `a`. + RUBY + + expect_correction(<<~RUBY) + foo = %r{a} + RUBY + end + end + + context 'with multiple character classes containing single characters' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY) + foo = %r{[a]b[c]d} + ^^^ Redundant single-element character class, `[a]` can be replaced with `a`. + ^^^ Redundant single-element character class, `[c]` can be replaced with `c`. + RUBY + + expect_correction(<<~RUBY) + foo = %r{abcd} + RUBY + end + end + + context 'with a character class containing a single character inside a group' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY) + foo = %r{([a])} + ^^^ Redundant single-element character class, `[a]` can be replaced with `a`. + RUBY + + expect_correction(<<~RUBY) + foo = %r{(a)} + RUBY + end + end + + context 'with a character class containing a single character before `+` quantifier' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY) + foo = %r{[a]+} + ^^^ Redundant single-element character class, `[a]` can be replaced with `a`. + RUBY + + expect_correction(<<~RUBY) + foo = %r{a+} + RUBY + end + end + + context 'with a character class containing a single character before `{n,m}` quantifier' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY) + foo = %r{[a]{2,10}} + ^^^ Redundant single-element character class, `[a]` can be replaced with `a`. + RUBY + + expect_correction(<<~RUBY) + foo = %r{a{2,10}} + RUBY + end + end + end + context 'with a character class containing a single range' do it 'does not register an offense' do expect_no_offenses('foo = /[a-z]/')