Merge pull request #909 from twalpole/capybara_visibility_matchers

Support multiple matchers for Capybara/VisibilityMatcher cop

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

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

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

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)