Fix an incorrect autocorrect for Capybara/RedundantWithinFind when …

…escape required css selector

Fix: #136

Previously, the following automatic corrections were made.

before:
```ruby
within find_by_id("array-form-session.dates") do
  expect(page).to have_text(:visible, "YYYY/MM/DD")
end
```

after:
```ruby
within "#array-form-session.dates" do
  expect(page).to have_text(:visible, "YYYY/MM/DD")
end
```

This is `.' in find_by_id. ` has the same meaning as the escaped id.
In other words, when replacing within, the `. ` must be escaped.

This PR has been modified so that escaping is correctly done in selectors that require escaping.
This will prevent the behavior from changing before and after the automatic correction.

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,47 @@ 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
+        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)