Skip to content

Commit

Permalink
Fix a false positive for RSpec/Capybara/SpecificMatcher when pseudo…
Browse files Browse the repository at this point in the history
…-classes

Resolve: #1351
  • Loading branch information
ydah committed Sep 1, 2022
1 parent 6b183b7 commit 808391d
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -17,6 +17,7 @@
* Fix incorrect documentation URLs when using `rubocop --show-docs-url`. ([@r7kamura][])
* Add `AllowedGroups` configuration option to `RSpec/NestedGroups`. ([@ydah][])
* Deprecate `IgnoredPatterns` option in favor of the `AllowedPatterns` options. ([@ydah][])
* Fix a false positive for `RSpec/Capybara/SpecificMatcher` when pseudo-classes. ([@ydah][])

## 2.12.1 (2022-07-03)

Expand Down
35 changes: 30 additions & 5 deletions lib/rubocop/cop/rspec/capybara/specific_matcher.rb
Expand Up @@ -26,7 +26,7 @@ module Capybara
# expect(page).to have_select
# expect(page).to have_field('foo')
#
class SpecificMatcher < Base
class SpecificMatcher < Base # rubocop:disable Metrics/ClassLength
MSG = 'Prefer `%<good_matcher>s` over `%<bad_matcher>s`.'
RESTRICT_ON_SEND = %i[have_selector have_no_selector have_css
have_no_css].freeze
Expand Down Expand Up @@ -62,6 +62,9 @@ class SpecificMatcher < Base
]
).freeze
}.freeze
SPECIFIC_MATCHER_PSEUDO_CLASSES = %w[
not() disabled enabled checked unchecked
].freeze

# @!method first_argument(node)
def_node_matcher :first_argument, <<-PATTERN
Expand All @@ -78,6 +81,7 @@ def on_send(node)
next unless (matcher = specific_matcher(arg))
next if CssSelector.multiple_selectors?(arg)
next unless specific_matcher_option?(node, arg, matcher)
next unless specific_matcher_pseudo_classes?(arg)

add_offense(node, message: message(node, matcher))
end
Expand All @@ -90,10 +94,6 @@ def specific_matcher(arg)
SPECIFIC_MATCHER[splitted_arg]
end

def acceptable_pattern?(arg)
arg.match?(/[ >,+]/)
end

def specific_matcher_option?(node, arg, matcher)
attrs = CssSelector.attributes(arg).keys
return true if attrs.empty?
Expand All @@ -104,6 +104,31 @@ def specific_matcher_option?(node, arg, matcher)
end
end

def specific_matcher_pseudo_classes?(arg)
pseudo_classes = CssSelector.pseudo_classes(arg)
return true if pseudo_classes.empty?

pseudo_classes.each.all? do |pseudo_class|
SPECIFIC_MATCHER_PSEUDO_CLASSES.include?(pseudo_class) &&
replaceable_pseudo_class?(pseudo_class, arg)
end
end

def replaceable_pseudo_class?(pseudo_class, arg)
case pseudo_class
when 'not()' then replaceable_pseudo_class_not?(arg)
else true
end
end

def replaceable_pseudo_class_not?(arg)
arg.scan(/not\(.*?\)/).all? do |not_arg|
CssSelector.attributes(not_arg).values.all? do |v|
v.is_a?(TrueClass) || v.is_a?(FalseClass)
end
end
end

def replaceable_matcher?(node, matcher, attrs)
case matcher
when 'link' then replaceable_to_have_link?(node, attrs)
Expand Down
11 changes: 11 additions & 0 deletions lib/rubocop/cop/rspec/mixin/css_selector.rb
Expand Up @@ -55,6 +55,17 @@ def common_attributes?(selector)
attributes(selector).keys.difference(COMMON_OPTIONS).none?
end

# @param selector [String]
# @return [Array<String>]
# @example
# pseudo_classes('button:not([disabled])') # => ['not()']
# pseudo_classes('a:enabled:not([valid])') # => ['enabled', 'not()']
def pseudo_classes(selector)
# Attributes must be excluded or else the colon in the `href`s URL
# will also be picked up as pseudo classes.
selector.gsub(/\[.*?\]/, '').scan(/:(.*)/).flatten.join.split(':')
end

# @param selector [String]
# @return [Boolean]
# @example
Expand Down
43 changes: 41 additions & 2 deletions spec/rubocop/cop/rspec/capybara/specific_matcher_spec.rb
Expand Up @@ -194,17 +194,56 @@ class style visible obscured exact exact_text normalize_ws match wait
expect_offense(<<-RUBY)
expect(page).to have_css('button[disabled][name="foo"]', exact_text: 'foo')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`.
expect(page).to have_css('button:not([name="foo"][disabled])', exact_text: 'bar')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`.
RUBY
end

it 'registers an offense when using abstract matcher with state' do
expect_offense(<<-RUBY)
expect(page).to have_css('button[disabled]', exact_text: 'foo')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`.
RUBY
end

it 'registers an offense when using abstract matcher with ' \
'first argument is element with replaceable pseudo-classes' do
expect_offense(<<-RUBY)
expect(page).to have_css('button:not([disabled])', exact_text: 'bar')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`.
expect(page).to have_css('button:not([disabled][visible])')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`.
RUBY
end

it 'registers an offense when using abstract matcher with ' \
'first argument is element with multiple replaceable pseudo-classes' do
expect_offense(<<-RUBY)
expect(page).to have_css('button:not([disabled]):enabled')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`.
expect(page).to have_css('button:not([disabled=false]):disabled')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`.
expect(page).to have_css('button:not([disabled]):not([disabled])')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`.
expect(page).to have_css('input:not([unchecked]):checked')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_field` over `have_css`.
expect(page).to have_css('input:not([unchecked=false]):unchecked')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_field` over `have_css`.
expect(page).to have_css('input:not([unchecked]):not([unchecked])')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_field` over `have_css`.
RUBY
end

it 'does not register an offense when using abstract matcher with ' \
'first argument is element with replaceable pseudo-classes' \
'and not boolean attributes' do
expect_no_offenses(<<-RUBY)
expect(page).to have_css('button:not([name="foo"][disabled])')
RUBY
end

it 'does not register an offense when using abstract matcher with ' \
'first argument is element with multiple nonreplaceable pseudo-classes' do
expect_no_offenses(<<-RUBY)
expect(page).to have_css('button:first-of-type:not([disabled])')
RUBY
end

Expand Down

0 comments on commit 808391d

Please sign in to comment.