diff --git a/changelog/fix_improve_redundant_regex_escape_catching.md b/changelog/fix_improve_redundant_regex_escape_catching.md new file mode 100644 index 00000000000..92ff4040a14 --- /dev/null +++ b/changelog/fix_improve_redundant_regex_escape_catching.md @@ -0,0 +1 @@ +* [#11150](https://github.com/rubocop/rubocop/issues/11150): Improve `Style/RedundantRegexpEscape` to catch unnecessarily escaped hyphens within a character class. ([@si-lens][]) diff --git a/lib/rubocop/cop/style/redundant_regexp_escape.rb b/lib/rubocop/cop/style/redundant_regexp_escape.rb index a8ba38da93a..7307a5b1f7e 100644 --- a/lib/rubocop/cop/style/redundant_regexp_escape.rb +++ b/lib/rubocop/cop/style/redundant_regexp_escape.rb @@ -44,7 +44,7 @@ class RedundantRegexpEscape < Base def on_regexp(node) each_escape(node) do |char, index, within_character_class| - next if allowed_escape?(node, char, within_character_class) + next if allowed_escape?(node, char, index, within_character_class) location = escape_range_at_index(node, index) @@ -56,7 +56,7 @@ def on_regexp(node) private - def allowed_escape?(node, char, within_character_class) + def allowed_escape?(node, char, index, within_character_class) # Strictly speaking a few single-letter metachars are currently # unnecessary to "escape", e.g. i, E, F, but enumerating them is # rather difficult, and their behavior could change over time with @@ -65,12 +65,21 @@ def allowed_escape?(node, char, within_character_class) return true if ALLOWED_ALWAYS_ESCAPES.include?(char) || delimiter?(node, char) if within_character_class - ALLOWED_WITHIN_CHAR_CLASS_METACHAR_ESCAPES.include?(char) + ALLOWED_WITHIN_CHAR_CLASS_METACHAR_ESCAPES.include?(char) && + !char_class_begins_or_ends_with_escaped_hyphen?(node, index) else ALLOWED_OUTSIDE_CHAR_CLASS_METACHAR_ESCAPES.include?(char) end end + def char_class_begins_or_ends_with_escaped_hyphen?(node, index) + # The hyphen character is allowed to be escaped within a character class + # but it's not necessry to escape hyphen if it's the first or last character + # within the character class. This method checks if that's the case. + # e.g. "[0-9\\-]" or "[\\-0-9]" would return true + node.source[index] == '[' || node.source[index + 3] == ']' + end + def delimiter?(node, char) delimiters = [node.loc.begin.source[-1], node.loc.end.source[0]] diff --git a/spec/rubocop/cop/style/redundant_regexp_escape_spec.rb b/spec/rubocop/cop/style/redundant_regexp_escape_spec.rb index c7ddb38a879..5ffcf1a72ad 100644 --- a/spec/rubocop/cop/style/redundant_regexp_escape_spec.rb +++ b/spec/rubocop/cop/style/redundant_regexp_escape_spec.rb @@ -111,6 +111,32 @@ end end + context "with an escaped '-' character being the last character inside a character class" do + it 'registers an offense and corrects' do + expect_offense(<<~'RUBY') + foo = /[0-9\-]/ + ^^ Redundant escape inside regexp literal + RUBY + + expect_correction(<<~RUBY) + foo = /[0-9-]/ + RUBY + end + end + + context "with an escaped '-' character being the first character inside a character class" do + it 'registers an offense and corrects' do + expect_offense(<<~'RUBY') + foo = /[\-0-9]/ + ^^ Redundant escape inside regexp literal + RUBY + + expect_correction(<<~RUBY) + foo = /[-0-9]/ + RUBY + end + end + context 'with an escaped character class and following escaped char' do it 'does not register an offense' do expect_no_offenses('foo = /\[\+/') @@ -125,7 +151,7 @@ context 'with a nested character class then allowed escape' do it 'does not register an offense' do - expect_no_offenses('foo = /[a-w&&[^c-g]\-]/') + expect_no_offenses('foo = /[a-w&&[^c-g]\-1-9]/') end end @@ -333,8 +359,15 @@ end context "with an escaped '#{char}' inside a character class" do - it 'does not register an offense' do - expect_no_offenses("foo = /a[\\#{char}]b/") + it 'registers an offense and corrects' do + expect_offense(<<~RUBY) + foo = /a[\\#{char}]b/ + ^^ Redundant escape inside regexp literal + RUBY + + expect_correction(<<~RUBY) + foo = /a[#{char}]b/ + RUBY end end end