Merge pull request #1402 from ydah/feature/1326

Add new `RSpec/Capybara/SpecificActions` cop

.rubocop.yml

@@ -126,6 +126,8 @@ RSpec/VerifiedDoubleReference:
   Enabled: true
 RSpec/Capybara/NegationMatcher:
   Enabled: true
+RSpec/Capybara/SpecificActions:
+  Enabled: true
 RSpec/Capybara/SpecificFinders:
   Enabled: true
 RSpec/Capybara/SpecificMatcher:

CHANGELOG.md

@@ -10,6 +10,7 @@
 * Improve `RSpec/NoExpectationExample` cop to ignore examples skipped or pending via metatada.  ([@pirj][])
 * Add `RSpec/FactoryBot/ConsistentParenthesesStyle` cop. ([@Liberatys][])
 * Add `RSpec/Rails/InferredSpecType` cop. ([@r7kamura][])
+* Add new `RSpec/Capybara/SpecificActions` cop. ([@ydah][])
 
 ## 2.13.2 (2022-09-23)
 

config/default.yml

@@ -883,6 +883,12 @@ RSpec/Capybara/NegationMatcher:
     - not_to
   Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/NegationMatcher
 
+RSpec/Capybara/SpecificActions:
+  Description: Checks for there is a more specific actions offered by Capybara.
+  Enabled: pending
+  VersionAdded: '2.14'
+  Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/SpecificActions
+
 RSpec/Capybara/SpecificFinders:
   Description: Checks if there is a more specific finder offered by Capybara.
   Enabled: pending

docs/modules/ROOT/pages/cops.adoc

@@ -96,6 +96,7 @@
 * xref:cops_rspec_capybara.adoc#rspeccapybara/currentpathexpectation[RSpec/Capybara/CurrentPathExpectation]
 * xref:cops_rspec_capybara.adoc#rspeccapybara/featuremethods[RSpec/Capybara/FeatureMethods]
 * xref:cops_rspec_capybara.adoc#rspeccapybara/negationmatcher[RSpec/Capybara/NegationMatcher]
+* xref:cops_rspec_capybara.adoc#rspeccapybara/specificactions[RSpec/Capybara/SpecificActions]
 * xref:cops_rspec_capybara.adoc#rspeccapybara/specificfinders[RSpec/Capybara/SpecificFinders]
 * xref:cops_rspec_capybara.adoc#rspeccapybara/specificmatcher[RSpec/Capybara/SpecificMatcher]
 * xref:cops_rspec_capybara.adoc#rspeccapybara/visibilitymatcher[RSpec/Capybara/VisibilityMatcher]

docs/modules/ROOT/pages/cops_rspec_capybara.adoc

@@ -168,6 +168,41 @@ expect(page).to have_no_css('a')
 
 * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/NegationMatcher
 
+== RSpec/Capybara/SpecificActions
+
+|===
+| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed
+
+| Pending
+| Yes
+| No
+| 2.14
+| -
+|===
+
+Checks for there is a more specific actions offered by Capybara.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+find('a').click
+find('button.cls').click
+find('a', exact_text: 'foo').click
+find('div button').click
+
+# good
+click_link
+click_button(class: 'cls')
+click_link(exact_text: 'foo')
+find('div').click_button
+----
+
+=== References
+
+* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/SpecificActions
+
 == RSpec/Capybara/SpecificFinders
 
 |===

lib/rubocop-rspec.rb

@@ -24,6 +24,7 @@
 require_relative 'rubocop/cop/rspec/mixin/namespace'
 require_relative 'rubocop/cop/rspec/mixin/css_selector'
 require_relative 'rubocop/cop/rspec/mixin/skip_or_pending'
+require_relative 'rubocop/cop/rspec/mixin/capybara_help'
 
 require_relative 'rubocop/rspec/concept'
 require_relative 'rubocop/rspec/example_group'

lib/rubocop/cop/rspec/capybara/specific_actions.rb

@@ -0,0 +1,85 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      module Capybara
+        # Checks for there is a more specific actions offered by Capybara.
+        #
+        # @example
+        #
+        #   # bad
+        #   find('a').click
+        #   find('button.cls').click
+        #   find('a', exact_text: 'foo').click
+        #   find('div button').click
+        #
+        #   # good
+        #   click_link
+        #   click_button(class: 'cls')
+        #   click_link(exact_text: 'foo')
+        #   find('div').click_button
+        #
+        class SpecificActions < Base
+          MSG = "Prefer `%<good_action>s` over `find('%<selector>s').click`."
+          RESTRICT_ON_SEND = %i[click].freeze
+          SPECIFIC_ACTION = {
+            'button' => 'button',
+            'a' => 'link'
+          }.freeze
+
+          # @!method click_on_selector(node)
+          def_node_matcher :click_on_selector, <<-PATTERN
+            (send _ :find (str $_) ...)
+          PATTERN
+
+          def on_send(node)
+            click_on_selector(node.receiver) do |arg|
+              next unless supported_selector?(arg)
+              # Always check the last selector in the case of multiple selectors
+              # separated by whitespace.
+              # because the `.click` is executed on the element to
+              # which the last selector points.
+              next unless (selector = last_selector(arg))
+              next unless (action = specific_action(selector))
+              next unless CapybaraHelp.specific_option?(node.receiver, arg,
+                                                        action)
+              next unless CapybaraHelp.specific_pseudo_classes?(arg)
+
+              range = offense_range(node, node.receiver)
+              add_offense(range, message: message(action, selector))
+            end
+          end
+
+          private
+
+          def specific_action(selector)
+            SPECIFIC_ACTION[last_selector(selector)]
+          end
+
+          def supported_selector?(selector)
+            !selector.match?(/[>,+~]/)
+          end
+
+          def last_selector(arg)
+            arg.split.last[/^\w+/, 0]
+          end
+
+          def offense_range(node, receiver)
+            receiver.loc.selector.with(end_pos: node.loc.expression.end_pos)
+          end
+
+          def message(action, selector)
+            format(MSG,
+                   good_action: good_action(action),
+                   selector: selector)
+          end
+
+          def good_action(action)
+            "click_#{action}"
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rspec/capybara/specific_matcher.rb

@@ -26,7 +26,9 @@ module Capybara
         #   expect(page).to have_select
         #   expect(page).to have_field('foo')
         #
-        class SpecificMatcher < Base # rubocop:disable Metrics/ClassLength
+        class SpecificMatcher < Base
+          include CapybaraHelp
+
           MSG = 'Prefer `%<good_matcher>s` over `%<bad_matcher>s`.'
           RESTRICT_ON_SEND = %i[have_selector have_no_selector have_css
                                 have_no_css].freeze
@@ -37,51 +39,18 @@ class SpecificMatcher < Base # rubocop:disable Metrics/ClassLength
             'select' => 'select',
             'input' => 'field'
           }.freeze
-          SPECIFIC_MATCHER_OPTIONS = {
-            'button' => (
-              CssSelector::COMMON_OPTIONS + %w[disabled name value title type]
-            ).freeze,
-            'link' => (
-              CssSelector::COMMON_OPTIONS + %w[href alt title download]
-            ).freeze,
-            'table' => (
-              CssSelector::COMMON_OPTIONS + %w[
-                caption with_cols cols with_rows rows
-              ]
-            ).freeze,
-            'select' => (
-              CssSelector::COMMON_OPTIONS + %w[
-                disabled name placeholder options enabled_options
-                disabled_options selected with_selected multiple with_options
-              ]
-            ).freeze,
-            'field' => (
-              CssSelector::COMMON_OPTIONS + %w[
-                checked unchecked disabled valid name placeholder
-                validation_message readonly with type multiple
-              ]
-            ).freeze
-          }.freeze
-          SPECIFIC_MATCHER_PSEUDO_CLASSES = %w[
-            not() disabled enabled checked unchecked
-          ].freeze
 
           # @!method first_argument(node)
           def_node_matcher :first_argument, <<-PATTERN
             (send nil? _ (str $_) ... )
           PATTERN
 
-          # @!method option?(node)
-          def_node_search :option?, <<-PATTERN
-            (pair (sym %) _)
-          PATTERN
-
           def on_send(node)
             first_argument(node) do |arg|
               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)
+              next unless specific_option?(node, arg, matcher)
+              next unless specific_pseudo_classes?(arg)
 
               add_offense(node, message: message(node, matcher))
             end
@@ -94,51 +63,6 @@ def specific_matcher(arg)
             SPECIFIC_MATCHER[splitted_arg]
           end
 
-          def specific_matcher_option?(node, arg, matcher)
-            attrs = CssSelector.attributes(arg).keys
-            return false unless replaceable_matcher?(node, matcher, attrs)
-
-            attrs.all? do |attr|
-              SPECIFIC_MATCHER_OPTIONS.fetch(matcher, []).include?(attr)
-            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)
-            else true
-            end
-          end
-
-          def replaceable_to_have_link?(node, attrs)
-            option?(node, :href) || attrs.include?('href')
-          end
-
           def message(node, matcher)
             format(MSG,
                    good_matcher: good_matcher(node, matcher),

lib/rubocop/cop/rspec/mixin/capybara_help.rb

@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      # Help methods for capybara.
+      module CapybaraHelp
+        module_function
+
+        # @param node [RuboCop::AST::SendNode]
+        # @param locator [String]
+        # @param element [String]
+        # @return [Boolean]
+        def specific_option?(node, locator, element)
+          attrs = CssSelector.attributes(locator).keys
+          return false unless replaceable_element?(node, element, attrs)
+
+          attrs.all? do |attr|
+            CssSelector.specific_options?(element, attr)
+          end
+        end
+
+        # @param locator [String]
+        # @return [Boolean]
+        def specific_pseudo_classes?(locator)
+          CssSelector.pseudo_classes(locator).all? do |pseudo_class|
+            replaceable_pseudo_class?(pseudo_class, locator)
+          end
+        end
+
+        # @param pseudo_class [String]
+        # @param locator [String]
+        # @return [Boolean]
+        def replaceable_pseudo_class?(pseudo_class, locator)
+          return false unless CssSelector.specific_pesudo_classes?(pseudo_class)
+
+          case pseudo_class
+          when 'not()' then replaceable_pseudo_class_not?(locator)
+          else true
+          end
+        end
+
+        # @param locator [String]
+        # @return [Boolean]
+        def replaceable_pseudo_class_not?(locator)
+          locator.scan(/not\(.*?\)/).all? do |negation|
+            CssSelector.attributes(negation).values.all? do |v|
+              v.is_a?(TrueClass) || v.is_a?(FalseClass)
+            end
+          end
+        end
+
+        # @param node [RuboCop::AST::SendNode]
+        # @param element [String]
+        # @param attrs [Array<String>]
+        # @return [Boolean]
+        def replaceable_element?(node, element, attrs)
+          case element
+          when 'link' then replaceable_to_link?(node, attrs)
+          else true
+          end
+        end
+
+        # @param node [RuboCop::AST::SendNode]
+        # @param attrs [Array<String>]
+        # @return [Boolean]
+        def replaceable_to_link?(node, attrs)
+          include_option?(node, :href) || attrs.include?('href')
+        end
+
+        # @param node [RuboCop::AST::SendNode]
+        # @param option [Symbol]
+        # @return [Boolean]
+        def include_option?(node, option)
+          node.each_descendant(:sym).find { |opt| opt.value == option }
+        end
+      end
+    end
+  end
+end