diff --git a/CHANGELOG.md b/CHANGELOG.md index 41ec9ac3f..abb217c09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Master (Unreleased) +- Fix a false positive and incorrect autocorrect for `RSpec/Capybara/SpecificActions`, `RSpec/Capybara/SpecificFinders` and `RSpec/Capybara/SpecificMatcher`. ([@ydah]) - Add new `RSpec/FactoryBot/FactoryNameStyle` cop. ([@ydah]) - Improved processing speed for `RSpec/Be`, `RSpec/ExpectActual`, `RSpec/ImplicitExpect`, `RSpec/MessageSpies`, `RSpec/PredicateMatcher` and `RSpec/Rails/HaveHttpStatus`. ([@ydah]) - Fix wrong autocorrection in `n_times` style on `RSpec/FactoryBot/CreateList`. ([@r7kamura]) diff --git a/lib/rubocop/cop/rspec/capybara/specific_actions.rb b/lib/rubocop/cop/rspec/capybara/specific_actions.rb index d662e925f..3436e1ac5 100644 --- a/lib/rubocop/cop/rspec/capybara/specific_actions.rb +++ b/lib/rubocop/cop/rspec/capybara/specific_actions.rb @@ -42,9 +42,7 @@ def on_send(node) # which the last selector points. next unless (selector = last_selector(arg)) next unless (action = specific_action(selector)) - next unless CapybaraHelp.specific_option?(node.receiver, arg, - action) - next unless CapybaraHelp.specific_pseudo_classes?(arg) + next unless replaceable?(node, arg, action) range = offense_range(node, node.receiver) add_offense(range, message: message(action, selector)) @@ -57,6 +55,18 @@ def specific_action(selector) SPECIFIC_ACTION[last_selector(selector)] end + def replaceable?(node, arg, action) + replaceable_attributes?(arg) && + CapybaraHelp.replaceable_option?(node.receiver, arg, action) && + CapybaraHelp.replaceable_pseudo_classes?(arg) + end + + def replaceable_attributes?(selector) + CapybaraHelp.replaceable_attributes?( + CssSelector.attributes(selector) + ) + end + def supported_selector?(selector) !selector.match?(/[>,+~]/) end diff --git a/lib/rubocop/cop/rspec/capybara/specific_finders.rb b/lib/rubocop/cop/rspec/capybara/specific_finders.rb index 54acb006c..f3fdbe912 100644 --- a/lib/rubocop/cop/rspec/capybara/specific_finders.rb +++ b/lib/rubocop/cop/rspec/capybara/specific_finders.rb @@ -28,8 +28,14 @@ class SpecificFinders < Base (send _ :find (str $_) ...) PATTERN + # @!method class_options(node) + def_node_search :class_options, <<~PATTERN + (pair (sym :class) $_ ...) + PATTERN + def on_send(node) find_argument(node) do |arg| + next if CssSelector.pseudo_classes(arg).any? next if CssSelector.multiple_selectors?(arg) on_attr(node, arg) if attribute?(arg) @@ -40,28 +46,57 @@ def on_send(node) private def on_attr(node, arg) - return unless (id = CssSelector.attributes(arg)['id']) + attrs = CssSelector.attributes(arg) + return unless (id = attrs['id']) + return if attrs['class'] register_offense(node, replaced_arguments(arg, id)) end def on_id(node, arg) - register_offense(node, "'#{arg.to_s.delete('#')}'") + return if CssSelector.attributes(arg).any? + + id = CssSelector.id(arg) + register_offense(node, "'#{id.delete('#')}'", + CssSelector.classes(arg.sub(id, ''))) end def attribute?(arg) CssSelector.attribute?(arg) && - CssSelector.common_attributes?(arg) + CapybaraHelp.common_attributes?(arg) end - def register_offense(node, arg_replacement) + def register_offense(node, id, classes = []) add_offense(offense_range(node)) do |corrector| corrector.replace(node.loc.selector, 'find_by_id') corrector.replace(node.first_argument.loc.expression, - arg_replacement) + id.delete('\\')) + unless classes.compact.empty? + autocorrect_classes(corrector, node, classes) + end end end + def autocorrect_classes(corrector, node, classes) + if (options = class_options(node).first) + append_options(classes, options) + corrector.replace(options, classes.to_s) + else + corrector.insert_after(node.first_argument, + keyword_argument_class(classes)) + end + end + + def append_options(classes, options) + classes << options.value if options.str_type? + options.each_value { |v| classes << v.value } if options.array_type? + end + + def keyword_argument_class(classes) + value = classes.size > 1 ? classes.to_s : "'#{classes.first}'" + ", class: #{value}" + end + def replaced_arguments(arg, id) options = to_options(CssSelector.attributes(arg)) options.empty? ? id : "#{id}, #{options}" diff --git a/lib/rubocop/cop/rspec/capybara/specific_matcher.rb b/lib/rubocop/cop/rspec/capybara/specific_matcher.rb index 15935700b..f78b3be7c 100644 --- a/lib/rubocop/cop/rspec/capybara/specific_matcher.rb +++ b/lib/rubocop/cop/rspec/capybara/specific_matcher.rb @@ -47,8 +47,7 @@ def on_send(node) first_argument(node) do |arg| next unless (matcher = specific_matcher(arg)) next if CssSelector.multiple_selectors?(arg) - next unless CapybaraHelp.specific_option?(node, arg, matcher) - next unless CapybaraHelp.specific_pseudo_classes?(arg) + next unless replaceable?(node, arg, matcher) add_offense(node, message: message(node, matcher)) end @@ -61,6 +60,18 @@ def specific_matcher(arg) SPECIFIC_MATCHER[splitted_arg] end + def replaceable?(node, arg, matcher) + replaceable_attributes?(arg) && + CapybaraHelp.replaceable_option?(node, arg, matcher) && + CapybaraHelp.replaceable_pseudo_classes?(arg) + end + + def replaceable_attributes?(selector) + CapybaraHelp.replaceable_attributes?( + CssSelector.attributes(selector) + ) + end + def message(node, matcher) format(MSG, good_matcher: good_matcher(node, matcher), diff --git a/lib/rubocop/cop/rspec/mixin/capybara_help.rb b/lib/rubocop/cop/rspec/mixin/capybara_help.rb index 95a0acbf2..d06630f70 100644 --- a/lib/rubocop/cop/rspec/mixin/capybara_help.rb +++ b/lib/rubocop/cop/rspec/mixin/capybara_help.rb @@ -5,24 +5,69 @@ module Cop module RSpec # Help methods for capybara. module CapybaraHelp + COMMON_OPTIONS = %w[ + id class style + ].freeze + SPECIFIC_OPTIONS = { + 'button' => ( + COMMON_OPTIONS + %w[disabled name value title type] + ).freeze, + 'link' => ( + COMMON_OPTIONS + %w[href alt title download] + ).freeze, + 'table' => ( + COMMON_OPTIONS + %w[cols rows] + ).freeze, + 'select' => ( + COMMON_OPTIONS + %w[ + disabled name placeholder + selected multiple + ] + ).freeze, + 'field' => ( + COMMON_OPTIONS + %w[ + checked disabled name placeholder + readonly type multiple + ] + ).freeze + }.freeze + SPECIFIC_PSEUDO_CLASSES = %w[ + not() disabled enabled checked unchecked + ].freeze + module_function # @param node [RuboCop::AST::SendNode] # @param locator [String] # @param element [String] # @return [Boolean] - def specific_option?(node, locator, element) + def replaceable_option?(node, locator, element) attrs = CssSelector.attributes(locator).keys return false unless replaceable_element?(node, element, attrs) attrs.all? do |attr| - CssSelector.specific_options?(element, attr) + SPECIFIC_OPTIONS.fetch(element, []).include?(attr) end end + # @param selector [String] + # @return [Boolean] + # @example + # common_attributes?('a[focused]') # => true + # common_attributes?('button[focused][visible]') # => true + # common_attributes?('table[id=some-id]') # => true + # common_attributes?('h1[invalid]') # => false + def common_attributes?(selector) + CssSelector.attributes(selector).keys.difference(COMMON_OPTIONS).none? + end + + def replaceable_attributes?(attrs) + attrs.values.none?(&:nil?) + end + # @param locator [String] # @return [Boolean] - def specific_pseudo_classes?(locator) + def replaceable_pseudo_classes?(locator) CssSelector.pseudo_classes(locator).all? do |pseudo_class| replaceable_pseudo_class?(pseudo_class, locator) end @@ -32,7 +77,7 @@ def specific_pseudo_classes?(locator) # @param locator [String] # @return [Boolean] def replaceable_pseudo_class?(pseudo_class, locator) - return false unless CssSelector.specific_pesudo_classes?(pseudo_class) + return false unless SPECIFIC_PSEUDO_CLASSES.include?(pseudo_class) case pseudo_class when 'not()' then replaceable_pseudo_class_not?(locator) diff --git a/lib/rubocop/cop/rspec/mixin/css_selector.rb b/lib/rubocop/cop/rspec/mixin/css_selector.rb index 3fc2603fe..8923b0d51 100644 --- a/lib/rubocop/cop/rspec/mixin/css_selector.rb +++ b/lib/rubocop/cop/rspec/mixin/css_selector.rb @@ -5,59 +5,18 @@ module Cop module RSpec # Helps parsing css selector. module CssSelector - COMMON_OPTIONS = %w[ - above below left_of right_of near count minimum maximum between text - id class style visible obscured exact exact_text normalize_ws match - wait filter_set focused - ].freeze - SPECIFIC_OPTIONS = { - 'button' => ( - COMMON_OPTIONS + %w[disabled name value title type] - ).freeze, - 'link' => ( - COMMON_OPTIONS + %w[href alt title download] - ).freeze, - 'table' => ( - COMMON_OPTIONS + %w[ - caption with_cols cols with_rows rows - ] - ).freeze, - 'select' => ( - COMMON_OPTIONS + %w[ - disabled name placeholder options enabled_options - disabled_options selected with_selected multiple with_options - ] - ).freeze, - 'field' => ( - COMMON_OPTIONS + %w[ - checked unchecked disabled valid name placeholder - validation_message readonly with type multiple - ] - ).freeze - }.freeze - SPECIFIC_PSEUDO_CLASSES = %w[ - not() disabled enabled checked unchecked - ].freeze - module_function - # @param element [String] - # @param attribute [String] - # @return [Boolean] + # @param selector [String] + # @return [String] # @example - # specific_pesudo_classes?('button', 'name') # => true - # specific_pesudo_classes?('link', 'invalid') # => false - def specific_options?(element, attribute) - SPECIFIC_OPTIONS.fetch(element, []).include?(attribute) - end + # id('#some-id') # => some-id + # id('.some-class') # => nil + # id('#some-id.cls') # => some-id + def id(selector) + return unless id?(selector) - # @param pseudo_class [String] - # @return [Boolean] - # @example - # specific_pesudo_classes?('disabled') # => true - # specific_pesudo_classes?('first-of-type') # => false - def specific_pesudo_classes?(pseudo_class) - SPECIFIC_PSEUDO_CLASSES.include?(pseudo_class) + selector.gsub(selector.scan(/[^\\]([>,+~.].*)/).join, '') end # @param selector [String] @@ -69,6 +28,17 @@ def id?(selector) selector.start_with?('#') end + # @param selector [String] + # @return [Array] + # @example + # classes('#some-id') # => [] + # classes('.some-class') # => ['some-class'] + # classes('#some-id.some-class') # => ['some-class'] + # classes('#some-id.cls1.cls2') # => ['cls1', 'cls2'] + def classes(selector) + selector.scan(/.([\w-]*)/).flatten + end + # @param selector [String] # @return [Boolean] # @example @@ -87,21 +57,11 @@ def attribute?(selector) def attributes(selector) selector.scan(/\[(.*?)\]/).flatten.to_h do |attr| key, value = attr.split('=') + value << ']' if value&.include?('[') [key, normalize_value(value)] end end - # @param selector [String] - # @return [Boolean] - # @example - # common_attributes?('a[focused]') # => true - # common_attributes?('button[focused][visible]') # => true - # common_attributes?('table[id=some-id]') # => true - # common_attributes?('h1[invalid]') # => false - def common_attributes?(selector) - attributes(selector).keys.difference(COMMON_OPTIONS).none? - end - # @param selector [String] # @return [Array] # @example @@ -122,7 +82,8 @@ def pseudo_classes(selector) # multiple_selectors?('a.cls b#id') # => true # multiple_selectors?('a.cls') # => false def multiple_selectors?(selector) - selector.match?(/[ >,+~]/) + normalize = selector.gsub(/(\\[>,+~]|\(.*\))/, '') + normalize.match?(/[ >,+~]/) end # @param value [String] @@ -130,14 +91,14 @@ def multiple_selectors?(selector) # @example # normalize_value('true') # => true # normalize_value('false') # => false - # normalize_value(nil) # => false + # normalize_value(nil) # => nil # normalize_value("foo") # => "'foo'" def normalize_value(value) case value when 'true' then true when 'false' then false - when nil then true - else "'#{value}'" + when nil then nil + else "'#{value.gsub(/["']/, '')}'" end end end diff --git a/spec/rubocop/cop/rspec/capybara/specific_actions_spec.rb b/spec/rubocop/cop/rspec/capybara/specific_actions_spec.rb index 84ee67d26..a16d717ea 100644 --- a/spec/rubocop/cop/rspec/capybara/specific_actions_spec.rb +++ b/spec/rubocop/cop/rspec/capybara/specific_actions_spec.rb @@ -13,8 +13,6 @@ expect_offense(<<~RUBY) find('a', href: 'http://example.com').click ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. - find("a[href]").click - ^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. find("a[href='http://example.com']").click ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. RUBY @@ -92,25 +90,18 @@ RUBY end - %i[above below left_of right_of near count minimum maximum between - text id class style visible obscured exact exact_text normalize_ws - match wait filter_set focused disabled name value - title type].each do |attr| + %i[id class style disabled name value title type].each do |attr| it 'registers an offense for abstract action when ' \ "first argument is element with replaceable attributes #{attr} " \ 'for `click_button`' do expect_offense(<<-RUBY, attr: attr) find("button[#{attr}=foo]").click ^^^^^^^^^^^^^^{attr}^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - find("button[#{attr}]").click - ^^^^^^^^^^^^^^{attr}^^^^^^^^^ Prefer `click_button` over `find('button').click`. RUBY end end - %i[above below left_of right_of near count minimum maximum between text id - class style visible obscured exact exact_text normalize_ws match wait - filter_set focused alt title download].each do |attr| + %i[id class style alt title download].each do |attr| it 'does not register an offense for abstract action when ' \ "first argument is element with replaceable attributes #{attr} " \ 'for `click_link` without `href`' do @@ -124,10 +115,8 @@ class style visible obscured exact exact_text normalize_ws match wait "first argument is element with replaceable attributes #{attr} " \ 'for `click_link` with attribute `href`' do expect_offense(<<-RUBY, attr: attr) - find("a[#{attr}=foo][href]").click - ^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. - find("a[#{attr}][href='http://example.com']").click - ^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. + find("a[#{attr}=foo][href='http://example.com']").click + ^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. RUBY end @@ -137,8 +126,8 @@ class style visible obscured exact exact_text normalize_ws match wait expect_offense(<<-RUBY, attr: attr) find("a[#{attr}=foo]", href: 'http://example.com').click ^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. - find("a[#{attr}]", text: 'foo', href: 'http://example.com').click - ^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. + find("a[#{attr}=foo]", text: 'foo', href: 'http://example.com').click + ^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_link` over `find('a').click`. RUBY end end @@ -146,37 +135,42 @@ class style visible obscured exact exact_text normalize_ws match wait it 'registers an offense when using abstract action with ' \ 'first argument is element with multiple replaceable attributes' do expect_offense(<<-RUBY) - find('button[disabled][name="foo"]', exact_text: 'foo').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. + find('button[disabled=true][name="foo"]', exact_text: 'foo').click + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. RUBY end it 'registers an offense when using abstract action with state' do expect_offense(<<-RUBY) - find('button[disabled]', exact_text: 'foo').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. + find('button[disabled=false]', exact_text: 'foo').click + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. RUBY end it 'registers an offense when using abstract action with ' \ 'first argument is element with replaceable pseudo-classes' do expect_offense(<<-RUBY) - find('button:not([disabled])', exact_text: 'bar').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - find('button:not([disabled][visible])').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. + find('button:not([disabled=true])', exact_text: 'bar').click + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. RUBY end it 'registers an offense when using abstract action with ' \ 'first argument is element with multiple replaceable pseudo-classes' do expect_offense(<<-RUBY) - find('button:not([disabled]):enabled').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. + find('button:not([disabled=true]):enabled').click + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. find('button:not([disabled=false]):disabled').click ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. - find('button:not([disabled]):not([disabled])').click - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `click_button` over `find('button').click`. + RUBY + end + + it 'does not register an offense when using abstract action with' \ + 'first argument is element with not replaceable attributes' do + expect_no_offenses(<<-RUBY) + find('button[disabled]').click + find('button[id=some-id][disabled]').click + find('button[visible]').click RUBY end @@ -184,21 +178,21 @@ class style visible obscured exact exact_text normalize_ws match wait 'first argument is element with replaceable pseudo-classes' \ 'and not boolean attributes' do expect_no_offenses(<<-RUBY) - find('button:not([name="foo"][disabled])').click + find('button:not([name="foo"][disabled=true])').click RUBY end it 'does not register an offense when using abstract action with ' \ 'first argument is element with multiple nonreplaceable pseudo-classes' do expect_no_offenses(<<-RUBY) - find('button:first-of-type:not([disabled])').click + find('button:first-of-type:not([disabled=true])').click RUBY end it 'does not register an offense for abstract action when ' \ 'first argument is element with nonreplaceable attributes' do expect_no_offenses(<<-RUBY) - find('button[data-disabled]').click + find('button[data-disabled=true]').click find('button[foo=bar]').click find('button[foo-bar=baz]', exact_text: 'foo').click RUBY @@ -207,11 +201,11 @@ class style visible obscured exact exact_text normalize_ws match wait it 'does not register an offense for abstract action when ' \ 'first argument is element with multiple nonreplaceable attributes' do expect_no_offenses(<<-RUBY) - find('button[disabled][foo]').click - find('button[foo][disabled]').click - find('button[foo][disabled][bar]').click - find('button[disabled][foo=bar]').click - find('button[disabled=foo][bar]', exact_text: 'foo').click + find('button[disabled=true][foo=bar]').click + find('button[foo=bar][disabled=true]').click + find('button[foo=bar][disabled=true][bar=baz]').click + find('button[disabled=true][foo=bar]').click + find('button[disabled=foo][bar=bar]', exact_text: 'foo').click RUBY end diff --git a/spec/rubocop/cop/rspec/capybara/specific_finders_spec.rb b/spec/rubocop/cop/rspec/capybara/specific_finders_spec.rb index fc2bda62a..c986f7494 100644 --- a/spec/rubocop/cop/rspec/capybara/specific_finders_spec.rb +++ b/spec/rubocop/cop/rspec/capybara/specific_finders_spec.rb @@ -23,6 +23,87 @@ RUBY end + it 'registers an offense when using `find` with id and class' do + expect_offense(<<~RUBY) + find('#some-id.some-cls') + ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. + RUBY + + expect_correction(<<~RUBY) + find_by_id('some-id', class: 'some-cls') + RUBY + end + + it 'registers an offense when using `find` with id include `\.`' do + expect_offense(<<~RUBY) + find('#some-id\\.some-cls') + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. + find('#some-id\\>some-cls') + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. + find('#some-id\\,some-cls') + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. + find('#some-id\\+some-cls') + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. + find('#some-id\\~some-cls') + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. + RUBY + + expect_correction(<<~RUBY) + find_by_id('some-id.some-cls') + find_by_id('some-id>some-cls') + find_by_id('some-id,some-cls') + find_by_id('some-id+some-cls') + find_by_id('some-id~some-cls') + RUBY + end + + it 'registers an offense when using `find` with multiple classes' do + expect_offense(<<~RUBY) + find('#some-id.some-cls.other-cls') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. + RUBY + + expect_correction(<<~RUBY) + find_by_id('some-id', class: ["some-cls", "other-cls"]) + RUBY + end + + it 'registers an offense when using `find` with multiple classes ' \ + "and class: 'other-cls'" do + expect_offense(<<~RUBY) + find('#some-id.some-cls', class: 'other-cls') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. + RUBY + + expect_correction(<<~RUBY) + find_by_id('some-id', class: ["some-cls", "other-cls"]) + RUBY + end + + it 'registers an offense when using `find` with multiple classes ' \ + "and class: ['other-cls1', 'other-cls2']" do + expect_offense(<<~RUBY) + find('#some-id.some-cls', class: ['other-cls1', 'other-cls2']) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. + RUBY + + expect_correction(<<~RUBY) + find_by_id('some-id', class: ["some-cls", "other-cls1", "other-cls2"]) + RUBY + end + + it 'registers an offense when using `find` with multiple classes ' \ + "and exact_text: 'foo', class: 'other-cls'" do + expect_offense(<<~RUBY) + find('#some-id.some-cls', exact_text: 'foo', class: 'other-cls') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. + RUBY + + expect_correction(<<~RUBY) + find_by_id('some-id', exact_text: 'foo', class: ["some-cls", "other-cls"]) + RUBY + end + it 'registers an offense when using `find` and other args' do expect_offense(<<~RUBY) find('#some-id', exact_text: 'foo') @@ -64,24 +145,42 @@ expect_offense(<<~RUBY) find('[id=some-id]') ^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. - find('[visible][id=some-id]') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. - find('[id=some-id][class=some-cls][focused]') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. RUBY expect_correction(<<~RUBY) find_by_id('some-id') - find_by_id('some-id', visible: true) - find_by_id('some-id', class: 'some-cls', focused: true) + RUBY + end + + it 'registers an offense when using `find ' \ + 'with argument is attribute specified id surrounded by quotation' do + expect_offense(<<~RUBY) + find('[id="foo"]') + ^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. + find('[id="foo[bar]"]') + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `find_by` over `find`. + RUBY + + expect_correction(<<~RUBY) + find_by_id('foo') + find_by_id('foo[bar]') RUBY end it 'does not register an offense when using `find ' \ 'with argument is attribute not specified id' do expect_no_offenses(<<~RUBY) - find('[visible]') - find('[class=some-cls][visible]') + find('[id]') + find('[disabled=true]') + find('[class=some-cls][disabled]') + RUBY + end + + it 'does not register an offense when using `find ' \ + 'with argument is attribute specified id and class' do + expect_no_offenses(<<~RUBY) + find('[class=some-cls][id=some-id]') + find('[id=some-id][disabled][class=some-cls]') RUBY end @@ -122,4 +221,21 @@ find('#some-id+option') RUBY end + + it 'does not register an offense when using `find` ' \ + 'with id and attribute' do + expect_no_offenses(<<~RUBY) + find('#foo[hidden]') + find('#foo[class="some-cls"]') + RUBY + end + + it 'does not register an offense when using `find` ' \ + 'with id and pseudo class' do + expect_no_offenses(<<~RUBY) + find('#foo:enabled') + find('#foo:not(:disabled)') + find('#foo:is(:enabled,:checked)') + RUBY + end end diff --git a/spec/rubocop/cop/rspec/capybara/specific_matcher_spec.rb b/spec/rubocop/cop/rspec/capybara/specific_matcher_spec.rb index 0a605cdd4..d7bbaea1f 100644 --- a/spec/rubocop/cop/rspec/capybara/specific_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/capybara/specific_matcher_spec.rb @@ -78,25 +78,18 @@ RUBY end - %i[above below left_of right_of near count minimum maximum between - text id class style visible obscured exact exact_text normalize_ws - match wait filter_set focused disabled name value - title type].each do |attr| + %i[id class style disabled name value title type].each do |attr| it 'registers an offense for abstract matcher when ' \ "first argument is element with replaceable attributes #{attr} " \ 'for `have_button`' do expect_offense(<<-RUBY, attr: attr) expect(page).to have_css("button[#{attr}=foo]") ^^^^^^^^^^^^^^^^^^{attr}^^^^^^^ Prefer `have_button` over `have_css`. - expect(page).to have_css("button[#{attr}]") - ^^^^^^^^^^^^^^^^^^{attr}^^^ Prefer `have_button` over `have_css`. RUBY end end - %i[above below left_of right_of near count minimum maximum between text id - class style visible obscured exact exact_text normalize_ws match wait - filter_set focused alt title download].each do |attr| + %i[id class style alt title download].each do |attr| it 'does not register an offense for abstract matcher when ' \ "first argument is element with replaceable attributes #{attr} " \ 'for `have_link` without `href`' do @@ -111,10 +104,8 @@ class style visible obscured exact exact_text normalize_ws match wait "first argument is element with replaceable attributes #{attr} " \ 'for `have_link` with attribute `href`' do expect_offense(<<-RUBY, attr: attr) - expect(page).to have_css("a[#{attr}=foo][href]") - ^^^^^^^^^^^^^{attr}^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. - expect(page).to have_css("a[#{attr}][href='http://example.com']") - ^^^^^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. + expect(page).to have_css("a[#{attr}=foo][href='http://example.com']") + ^^^^^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. RUBY end @@ -124,8 +115,8 @@ class style visible obscured exact exact_text normalize_ws match wait expect_offense(<<-RUBY, attr: attr) expect(page).to have_css("a[#{attr}=foo]", href: 'http://example.com') ^^^^^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. - expect(page).to have_css("a[#{attr}]", text: 'foo', href: 'http://example.com') - ^^^^^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. + expect(page).to have_css("a[#{attr}=foo]", text: 'foo', href: 'http://example.com') + ^^^^^^^^^^^^^{attr}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. RUBY end end @@ -134,58 +125,40 @@ class style visible obscured exact exact_text normalize_ws match wait 'first argument is element with replaceable attributes href ' \ 'for `have_link`' do expect_offense(<<-RUBY) - expect(page).to have_css("a[href]") - ^^^^^^^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. expect(page).to have_css("a[href='http://example.com']") ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_link` over `have_css`. RUBY end - %i[above below left_of right_of near count minimum maximum between - text id class style visible obscured exact exact_text normalize_ws - match wait filter_set focused caption with_cols cols with_rows - rows].each do |attr| + %i[id class style cols rows].each do |attr| it 'registers an offense for abstract matcher when ' \ "first argument is element with replaceable attributes #{attr} " \ 'for `have_table`' do expect_offense(<<-RUBY, attr: attr) expect(page).to have_css("table[#{attr}=foo]") ^^^^^^^^^^^^^^^^^{attr}^^^^^^^ Prefer `have_table` over `have_css`. - expect(page).to have_css("table[#{attr}]") - ^^^^^^^^^^^^^^^^^{attr}^^^ Prefer `have_table` over `have_css`. RUBY end end - %i[above below left_of right_of near count minimum maximum between - text id class style visible obscured exact exact_text normalize_ws - match wait filter_set focused disabled name placeholder options - enabled_options disabled_options selected with_selected - multiple with_options].each do |attr| + %i[id class style disabled name placeholder selected multiple].each do |attr| it 'registers an offense for abstract matcher when ' \ "first argument is element with replaceable attributes #{attr} " \ 'for `have_select`' do expect_offense(<<-RUBY, attr: attr) expect(page).to have_css("select[#{attr}=foo]") ^^^^^^^^^^^^^^^^^^{attr}^^^^^^^ Prefer `have_select` over `have_css`. - expect(page).to have_css("select[#{attr}]") - ^^^^^^^^^^^^^^^^^^{attr}^^^ Prefer `have_select` over `have_css`. RUBY end end - %i[above below left_of right_of near count minimum maximum between - text id class style visible obscured exact exact_text normalize_ws - match wait filter_set checked unchecked disabled valid name - placeholder validation_message ].each do |attr| + %i[id class style checked disabled name placeholder].each do |attr| it 'registers an offense for abstract matcher when ' \ "first argument is element with replaceable attributes #{attr} " \ 'for `have_field`' do expect_offense(<<-RUBY, attr: attr) expect(page).to have_css("input[#{attr}=foo]") ^^^^^^^^^^^^^^^^^^{attr}^^^^^^ Prefer `have_field` over `have_css`. - expect(page).to have_css("input[#{attr}]") - ^^^^^^^^^^^^^^^^^^{attr}^^ Prefer `have_field` over `have_css`. RUBY end end @@ -193,43 +166,41 @@ class style visible obscured exact exact_text normalize_ws match wait it 'registers an offense when using abstract matcher with ' \ 'first argument is element with multiple replaceable attributes' do expect_offense(<<-RUBY) - expect(page).to have_css('button[disabled][name="foo"]', exact_text: 'foo') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. + expect(page).to have_css('button[disabled=true][name="foo"]', exact_text: 'foo') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. RUBY end it 'registers an offense when using abstract matcher with state' do expect_offense(<<-RUBY) - expect(page).to have_css('button[disabled]', exact_text: 'foo') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. + expect(page).to have_css('button[disabled=true]', exact_text: 'foo') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. RUBY end it 'registers an offense when using abstract matcher with ' \ 'first argument is element with replaceable pseudo-classes' do expect_offense(<<-RUBY) - expect(page).to have_css('button:not([disabled])', exact_text: 'bar') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - expect(page).to have_css('button:not([disabled][visible])') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. + expect(page).to have_css('button:not([disabled=true])', exact_text: 'bar') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. RUBY end it 'registers an offense when using abstract matcher with ' \ 'first argument is element with multiple replaceable pseudo-classes' do expect_offense(<<-RUBY) - expect(page).to have_css('button:not([disabled]):enabled') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. + expect(page).to have_css('button:not([disabled=true]):enabled') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. expect(page).to have_css('button:not([disabled=false]):disabled') ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - expect(page).to have_css('button:not([disabled]):not([disabled])') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. - expect(page).to have_css('input:not([unchecked]):checked') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_field` over `have_css`. - expect(page).to have_css('input:not([unchecked=false]):unchecked') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_field` over `have_css`. - expect(page).to have_css('input:not([unchecked]):not([unchecked])') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_field` over `have_css`. + expect(page).to have_css('button:not([disabled=true]):not([disabled=true])') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`. + expect(page).to have_css('input:not([checked=false]):checked') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_field` over `have_css`. + expect(page).to have_css('input:not([checked=false]):unchecked') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_field` over `have_css`. + expect(page).to have_css('input:not([checked=true]):not([checked=true])') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_field` over `have_css`. RUBY end @@ -251,6 +222,7 @@ class style visible obscured exact exact_text normalize_ws match wait it 'does not register an offense for abstract matcher when ' \ 'first argument is element with nonreplaceable attributes' do expect_no_offenses(<<-RUBY) + expect(page).to have_css('button[disabled]') expect(page).to have_css('button[data-disabled]') expect(page).to have_css('button[foo=bar]') expect(page).to have_css('button[foo-bar=baz]', exact_text: 'foo') @@ -260,10 +232,10 @@ class style visible obscured exact exact_text normalize_ws match wait it 'does not register an offense for abstract matcher when ' \ 'first argument is element with multiple nonreplaceable attributes' do expect_no_offenses(<<-RUBY) - expect(page).to have_css('button[disabled][foo]') - expect(page).to have_css('button[foo][disabled]') - expect(page).to have_css('button[foo][disabled][bar]') - expect(page).to have_css('button[disabled][foo=bar]') + expect(page).to have_css('button[disabled=true][foo]') + expect(page).to have_css('button[foo][disabled=true]') + expect(page).to have_css('button[foo][disabled=true][bar]') + expect(page).to have_css('button[disabled=true][foo=bar]') expect(page).to have_css('button[disabled=foo][bar]', exact_text: 'foo') RUBY end