diff --git a/CHANGELOG.md b/CHANGELOG.md index fa954f0ff..32f51073c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/lib/rubocop/cop/rspec/capybara/specific_matcher.rb b/lib/rubocop/cop/rspec/capybara/specific_matcher.rb index 99594ef3a..68ca7f693 100644 --- a/lib/rubocop/cop/rspec/capybara/specific_matcher.rb +++ b/lib/rubocop/cop/rspec/capybara/specific_matcher.rb @@ -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 `%s` over `%s`.' RESTRICT_ON_SEND = %i[have_selector have_no_selector have_css have_no_css].freeze @@ -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 @@ -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 @@ -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? @@ -104,6 +104,31 @@ def specific_matcher_option?(node, arg, matcher) end end + def specific_matcher_pseudo_classes?(arg) + CssSelector.pseudo_classes(arg).all? do |pseudo_class| + replaceable_pseudo_class?(pseudo_class, arg) + end + end + + def replaceable_pseudo_class?(pseudo_class, arg) + unless SPECIFIC_MATCHER_PSEUDO_CLASSES.include?(pseudo_class) + return false + end + + 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) diff --git a/lib/rubocop/cop/rspec/mixin/css_selector.rb b/lib/rubocop/cop/rspec/mixin/css_selector.rb index 8d64358e9..fbdf9ddb6 100644 --- a/lib/rubocop/cop/rspec/mixin/css_selector.rb +++ b/lib/rubocop/cop/rspec/mixin/css_selector.rb @@ -55,6 +55,17 @@ def common_attributes?(selector) attributes(selector).keys.difference(COMMON_OPTIONS).none? end + # @param selector [String] + # @return [Array] + # @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 diff --git a/spec/rubocop/cop/rspec/capybara/specific_matcher_spec.rb b/spec/rubocop/cop/rspec/capybara/specific_matcher_spec.rb index 217731eb8..605c361ca 100644 --- a/spec/rubocop/cop/rspec/capybara/specific_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/capybara/specific_matcher_spec.rb @@ -194,8 +194,6 @@ 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 @@ -203,8 +201,49 @@ class style visible obscured exact exact_text normalize_ws match wait 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