diff --git a/CHANGELOG.md b/CHANGELOG.md index 32a41da..54baeb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Bump RuboCop requirement to +1.81. ([@ydah]) - Fix a false positive for `Capybara/FindAllFirst` when using logical operators with `all('...')[0]`. ([@ydah]) +- Add support for using `find` with `:link` and `:field` in `Capybara/SpecificFinders` cop. ([@ydah]) ## 2.22.1 (2025-03-12) diff --git a/docs/modules/ROOT/pages/cops_capybara.adoc b/docs/modules/ROOT/pages/cops_capybara.adoc index ef56958..113308d 100644 --- a/docs/modules/ROOT/pages/cops_capybara.adoc +++ b/docs/modules/ROOT/pages/cops_capybara.adoc @@ -479,9 +479,13 @@ find('#some-id') find('[id=some-id]') find(:css, '#some-id') find(:id, 'some-id') +find(:link, 'Home') +find(:field, 'Name') # good find_by_id('some-id') +find_link('Home') +find_field('Name') ---- [#references-capybaraspecificfinders] diff --git a/lib/rubocop/cop/capybara/specific_finders.rb b/lib/rubocop/cop/capybara/specific_finders.rb index d6d56b5..734fca3 100644 --- a/lib/rubocop/cop/capybara/specific_finders.rb +++ b/lib/rubocop/cop/capybara/specific_finders.rb @@ -11,20 +11,29 @@ module Capybara # find('[id=some-id]') # find(:css, '#some-id') # find(:id, 'some-id') + # find(:link, 'Home') + # find(:field, 'Name') # # # good # find_by_id('some-id') + # find_link('Home') + # find_field('Name') # - class SpecificFinders < ::RuboCop::Cop::Base + class SpecificFinders < ::RuboCop::Cop::Base # rubocop:disable Metrics/ClassLength extend AutoCorrector include RangeHelp - MSG = 'Prefer `find_by_id` over `find`.' + MSG = 'Prefer `%s` over `find`.' RESTRICT_ON_SEND = %i[find].freeze # @!method find_argument(node) def_node_matcher :find_argument, <<~PATTERN - (send _ :find $(sym {:css :id})? (str $_) ...) + (send _ :find $(sym {:css :id :link :field})? (str $_) ...) + PATTERN + + # @!method find_argument_without_locator(node) + def_node_matcher :find_argument_without_locator, <<~PATTERN + (send _ :find $(sym {:link :field})) PATTERN # @!method class_options(node) @@ -32,7 +41,7 @@ class SpecificFinders < ::RuboCop::Cop::Base (pair (sym :class) $_ ...) PATTERN - def on_send(node) + def on_send(node) # rubocop:disable Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity find_argument(node) do |sym, arg| next if CssSelector.pseudo_classes(arg).any? next if CssSelector.multiple_selectors?(arg) @@ -40,11 +49,39 @@ def on_send(node) on_attr(node, sym, arg) if attribute?(arg) on_id(node, sym, arg) if CssSelector.id?(arg) on_sym_id(node, sym, arg) if sym.first&.value == :id + on_sym_selector(node, sym) if sym.first && selector?(sym) end end private + def selector?(sym) + %i[link field].include?(sym.first.value) + end + + def on_sym_selector(node, sym) + replacement = "find_#{sym.first.value}" + register_selector_offense(node, replacement) + end + + def register_selector_offense(node, replacement) + message = format(MSG, replacement: replacement) + add_offense(offense_range(node), message: message) do |corrector| + corrector.replace(node.loc.selector, replacement) + remove_selector_argument(corrector, node) + end + end + + def remove_selector_argument(corrector, node) + if node.arguments.size == 1 + corrector.remove(node.first_argument) + else + range = range_between(node.first_argument.source_range.begin_pos, + node.arguments[1].source_range.begin_pos) + corrector.remove(range) + end + end + def on_attr(node, sym, arg) attrs = CssSelector.attributes(arg) return unless (id = attrs['id']) @@ -71,16 +108,19 @@ def attribute?(arg) end def register_offense(node, sym, id, classes = []) - add_offense(offense_range(node)) do |corrector| + message = format(MSG, replacement: 'find_by_id') + add_offense(offense_range(node), message: message) do |corrector| corrector.replace(node.loc.selector, 'find_by_id') corrector.replace(node.first_argument, id.delete('\\')) - unless classes.compact.empty? - autocorrect_classes(corrector, node, classes) - end + autocorrect_id_classes(corrector, node, classes) corrector.remove(deletion_range(node)) unless sym.empty? end end + def autocorrect_id_classes(corrector, node, classes) + autocorrect_classes(corrector, node, classes) if classes.compact.any? + end + def deletion_range(node) range_between(node.first_argument.source_range.end_pos, node.arguments[1].source_range.end_pos) diff --git a/spec/rubocop/cop/capybara/specific_finders_spec.rb b/spec/rubocop/cop/capybara/specific_finders_spec.rb index 84de16e..42093b7 100644 --- a/spec/rubocop/cop/capybara/specific_finders_spec.rb +++ b/spec/rubocop/cop/capybara/specific_finders_spec.rb @@ -237,6 +237,18 @@ RUBY end + it 'registers an offense when using `find` ' \ + 'with argument is attribute specified id and style attribute' do + expect_offense(<<~RUBY) + find('[id=some-id][style=display:none]') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by_id` over `find`. + RUBY + + expect_correction(<<~RUBY) + find_by_id('some-id', style: 'display:none') + RUBY + end + it 'does not register an offense when using `find ' \ 'with argument is attribute not specified id' do expect_no_offenses(<<~RUBY) @@ -308,4 +320,62 @@ find('#foo:is(:enabled,:checked)') RUBY end + + it 'registers an offense when using `find` with `:link`' do + expect_offense(<<~RUBY) + find(:link, 'Home') + ^^^^^^^^^^^^^^^^^^^ Prefer `find_link` over `find`. + RUBY + + expect_correction(<<~RUBY) + find_link('Home') + RUBY + end + + it 'registers an offense when using `find` with `:field`' do + expect_offense(<<~RUBY) + find(:field, 'Name') + ^^^^^^^^^^^^^^^^^^^^ Prefer `find_field` over `find`. + RUBY + + expect_correction(<<~RUBY) + find_field('Name') + RUBY + end + + it 'registers an offense when using `find` with `:link` ' \ + 'and option' do + expect_offense(<<~RUBY) + find(:link, 'Home', class: 'navbar-link') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_link` over `find`. + RUBY + + expect_correction(<<~RUBY) + find_link('Home', class: 'navbar-link') + RUBY + end + + it 'registers an offense when using `find` with `:field` ' \ + 'and option' do + expect_offense(<<~RUBY) + find(:field, 'Email', visible: false) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_field` over `find`. + RUBY + + expect_correction(<<~RUBY) + find_field('Email', visible: false) + RUBY + end + + it 'does not register an offense when using `find_link`' do + expect_no_offenses(<<~RUBY) + find_link('Home') + RUBY + end + + it 'does not register an offense when using `find_field`' do + expect_no_offenses(<<~RUBY) + find_field('Name') + RUBY + end end