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.

Author: ydah

CHANGELOG.md

@@ -16,6 +16,7 @@
 - Add a new `Capybara/NegationMatcherAfterVisit` cop. ([@ydah])
 - Fix an error for `Capybara/RSpec/HaveSelector` when passing no arguments. ([@earlopain])
 - Make RuboCop Capybara work as a RuboCop plugin. ([@bquorning])
+- 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

@@ -83,6 +83,18 @@ def multiple_selectors?(selector)
           normalize = selector.gsub(/(\\[>,+~]|\(.*\))/, '')
           normalize.match?(/[ >,+~]/)
         end
+
+        # @param str [String]
+        # @param quote [String] the quote character used (' or ")
+        # @return [String]
+        # @example
+        #   css_escape('foo.bar', "'") # => 'foo\.bar'
+        #   css_escape('foo.bar', '"') # => 'foo\\.bar'
+        #   css_escape('foo', "'") # => 'foo'
+        def css_escape(str, quote = "'")
+          escaped_dot = quote == '"' ? '\\\\\\\\.' : '\\\\.'
+          str.gsub('.', escaped_dot)
+        end
       end
     end
   end

lib/rubocop/cop/capybara/redundant_within_find.rb

@@ -27,6 +27,7 @@ module Capybara
       #   end
       #
       class RedundantWithinFind < ::RuboCop::Cop::Base
+        include CssSelector
         extend AutoCorrector
         MSG = 'Redundant `within %<method>s(...)` call detected.'
         RESTRICT_ON_SEND = %i[within].freeze
@@ -53,13 +54,25 @@ def msg(node)
         end
 
         def replaced(node)
-          replaced = node.arguments.map(&:source).join(', ')
-          if node.method?(:find_by_id)
-            replaced.sub(/\A(["'])/, '\1#')
+          unless node.method?(:find_by_id)
+            return node.arguments.map(&:source).join(', ')
+          end
+
+          if node.first_argument.str_type?
+            build_escaped_selector(node.first_argument, node)
           else
-            replaced
+            node.arguments.map(&:source).join(', ')
+              .sub(/\A(["'])/, '\1#')
           end
         end
+
+        def build_escaped_selector(first_arg, node)
+          quote = first_arg.source[0]
+          escaped_id = CssSelector.css_escape(first_arg.value, quote)
+          rest_args = node.arguments.drop(1).map(&:source)
+
+          ["#{quote}##{escaped_id}#{quote}", *rest_args].join(', ')
+        end
       end
     end
   end

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

@@ -143,4 +143,40 @@
       expect(described_class.multiple_selectors?('a.cls\>b')).to be false
     end
   end
+
+  describe 'CssSelector.css_escape' do
+    context 'with single quote' do
+      it 'escapes dots with single backslash' do
+        expect(described_class.css_escape('foo.bar', "'")).to eq 'foo\\.bar'
+      end
+
+      it 'does not escape when no dots' do
+        expect(described_class.css_escape('foo', "'")).to eq 'foo'
+      end
+
+      it 'escapes multiple dots' do
+        expect(described_class.css_escape('foo.bar.baz', "'")).to eq 'foo\\.bar\\.baz'
+      end
+    end
+
+    context 'with double quote' do
+      it 'escapes dots with double backslash for double-quoted strings' do
+        expect(described_class.css_escape('foo.bar', '"')).to eq 'foo\\\\.bar'
+      end
+
+      it 'does not escape when no dots' do
+        expect(described_class.css_escape('foo', '"')).to eq 'foo'
+      end
+
+      it 'escapes multiple dots' do
+        expect(described_class.css_escape('foo.bar.baz', '"')).to eq 'foo\\\\.bar\\\\.baz'
+      end
+    end
+
+    context 'with default quote (single quote)' do
+      it 'uses single quote escaping by default' do
+        expect(described_class.css_escape('foo.bar')).to eq 'foo\\.bar'
+      end
+    end
+  end
 end

spec/rubocop/cop/capybara/redundant_within_find_spec.rb

@@ -66,6 +66,24 @@
     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
+      within find_by_id("foo.bar") do
+             ^^^^^^^^^^^^^^^^^^^^^ Redundant `within find_by_id(...)` call detected.
+      end
+    RUBY
+
+    expect_correction(<<~'RUBY')
+      within '#foo\.bar' do
+      end
+      within "#foo\\.bar" do
+      end
+    RUBY
+  end
+
   it 'registers an offense when using `within find_by_id(...)` with ' \
      'other argument' do
     expect_offense(<<~RUBY)
@@ -80,6 +98,19 @@
     RUBY
   end
 
+  it 'registers an offense when using `within find_by_id(variable)`' do
+    expect_offense(<<~RUBY)
+      within find_by_id(id_variable) do
+             ^^^^^^^^^^^^^^^^^^^^^^^ Redundant `within find_by_id(...)` call detected.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      within id_variable do
+      end
+    RUBY
+  end
+
   it 'does not register an offense when using `within` without `find`' do
     expect_no_offenses(<<~RUBY)
       within 'foo.bar' do