General code cleanup

lib/capybara/node/actions.rb

@@ -256,9 +256,9 @@ module Capybara
       def select_datalist_option(input, value)
         datalist_options = session.evaluate_script(DATALIST_OPTIONS_SCRIPT, input)
         if (option = datalist_options.find { |o| o['value'] == value || o['label'] == value })
-          input.set(option["value"])
+          input.set(option['value'])
         else
-          raise ::Capybara::ElementNotFound, "Unable to find datalist option \"#{value}\""
+          raise ::Capybara::ElementNotFound, %(Unable to find datalist option "#{value}")
         end
       rescue ::Capybara::NotSupportedByDriverError
         # Implement for drivers that don't support JS
@@ -298,8 +298,7 @@ module Capybara
             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
+              find(:label, for: el, visible: true).click unless el.checked? == checked
-              res
             rescue StandardError # swallow extra errors - raise original
               raise e
             end
@@ -307,7 +306,7 @@ module Capybara
         end
       end
 
-      UPDATE_STYLE_SCRIPT = <<-'JS'
+      UPDATE_STYLE_SCRIPT = <<~'JS'
         var el = arguments[0];
         el.capybara_style_cache = el.style.cssText;
         var css = arguments[1];
@@ -318,7 +317,7 @@ module Capybara
         }
       JS
 
-      RESET_STYLE_SCRIPT = <<-'JS'
+      RESET_STYLE_SCRIPT = <<~'JS'
         var el = arguments[0];
         if (el.hasOwnProperty('capybara_style_cache')) {
           el.style.cssText = el.capybara_style_cache;
@@ -326,7 +325,7 @@ module Capybara
         }
       JS
 
-      DATALIST_OPTIONS_SCRIPT = <<-'JS'
+      DATALIST_OPTIONS_SCRIPT = <<~'JS'
         Array.prototype.slice.call((arguments[0].list||{}).options || []).
           filter(function(el){ return !el.disabled }).
           map(function(el){ return { "value": el.value, "label": el.label} })

lib/capybara/node/base.rb

@@ -86,7 +86,7 @@ module Capybara
             raise e unless driver.wait? && catch_error?(e, errors)
             raise e if (Capybara::Helpers.monotonic_time - start_time) >= seconds
             sleep(0.05)
-            raise Capybara::FrozenInTime, "time appears to be frozen, Capybara does not work with libraries which freeze time, consider using time travelling instead" if Capybara::Helpers.monotonic_time == start_time
+            raise Capybara::FrozenInTime, "Time appears to be frozen. Capybara does not work with libraries which freeze time, consider using time travelling instead" if Capybara::Helpers.monotonic_time == start_time
             reload if session_options.automatic_reload
             retry
           ensure

lib/capybara/node/element.rb

@@ -55,13 +55,7 @@ module Capybara
       #
       def text(type = nil)
         type ||= :all unless session_options.ignore_hidden_elements || session_options.visible_text_only
-        synchronize do
+        synchronize { type == :all ? base.all_text : base.visible_text }
-          if type == :all
-            base.all_text
-          else
-            base.visible_text
-          end
-        end
       end
 
       ##

lib/capybara/node/matchers.rb

@@ -91,7 +91,7 @@ module Capybara
       #
       def assert_selector(*args, &optional_filter_block)
         _verify_selector_result(args, optional_filter_block) do |result, query|
-          unless result.matches_count? && (!result.empty? || query.expects_none?)
+          unless result.matches_count? && (result.any? || query.expects_none?)
             raise Capybara::ExpectationNotMet, result.failure_message
           end
         end
@@ -114,7 +114,7 @@ module Capybara
       #
       def assert_all_of_selectors(*args, wait: nil, **options, &optional_filter_block)
         wait = session_options.default_max_wait_time if wait.nil?
-        selector = args.first.is_a?(Symbol) ? args.shift : session_options.default_selector
+        selector = extract_selector(args)
         synchronize(wait) do
           args.each do |locator|
             assert_selector(selector, locator, options, &optional_filter_block)
@@ -139,7 +139,7 @@ module Capybara
       #
       def assert_none_of_selectors(*args, wait: nil, **options, &optional_filter_block)
         wait = session_options.default_max_wait_time if wait.nil?
-        selector = args.first.is_a?(Symbol) ? args.shift : session_options.default_selector
+        selector = extract_selector(args)
         synchronize(wait) do
           args.each do |locator|
             assert_no_selector(selector, locator, options, &optional_filter_block)
@@ -659,12 +659,15 @@ module Capybara
 
     private
 
+      def extract_selector(args)
+        args.first.is_a?(Symbol) ? args.shift : session_options.default_selector
+      end
+
       def _verify_selector_result(query_args, optional_filter_block)
         query_args = _set_query_session_options(*query_args)
         query = Capybara::Queries::SelectorQuery.new(*query_args, &optional_filter_block)
         synchronize(query.wait) do
-          result = query.resolve_for(self)
+          yield query.resolve_for(self), query
-          yield result, query
         end
         true
       end
@@ -673,8 +676,7 @@ module Capybara
         query_args = _set_query_session_options(*query_args)
         query = Capybara::Queries::MatchQuery.new(*query_args, &optional_filter_block)
         synchronize(query.wait) do
-          result = query.resolve_for(query_scope)
+          yield query.resolve_for(query_scope)
-          yield result
         end
         true
       end
@@ -683,8 +685,7 @@ module Capybara
         query_args = _set_query_session_options(*query_args)
         query = Capybara::Queries::TextQuery.new(*query_args)
         synchronize(query.wait) do
-          count = query.resolve_for(self)
+          yield query.resolve_for(self), query
-          yield(count, query)
         end
         true
       end

lib/capybara/node/simple.rb

@@ -78,7 +78,7 @@ module Capybara
         if tag_name == 'textarea'
           native['_capybara_raw_value']
         elsif tag_name == 'select'
-          if native['multiple'] == 'multiple'
+          if multiple?
             native.xpath(".//option[@selected='selected']").map { |option| option[:value] || option.content }
           else
             option = native.xpath(".//option[@selected='selected']").first || native.xpath(".//option").first
@@ -139,6 +139,10 @@ module Capybara
         native.has_attribute?('selected')
       end
 
+      def multiple?
+        native.has_attribute?('multiple')
+      end
+
       def synchronize(_seconds = nil)
         yield # simple nodes don't need to wait
       end

lib/capybara/queries/selector_query.rb

@@ -36,7 +36,7 @@ module Capybara
         @description << "non-visible " if visible == :hidden
         @description << "#{label} #{locator.inspect}"
         @description << " with#{' exact' if exact_text == true} text #{options[:text].inspect}" if options[:text]
-        @description << " with exact text #{options[:exact_text]}" if options[:exact_text].is_a?(String)
+        @description << " with exact text #{exact_text}" if exact_text.is_a?(String)
         @description << " with id #{options[:id]}" if options[:id]
         @description << " with classes [#{Array(options[:class]).join(',')}]" if options[:class]
         @description << selector.description(options)
@@ -172,28 +172,23 @@ module Capybara
       end
 
       def filtered_xpath(expr)
-        if options.key?(:id) || options.key?(:class)
+        expr = "(#{expr})[#{XPath.attr(:id) == options[:id]}]" if options.key?(:id) && !custom_keys.include?(:id)
-          expr = "(#{expr})"
-          expr = "#{expr}[#{XPath.attr(:id) == options[:id]}]" if options.key?(:id) && !custom_keys.include?(:id)
         if options.key?(:class) && !custom_keys.include?(:class)
           class_xpath = Array(options[:class]).map do |klass|
             XPath.attr(:class).contains_word(klass)
           end.reduce(:&)
-            expr = "#{expr}[#{class_xpath}]"
+          expr = "(#{expr})[#{class_xpath}]"
-          end
         end
         expr
       end
 
       def filtered_css(expr)
-        if options.key?(:id) || options.key?(:class)
         css_selectors = expr.split(',').map(&:rstrip)
         expr = css_selectors.map do |sel|
           sel += "##{Capybara::Selector::CSS.escape(options[:id])}" if options.key?(:id) && !custom_keys.include?(:id)
           sel += Array(options[:class]).map { |k| ".#{Capybara::Selector::CSS.escape(k)}" }.join if options.key?(:class) && !custom_keys.include?(:class)
           sel
         end.join(", ")
-        end
         expr
       end
 
@@ -219,25 +214,29 @@ module Capybara
       end
 
       def describe_within?
-        @resolved_node && !(@resolved_node.is_a?(::Capybara::Node::Document) ||
+        @resolved_node && !document?(@resolved_node) && !simple_root?(@resolved_node)
-                            (@resolved_node.is_a?(::Capybara::Node::Simple) && @resolved_node.path == '/'))
       end
 
-      def matches_text_filter(node, text_option)
+      def document?(node)
-        regexp = if text_option.is_a?(Regexp)
+        node.is_a?(::Capybara::Node::Document)
-          text_option
-        elsif exact_text == true
-          /\A#{Regexp.escape(text_option.to_s)}\z/
-        else
-          Regexp.escape(text_option.to_s)
-        end
-        text_visible = visible
-        text_visible = :all if text_visible == :hidden
-        node.text(text_visible).match(regexp)
       end
 
-      def matches_exact_text_filter(node, exact_text_option)
+      def simple_root?(node)
-        regexp = /\A#{Regexp.escape(exact_text_option)}\z/
+        node.is_a?(::Capybara::Node::Simple) && node.path == '/'
+      end
+
+      def matches_text_filter(node, value)
+        return matches_exact_text_filter(node, value) if exact_text == true
+        regexp = value.is_a?(Regexp) ? value : Regexp.escape(value.to_s)
+        matches_text_regexp(node, regexp)
+      end
+
+      def matches_exact_text_filter(node, value)
+        regexp = value.is_a?(Regexp) ? value : /\A#{Regexp.escape(value.to_s)}\z/
+        matches_text_regexp(node, regexp)
+      end
+
+      def matches_text_regexp(node, regexp)
         text_visible = visible
         text_visible = :all if text_visible == :hidden
         node.text(text_visible).match(regexp)

lib/capybara/queries/text_query.rb

@@ -57,7 +57,7 @@ module Capybara
         message << " in #{@actual_text.inspect}"
 
         details_message = []
-        details_message << case_insensitive_message if @node && !@expected_text.is_a?(Regexp)
+        details_message << case_insensitive_message if @node && check_case_insensitive?
         details_message << invisible_message if @node && check_visible_text? && report_on_invisible
         details_message.compact!
 
@@ -75,12 +75,11 @@ module Capybara
       def invisible_message
         invisible_text = text(@node, :all)
         invisible_count = invisible_text.scan(@search_regexp).size
-        if invisible_count != @count
+        return if invisible_count == @count
         "it was found #{invisible_count} #{Capybara::Helpers.declension('time', 'times', invisible_count)} including non-visible text"
-        end
       rescue StandardError
         # An error getting the non-visible text (if element goes out of scope) should not affect the response
-        ""
+        nil
       end
 
       def valid_keys
@@ -91,6 +90,10 @@ module Capybara
         @type == :visible
       end
 
+      def check_case_insensitive?
+        !@expected_text.is_a?(Regexp)
+      end
+
       def text(node, query_type)
         node.text(query_type)
       end

lib/capybara/rack_test/browser.rb

@@ -34,7 +34,7 @@ class Capybara::RackTest::Browser
   end
 
   def follow(method, path, **attributes)
-    return if path.gsub(/^#{Regexp.escape(request_path)}/, '').start_with?('#') || path.downcase.start_with?('javascript:')
+    return if fragment_or_script?(path)
     process_and_follow_redirects(method, path, attributes, 'HTTP_REFERER' => current_url)
   end
 
@@ -63,9 +63,7 @@ class Capybara::RackTest::Browser
     new_uri.host ||= @current_host
     new_uri.port ||= @current_port unless new_uri.default_port == @current_port
 
-    @current_scheme = new_uri.scheme
+    @current_scheme, @current_host, @current_port = new_uri.select(:scheme, :host, :port)
-    @current_host = new_uri.host
-    @current_port = new_uri.port
 
     reset_cache!
     send(method, new_uri.to_s, attributes, env.merge(options[:headers] || {}))
@@ -79,9 +77,7 @@ class Capybara::RackTest::Browser
 
   def reset_host!
     uri = URI.parse(driver.session_options.app_host || driver.session_options.default_host)
-    @current_scheme = uri.scheme
+    @current_scheme, @current_host, @current_port = uri.select(:scheme, :host, :port)
-    @current_host = uri.host
-    @current_port = uri.port
   end
 
   def reset_cache!
@@ -122,4 +118,10 @@ protected
   rescue Rack::Test::Error
     "/"
   end
+
+private
+
+  def fragment_or_script?(path)
+    path.gsub(/^#{Regexp.escape(request_path)}/, '').start_with?('#') || path.downcase.start_with?('javascript:')
+  end
 end

lib/capybara/rack_test/form.rb

@@ -30,12 +30,9 @@ class Capybara::RackTest::Form < Capybara::RackTest::Node
 
     native.xpath(form_elements_xpath).map do |field|
       case field.name
-      when 'input'
+      when 'input' then add_input_param(field, params)
-        add_input_param(field, params)
+      when 'select' then add_select_param(field, params)
-      when 'select'
+      when 'textarea' then add_textarea_param(field, params)
-        add_select_param(field, params)
-      when 'textarea'
-        add_textarea_param(field, params)
       end
     end
     merge_param!(params, button[:name], button[:value] || "") if button[:name]
@@ -44,8 +41,8 @@ class Capybara::RackTest::Form < Capybara::RackTest::Node
   end
 
   def submit(button)
-    action = (button && button['formaction']) || native['action']
+    action = button&.[]('formaction') || native['action']
-    method = (button && button['formmethod']) || request_method
+    method = button&.[]('formmethod') || request_method
     driver.submit(method, action.to_s, params(button))
   end
 
@@ -66,6 +63,7 @@ private
   end
 
   def merge_param!(params, key, value)
+    key = key.to_s
     if Rack::Utils.respond_to?(:default_query_parser)
       Rack::Utils.default_query_parser.normalize_params(params, key, value, Rack::Utils.param_depth_limit)
     else
@@ -85,7 +83,7 @@ private
     if %w[radio checkbox].include? field['type']
       if field['checked']
         node = Capybara::RackTest::Node.new(driver, field)
-        merge_param!(params, field['name'].to_s, node.value.to_s)
+        merge_param!(params, field['name'], node.value.to_s)
       end
     elsif %w[submit image].include? field['type']
       # TODO: identify the click button here (in document order, rather
@@ -96,29 +94,29 @@ private
           NilUploadedFile.new
         else
           mime_info = MiniMime.lookup_by_filename(value)
-          Rack::Test::UploadedFile.new(value, (mime_info && mime_info.content_type).to_s)
+          Rack::Test::UploadedFile.new(value, mime_info&.content_type&.to_s)
         end
-        merge_param!(params, field['name'].to_s, file)
+        merge_param!(params, field['name'], file)
       else
-        merge_param!(params, field['name'].to_s, File.basename(field['value'].to_s))
+        merge_param!(params, field['name'], File.basename(field['value'].to_s))
       end
     else
-      merge_param!(params, field['name'].to_s, field['value'].to_s)
+      merge_param!(params, field['name'], field['value'].to_s)
     end
   end
 
   def add_select_param(field, params)
-    if field['multiple'] == 'multiple'
+    if field.has_attribute?('multiple')
       field.xpath(".//option[@selected]").each do |option|
-        merge_param!(params, field['name'].to_s, (option['value'] || option.text).to_s)
+        merge_param!(params, field['name'], (option['value'] || option.text).to_s)
       end
     else
       option = field.xpath('.//option[@selected]').first || field.xpath('.//option').first
-      merge_param!(params, field['name'].to_s, (option['value'] || option.text).to_s) if option
+      merge_param!(params, field['name'], (option['value'] || option.text).to_s) if option
     end
   end
 
   def add_textarea_param(field, params)
-    merge_param!(params, field['name'].to_s, field['_capybara_raw_value'].to_s.gsub(/\n/, "\r\n"))
+    merge_param!(params, field['name'], field['_capybara_raw_value'].to_s.gsub(/\n/, "\r\n"))
   end
 end

lib/capybara/rack_test/node.rb

@@ -228,26 +228,27 @@ private
 protected
 
   def checkbox_or_radio?(field = self)
-    field && (field.checkbox? || field.radio?)
+    field&.checkbox? || field&.radio?
   end
 
   def checkbox?
     input_field? && type == 'checkbox'
   end
 
-  def input_field?
-    tag_name == 'input'
-  end
-
   def radio?
     input_field? && type == 'radio'
   end
 
+  def text_or_password?
+    input_field? && (type == 'text' || type == 'password')
+  end
+
+  def input_field?
+    tag_name == 'input'
+  end
+
   def textarea?
     tag_name == "textarea"
   end
 
-  def text_or_password?
-    input_field? && (type == 'text' || type == 'password')
-  end
 end

lib/capybara/result.rb

@@ -109,7 +109,7 @@ module Capybara
           break if @result_cache.size > max
           @result_cache << @results_enum.next
         end
-        return 0 if @query.options[:between] === @result_cache.size
+        return 0 if @query.options[:between].include? @result_cache.size
         return @result_cache.size <=> min
       end
 

lib/capybara/selector.rb

@@ -154,9 +154,9 @@ Capybara.add_selector(:link) do
       matchers = [XPath.attr(:id) == locator,
                   XPath.string.n.is(locator),
                   XPath.attr(:title).is(locator),
-                  XPath.descendant(:img)[XPath.attr(:alt).is(locator)]].reduce(:|)
+                  XPath.descendant(:img)[XPath.attr(:alt).is(locator)]]
-      matchers |= XPath.attr(:'aria-label').is(locator) if enable_aria_label
+      matchers << XPath.attr(:'aria-label').is(locator) if enable_aria_label
-      xpath = xpath[matchers]
+      xpath = xpath[matchers.reduce(:|)]
     end
 
     xpath = xpath[find_by_attr(:title, title)]
@@ -188,7 +188,7 @@ end
 # @filter [String] :value Matches the value of an input button
 #
 Capybara.add_selector(:button) do
-  xpath(:value, :title, :type) do |locator, **options|
+  xpath(:value, :title, :type) do |locator, enable_aria_label: false, **options|
     input_btn_xpath = XPath.descendant(:input)[XPath.attr(:type).one_of('submit', 'reset', 'image', 'button')]
     btn_xpath = XPath.descendant(:button)
     image_btn_xpath = XPath.descendant(:input)[XPath.attr(:type) == 'image']
@@ -196,14 +196,14 @@ Capybara.add_selector(:button) do
     unless locator.nil?
       locator = locator.to_s
       locator_matches = XPath.attr(:id).equals(locator) | XPath.attr(:value).is(locator) | XPath.attr(:title).is(locator)
-      locator_matches |= XPath.attr(:'aria-label').is(locator) if options[:enable_aria_label]
+      locator_matches |= XPath.attr(:'aria-label').is(locator) if enable_aria_label
 
       input_btn_xpath = input_btn_xpath[locator_matches]
 
       btn_xpath = btn_xpath[locator_matches | XPath.string.n.is(locator) | XPath.descendant(:img)[XPath.attr(:alt).is(locator)]]
 
       alt_matches = XPath.attr(:alt).is(locator)
-      alt_matches |= XPath.attr(:'aria-label').is(locator) if options[:enable_aria_label]
+      alt_matches |= XPath.attr(:'aria-label').is(locator) if enable_aria_label
       image_btn_xpath = image_btn_xpath[alt_matches]
     end
 
@@ -384,10 +384,9 @@ Capybara.add_selector(:select) do
   end
 
   expression_filter(:with_options) do |expr, options|
-    options.each do |option|
+    options.inject(expr) do |xpath, option|
-      expr = expr[Capybara::Selector.all[:option].call(option)]
+      xpath[Capybara::Selector.all[:option].call(option)]
     end
-    expr
   end
 
   filter(:selected) do |node, selected|
@@ -427,10 +426,9 @@ Capybara.add_selector(:datalist_input) do
   end
 
   expression_filter(:with_options) do |expr, options|
-    options.each do |option|
+    options.inject(expr) do |xpath, option|
-      expr = expr[XPath.attr(:list) == XPath.anywhere(:datalist)[Capybara::Selector.all[:datalist_option].call(option)].attr(:id)]
+      xpath[XPath.attr(:list) == XPath.anywhere(:datalist)[Capybara::Selector.all[:datalist_option].call(option)].attr(:id)]
     end
-    expr
   end
 
   describe do |options: nil, with_options: nil, **opts|
@@ -544,8 +542,7 @@ Capybara.add_selector(:label) do
         field_or_value.find_xpath('./ancestor::label[1]').include? node.base
       end
     else
-      # Non element values were handled through the expression filter
+      true # Non element values were handled through the expression filter
-      true
     end
   end
 
@@ -566,10 +563,10 @@ end
 # @filter [String, Array<String>] :class  Matches the class(es) provided
 #
 Capybara.add_selector(:table) do
-  xpath(:caption) do |locator, options|
+  xpath(:caption) do |locator, caption: nil, **_options|
     xpath = XPath.descendant(:table)
     xpath = xpath[(XPath.attr(:id) == locator.to_s) | XPath.descendant(:caption).is(locator.to_s)] unless locator.nil?
-    xpath = xpath[XPath.descendant(:caption) == options[:caption]] if options[:caption]
+    xpath = xpath[XPath.descendant(:caption) == caption] if caption
     xpath
   end
 

lib/capybara/selenium/node.rb

@@ -77,16 +77,12 @@ class Capybara::Selenium::Node < Capybara::Driver::Node
   end
 
   def click(keys = [], options = {})
-    if keys.empty? && !(options[:x] && options[:y])
+    if keys.empty? && !has_coords?(options)
       native.click
     else
       scroll_if_needed do
         action_with_modifiers(keys, options) do |a|
-          if options[:x] && options[:y]
+          has_coords?(options) ? a.click : a.click(native)
-            a.click
-          else
-            a.click(native)
-          end
         end
       end
     end
@@ -101,11 +97,7 @@ class Capybara::Selenium::Node < Capybara::Driver::Node
   def right_click(keys = [], options = {})
     scroll_if_needed do
       action_with_modifiers(keys, options) do |a|
-        if options[:x] && options[:y]
+        has_coords?(options) ? a.context_click : a.context_click(native)
-          a.context_click
-        else
-          a.context_click(native)
-        end
       end
     end
   end
@@ -113,11 +105,7 @@ class Capybara::Selenium::Node < Capybara::Driver::Node
   def double_click(keys = [], options = {})
     scroll_if_needed do
       action_with_modifiers(keys, options) do |a|
-        if options[:x] && options[:y]
+        has_coords?(options) ? a.double_click : a.double_click(native)
-          a.double_click
-        else
-          a.double_click(native)
-        end
       end
     end
   end
@@ -194,6 +182,10 @@ class Capybara::Selenium::Node < Capybara::Driver::Node
 
 private
 
+  def has_coords?(options)
+    options[:x] && options[:y]
+  end
+
   def boolean_attr(val)
     val && (val != "false")
   end

lib/capybara/session.rb

@@ -193,7 +193,7 @@ module Capybara
       return nil if uri&.scheme == "about"
 
       path = uri&.path
-      path if !path&.empty?
+      path unless path&.empty?
     end
 
     ##

spec/result_spec.rb

@@ -138,6 +138,12 @@ RSpec.describe Capybara::Result do
     expect(result.instance_variable_get('@result_cache').size).to be 4
   end
 
+  it "should only need to evaluate one result for any?" do
+    skip 'JRuby has an issue with lazy enumerator evaluation' if RUBY_PLATFORM == 'java'
+    result.any?
+    expect(result.instance_variable_get('@result_cache').size).to be 1
+  end
+
   it "should evaluate all elements when #to_a called" do
     # All cached when converted to array
     result.to_a