Cleanup some rubocop style warnings

.rubocop.yml

@@ -46,10 +46,8 @@ Lint/UnusedMethodArgument:
     - 'lib/capybara/driver/node.rb'
 
 Lint/HandleExceptions:
-  Enabled: false
-
-Lint/Loop:
-  Enabled: false
+  Exclude:
+    - 'spec/**/*'
 
 Layout/EndAlignment:
   EnforcedStyleAlignWith: variable
@@ -68,7 +66,9 @@ Naming/UncommunicativeMethodParamName:
     - 'on'
 
 Style/RescueStandardError:
-  Enabled: false
+    Exclude:
+      - 'spec/**/*'
+      - 'lib/capybara/spec/**/*'
 
 Style/ParallelAssignment:
   Enabled: false
@@ -79,19 +79,13 @@ Style/SingleLineMethods:
 Style/StringLiterals:
   Enabled: false
 
-Style/RegexpLiteral:
-  Enabled: false
-
-Style/RescueModifier:
-  Enabled: false
-
 Style/Alias:
   Enabled: false
 
-Style/AndOr:
-  Enabled: false
-
 Style/Documentation:
+  Exclude:
+    - 'lib/capybara/spec/**/*'
+    - 'spec/**/*'
   Enabled: false
 
 Style/DoubleNegation:
@@ -106,9 +100,6 @@ Style/GuardClause:
 Style/EmptyElse:
   EnforcedStyle: empty
 
-Style/RedundantReturn:
-  Enabled: false
-
 Style/ClassAndModuleChildren:
   Enabled: false
 

capybara.gemspec

@@ -13,7 +13,7 @@ Gem::Specification.new do |s|
   s.email = ["twalpole@gmail.com", "jonas.nicklas@gmail.com"]
   s.description = "Capybara is an integration testing tool for rack based web applications. It simulates how a user would interact with a website"
 
-  s.files = Dir.glob("{lib,spec,.yard}/**/*") + %w(README.md History.md License.txt .yardopts)
+  s.files = Dir.glob("{lib,spec,.yard}/**/*") + %w[README.md History.md License.txt .yardopts]
 
   s.homepage = "https://github.com/teamcapybara/capybara"
   s.require_paths = ["lib"]
@@ -27,7 +27,7 @@ Gem::Specification.new do |s|
   s.add_runtime_dependency("xpath", ["~>3.0"])
 
   s.add_development_dependency("cucumber", [">= 2.3.0"])
-  s.add_development_dependency("erubi")  # dependency specification needed by rbx
+  s.add_development_dependency("erubi") # dependency specification needed by rbx
   s.add_development_dependency("fuubar", [">= 1.0.0"])
   s.add_development_dependency("launchy", [">= 2.0.4"])
   s.add_development_dependency("minitest")
@@ -39,14 +39,12 @@ Gem::Specification.new do |s|
   s.add_development_dependency("webdrivers") if ENV['CI']
   s.add_development_dependency("yard", [">= 0.9.0"])
 
-  if RUBY_ENGINE == 'rbx' then
+  if RUBY_ENGINE == 'rbx'
     s.add_development_dependency("json")
     s.add_development_dependency("racc")
     s.add_development_dependency("rubysl")
   end
 
-  if File.exist?("gem-private_key.pem")
-    s.signing_key = 'gem-private_key.pem'
-  end
+  s.signing_key = 'gem-private_key.pem' if File.exist?("gem-private_key.pem")
   s.cert_chain = ['gem-public_cert.pem']
 end

features/step_definitions/capybara_steps.rb

@@ -21,5 +21,5 @@ When(/^I use a matcher that fails$/) do
 end
 
 Then(/^the failing exception should be nice$/) do
-  expect(@error_message).to match %r{expected to find visible css \"h1#doesnotexist\"}
+  expect(@error_message).to match(/expected to find visible css \"h1#doesnotexist\"/)
 end

lib/capybara/config.rb

@@ -81,7 +81,7 @@ module Capybara
 
     def deprecate(method, alternate_method, once = false)
       @deprecation_notified ||= {}
-      warn "DEPRECATED: ##{method} is deprecated, please use ##{alternate_method} instead" unless once and @deprecation_notified[method]
+      warn "DEPRECATED: ##{method} is deprecated, please use ##{alternate_method} instead" unless once && @deprecation_notified[method]
       @deprecation_notified[method] = true
     end
   end

lib/capybara/node/actions.rb

@@ -286,7 +286,7 @@ module Capybara
 
       def _reset_style(element)
         session.execute_script(RESET_STYLE_SCRIPT, element)
-      rescue # swallow extra errors
+      rescue StandardError # rubocop:disable Lint/HandleExceptions swallow extra errors
       end
 
       def _check_with_label(selector, checked, locator, allow_label_click: session_options.automatic_label_click, **options)
@@ -294,13 +294,13 @@ module Capybara
           begin
             el = find(selector, locator, options)
             el.set(checked)
-          rescue => e
+          rescue StandardError => e
             raise unless allow_label_click && catch_error?(e)
             begin
               el ||= find(selector, locator, options.merge(visible: :all))
               res = find(:label, for: el, visible: true).click unless el.checked? == checked
               res
-            rescue # swallow extra errors - raise original
+            rescue StandardError # swallow extra errors - raise original
               raise e
             end
           end

lib/capybara/node/base.rb

@@ -81,7 +81,7 @@ module Capybara
           start_time = Capybara::Helpers.monotonic_time
           begin
             yield
-          rescue => e
+          rescue StandardError => e
             session.raise_server_error!
             raise e unless driver.wait? && catch_error?(e, errors)
             raise e if (Capybara::Helpers.monotonic_time - start_time) >= seconds

lib/capybara/node/document_matchers.rb

@@ -40,7 +40,7 @@ module Capybara
       def has_title?(title, **options)
         assert_title(title, options)
       rescue Capybara::ExpectationNotMet
-        return false
+        false
       end
 
       ##
@@ -52,7 +52,7 @@ module Capybara
       def has_no_title?(title, **options)
         assert_no_title(title, options)
       rescue Capybara::ExpectationNotMet
-        return false
+        false
       end
 
     private
@@ -62,7 +62,7 @@ module Capybara
         synchronize(query.wait) do
           yield(query)
         end
-        return true
+        true
       end
     end
   end

lib/capybara/node/element.rb

@@ -54,7 +54,7 @@ module Capybara
       # @return [String]              The text of the element
       #
       def text(type = nil)
-        type ||= :all unless session_options.ignore_hidden_elements or session_options.visible_text_only
+        type ||= :all unless session_options.ignore_hidden_elements || session_options.visible_text_only
         synchronize do
           if type == :all
             base.all_text
@@ -355,7 +355,7 @@ module Capybara
           begin
             reloaded = query_scope.reload.first(@query.name, @query.locator, @query.options)
             @base = reloaded.base if reloaded
-          rescue => e
+          rescue StandardError => e
             raise e unless catch_error?(e)
           end
         end
@@ -366,7 +366,7 @@ module Capybara
         %(#<Capybara::Node::Element tag="#{base.tag_name}" path="#{base.path}">)
       rescue NotSupportedByDriverError
         %(#<Capybara::Node::Element tag="#{base.tag_name}">)
-      rescue => e
+      rescue StandardError => e
         raise unless session.driver.invalid_element_errors.any? { |et| e.is_a?(et) }
 
         %(Obsolete #<Capybara::Node::Element>)

lib/capybara/node/finders.rb

@@ -298,11 +298,11 @@ module Capybara
       end
 
       def ambiguous?(query, result)
-        query.match == :one or query.match == :smart and result.size > 1
+        ((query.match == :one) || (query.match == :smart)) && (result.size > 1)
       end
 
       def prefer_exact?(query)
-        query.match == :smart or query.match == :prefer_exact
+        (query.match == :smart) || (query.match == :prefer_exact)
       end
 
       def options_include_minimum?(opts)

lib/capybara/node/matchers.rb

@@ -508,7 +508,7 @@ module Capybara
       def matches_selector?(*args, &optional_filter_block)
         assert_matches_selector(*args, &optional_filter_block)
       rescue Capybara::ExpectationNotMet
-        return false
+        false
       end
 
       ##
@@ -544,7 +544,7 @@ module Capybara
       def not_matches_selector?(*args, &optional_filter_block)
         assert_not_matches_selector(*args, &optional_filter_block)
       rescue Capybara::ExpectationNotMet
-        return false
+        false
       end
 
       ##

lib/capybara/node/simple.rb

@@ -45,7 +45,7 @@ module Capybara
         attr_name = name.to_s
         if attr_name == 'value'
           value
-        elsif tag_name == 'input' and native[:type] == 'checkbox' and attr_name == 'checked'
+        elsif (tag_name == 'input') && (native[:type] == 'checkbox') && (attr_name == 'checked')
           native['checked'] == 'checked'
         else
           native[attr_name]

lib/capybara/queries/selector_query.rb

@@ -56,7 +56,7 @@ module Capybara
 
         matches_node_filters?(node) && matches_filter_block?(node)
       rescue *(node.respond_to?(:session) ? node.session.driver.invalid_element_errors : [])
-        return false
+        false
       end
 
       def visible

lib/capybara/queries/text_query.rb

@@ -57,8 +57,8 @@ module Capybara
         message << " in #{@actual_text.inspect}"
 
         details_message = []
-        details_message << case_insensitive_message if @node and !@expected_text.is_a? Regexp
-        details_message << invisible_message if @node and check_visible_text? and report_on_invisible
+        details_message << case_insensitive_message if @node && !@expected_text.is_a?(Regexp)
+        details_message << invisible_message if @node && check_visible_text? && report_on_invisible
         details_message.compact!
 
         message << ". (However, #{details_message.join(' and ')}.)" unless details_message.empty?
@@ -78,7 +78,9 @@ module Capybara
         if invisible_count != @count
           "it was found #{invisible_count} #{Capybara::Helpers.declension('time', 'times', invisible_count)} including non-visible text"
         end
-      rescue # An error getting the non-visible text (if element goes out of scope) should not affect the response
+      rescue StandardError
+        # An error getting the non-visible text (if element goes out of scope) should not affect the response
+        ""
       end
 
       def valid_keys

lib/capybara/rack_test/node.rb

@@ -218,11 +218,11 @@ private
   end
 
   def submits?
-    (tag_name == 'input' and %w[submit image].include?(type)) || (tag_name == 'button' and [nil, "submit"].include?(type))
+    (tag_name == 'input' && %w[submit image].include?(type)) || (tag_name == 'button' && [nil, "submit"].include?(type))
   end
 
   def checkable?
-    tag_name == 'input' and %w[checkbox radio].include?(type)
+    tag_name == 'input' && %w[checkbox radio].include?(type)
   end
 
 protected

lib/capybara/result.rb

@@ -97,10 +97,9 @@ module Capybara
 
       if @query.options[:maximum]
         max_opt = Integer(@query.options[:maximum])
-        begin
-          @result_cache << @results_enum.next while @result_cache.size <= max_opt
-          return 1
-        rescue StopIteration
+        loop do
+          return 1 if @result_cache.size > max_opt
+          @result_cache << @results_enum.next
         end
       end
 
@@ -115,7 +114,7 @@ module Capybara
         return 1
       end
 
-      return 0
+      0
     end
 
     def matches_count?
@@ -130,7 +129,7 @@ module Capybara
         message << ", found #{count} #{Capybara::Helpers.declension('match', 'matches', count)}: " << full_results.map(&:text).map(&:inspect).join(", ")
       end
       unless rest.empty?
-        elements = rest.map { |el| el.text rescue "<<ERROR>>" }.map(&:inspect).join(", ")
+        elements = rest.map { |el| el.text rescue "<<ERROR>>" }.map(&:inspect).join(", ") # rubocop:disable Style/RescueModifier
         message << ". Also found " << elements << ", which matched the selector but not all filters."
       end
       message

lib/capybara/rspec/compound.rb

@@ -44,7 +44,7 @@ module Capybara
               raise ::Capybara::ElementNotFound unless [matcher_1_matches?, matcher_2_matches?].all?
               true
             end
-          rescue
+          rescue StandardError
             false
           end
         end
@@ -72,7 +72,7 @@ module Capybara
               raise ::Capybara::ElementNotFound unless [matcher_1_matches?, matcher_2_matches?].any?
               true
             end
-          rescue
+          rescue StandardError
             false
           end
         end

lib/capybara/rspec/matchers.rb

@@ -25,14 +25,14 @@ module Capybara
         yield(wrap(actual))
       rescue Capybara::ExpectationNotMet => e
         @failure_message = e.message
-        return false
+        false
       end
 
       def wrap_does_not_match?(actual)
         yield(wrap(actual))
       rescue Capybara::ExpectationNotMet => e
         @failure_message_when_negated = e.message
-        return false
+        false
       end
 
       def session_query_args

lib/capybara/selector.rb

@@ -234,7 +234,7 @@ Capybara.add_selector(:link_or_button) do
     self.class.all.values_at(:link, :button).map { |selector| selector.xpath.call(locator, options) }.reduce(:union)
   end
 
-  filter(:disabled, :boolean, default: false, skip_if: :all) { |node, value| node.tag_name == "a" or !(value ^ node.disabled?) }
+  filter(:disabled, :boolean, default: false, skip_if: :all) { |node, value| node.tag_name == "a" || !(value ^ node.disabled?) }
 
   describe { |disabled: nil, **_options| " that is disabled" if disabled == true }
 end

lib/capybara/selenium/driver.rb

@@ -137,7 +137,7 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
         begin
           @browser.manage.delete_all_cookies
           clear_storage
-        rescue Selenium::WebDriver::Error::UnhandledError
+        rescue Selenium::WebDriver::Error::UnhandledError # rubocop:disable Lint/HandleExceptions
           # delete_all_cookies fails when we've previously gone
           # to about:blank, so we rescue this error and do nothing
           # instead.
@@ -167,7 +167,7 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
             @browser.navigate.to("about:blank")
             sleep 0.1 # slight wait for alert
             @browser.switch_to.alert.accept
-          rescue modal_error # rubocop:disable Metrics/BlockNesting
+          rescue modal_error # rubocop:disable Metrics/BlockNesting, Lint/HandleExceptions
             # alert now gone, should mean navigation happened
           end
         end
@@ -264,7 +264,7 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
 
   def quit
     @browser&.quit
-  rescue Selenium::WebDriver::Error::SessionNotCreatedError, Errno::ECONNREFUSED
+  rescue Selenium::WebDriver::Error::SessionNotCreatedError, Errno::ECONNREFUSED # rubocop:disable Lint/HandleExceptions
     # Browser must have already gone
   rescue Selenium::WebDriver::Error::UnknownError => e
     unless silenced_unknown_error_message?(e.message) # Most likely already gone
@@ -339,6 +339,7 @@ private
         warn "sessionStorage clear requested but is not available for this driver"
       end
     end
+
     if options[:clear_local_storage]
       if @browser.respond_to? :local_storage
         @browser.local_storage.clear

lib/capybara/selenium/node.rb

@@ -21,7 +21,7 @@ class Capybara::Selenium::Node < Capybara::Driver::Node
   end
 
   def value
-    if tag_name == "select" and multiple?
+    if tag_name == "select" && multiple?
       native.find_elements(:css, "option:checked").map { |n| n[:value] || n.text }
     else
       native[:value]
@@ -90,12 +90,13 @@ class Capybara::Selenium::Node < Capybara::Driver::Node
         end
       end
     end
-  rescue => e
+  rescue StandardError => e
     if e.is_a?(::Selenium::WebDriver::Error::ElementClickInterceptedError) ||
        e.message =~ /Other element would receive the click/
       begin
         driver.execute_script("arguments[0].scrollIntoView({behavior: 'instant', block: 'center', inline: 'center'})", self)
-      rescue # Swallow error if scrollIntoView with options isn't supported
+      rescue StandardError # rubocop:disable Lint/HandleExceptions
+        # Swallow error if scrollIntoView with options isn't supported
       end
     end
     raise e
@@ -198,7 +199,7 @@ class Capybara::Selenium::Node < Capybara::Driver::Node
 private
 
   def boolean_attr(val)
-    val and val != "false"
+    val && (val != "false")
   end
 
   # a reference to the select node if this is an option node

lib/capybara/server.rb

@@ -99,11 +99,11 @@ module Capybara
         @using_ssl = true
       end
 
-      if res.is_a?(Net::HTTPSuccess) or res.is_a?(Net::HTTPRedirection)
+      if res.is_a?(Net::HTTPSuccess) || res.is_a?(Net::HTTPRedirection)
         return res.body == app.object_id.to_s
       end
     rescue SystemCallError
-      return false
+      false
     end
 
     def wait_for_pending_requests

lib/capybara/session.rb

@@ -83,7 +83,7 @@ module Capybara
         raise "A configuration block is only accepted when Capybara.threadsafe == true" unless Capybara.threadsafe
         yield config
       end
-      @server = if config.run_server and @app and driver.needs_server?
+      @server = if config.run_server && @app && driver.needs_server?
         Capybara::Server.new(@app, port: config.server_port, host: config.server_host, reportable_errors: config.server_errors).boot
       else
         nil
@@ -878,7 +878,7 @@ module Capybara
               driver.switch_to_window handle
               return Window.new(self, handle) if yield
             end
-          rescue => e
+          rescue StandardError => e
             driver.switch_to_window(original_window_handle)
             raise e
           else

lib/capybara/session/matchers.rb

@@ -49,7 +49,7 @@ module Capybara
     def has_current_path?(path, **options)
       assert_current_path(path, options)
     rescue Capybara::ExpectationNotMet
-      return false
+      false
     end
 
     ##
@@ -64,7 +64,7 @@ module Capybara
     def has_no_current_path?(path, **options)
       assert_no_current_path(path, options)
     rescue Capybara::ExpectationNotMet
-      return false
+      false
     end
 
   private
@@ -74,7 +74,7 @@ module Capybara
       document.synchronize(query.wait) do
         yield(query)
       end
-      return true
+      true
     end
   end
 end

lib/capybara/spec/session/assert_text_spec.rb

@@ -76,7 +76,7 @@ Capybara::SpecHelper.spec '#assert_text' do
     @session.visit('/with_html')
     expect do
       @session.assert_text(/xxxxyzzz/)
-    end.to raise_error(Capybara::ExpectationNotMet, /\Aexpected to find text matching \/xxxxyzzz\/ in "This is a test\\nHeader Class(.+)"\Z/)
+    end.to raise_error(Capybara::ExpectationNotMet, %r{\Aexpected to find text matching /xxxxyzzz/ in "This is a test\\nHeader Class(.+)"\Z})
   end
 
   it "should escape any characters that would have special meaning in a regexp" do

lib/capybara/spec/session/click_link_spec.rb

@@ -93,12 +93,12 @@ Capybara::SpecHelper.spec '#click_link' do
     end
 
     it "should find a link matching an exact regex pattern" do
-      @session.click_link('labore', href: /\/with_simple_html/)
+      @session.click_link('labore', href: %r{/with_simple_html})
       expect(@session).to have_content('Bar')
     end
 
     it "should find a link matching a partial regex pattern" do
-      @session.click_link('labore', href: /\/with_simple/)
+      @session.click_link('labore', href: %r{/with_simple})
       expect(@session).to have_content('Bar')
     end
 
@@ -137,7 +137,7 @@ Capybara::SpecHelper.spec '#click_link' do
 
   it "should follow redirects back to itself" do
     @session.click_link('BackToMyself')
-    expect(@session).to have_css('#referrer', text: /\/with_html$/)
+    expect(@session).to have_css('#referrer', text: %r{/with_html$})
     expect(@session).to have_content('This is a test')
   end
 

lib/capybara/spec/session/has_link_spec.rb

@@ -10,7 +10,7 @@ Capybara::SpecHelper.spec '#has_link?' do
     expect(@session).to have_link('awesome title')
     expect(@session).to have_link('A link', href: '/with_simple_html')
     expect(@session).to have_link(:'A link', href: :'/with_simple_html')
-    expect(@session).to have_link('A link', href: /\/with_simple_html/)
+    expect(@session).to have_link('A link', href: %r{/with_simple_html})
   end
 
   it "should be false if the given link is not on the page" do
@@ -34,6 +34,6 @@ Capybara::SpecHelper.spec '#has_no_link?' do
   it "should be true if the given link is not on the page" do
     expect(@session).to have_no_link('monkey')
     expect(@session).to have_no_link('A link', href: '/nonexistent-href')
-    expect(@session).to have_no_link('A link', href: /\/nonexistent-href/)
+    expect(@session).to have_no_link('A link', href: %r{/nonexistent-href})
   end
 end

lib/capybara/spec/session/save_and_open_screenshot_spec.rb

@@ -8,7 +8,7 @@ Capybara::SpecHelper.spec '#save_and_open_screenshot' do
   end
 
   it 'opens file from the default directory', requires: [:screenshot] do
-    expected_file_regex = %r{capybara-\d+\.png}
+    expected_file_regex = /capybara-\d+\.png/
     allow(@session.driver).to receive(:save_screenshot)
     allow(Launchy).to receive(:open)
 

lib/capybara/spec/spec_helper.rb

@@ -33,7 +33,7 @@ module Capybara
       end
 
       def filter(requires, metadata)
-        if requires and metadata[:capybara_skip]
+        if requires && metadata[:capybara_skip]
           requires.any? do |require|
             metadata[:capybara_skip].include?(require)
           end

lib/capybara/window.rb

@@ -120,12 +120,13 @@ module Capybara
       res = yield if block_given?
       prev_size = size
       start_time = Capybara::Helpers.monotonic_time
-      begin
+      loop do
         sleep 0.05
         cur_size = size
         return res if cur_size == prev_size
         prev_size = cur_size
-      end while (Capybara::Helpers.monotonic_time - start_time) < seconds
+        break if (Capybara::Helpers.monotonic_time - start_time) >= seconds
+      end
       raise Capybara::WindowError, "Window size not stable within #{seconds} seconds."
     end
 

spec/rspec/shared_spec_matchers.rb

@@ -215,7 +215,7 @@ RSpec.shared_examples Capybara::RSpecMatchers do |session, _mode|
         it "fails if has_no_selector? returns false" do
           expect do
             expect("<h1>Text</h1>").not_to have_selector(:css, 'h1')
-          end.to raise_error(%r{expected not to find visible css "h1"})
+          end.to raise_error(/expected not to find visible css "h1"/)
         end
       end
     end
@@ -251,7 +251,7 @@ RSpec.shared_examples Capybara::RSpecMatchers do |session, _mode|
         it "fails if has_no_selector? returns false" do
           expect do
             expect(page).not_to have_selector(:css, 'h1', text: 'test')
-          end.to raise_error(%r{expected not to find visible css "h1" with text "test"})
+          end.to raise_error(/expected not to find visible css "h1" with text "test"/)
         end
       end
     end

spec/server_spec.rb

@@ -35,7 +35,7 @@ RSpec.describe Capybara::Server do
     ensure
       Capybara.server_host = nil
     end
-  end unless ENV['TRAVIS'] && (RUBY_ENGINE == 'jruby') or Gem.win_platform?
+  end unless (ENV['TRAVIS'] && (RUBY_ENGINE == 'jruby')) || Gem.win_platform?
 
   it "should use specified port" do
     Capybara.server_port = 22789