Merge pull request #77 from rubocop/feature/75

Add new `Capybara/RedundantWithinFind` cop

Author: ydah

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - Change to default `EnforcedStyle: link_or_button` for `Capybara/ClickLinkOrButtonStyle` cop. ([@ydah])
 - Fix a false negative for `RSpec/HaveSelector` when first argument is dstr node. ([@ydah])
+- Add new `Capybara/RedundantWithinFind` cop. ([@ydah])
 - Fix an invalid attributes parse when name with multiple `[]` for `Capybara/SpecificFinders` and `Capybara/SpecificActions` and `Capybara/SpecificMatcher`. ([@ydah])
 
 ## 2.19.0 (2023-09-20)

config/default.yml

@@ -43,6 +43,12 @@ Capybara/NegationMatcher:
     - not_to
   Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/NegationMatcher
 
+Capybara/RedundantWithinFind:
+  Description: Checks for redundant `within find(...)` calls.
+  Enabled: pending
+  VersionAdded: "<<next>>"
+  Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/RedundantWithinFind
+
 Capybara/SpecificActions:
   Description: Checks for there is a more specific actions offered by Capybara.
   Enabled: pending

docs/modules/ROOT/pages/cops.adoc

@@ -6,6 +6,7 @@
 * xref:cops_capybara.adoc#capybaracurrentpathexpectation[Capybara/CurrentPathExpectation]
 * xref:cops_capybara.adoc#capybaramatchstyle[Capybara/MatchStyle]
 * xref:cops_capybara.adoc#capybaranegationmatcher[Capybara/NegationMatcher]
+* xref:cops_capybara.adoc#capybararedundantwithinfind[Capybara/RedundantWithinFind]
 * xref:cops_capybara.adoc#capybaraspecificactions[Capybara/SpecificActions]
 * xref:cops_capybara.adoc#capybaraspecificfinders[Capybara/SpecificFinders]
 * xref:cops_capybara.adoc#capybaraspecificmatcher[Capybara/SpecificMatcher]

docs/modules/ROOT/pages/cops_capybara.adoc

@@ -217,6 +217,49 @@ expect(page).to have_no_css('a')
 
 * https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/NegationMatcher
 
+== Capybara/RedundantWithinFind
+
+|===
+| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed
+
+| Pending
+| Yes
+| Yes
+| <<next>>
+| -
+|===
+
+Checks for redundant `within find(...)` calls.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+within find('foo.bar') do
+  # ...
+end
+
+# good
+within 'foo.bar' do
+  # ...
+end
+
+# bad
+within find_by_id('foo') do
+  # ...
+end
+
+# good
+within '#foo' do
+  # ...
+end
+----
+
+=== References
+
+* https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/RedundantWithinFind
+
 == Capybara/SpecificActions
 
 |===

lib/rubocop/cop/capybara/redundant_within_find.rb

@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Capybara
+      # Checks for redundant `within find(...)` calls.
+      #
+      # @example
+      #   # bad
+      #   within find('foo.bar') do
+      #     # ...
+      #   end
+      #
+      #   # good
+      #   within 'foo.bar' do
+      #     # ...
+      #   end
+      #
+      #   # bad
+      #   within find_by_id('foo') do
+      #     # ...
+      #   end
+      #
+      #   # good
+      #   within '#foo' do
+      #     # ...
+      #   end
+      #
+      class RedundantWithinFind < ::RuboCop::Cop::Base
+        extend AutoCorrector
+        MSG = 'Redundant `within %<method>s(...)` call detected.'
+        RESTRICT_ON_SEND = %i[within].freeze
+        FIND_METHODS = Set.new(%i[find find_by_id]).freeze
+
+        # @!method within_find(node)
+        def_node_matcher :within_find, <<~PATTERN
+          (send nil? :within
+            $(send nil? %FIND_METHODS ...))
+        PATTERN
+
+        def on_send(node)
+          within_find(node) do |find_node|
+            add_offense(find_node, message: msg(find_node)) do |corrector|
+              corrector.replace(find_node, replaced(find_node))
+            end
+          end
+        end
+
+        private
+
+        def msg(node)
+          format(MSG, method: node.method_name)
+        end
+
+        def replaced(node)
+          replaced = node.arguments.map(&:source).join(', ')
+          if node.method?(:find_by_id)
+            replaced.sub(/\A(["'])/, '\1#')
+          else
+            replaced
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/capybara_cops.rb

@@ -7,6 +7,7 @@
 require_relative 'capybara/current_path_expectation'
 require_relative 'capybara/match_style'
 require_relative 'capybara/negation_matcher'
+require_relative 'capybara/redundant_within_find'
 require_relative 'capybara/specific_actions'
 require_relative 'capybara/specific_finders'
 require_relative 'capybara/specific_matcher'

spec/rubocop/cop/capybara/redundant_within_find_spec.rb

@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Capybara::RedundantWithinFind, :config do
+  it 'registers an offense when using `within find(...)`' do
+    expect_offense(<<~RUBY)
+      within find('foo.bar') do
+             ^^^^^^^^^^^^^^^ Redundant `within find(...)` call detected.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      within 'foo.bar' do
+      end
+    RUBY
+  end
+
+  it 'registers an offense when using `within(find ...)`' do
+    expect_offense(<<~RUBY)
+      within(find 'foo.bar') do
+             ^^^^^^^^^^^^^^ Redundant `within find(...)` call detected.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      within('foo.bar') do
+      end
+    RUBY
+  end
+
+  it 'registers an offense when using `within find(...)` with other argument' do
+    expect_offense(<<~RUBY)
+      within find('foo.bar', visible: false) do
+             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Redundant `within find(...)` call detected.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      within 'foo.bar', visible: false do
+      end
+    RUBY
+  end
+
+  it 'registers an offense when using `within find_by_id(...)`' do
+    expect_offense(<<~RUBY)
+      within find_by_id('foo') do
+             ^^^^^^^^^^^^^^^^^ Redundant `within find_by_id(...)` call detected.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      within '#foo' do
+      end
+    RUBY
+  end
+
+  it 'registers an offense when using `within(find_by_id ...)`' do
+    expect_offense(<<~RUBY)
+      within(find_by_id 'foo') do
+             ^^^^^^^^^^^^^^^^ Redundant `within find_by_id(...)` call detected.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      within('#foo') do
+      end
+    RUBY
+  end
+
+  it 'registers an offense when using `within find_by_id(...)` with ' \
+     'other argument' do
+    expect_offense(<<~RUBY)
+      within find_by_id('foo', visible: false) do
+             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Redundant `within find_by_id(...)` call detected.
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      within '#foo', visible: false do
+      end
+    RUBY
+  end
+
+  it 'does not register an offense when using `within` without `find`' do
+    expect_no_offenses(<<~RUBY)
+      within 'foo.bar' do
+      end
+    RUBY
+  end
+end