From 4a00173fd74869cbcfc713b2cd50ebceeb14d451 Mon Sep 17 00:00:00 2001 From: ydah Date: Thu, 11 Apr 2024 18:52:09 +0900 Subject: [PATCH] Add new `Capybara/FindAllFirst` cop Fix: https://github.com/rubocop/rubocop-capybara/issues/115 --- CHANGELOG.md | 1 + config/default.yml | 6 + docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_capybara.adoc | 32 +++++ lib/rubocop/cop/capybara/find_all_first.rb | 78 +++++++++++ lib/rubocop/cop/capybara_cops.rb | 1 + .../cop/capybara/find_all_first_spec.rb | 121 ++++++++++++++++++ 7 files changed, 240 insertions(+) create mode 100644 lib/rubocop/cop/capybara/find_all_first.rb create mode 100644 spec/rubocop/cop/capybara/find_all_first_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 83011af..1744894 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/config/default.yml b/config/default.yml index c02ce69..e23f889 100644 --- a/config/default.yml +++ b/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: "<>" + Reference: https://www.rubydoc.info/gems/rubocop-capybara/RuboCop/Cop/Capybara/FindAllFirst + Capybara/MatchStyle: Description: Checks for usage of deprecated style methods. Enabled: pending diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 23818d1..c6fe85f 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/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] diff --git a/docs/modules/ROOT/pages/cops_capybara.adoc b/docs/modules/ROOT/pages/cops_capybara.adoc index 2d4b65e..56d92d5 100644 --- a/docs/modules/ROOT/pages/cops_capybara.adoc +++ b/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 +| <> +| - +|=== + +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 |=== diff --git a/lib/rubocop/cop/capybara/find_all_first.rb b/lib/rubocop/cop/capybara/find_all_first.rb new file mode 100644 index 0000000..06de641 --- /dev/null +++ b/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(%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 diff --git a/lib/rubocop/cop/capybara_cops.rb b/lib/rubocop/cop/capybara_cops.rb index 89a7294..180a767 100644 --- a/lib/rubocop/cop/capybara_cops.rb +++ b/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' diff --git a/spec/rubocop/cop/capybara/find_all_first_spec.rb b/spec/rubocop/cop/capybara/find_all_first_spec.rb new file mode 100644 index 0000000..e271189 --- /dev/null +++ b/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