diff --git a/CHANGELOG.md b/CHANGELOG.md index 32a41da1..22a4c568 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/lib/rubocop/cop/capybara/mixin/css_selector.rb b/lib/rubocop/cop/capybara/mixin/css_selector.rb index 43a264ef..a4caf97a 100644 --- a/lib/rubocop/cop/capybara/mixin/css_selector.rb +++ b/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 diff --git a/lib/rubocop/cop/capybara/redundant_within_find.rb b/lib/rubocop/cop/capybara/redundant_within_find.rb index 8e0085b2..db9da94a 100644 --- a/lib/rubocop/cop/capybara/redundant_within_find.rb +++ b/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 %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 diff --git a/spec/rubocop/cop/capybara/mixin/css_selector_spec.rb b/spec/rubocop/cop/capybara/mixin/css_selector_spec.rb index af7cda24..5b0f5b3b 100644 --- a/spec/rubocop/cop/capybara/mixin/css_selector_spec.rb +++ b/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 diff --git a/spec/rubocop/cop/capybara/redundant_within_find_spec.rb b/spec/rubocop/cop/capybara/redundant_within_find_spec.rb index 9de7f973..dc51cc34 100644 --- a/spec/rubocop/cop/capybara/redundant_within_find_spec.rb +++ b/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