diff --git a/CHANGELOG.md b/CHANGELOG.md index e5f12d438e9..4f776e76a46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * [#6289](https://github.com/rubocop-hq/rubocop/issues/6289): Add new `CheckDefinitionPathHierarchy` option for `Naming/FileName`. ([@jschneid][]) * [#8069](https://github.com/rubocop-hq/rubocop/issues/8069): New option for `expect_offense` to help format offense templates. ([@marcandre][]) +* [#7908](https://github.com/rubocop-hq/rubocop/pull/7908): Add new `Style/RedundantRegexpEscape` cop. ([@owst][]) ### Bug fixes diff --git a/config/default.yml b/config/default.yml index 74522cba808..89d8b968f7d 100644 --- a/config/default.yml +++ b/config/default.yml @@ -3607,6 +3607,11 @@ Style/RedundantPercentQ: Enabled: true VersionAdded: '0.76' +Style/RedundantRegexpEscape: + Description: 'Checks for redundant escapes in Regexps.' + Enabled: pending + VersionAdded: '0.85' + Style/RedundantReturn: Description: "Don't use return where it's not required." StyleGuide: '#no-explicit-return' diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 7ac453da049..46c111dff1c 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -437,6 +437,7 @@ In the following section you find all available cops: * xref:cops_style.adoc#styleredundantinterpolation[Style/RedundantInterpolation] * xref:cops_style.adoc#styleredundantparentheses[Style/RedundantParentheses] * xref:cops_style.adoc#styleredundantpercentq[Style/RedundantPercentQ] +* xref:cops_style.adoc#styleredundantregexpescape[Style/RedundantRegexpEscape] * xref:cops_style.adoc#styleredundantreturn[Style/RedundantReturn] * xref:cops_style.adoc#styleredundantself[Style/RedundantSelf] * xref:cops_style.adoc#styleredundantsort[Style/RedundantSort] diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index 702ecbafb26..863667d660d 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -7116,6 +7116,49 @@ question = '"What did you say?"' * https://rubystyle.guide#percent-q +== Style/RedundantRegexpEscape + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| Yes +| 0.85 +| - +|=== + +This cop checks for redundant escapes inside Regexp literals. + +=== Examples + +[source,ruby] +---- +# bad +%r{foo\/bar} + +# good +%r{foo/bar} + +# good +/foo\/bar/ + +# good +%r/foo\/bar/ + +# bad +/a\-b/ + +# good +/a-b/ + +# bad +/[\+\-]\d/ + +# good +/[+\-]\d/ +---- + == Style/RedundantReturn |=== diff --git a/lib/rubocop.rb b/lib/rubocop.rb index cf82d5de1d6..6851359ad80 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -475,6 +475,7 @@ require_relative 'rubocop/cop/style/redundant_interpolation' require_relative 'rubocop/cop/style/redundant_parentheses' require_relative 'rubocop/cop/style/redundant_percent_q' +require_relative 'rubocop/cop/style/redundant_regexp_escape' require_relative 'rubocop/cop/style/redundant_return' require_relative 'rubocop/cop/style/redundant_self' require_relative 'rubocop/cop/style/redundant_sort' diff --git a/lib/rubocop/cop/layout/first_array_element_line_break.rb b/lib/rubocop/cop/layout/first_array_element_line_break.rb index 8f676af3390..9a8b1784e46 100644 --- a/lib/rubocop/cop/layout/first_array_element_line_break.rb +++ b/lib/rubocop/cop/layout/first_array_element_line_break.rb @@ -37,7 +37,7 @@ def autocorrect(node) def assignment_on_same_line?(node) source = node.source_range.source_line[0...node.loc.column] - source =~ /\s*\=\s*$/ + source =~ /\s*=\s*$/ end end end diff --git a/lib/rubocop/cop/layout/space_around_keyword.rb b/lib/rubocop/cop/layout/space_around_keyword.rb index f3dc4fdd173..4fa9bb223b4 100644 --- a/lib/rubocop/cop/layout/space_around_keyword.rb +++ b/lib/rubocop/cop/layout/space_around_keyword.rb @@ -186,7 +186,7 @@ def space_before_missing?(range) pos = range.begin_pos - 1 return false if pos.negative? - range.source_buffer.source[pos] !~ /[\s\(\|\{\[;,\*\=]/ + range.source_buffer.source[pos] !~ /[\s(|{\[;,*=]/ end def space_after_missing?(range) @@ -198,7 +198,7 @@ def space_after_missing?(range) return false if accept_namespace_operator?(range) && namespace_operator?(range, pos) - char !~ /[\s;,#\\\)\}\]\.]/ + char !~ /[\s;,#\\)}\].]/ end def accepted_opening_delimiter?(range, char) diff --git a/lib/rubocop/cop/naming/file_name.rb b/lib/rubocop/cop/naming/file_name.rb index da46a5d5a97..15f9399e230 100644 --- a/lib/rubocop/cop/naming/file_name.rb +++ b/lib/rubocop/cop/naming/file_name.rb @@ -119,7 +119,7 @@ def allowed_acronyms def filename_good?(basename) basename = basename.sub(/^\./, '') - basename = basename.sub(/\.[^\.]+$/, '') + basename = basename.sub(/\.[^.]+$/, '') # special handling for Action Pack Variants file names like # some_file.xlsx+mobile.axlsx basename = basename.sub('+', '_') diff --git a/lib/rubocop/cop/style/multiline_memoization.rb b/lib/rubocop/cop/style/multiline_memoization.rb index b8e85ae2db3..4baa0620f95 100644 --- a/lib/rubocop/cop/style/multiline_memoization.rb +++ b/lib/rubocop/cop/style/multiline_memoization.rb @@ -82,7 +82,7 @@ def keyword_begin_str(node, node_buf) end def keyword_end_str(node, node_buf) - if /[^\s\)]/.match?(node_buf.source_line(node.loc.end.line)) + if /[^\s)]/.match?(node_buf.source_line(node.loc.end.line)) "\n" + (' ' * node.loc.column) + 'end' else 'end' diff --git a/lib/rubocop/cop/style/redundant_regexp_escape.rb b/lib/rubocop/cop/style/redundant_regexp_escape.rb new file mode 100644 index 00000000000..6795c05801b --- /dev/null +++ b/lib/rubocop/cop/style/redundant_regexp_escape.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # This cop checks for redundant escapes inside Regexp literals. + # + # @example + # # bad + # %r{foo\/bar} + # + # # good + # %r{foo/bar} + # + # # good + # /foo\/bar/ + # + # # good + # %r/foo\/bar/ + # + # # bad + # /a\-b/ + # + # # good + # /a-b/ + # + # # bad + # /[\+\-]\d/ + # + # # good + # /[+\-]\d/ + class RedundantRegexpEscape < Cop + include RangeHelp + + MSG_REDUNDANT_ESCAPE = 'Redundant escape inside regexp literal' + + ALLOWED_ALWAYS_ESCAPES = ' []^\\#'.chars.freeze + ALLOWED_WITHIN_CHAR_CLASS_METACHAR_ESCAPES = '-'.chars.freeze + ALLOWED_OUTSIDE_CHAR_CLASS_METACHAR_ESCAPES = '.*+?{}()|$'.chars.freeze + + def on_regexp(node) + each_escape(node) do |char, index, within_character_class| + next if allowed_escape?(node, char, within_character_class) + + add_offense( + node, + location: escape_range_at_index(node, index), + message: MSG_REDUNDANT_ESCAPE + ) + end + end + + def autocorrect(node) + lambda do |corrector| + each_escape(node) do |char, index, within_character_class| + next if allowed_escape?(node, char, within_character_class) + + corrector.remove_leading(escape_range_at_index(node, index), 1) + end + end + end + + private + + def slash_literal?(node) + ['/', '%r/'].include?(node.loc.begin.source) + end + + def allowed_escape?(node, char, within_character_class) + # Strictly speaking a few single-letter metachars are currently + # unneccessary to "escape", e.g. g, i, E, F, but enumerating them is + # rather difficult, and their behaviour could change over time with + # different versions of Ruby so that e.g. /\g/ != /g/ + return true if /[[:alnum:]]/.match?(char) + return true if ALLOWED_ALWAYS_ESCAPES.include?(char) + + if char == '/' + slash_literal?(node) + elsif within_character_class + ALLOWED_WITHIN_CHAR_CLASS_METACHAR_ESCAPES.include?(char) + else + ALLOWED_OUTSIDE_CHAR_CLASS_METACHAR_ESCAPES.include?(char) + end + end + + def each_escape(node) + pattern_source(node).each_char.with_index.reduce( + [nil, false] + ) do |(previous, within_character_class), (current, index)| + if previous == '\\' + yield [current, index - 1, within_character_class] + + [nil, within_character_class] + elsif previous == '[' && current != ':' + [current, true] + elsif previous != ':' && current == ']' + [current, false] + else + [current, within_character_class] + end + end + end + + def escape_range_at_index(node, index) + regexp_begin = node.loc.begin.end_pos + + start = regexp_begin + index + + range_between(start, start + 2) + end + + def pattern_source(node) + node.children.reject(&:regopt_type?).map(&:source).join + end + end + end + end +end diff --git a/lib/rubocop/cop/style/symbol_array.rb b/lib/rubocop/cop/style/symbol_array.rb index e614c00c942..31be5184726 100644 --- a/lib/rubocop/cop/style/symbol_array.rb +++ b/lib/rubocop/cop/style/symbol_array.rb @@ -106,7 +106,7 @@ def symbol_without_quote?(string) # method name string =~ /\A[a-zA-Z_]\w*[!?]?\z/ || # instance / class variable - string =~ /\A\@\@?[a-zA-Z_]\w*\z/ || + string =~ /\A@@?[a-zA-Z_]\w*\z/ || # global variable string =~ /\A\$[1-9]\d*\z/ || string =~ /\A\$[a-zA-Z_]\w*\z/ || diff --git a/lib/rubocop/magic_comment.rb b/lib/rubocop/magic_comment.rb index 3d3868ac81b..0b237384412 100644 --- a/lib/rubocop/magic_comment.rb +++ b/lib/rubocop/magic_comment.rb @@ -133,7 +133,7 @@ def tokens # @see https://www.gnu.org/software/emacs/manual/html_node/emacs/Specify-Coding.html # @see https://git.io/vMCXh Emacs handling in Ruby's parse.y class EmacsComment < EditorComment - FORMAT = /\-\*\-(.+)\-\*\-/.freeze + FORMAT = /-\*-(.+)-\*-/.freeze SEPARATOR = ';' OPERATOR = ':' diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 783bd3d70fd..b519e685191 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -150,7 +150,7 @@ describe 'link to related issue' do let(:issues) do entries.map do |entry| - entry.match(/\[(?[#\d]+)\]\((?[^\)]+)\)/) + entry.match(/\[(?[#\d]+)\]\((?[^)]+)\)/) end.compact end @@ -191,7 +191,7 @@ entry .gsub(/`[^`]+`/, '``') .sub(/^\*\s*(?:\[.+?\):\s*)?/, '') - .sub(/\s*\([^\)]+\)$/, '') + .sub(/\s*\([^)]+\)$/, '') end end @@ -202,7 +202,7 @@ end it 'ends with a punctuation' do - expect(bodies).to all(match(/[\.\!]$/)) + expect(bodies).to all(match(/[.!]$/)) end end end diff --git a/spec/rubocop/cli/cli_options_spec.rb b/spec/rubocop/cli/cli_options_spec.rb index a52828f36f8..2552b108ea9 100644 --- a/spec/rubocop/cli/cli_options_spec.rb +++ b/spec/rubocop/cli/cli_options_spec.rb @@ -1005,8 +1005,8 @@ def full_description_of_cop(cop) ' Description: Consistent indentation either with tabs only or spaces only.', /^ StyleGuide: ('|")#spaces-indentation('|")$/, ' Enabled: true', - /^ VersionAdded: '[0-9\.]+'$/, - /^ VersionChanged: '[0-9\.]+'$/, + /^ VersionAdded: '[0-9.]+'$/, + /^ VersionChanged: '[0-9.]+'$/, ' IndentationWidth:'].join("\n") ) end diff --git a/spec/rubocop/cop/style/redundant_regexp_escape_spec.rb b/spec/rubocop/cop/style/redundant_regexp_escape_spec.rb new file mode 100644 index 00000000000..119d0a34821 --- /dev/null +++ b/spec/rubocop/cop/style/redundant_regexp_escape_spec.rb @@ -0,0 +1,290 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::RedundantRegexpEscape do + subject(:cop) { described_class.new } + + context 'with a single-line `//` regexp' do + context 'without escapes' do + it 'does not register an offense' do + expect_no_offenses('foo = /a/') + end + end + + context 'with escaped slashes' do + it 'does not register an offense' do + expect_no_offenses('foo = /\/a\//') + end + end + + [ + ('a'..'z').to_a - %w[c n p u x], + ('A'..'Z').to_a - %w[C M P], + %w[n101 x41 u0041 u{0041} cc C-c p{alpha} P{alpha}] + ].flatten.each do |escape| + context "with an escaped '#{escape}' outside a character class" do + it 'does not register an offense' do + expect_no_offenses("foo = /\\#{escape}/") + end + end + + context "with an escaped '#{escape}' inside a character class" do + it 'does not register an offense' do + expect_no_offenses("foo = /[\\#{escape}]/") + end + end + end + + context "with an escaped 'M-a' outside a character class" do + it 'does not register an offense' do + expect_no_offenses('foo = /\\M-a/n') + end + end + + context "with an escaped 'M-a' inside a character class" do + it 'does not register an offense' do + expect_no_offenses('foo = /[\\M-a]/n') + end + end + + described_class::ALLOWED_OUTSIDE_CHAR_CLASS_METACHAR_ESCAPES.each do |char| + context "with an escaped '#{char}' outside a character class" do + it 'does not register an offense' do + expect_no_offenses("foo = /\\#{char}/") + end + end + + context "with an escaped '#{char}' inside a character class" do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY) + foo = /[\\#{char}]/ + ^^ Redundant escape inside regexp literal + RUBY + + expect_correction(<<~RUBY) + foo = /[#{char}]/ + RUBY + end + end + end + + context 'with an escaped character class and following escaped char' do + it 'does not register an offense' do + expect_no_offenses('foo = /\[\+/') + end + end + + context 'with a character class and following escaped char' do + it 'does not register an offense' do + expect_no_offenses('foo = /[a]\+/') + end + end + + context 'with a POSIX character class inside a character class' do + it 'does not register an offense' do + expect_no_offenses('foo = /[[:alnum:]\-_]+/') + end + end + + context 'with a backreference' do + it 'does not register an offense' do + expect_no_offenses('foo = /([a-z])\s*\1/') + end + end + + context 'with an escaped interpolation outside a character class' do + it 'does not register an offense' do + expect_no_offenses('foo = /\#\{[a-z_]+\}/') + end + end + + context 'with an escaped interpolation inside a character class' do + it 'does not register an offense' do + expect_no_offenses('foo = /[\#{}]/') + end + end + + context 'with an uppercase metacharacter inside a character class' do + it 'does not register an offense' do + expect_no_offenses('foo = /[\H]/') + end + end + + context 'with an uppercase metacharacter outside a character class' do + it 'does not register an offense' do + expect_no_offenses('foo = /\H/') + end + end + + context 'with regexp options and a redundant escape' do + it 'registers offenses and corrects' do + expect_offense(<<~'RUBY') + r = /\-/i + ^^ Redundant escape inside regexp literal + RUBY + + expect_correction(<<~'RUBY') + r = /-/i + RUBY + end + end + + context 'with an interpolation followed by redundant escapes' do + it 'registers offenses and corrects' do + expect_offense(<<~'RUBY') + METHOD_NAME = /\#?#{IDENTIFIER}[\!\?]?\(?/.freeze + ^^ Redundant escape inside regexp literal + ^^ Redundant escape inside regexp literal + RUBY + + expect_correction(<<~'RUBY') + METHOD_NAME = /\#?#{IDENTIFIER}[!?]?\(?/.freeze + RUBY + end + end + + context 'with multiple escaped metachars inside a character class' do + it 'registers offenses and corrects' do + expect_offense(<<~'RUBY') + foo = /[\s\(\|\{\[;,\*\=]/ + ^^ Redundant escape inside regexp literal + ^^ Redundant escape inside regexp literal + ^^ Redundant escape inside regexp literal + ^^ Redundant escape inside regexp literal + ^^ Redundant escape inside regexp literal + RUBY + + expect_correction(<<~'RUBY') + foo = /[\s(|{\[;,*=]/ + RUBY + end + end + + described_class::ALLOWED_ALWAYS_ESCAPES.each do |char| + context "with an escaped '#{char}' outside a character class" do + it 'does not register an offense' do + expect_no_offenses("foo = /\\#{char}/") + end + end + + context "with an escaped '#{char}' inside a character class" do + it 'does not register an offense' do + expect_no_offenses("foo = /[\\#{char}]/") + end + end + end + + described_class::ALLOWED_WITHIN_CHAR_CLASS_METACHAR_ESCAPES.each do |char| + context "with an escaped '#{char}' outside a character class" do + 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 + + context "with an escaped '#{char}' inside a character class" do + it 'does not register an offense' do + expect_no_offenses("foo = /a[\\#{char}]b/") + end + end + end + end + + context 'with a single-line %r{} regexp' do + context 'without escapes' do + it 'does not register an offense' do + expect_no_offenses('foo = %r{a}') + end + end + + context 'with redundantly-escaped slashes' do + it 'registers an offense and corrects' do + expect_offense(<<~'RUBY') + foo = %r{\/a\/} + ^^ Redundant escape inside regexp literal + ^^ Redundant escape inside regexp literal + RUBY + + expect_correction(<<~RUBY) + foo = %r{/a/} + RUBY + end + end + end + + context 'with a single-line %r// regexp' do + context 'without escapes' do + it 'does not register an offense' do + expect_no_offenses('foo = %r/a/') + end + end + + context 'with escaped slashes' do + it 'does not register an offense' do + expect_no_offenses('foo = %r/\/a\//') + end + end + end + + context 'with a multi-line %r{} regexp' do + context 'without escapes' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + foo = %r{ + a + b + }x + RUBY + end + end + + context 'with redundantly-escaped slashes' do + it 'registers an offense and corrects' do + expect_offense(<<~'RUBY') + foo = %r{ + \/a + ^^ Redundant escape inside regexp literal + b\/ + ^^ Redundant escape inside regexp literal + }x + RUBY + + expect_correction(<<~RUBY) + foo = %r{ + /a + b/ + }x + RUBY + end + end + end + + context 'with a multi-line %r// regexp' do + context 'without escapes' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + foo = %r/ + a + b + /x + RUBY + end + end + + context 'with escaped slashes' do + it 'does not register an offense' do + expect_no_offenses(<<~'RUBY') + foo = %r/ + \/a + b\/ + /x + RUBY + end + end + end +end diff --git a/spec/rubocop/cop/style/word_array_spec.rb b/spec/rubocop/cop/style/word_array_spec.rb index 240fa2ce8d7..843dea3ddb8 100644 --- a/spec/rubocop/cop/style/word_array_spec.rb +++ b/spec/rubocop/cop/style/word_array_spec.rb @@ -350,7 +350,7 @@ end context 'with a treacherous WordRegex configuration' do - let(:cop_config) { { 'MinSize' => 0, 'WordRegex' => /[\w \[\]\(\)]/ } } + let(:cop_config) { { 'MinSize' => 0, 'WordRegex' => /[\w \[\]()]/ } } it "doesn't break when words contain whitespace" do new_source = autocorrect_source("['hi there', 'something\telse']")