Merge pull request #118 from rubocop/add-new-cop

Add new `Capybara/FindAllFirst` cop

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Edge (Unreleased)
 
 - Add `Capybara/AmbiguousClick` cop and make soft-deprecated `Capybara/ClickLinkOrButtonStyle` cop. If you want to use `EnforcedStyle: strict`, use `Capybara/AmbiguousClick` cop instead. ([@ydah])
+- Add new `Capybara/FindAllFirst` cop. ([@ydah])
 
 ## 2.21.0 (2024-06-08)
 

config/default.yml

@@ -33,6 +33,12 @@ Capybara/CurrentPathExpectation:
   VersionChanged: '2.0'
   Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/CurrentPathExpectation
 
+Capybara/FindAllFirst:
+  Description: Enforces use of `first` instead of `all` with `first` or `[0]`.
+  Enabled: pending
+  VersionAdded: "<<next>>"
+  Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/FindAllFirst
+
 Capybara/MatchStyle:
   Description: Checks for usage of deprecated style methods.
   Enabled: pending

docs/modules/ROOT/pages/cops.adoc

@@ -5,6 +5,7 @@
 * xref:cops_capybara.adoc#capybaraambiguousclick[Capybara/AmbiguousClick]
 * xref:cops_capybara.adoc#capybaraclicklinkorbuttonstyle[Capybara/ClickLinkOrButtonStyle]
 * xref:cops_capybara.adoc#capybaracurrentpathexpectation[Capybara/CurrentPathExpectation]
+* xref:cops_capybara.adoc#capybarafindallfirst[Capybara/FindAllFirst]
 * xref:cops_capybara.adoc#capybaramatchstyle[Capybara/MatchStyle]
 * xref:cops_capybara.adoc#capybaranegationmatcher[Capybara/NegationMatcher]
 * xref:cops_capybara.adoc#capybararedundantwithinfind[Capybara/RedundantWithinFind]

docs/modules/ROOT/pages/cops_capybara.adoc

@@ -154,6 +154,38 @@ expect(page).to match(variable)
 
 * https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/CurrentPathExpectation
 
+== Capybara/FindAllFirst
+
+|===
+| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed
+
+| Pending
+| Yes
+| Always
+| <<next>>
+| -
+|===
+
+Enforces use of `first` instead of `all` with `first` or `[0]`.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+all('a').first
+all('a')[0]
+find('a', match: :first)
+all('a', match: :first)
+
+# good
+first('a')
+----
+
+=== References
+
+* https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/FindAllFirst
+
 == Capybara/MatchStyle
 
 |===

lib/rubocop/cop/capybara/find_all_first.rb

@@ -0,0 +1,78 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Capybara
+      # Enforces use of `first` instead of `all` with `first` or `[0]`.
+      #
+      # @example
+      #
+      #   # bad
+      #   all('a').first
+      #   all('a')[0]
+      #   find('a', match: :first)
+      #   all('a', match: :first)
+      #
+      #   # good
+      #   first('a')
+      #
+      class FindAllFirst < ::RuboCop::Cop::Base
+        extend AutoCorrector
+        include RangeHelp
+
+        MSG = 'Use `first(%<selector>s)`.'
+        RESTRICT_ON_SEND = %i[all find].freeze
+
+        # @!method find_all_first?(node)
+        def_node_matcher :find_all_first?, <<~PATTERN
+          {
+            (send (send _ :all _ ...) :first)
+            (send (send _ :all _ ...) :[] (int 0))
+          }
+        PATTERN
+
+        # @!method include_match_first?(node)
+        def_node_matcher :include_match_first?, <<~PATTERN
+          (send _ {:find :all} _ $(hash <(pair (sym :match) (sym :first)) ...>))
+        PATTERN
+
+        def on_send(node)
+          on_all_first(node)
+          on_match_first(node)
+        end
+
+        private
+
+        def on_all_first(node)
+          return unless (parent = node.parent)
+          return unless find_all_first?(parent)
+
+          range = range_between(node.loc.selector.begin_pos,
+                                parent.loc.selector.end_pos)
+          selector = node.arguments.map(&:source).join(', ')
+          add_offense(range,
+                      message: format(MSG, selector: selector)) do |corrector|
+            corrector.replace(range, "first(#{selector})")
+          end
+        end
+
+        def on_match_first(node)
+          include_match_first?(node) do |hash|
+            selector = ([node.first_argument.source] + replaced_hash(hash))
+              .join(', ')
+            add_offense(node,
+                        message: format(MSG, selector: selector)) do |corrector|
+              corrector.replace(node, "first(#{selector})")
+            end
+          end
+        end
+
+        def replaced_hash(hash)
+          hash.child_nodes.flat_map(&:source).reject do |arg|
+            arg == 'match: :first'
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/capybara_cops.rb

@@ -6,6 +6,7 @@
 require_relative 'capybara/ambiguous_click'
 require_relative 'capybara/click_link_or_button_style'
 require_relative 'capybara/current_path_expectation'
+require_relative 'capybara/find_all_first'
 require_relative 'capybara/match_style'
 require_relative 'capybara/negation_matcher'
 require_relative 'capybara/redundant_within_find'

spec/rubocop/cop/capybara/find_all_first_spec.rb

@@ -0,0 +1,121 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Capybara::FindAllFirst, :config do
+  it 'registers an offense when using `all` with `first`' do
+    expect_offense(<<~RUBY)
+      all('a').first
+      ^^^^^^^^^^^^^^ Use `first('a')`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      first('a')
+    RUBY
+  end
+
+  it 'registers an offense when using `all` with `[0]`' do
+    expect_offense(<<~RUBY)
+      all('a')[0]
+      ^^^^^^^^^^^ Use `first('a')`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      first('a')
+    RUBY
+  end
+
+  it 'registers an offense when using `find` with `match: :first`' do
+    expect_offense(<<~RUBY)
+      find('a', match: :first)
+      ^^^^^^^^^^^^^^^^^^^^^^^^ Use `first('a')`.
+      find('a', text: 'b', match: :first)
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `first('a', text: 'b')`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      first('a')
+      first('a', text: 'b')
+    RUBY
+  end
+
+  it 'registers an offense when using `all` with `match: :first`' do
+    expect_offense(<<~RUBY)
+      all('a', match: :first)
+      ^^^^^^^^^^^^^^^^^^^^^^^ Use `first('a')`.
+      all('a', text: 'b', match: :first)
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `first('a', text: 'b')`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      first('a')
+      first('a', text: 'b')
+    RUBY
+  end
+
+  it 'registers an offense when using `all` with argument and `first`' do
+    expect_offense(<<~RUBY)
+      all('a', text: 'b')[0]
+      ^^^^^^^^^^^^^^^^^^^^^^ Use `first('a', text: 'b')`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      first('a', text: 'b')
+    RUBY
+  end
+
+  it 'registers an offense when using `all` with `first` and receiver' do
+    expect_offense(<<~RUBY)
+      page.all('a').first
+           ^^^^^^^^^^^^^^ Use `first('a')`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      page.first('a')
+    RUBY
+  end
+
+  it 'registers an offense when using nested `all` with `first` and receiver' do
+    expect_offense(<<~RUBY)
+      find('a')
+        .all('div')
+         ^^^^^^^^^^ Use `first('div')`.
+          .first
+    RUBY
+
+    expect_correction(<<~RUBY)
+      find('a')
+        .first('div')
+    RUBY
+  end
+
+  it 'does not register an offense when using `first`' do
+    expect_no_offenses(<<~RUBY)
+      first('a')
+    RUBY
+  end
+
+  it 'does not register an offense when using `all` without `first`' do
+    expect_no_offenses(<<~RUBY)
+      all('a').map(&:text)
+    RUBY
+  end
+
+  it 'does not register an offense when using `all` with `[1]`' do
+    expect_no_offenses(<<~RUBY)
+      all('a')[1]
+    RUBY
+  end
+
+  it 'does not register an offense when using `all` with argument' \
+     ' without `first`' do
+    expect_no_offenses(<<~RUBY)
+      all('a', text: 'b')
+    RUBY
+  end
+
+  it 'does not register an offense when using no argument `all`' do
+    expect_no_offenses(<<~RUBY)
+      all.first
+      all[0]
+    RUBY
+  end
+end