Skip to content

Commit afe96d5

Browse files
committed
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.
1 parent cc0a743 commit afe96d5

File tree

4 files changed

+48
-4
lines changed

4 files changed

+48
-4
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
- Add a new `Capybara/NegationMatcherAfterVisit` cop. ([@ydah])
1717
- Fix an error for `Capybara/RSpec/HaveSelector` when passing no arguments. ([@earlopain])
1818
- Make RuboCop Capybara work as a RuboCop plugin. ([@bquorning])
19+
- Fix an incorrect autocorrect for `Capybara/RedundantWithinFind` when escape required css selector. ([@ydah])
1920

2021
## 2.21.0 (2024-06-08)
2122

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,18 @@ def multiple_selectors?(selector)
8383
normalize = selector.gsub(/(\\[>,+~]|\(.*\))/, '')
8484
normalize.match?(/[ >,+~]/)
8585
end
86+
87+
# @param str [String]
88+
# @param quote [String] the quote character used (' or ")
89+
# @return [String]
90+
# @example
91+
# css_escape('foo.bar', "'") # => 'foo\.bar'
92+
# css_escape('foo.bar', '"') # => 'foo\\.bar'
93+
# css_escape('foo', "'") # => 'foo'
94+
def css_escape(str, quote = "'")
95+
escaped_dot = quote == '"' ? '\\\\\\\\.' : '\\\\.'
96+
str.gsub('.', escaped_dot)
97+
end
8698
end
8799
end
88100
end

lib/rubocop/cop/capybara/redundant_within_find.rb

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ module Capybara
2727
# end
2828
#
2929
class RedundantWithinFind < ::RuboCop::Cop::Base
30+
include CssSelector
3031
extend AutoCorrector
3132
MSG = 'Redundant `within %<method>s(...)` call detected.'
3233
RESTRICT_ON_SEND = %i[within].freeze
@@ -53,13 +54,25 @@ def msg(node)
5354
end
5455

5556
def replaced(node)
56-
replaced = node.arguments.map(&:source).join(', ')
57-
if node.method?(:find_by_id)
58-
replaced.sub(/\A(["'])/, '\1#')
57+
unless node.method?(:find_by_id)
58+
return node.arguments.map(&:source).join(', ')
59+
end
60+
61+
if node.first_argument.str_type?
62+
build_escaped_selector(node.first_argument, node)
5963
else
60-
replaced
64+
node.arguments.map(&:source).join(', ').sub(/\A(["'])/,
65+
'\1#')
6166
end
6267
end
68+
69+
def build_escaped_selector(first_arg, node)
70+
quote = first_arg.source[0]
71+
escaped_id = CssSelector.css_escape(first_arg.value, quote)
72+
rest_args = node.arguments.drop(1).map(&:source)
73+
74+
["#{quote}##{escaped_id}#{quote}", *rest_args].join(', ')
75+
end
6376
end
6477
end
6578
end

spec/rubocop/cop/capybara/redundant_within_find_spec.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,24 @@
6666
RUBY
6767
end
6868

69+
it 'registers an offense when using `within find_by_id("foo.bar")`' do
70+
expect_offense(<<~RUBY)
71+
within find_by_id('foo.bar') do
72+
^^^^^^^^^^^^^^^^^^^^^ Redundant `within find_by_id(...)` call detected.
73+
end
74+
within find_by_id("foo.bar") do
75+
^^^^^^^^^^^^^^^^^^^^^ Redundant `within find_by_id(...)` call detected.
76+
end
77+
RUBY
78+
79+
expect_correction(<<~'RUBY')
80+
within '#foo\.bar' do
81+
end
82+
within "#foo\\.bar" do
83+
end
84+
RUBY
85+
end
86+
6987
it 'registers an offense when using `within find_by_id(...)` with ' \
7088
'other argument' do
7189
expect_offense(<<~RUBY)

0 commit comments

Comments
 (0)