diff --git a/CHANGELOG.md b/CHANGELOG.md index 682b16909..97edb53c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * Add new `RSpec/VariableName` cop. ([@tejasbubane][]) * Add new `RSpec/VariableDefinition` cop. ([@tejasbubane][]) +* Expand `Capybara/VisibilityMatcher` to support more than just `have_selector`. ([@twalpole][]) ## 1.39.0 (2020-05-01) @@ -506,3 +507,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features. [@aried3r]: https://github.com/aried3r [@AlexWayfer]: https://github.com/AlexWayfer [@tejasbubane]: https://github.com/tejasbubane +[@twalpole]: https://github.com/twalpole diff --git a/lib/rubocop/cop/rspec/capybara/visibility_matcher.rb b/lib/rubocop/cop/rspec/capybara/visibility_matcher.rb index 35f4579e1..4e23c12c9 100644 --- a/lib/rubocop/cop/rspec/capybara/visibility_matcher.rb +++ b/lib/rubocop/cop/rspec/capybara/visibility_matcher.rb @@ -18,35 +18,50 @@ module Capybara # # # bad # expect(page).to have_selector('.foo', visible: false) - # - # # bad - # expect(page).to have_selector('.foo', visible: true) - # - # # good - # expect(page).to have_selector('.foo', visible: :all) - # - # # good - # expect(page).to have_selector('.foo', visible: :hidden) + # expect(page).to have_css('.foo', visible: true) + # expect(page).to have_link('my link', visible: false) # # # good # expect(page).to have_selector('.foo', visible: :visible) + # expect(page).to have_css('.foo', visible: :all) + # expect(page).to have_link('my link', visible: :hidden) # class VisibilityMatcher < Cop MSG_FALSE = 'Use `:all` or `:hidden` instead of `false`.' MSG_TRUE = 'Use `:visible` instead of `true`.' + CAPYBARA_MATCHER_METHODS = %i[ + have_selector + have_css + have_xpath + have_link + have_button + have_field + have_select + have_table + have_checked_field + have_unchecked_field + have_text + have_content + ].freeze def_node_matcher :visible_true?, <<~PATTERN - (send nil? :have_selector ... (hash <$(pair (sym :visible) true) ...>)) + (send nil? #capybara_matcher? ... (hash <$(pair (sym :visible) true) ...>)) PATTERN def_node_matcher :visible_false?, <<~PATTERN - (send nil? :have_selector ... (hash <$(pair (sym :visible) false) ...>)) + (send nil? #capybara_matcher? ... (hash <$(pair (sym :visible) false) ...>)) PATTERN def on_send(node) visible_false?(node) { |arg| add_offense(arg, message: MSG_FALSE) } visible_true?(node) { |arg| add_offense(arg, message: MSG_TRUE) } end + + private + + def capybara_matcher?(method_name) + CAPYBARA_MATCHER_METHODS.include? method_name + end end end end diff --git a/manual/cops_capybara.md b/manual/cops_capybara.md index 53922f6d0..9ffda544f 100644 --- a/manual/cops_capybara.md +++ b/manual/cops_capybara.md @@ -109,18 +109,13 @@ symbol values, `:all`, `:hidden` or `:visible`. ```ruby # bad expect(page).to have_selector('.foo', visible: false) - -# bad -expect(page).to have_selector('.foo', visible: true) - -# good -expect(page).to have_selector('.foo', visible: :all) - -# good -expect(page).to have_selector('.foo', visible: :hidden) +expect(page).to have_css('.foo', visible: true) +expect(page).to have_link('my link', visible: false) # good expect(page).to have_selector('.foo', visible: :visible) +expect(page).to have_css('.foo', visible: :all) +expect(page).to have_link('my link', visible: :hidden) ``` ### Configurable attributes diff --git a/spec/rubocop/cop/rspec/capybara/visibility_matcher_spec.rb b/spec/rubocop/cop/rspec/capybara/visibility_matcher_spec.rb index 411eddee4..e108da5be 100644 --- a/spec/rubocop/cop/rspec/capybara/visibility_matcher_spec.rb +++ b/spec/rubocop/cop/rspec/capybara/visibility_matcher_spec.rb @@ -17,6 +17,33 @@ RUBY end + it 'recognizes multiple matchers' do + expect_offense(<<-RUBY) + expect(page).to have_css('.profile', visible: false) + ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. + expect(page).to have_xpath('.//profile', visible: false) + ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. + expect(page).to have_link('news', visible: false) + ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. + expect(page).to have_button('login', visible: false) + ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. + expect(page).to have_field('name', visible: false) + ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. + expect(page).to have_select('sauce', visible: false) + ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. + expect(page).to have_table('arrivals', visible: false) + ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. + expect(page).to have_checked_field('cat', visible: false) + ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. + expect(page).to have_unchecked_field('cat', visible: false) + ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. + expect(page).to have_text('My homepage', visible: false) + ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. + expect(page).to have_content('Success', visible: false) + ^^^^^^^^^^^^^^ Use `:all` or `:hidden` instead of `false`. + RUBY + end + it 'registers an offense when using a selector`' do expect_offense(<<-RUBY) expect(page).to have_selector(:css, '.my_element', visible: false)