Fix an incorrect autocorrect for Capybara/RedundantWithinFind when escape required css selector

CHANGELOG.md

@@ -5,6 +5,7 @@
 - Add `Capybara/AmbiguousClick` cop and make soft-deprecated `Capybara/ClickLinkOrButtonStyle` cop. If you want to use `EnforcedStyle: strict`, use `Capybara/AmbiguousClick` cop instead. ([@ydah])
 - Add new `Capybara/FindAllFirst` cop. ([@ydah])
 - Fix an error for `Capybara/RSpec/HaveSelector` when passing no arguments. ([@earlopain])
+- Fix an incorrect autocorrect for `Capybara/RedundantWithinFind` when escape required css selector. ([@ydah])
 
 ## 2.21.0 (2024-06-08)
 

lib/rubocop/cop/capybara/mixin/css_selector.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'strscan'
+
 module RuboCop
   module Cop
     module Capybara
@@ -83,6 +85,49 @@ def multiple_selectors?(selector)
           normalize = selector.gsub(/(\\[>,+~]|\(.*\))/, '')
           normalize.match?(/[ >,+~]/)
         end
+
+        # @param selector [String]
+        # @return [String]
+        # @example
+        #   css_escape('some-id') # => some-id
+        #   css_escape('some-id.with-priod') # => some-id\.with-priod
+        # @reference
+        #   https://github.com/mathiasbynens/CSS.escape/blob/master/css.escape.js
+        def css_escape(selector) # rubocop:disable Metrics/AbcSize,Metrics/CyclomaticComplexity,Metrics/MethodLength,Metrics/PerceivedComplexity
+          scanner = StringScanner.new(selector)
+          result = +''
+
+          # Special case: if the selector is of length 1 and
+          # the first character is `-`
+          if selector.length == 1 && scanner.peek(1) == '-'
+            return "\\#{selector}"
+          end
+
+          until scanner.eos?
+            # NULL character (U+0000)
+            if scanner.scan(/\0/)
+              result << "\uFFFD"
+            # Control characters (U+0001 to U+001F, U+007F)
+            elsif scanner.scan(/[\x01-\x1F\x7F]/)
+              result << "\\#{scanner.matched.ord.to_s(16)} "
+            # First character is a digit (U+0030 to U+0039)
+            elsif scanner.pos.zero? && scanner.scan(/\d/)
+              result << "\\#{scanner.matched.ord.to_s(16)} "
+            # Second character is a digit and first is `-`
+            elsif scanner.pos == 1 && scanner.scan(/\d/) &&
+                scanner.pre_match == '-'
+              result << "\\#{scanner.matched.ord.to_s(16)} "
+            # Alphanumeric characters, `-`, `_`
+            elsif scanner.scan(/[a-zA-Z0-9\-_]/)
+              result << scanner.matched
+            # Any other characters, escape them
+            elsif scanner.scan(/./)
+              result << "\\#{scanner.matched}"
+            end
+          end
+
+          result
+        end
       end
     end
   end

lib/rubocop/cop/capybara/redundant_within_find.rb

@@ -53,11 +53,15 @@ def msg(node)
         end
 
         def replaced(node)
-          replaced = node.arguments.map(&:source).join(', ')
           if node.method?(:find_by_id)
-            replaced.sub(/\A(["'])/, '\1#')
+            node.first_argument.source.match(/\A(['"])(.*)['"]\z/) do
+              quote = ::Regexp.last_match(1)
+              css = ::Regexp.last_match(2)
+              return ["#{quote}##{CssSelector.css_escape(css)}#{quote}",
+                      *node.arguments[1..].map(&:source)].join(', ')
+            end
           else
-            replaced
+            node.arguments.map(&:source).join(', ')
           end
         end
       end

spec/rubocop/cop/capybara/mixin/css_selector_spec.rb

@@ -143,4 +143,73 @@
       expect(described_class.multiple_selectors?('a.cls\>b')).to be false
     end
   end
+
+  describe 'CssSelector.css_escape' do
+    context 'when selector is a single hyphen' do
+      let(:selector) { '-' }
+
+      it 'escapes the hyphen character' do
+        expect(described_class.css_escape(selector)).to eq('\\-')
+      end
+    end
+
+    context 'when selector contains NULL character' do
+      let(:selector) { "abc\0def" }
+
+      it 'replaces NULL character with U+FFFD' do
+        expect(described_class.css_escape(selector)).to eq('abc�def')
+      end
+    end
+
+    context 'when selector contains control characters' do
+      let(:selector) { "abc\x01\x1F\x7Fdef" }
+
+      it 'escapes control characters as hexadecimal with a trailing space' do
+        expect(described_class.css_escape(selector))
+          .to eq('abc\\1 \\1f \\7f def')
+      end
+    end
+
+    context 'when selector starts with a digit' do
+      let(:selector) { '1abc' }
+
+      it 'escapes the starting digit as hexadecimal with a trailing space' do
+        expect(described_class.css_escape(selector)).to eq('\\31 abc')
+      end
+    end
+
+    context 'when selector starts with a hyphen followed by a digit' do
+      let(:selector) { '-1abc' }
+
+      it 'escapes the digit following a hyphen as hexadecimal with a ' \
+         'trailing space' do
+        expect(described_class.css_escape(selector)).to eq('-\\31 abc')
+      end
+    end
+
+    context 'when selector contains alphanumeric characters, hyphen, or ' \
+            'underscore' do
+      let(:selector) { 'a-Z_0-9' }
+
+      it 'does not escape alphanumeric characters, hyphen, or underscore' do
+        expect(described_class.css_escape(selector)).to eq('a-Z_0-9')
+      end
+    end
+
+    context 'when selector contains special characters needing escape' do
+      let(:selector) { 'a!b@c#d$' }
+
+      it 'escapes special characters' do
+        expect(described_class.css_escape(selector)).to eq('a\\!b\\@c\\#d\\$')
+      end
+    end
+
+    context 'when selector contains mixed cases' do
+      let(:selector) { "ab\x00\x7F" }
+
+      it 'handles mixed cases appropriately' do
+        expect(described_class.css_escape(selector)).to eq('ab�\\7f ')
+      end
+    end
+  end
 end

spec/rubocop/cop/capybara/redundant_within_find_spec.rb

@@ -66,6 +66,19 @@
     RUBY
   end
 
+  it 'registers an offense when using `within find_by_id("foo.bar")`' do
+    expect_offense(<<~RUBY)
+      within find_by_id('foo.bar') do
+             ^^^^^^^^^^^^^^^^^^^^^ Redundant `within find_by_id(...)` call detected.
+      end
+    RUBY
+
+    expect_correction(<<~'RUBY')
+      within '#foo\.bar' do
+      end
+    RUBY
+  end
+
   it 'registers an offense when using `within find_by_id(...)` with ' \
      'other argument' do
     expect_offense(<<~RUBY)