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

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,42 @@
       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