From 8f6eb29b14190d9ee2b4c910e080b2f6e85bc36c Mon Sep 17 00:00:00 2001 From: ydah <13041216+ydah@users.noreply.github.com> Date: Mon, 2 Oct 2023 23:49:40 +0900 Subject: [PATCH] Add new `Capybara/RedundantWithinFind` cop Resolve: https://github.com/rubocop/rubocop-capybara/issues/75 --- CHANGELOG.md | 2 + config/default.yml | 6 ++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_capybara.adoc | 39 ++++++++ .../cop/capybara/redundant_within_find.rb | 66 ++++++++++++++ lib/rubocop/cop/capybara_cops.rb | 1 + .../capybara/redundant_within_find_spec.rb | 89 +++++++++++++++++++ 7 files changed, 204 insertions(+) create mode 100644 lib/rubocop/cop/capybara/redundant_within_find.rb create mode 100644 spec/rubocop/cop/capybara/redundant_within_find_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dffc14..aa1b69a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Edge (Unreleased) +- Add new `Capybara/RedundantWithinFind` cop. ([@ydah]) + ## 2.19.0 (2023-09-20) - Add new `Capybara/RSpec/PredicateMatcher` cop. ([@ydah]) diff --git a/config/default.yml b/config/default.yml index 4f6506e..9199d2a 100644 --- a/config/default.yml +++ b/config/default.yml @@ -42,6 +42,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: "<>" + 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 diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index ace8a67..65c23c4 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/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] diff --git a/docs/modules/ROOT/pages/cops_capybara.adoc b/docs/modules/ROOT/pages/cops_capybara.adoc index ca301ff..2e9c71b 100644 --- a/docs/modules/ROOT/pages/cops_capybara.adoc +++ b/docs/modules/ROOT/pages/cops_capybara.adoc @@ -210,6 +210,45 @@ 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 +| <> +| - +|=== + +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 +---- + == Capybara/SpecificActions |=== diff --git a/lib/rubocop/cop/capybara/redundant_within_find.rb b/lib/rubocop/cop/capybara/redundant_within_find.rb new file mode 100644 index 0000000..8e0085b --- /dev/null +++ b/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 %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 diff --git a/lib/rubocop/cop/capybara_cops.rb b/lib/rubocop/cop/capybara_cops.rb index 9dd1fb7..1c38654 100644 --- a/lib/rubocop/cop/capybara_cops.rb +++ b/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' diff --git a/spec/rubocop/cop/capybara/redundant_within_find_spec.rb b/spec/rubocop/cop/capybara/redundant_within_find_spec.rb new file mode 100644 index 0000000..9de7f97 --- /dev/null +++ b/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