Merge pull request #65 from rubocop/fix-specific-finders

Fix a false positive for `Capybara/SpecificFinders` when `find` with kind option

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - Drop Ruby 2.6 support. ([@ydah])
 - Add new `Capybara/RSpec/PredicateMatcher` cop. ([@ydah])
+- Fix a false positive for `Capybara/SpecificFinders` when `find` with kind option. ([@ydah])
 
 ## 2.18.0 (2023-04-21)
 

docs/modules/ROOT/pages/cops_capybara.adoc

@@ -212,6 +212,7 @@ Checks if there is a more specific finder offered by Capybara.
 # bad
 find('#some-id')
 find('[id=some-id]')
+find(:css, '#some-id')
 
 # good
 find_by_id('some-id')

lib/rubocop/cop/capybara/specific_finders.rb

@@ -9,21 +9,21 @@ module Capybara
       #   # bad
       #   find('#some-id')
       #   find('[id=some-id]')
+      #   find(:css, '#some-id')
       #
       #   # good
       #   find_by_id('some-id')
       #
       class SpecificFinders < ::RuboCop::Cop::Base
         extend AutoCorrector
-
         include RangeHelp
 
         MSG = 'Prefer `find_by_id` over `find`.'
         RESTRICT_ON_SEND = %i[find].freeze
 
         # @!method find_argument(node)
         def_node_matcher :find_argument, <<~PATTERN
-          (send _ :find (str $_) ...)
+          (send _ :find $(sym :css)? (str $_) ...)
         PATTERN
 
         # @!method class_options(node)
@@ -32,30 +32,30 @@ class SpecificFinders < ::RuboCop::Cop::Base
         PATTERN
 
         def on_send(node)
-          find_argument(node) do |arg|
+          find_argument(node) do |sym, arg|
             next if CssSelector.pseudo_classes(arg).any?
             next if CssSelector.multiple_selectors?(arg)
 
-            on_attr(node, arg) if attribute?(arg)
-            on_id(node, arg) if CssSelector.id?(arg)
+            on_attr(node, sym, arg) if attribute?(arg)
+            on_id(node, sym, arg) if CssSelector.id?(arg)
           end
         end
 
         private
 
-        def on_attr(node, arg)
+        def on_attr(node, sym, arg)
           attrs = CssSelector.attributes(arg)
           return unless (id = attrs['id'])
           return if attrs['class']
 
-          register_offense(node, replaced_arguments(arg, id))
+          register_offense(node, sym, replaced_arguments(arg, id))
         end
 
-        def on_id(node, arg)
+        def on_id(node, sym, arg)
           return if CssSelector.attributes(arg).any?
 
           id = CssSelector.id(arg)
-          register_offense(node, "'#{id}'",
+          register_offense(node, sym, "'#{id}'",
                            CssSelector.classes(arg.sub("##{id}", '')))
         end
 
@@ -64,17 +64,22 @@ def attribute?(arg)
             CapybaraHelp.common_attributes?(arg)
         end
 
-        def register_offense(node, id, classes = [])
+        def register_offense(node, sym, id, classes = [])
           add_offense(offense_range(node)) do |corrector|
             corrector.replace(node.loc.selector, 'find_by_id')
-            corrector.replace(node.first_argument,
-                              id.delete('\\'))
+            corrector.replace(node.first_argument, id.delete('\\'))
             unless classes.compact.empty?
               autocorrect_classes(corrector, node, classes)
             end
+            corrector.remove(deletion_range(node)) unless sym.empty?
           end
         end
 
+        def deletion_range(node)
+          range_between(node.arguments[0].source_range.end_pos,
+                        node.arguments[1].source_range.end_pos)
+        end
+
         def autocorrect_classes(corrector, node, classes)
           if (options = class_options(node).first)
             append_options(classes, options)

spec/rubocop/cop/capybara/specific_finders_spec.rb

@@ -12,6 +12,29 @@
     RUBY
   end
 
+  it 'registers an offense when using `find` with kind' do
+    expect_offense(<<~RUBY)
+      find(:css, '#some-id')
+      ^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by_id` over `find`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      find_by_id('some-id')
+    RUBY
+  end
+
+  it 'registers an offense when using `find` with kind ' \
+     'and option' do
+    expect_offense(<<~RUBY)
+      find(:css, '#some-id', class: 'some-cls')
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by_id` over `find`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      find_by_id('some-id', class: 'some-cls')
+    RUBY
+  end
+
   it 'registers an offense when using `find` with no parentheses' do
     expect_offense(<<~RUBY)
       find "#some-id"
@@ -152,6 +175,30 @@
     RUBY
   end
 
+  it 'registers an offense when using `find` ' \
+     'with argument is kind and attribute specified id' do
+    expect_offense(<<~RUBY)
+      find(:css, '[id=some-id]')
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by_id` over `find`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      find_by_id('some-id')
+    RUBY
+  end
+
+  it 'registers an offense when using `find` ' \
+     'with argument is kind and attribute specified id and option' do
+    expect_offense(<<~RUBY)
+      find(:css, '[id=some-id]', class: 'some-cls')
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by_id` over `find`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      find_by_id('some-id', class: 'some-cls')
+    RUBY
+  end
+
   it 'registers an offense when using `find ' \
      'with argument is attribute specified id surrounded by quotation' do
     expect_offense(<<~RUBY)