Merge pull request #2258 from teamcapybara/cleanup2

Fix for RuboCop Style/IfUnlessModifier changes

.rubocop.yml

@@ -111,6 +111,10 @@ Style/SpecialGlobalVars:
   Exclude:
     - 'capybara.gemspec'
 
+Style/IfUnlessModifier:
+  Exclude:
+    - 'spec/**/*'
+
 Layout/EmptyLineBetweenDefs:
   AllowAdjacentOneLineDefs: true
 

lib/capybara/config.rb

@@ -27,7 +27,9 @@ module Capybara
     attr_writer :reuse_server
 
     def threadsafe=(bool)
-      raise 'Threadsafe setting cannot be changed once a session is created' if (bool != threadsafe) && Session.instance_created?
+      if (bool != threadsafe) && Session.instance_created?
+        raise 'Threadsafe setting cannot be changed once a session is created'
+      end
 
       @threadsafe = bool
     end
@@ -83,7 +85,9 @@ module Capybara
 
     def deprecate(method, alternate_method, once = false)
       @deprecation_notified ||= {}
-      warn "DEPRECATED: ##{method} is deprecated, please use ##{alternate_method} instead" unless once && @deprecation_notified[method]
+      unless once && @deprecation_notified[method]
+        warn "DEPRECATED: ##{method} is deprecated, please use ##{alternate_method} instead"
+      end
       @deprecation_notified[method] = true
     end
   end

lib/capybara/helpers.rb

@@ -87,7 +87,9 @@ module Capybara
       end
 
       def expired?
-        raise Capybara::FrozenInTime, 'Time appears to be frozen. Capybara does not work with libraries which freeze time, consider using time travelling instead' if stalled?
+        if stalled?
+          raise Capybara::FrozenInTime, 'Time appears to be frozen. Capybara does not work with libraries which freeze time, consider using time travelling instead'
+        end
 
         current - @start >= @expire_in
       end

lib/capybara/node/actions.rb

@@ -276,7 +276,9 @@ module Capybara
       #   @yield Block whose actions will trigger the system file chooser to be shown
       # @return [Capybara::Node::Element]  The file field element
       def attach_file(locator = nil, paths, make_visible: nil, **options) # rubocop:disable Style/OptionalArguments
-        raise ArgumentError, '``#attach_file` does not support passing both a locator and a block' if locator && block_given?
+        if locator && block_given?
+          raise ArgumentError, '``#attach_file` does not support passing both a locator and a block'
+        end
 
         Array(paths).each do |path|
           raise Capybara::FileNotFound, "cannot attach file, #{path} does not exist" unless File.exist?(path.to_s)
@@ -338,7 +340,9 @@ module Capybara
           visible_css = { opacity: 1, display: 'block', visibility: 'visible', width: 'auto', height: 'auto' }
         end
         _update_style(element, visible_css)
-        raise ExpectationNotMet, 'The style changes in :make_visible did not make the file input visible' unless element.visible?
+        unless element.visible?
+          raise ExpectationNotMet, 'The style changes in :make_visible did not make the file input visible'
+        end
 
         begin
           yield element

lib/capybara/node/element.rb

@@ -113,7 +113,9 @@ module Capybara
       #
       # @return [Capybara::Node::Element]  The element
       def set(value, **options)
-        raise Capybara::ReadOnlyElementError, "Attempt to set readonly element with value: #{value}" if ENV['CAPYBARA_THOROUGH'] && readonly?
+        if ENV['CAPYBARA_THOROUGH'] && readonly?
+          raise Capybara::ReadOnlyElementError, "Attempt to set readonly element with value: #{value}"
+        end
 
         options = session_options.default_set_options.to_h.merge(options)
         synchronize { base.set(value, options) }

lib/capybara/node/finders.rb

@@ -292,7 +292,9 @@ module Capybara
             result = query.resolve_for(self)
           end
 
-          raise Capybara::Ambiguous, "Ambiguous match, found #{result.size} elements matching #{query.applied_description}" if ambiguous?(query, result)
+          if ambiguous?(query, result)
+            raise Capybara::Ambiguous, "Ambiguous match, found #{result.size} elements matching #{query.applied_description}"
+          end
           raise Capybara::ElementNotFound, "Unable to find #{query.applied_description}" if result.empty?
 
           result.first

lib/capybara/node/matchers.rb

@@ -107,7 +107,9 @@ module Capybara
       #
       def assert_selector(*args, &optional_filter_block)
         _verify_selector_result(args, optional_filter_block) do |result, query|
-          raise Capybara::ExpectationNotMet, result.failure_message unless result.matches_count? && (result.any? || query.expects_none?)
+          unless result.matches_count? && (result.any? || query.expects_none?)
+            raise Capybara::ExpectationNotMet, result.failure_message
+          end
         end
       end
 
@@ -738,7 +740,9 @@ module Capybara
       #
       def assert_ancestor(*args, &optional_filter_block)
         _verify_selector_result(args, optional_filter_block, Capybara::Queries::AncestorQuery) do |result, query|
-          raise Capybara::ExpectationNotMet, result.failure_message unless result.matches_count? && (result.any? || query.expects_none?)
+          unless result.matches_count? && (result.any? || query.expects_none?)
+            raise Capybara::ExpectationNotMet, result.failure_message
+          end
         end
       end
 
@@ -779,7 +783,9 @@ module Capybara
       #
       def assert_sibling(*args, &optional_filter_block)
         _verify_selector_result(args, optional_filter_block, Capybara::Queries::SiblingQuery) do |result, query|
-          raise Capybara::ExpectationNotMet, result.failure_message unless result.matches_count? && (result.any? || query.expects_none?)
+          unless result.matches_count? && (result.any? || query.expects_none?)
+            raise Capybara::ExpectationNotMet, result.failure_message
+          end
         end
       end
 

lib/capybara/queries/selector_query.rb

@@ -77,7 +77,9 @@ module Capybara
         end
 
         %i[above below left_of right_of near].each do |spatial_filter|
-          desc << " #{spatial_filter} #{options[spatial_filter] rescue '<ERROR>'}" if options[spatial_filter] && show_for[:spatial] # rubocop:disable Style/RescueModifier
+          if options[spatial_filter] && show_for[:spatial]
+            desc << " #{spatial_filter} #{options[spatial_filter] rescue '<ERROR>'}" # rubocop:disable Style/RescueModifier
+          end
         end
 
         desc << selector.description(node_filters: show_for[:node], **options)

lib/capybara/rack_test/browser.rb

@@ -53,7 +53,10 @@ class Capybara::RackTest::Browser
         end
       end
     end
-    raise Capybara::InfiniteRedirectError, "redirected more than #{driver.redirect_limit} times, check for infinite redirects." if last_response.redirect?
+
+    if last_response.redirect? # rubocop:disable Style/GuardClause
+      raise Capybara::InfiniteRedirectError, "redirected more than #{driver.redirect_limit} times, check for infinite redirects."
+    end
   end
 
   def process(method, path, attributes = {}, env = {})

lib/capybara/selector/definition/css.rb

@@ -2,7 +2,9 @@
 
 Capybara.add_selector(:css, locator_type: [String, Symbol], raw_locator: true) do
   css do |css|
-    warn "DEPRECATED: Passing a symbol (#{css.inspect}) as the CSS locator is deprecated - please pass a string instead." if css.is_a? Symbol
+    if css.is_a? Symbol
+      warn "DEPRECATED: Passing a symbol (#{css.inspect}) as the CSS locator is deprecated - please pass a string instead."
+    end
     css
   end
 end

lib/capybara/selector/filter_set.rb

@@ -112,7 +112,9 @@ module Capybara
 
       def add_filter(name, filter_class, *types, matcher: nil, **options, &block)
         types.each { |type| options[type] = true }
-        raise 'ArgumentError', ':default option is not supported for filters with a :matcher option' if matcher && options[:default]
+        if matcher && options[:default]
+          raise 'ArgumentError', ':default option is not supported for filters with a :matcher option'
+        end
 
         filter = filter_class.new(name, matcher, block, options)
         (filter_class <= Filters::ExpressionFilter ? @expression_filters : @node_filters)[name] = filter

lib/capybara/selector/filters/base.rb

@@ -48,7 +48,12 @@ module Capybara
 
         def apply(subject, name, value, skip_value, ctx)
           return skip_value if skip?(value)
-          raise ArgumentError, "Invalid value #{value.inspect} passed to #{self.class.name.split('::').last} #{name}#{" : #{@name}" if @name.is_a?(Regexp)}" unless valid_value?(value)
+
+          unless valid_value?(value)
+            raise ArgumentError,
+                  "Invalid value #{value.inspect} passed to #{self.class.name.split('::').last} #{name}" \
+                  "#{" : #{name}" if @name.is_a?(Regexp)}"
+          end
 
           if @block.arity == 2
             filter_context(ctx).instance_exec(subject, value, &@block)

lib/capybara/selector/selector.rb

@@ -61,7 +61,9 @@ module Capybara
         warn 'Selector has no format'
       end
     ensure
-      warn "Locator #{locator.class}:#{locator.inspect} for selector #{name.inspect} must #{locator_description}. This will raise an error in a future version of Capybara." unless locator_valid?(locator)
+      unless locator_valid?(locator)
+        warn "Locator #{locator.class}:#{locator.inspect} for selector #{name.inspect} must #{locator_description}. This will raise an error in a future version of Capybara."
+      end
     end
 
     def add_error(error_msg)

lib/capybara/selenium/driver.rb

@@ -20,7 +20,9 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
       require 'capybara/selenium/logger_suppressor'
       require 'capybara/selenium/patches/atoms'
       require 'capybara/selenium/patches/is_displayed'
-      warn "Warning: You're using an unsupported version of selenium-webdriver, please upgrade." if Gem.loaded_specs['selenium-webdriver'].version < Gem::Version.new('3.5.0')
+      if Gem.loaded_specs['selenium-webdriver'].version < Gem::Version.new('3.5.0')
+        warn "Warning: You're using an unsupported version of selenium-webdriver, please upgrade."
+      end
     rescue LoadError => e
       raise e unless e.message.match?(/selenium-webdriver/)
 
@@ -325,7 +327,9 @@ private
       begin
         @browser&.execute_script('window.sessionStorage.clear()')
       rescue # rubocop:disable Style/RescueStandardError
-        warn 'sessionStorage clear requested but is not supported by this driver' unless options[:clear_session_storage].nil?
+        unless options[:clear_session_storage].nil?
+          warn 'sessionStorage clear requested but is not supported by this driver'
+        end
       end
     end
   end
@@ -337,7 +341,9 @@ private
       begin
         @browser&.execute_script('window.localStorage.clear()')
       rescue # rubocop:disable Style/RescueStandardError
-        warn 'localStorage clear requested but is not supported by this driver' unless options[:clear_local_storage].nil?
+        unless options[:clear_local_storage].nil?
+          warn 'localStorage clear requested but is not supported by this driver'
+        end
       end
     end
   end
@@ -378,7 +384,9 @@ private
         alert = @browser.switch_to.alert
         regexp = text.is_a?(Regexp) ? text : Regexp.new(Regexp.escape(text.to_s))
         matched = alert.text.match?(regexp)
-        raise Capybara::ModalNotFound, "Unable to find modal dialog with #{text} - found '#{alert.text}' instead." unless matched
+        unless matched
+          raise Capybara::ModalNotFound, "Unable to find modal dialog with #{text} - found '#{alert.text}' instead."
+        end
 
         alert
       end

lib/capybara/selenium/node.rb

@@ -54,7 +54,9 @@ class Capybara::Selenium::Node < Capybara::Driver::Node
   #   :backspace => send backspace keystrokes to clear the field <br/>
   #   Array => an array of keys to send before the value being set, e.g. [[:command, 'a'], :backspace]
   def set(value, **options)
-    raise ArgumentError, "Value cannot be an Array when 'multiple' attribute is not present. Not a #{value.class}" if value.is_a?(Array) && !multiple?
+    if value.is_a?(Array) && !multiple?
+      raise ArgumentError, "Value cannot be an Array when 'multiple' attribute is not present. Not a #{value.class}"
+    end
 
     tag_name, type = attrs(:tagName, :type).map { |val| val&.downcase }
     @tag_name ||= tag_name

lib/capybara/selenium/nodes/chrome_node.rb

@@ -25,7 +25,9 @@ class Capybara::Selenium::ChromeNode < Capybara::Selenium::Node
     end
     super
   rescue *file_errors => e
-    raise ArgumentError, "Selenium < 3.14 with remote Chrome doesn't support multiple file upload" if e.message.match?(/File not found : .+\n.+/m)
+    if e.message.match?(/File not found : .+\n.+/m)
+      raise ArgumentError, "Selenium < 3.14 with remote Chrome doesn't support multiple file upload"
+    end
 
     raise
   end
@@ -40,7 +42,9 @@ class Capybara::Selenium::ChromeNode < Capybara::Selenium::Node
     raise
   rescue ::Selenium::WebDriver::Error::WebDriverError => e
     # chromedriver 74 (at least on mac) raises the wrong error for this
-    raise ::Selenium::WebDriver::Error::ElementClickInterceptedError, e.message if e.message.match?(/element click intercepted/)
+    if e.message.match?(/element click intercepted/)
+      raise ::Selenium::WebDriver::Error::ElementClickInterceptedError, e.message
+    end
 
     raise
   end

lib/capybara/selenium/nodes/edge_node.rb

@@ -25,7 +25,9 @@ class Capybara::Selenium::EdgeNode < Capybara::Selenium::Node
     end
     super
   rescue *file_errors => e
-    raise ArgumentError, "Selenium < 3.14 with remote Chrome doesn't support multiple file upload" if e.message.match?(/File not found : .+\n.+/m)
+    if e.message.match?(/File not found : .+\n.+/m)
+      raise ArgumentError, "Selenium < 3.14 with remote Chrome doesn't support multiple file upload"
+    end
 
     raise
   end

lib/capybara/server.rb

@@ -24,7 +24,9 @@ module Capybara
                    host: Capybara.server_host,
                    reportable_errors: Capybara.server_errors,
                    extra_middleware: [])
-      warn 'Positional arguments, other than the application, to Server#new are deprecated, please use keyword arguments' unless deprecated_options.empty?
+      unless deprecated_options.empty?
+        warn 'Positional arguments, other than the application, to Server#new are deprecated, please use keyword arguments'
+      end
       @app = app
       @extra_middleware = extra_middleware
       @server_thread = nil # suppress warnings

lib/capybara/session.rb

@@ -75,7 +75,9 @@ module Capybara
     attr_accessor :synchronized
 
     def initialize(mode, app = nil)
-      raise TypeError, 'The second parameter to Session::new should be a rack app if passed.' if app && !app.respond_to?(:call)
+      if app && !app.respond_to?(:call)
+        raise TypeError, 'The second parameter to Session::new should be a rack app if passed.'
+      end
 
       @@instance_created = true # rubocop:disable Style/ClassVars
       @mode = mode

lib/capybara/session/config.rb

@@ -79,14 +79,18 @@ module Capybara
 
     remove_method :app_host=
     def app_host=(url)
-      raise ArgumentError, "Capybara.app_host should be set to a url (http://www.example.com). Attempted to set #{url.inspect}." unless url.nil? || url.match?(URI::DEFAULT_PARSER.make_regexp)
+      unless url.nil? || url.match?(URI::DEFAULT_PARSER.make_regexp)
+        raise ArgumentError, "Capybara.app_host should be set to a url (http://www.example.com). Attempted to set #{url.inspect}."
+      end
 
       @app_host = url
     end
 
     remove_method :default_host=
     def default_host=(url)
-      raise ArgumentError, "Capybara.default_host should be set to a url (http://www.example.com). Attempted to set #{url.inspect}." unless url.nil? || url.match?(URI::DEFAULT_PARSER.make_regexp)
+      unless url.nil? || url.match?(URI::DEFAULT_PARSER.make_regexp)
+        raise ArgumentError, "Capybara.default_host should be set to a url (http://www.example.com). Attempted to set #{url.inspect}."
+      end
 
       @default_host = url
     end