Style updates

.gitignore

@@ -22,3 +22,4 @@ bin
 gem-private_key.pem
 save_path_tmp
 vendor/bundle
+.byebug_history

.rubocop.yml

@@ -3,6 +3,7 @@ require:
   - rubocop-performance
 
 AllCops:
+  NewCops: enable
   DisabledByDefault: false
   TargetRubyVersion: 2.5
   Exclude:

Rakefile

@@ -81,7 +81,7 @@ end
 task :release do
   version = Capybara::VERSION
   puts "Releasing #{version}, y/n?"
-  exit(1) unless STDIN.gets.chomp == 'y'
+  exit(1) unless $stdin.gets.chomp == 'y'
   sh "git commit -am 'tagged #{version}' && " \
      "git tag #{version} && " \
      'gem build capybara.gemspec && ' \

lib/capybara/config.rb

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

lib/capybara/node/simple.rb

@@ -100,7 +100,7 @@ module Capybara
       # @param  [Boolean] check_ancestors  Whether to inherit visibility from ancestors
       # @return [Boolean]     Whether the element is visible
       #
-      def visible?(check_ancestors = true)
+      def visible?(check_ancestors = true) # rubocop:disable Style/OptionalBooleanParameter
         return false if (tag_name == 'input') && (native[:type] == 'hidden')
         return false if tag_name == 'template'
 

lib/capybara/queries/ancestor_query.rb

@@ -16,7 +16,7 @@ module Capybara
         end
       end
 
-      def description(applied = false)
+      def description(applied = false) # rubocop:disable Style/OptionalBooleanParameter
         child_query = @child_node&.instance_variable_get(:@query)
         desc = super
         desc += " that is an ancestor of #{child_query.description}" if child_query

lib/capybara/queries/selector_query.rb

@@ -55,7 +55,7 @@ module Capybara
       def name; selector.name; end
       def label; selector.label || selector.name; end
 
-      def description(only_applied = false)
+      def description(only_applied = false) # rubocop:disable Style/OptionalBooleanParameter
         desc = +''
         show_for = show_for_stage(only_applied)
 
@@ -95,11 +95,9 @@ module Capybara
         desc << ' that also matches the custom filter block' if @filter_block && show_for[:node]
 
         desc << " within #{@resolved_node.inspect}" if describe_within?
-        if locator.is_a?(String) && locator.start_with?('#', './/', '//')
+        if locator.is_a?(String) && locator.start_with?('#', './/', '//') && !selector.raw_locator?
-          unless selector.raw_locator?
+          desc << "\nNote: It appears you may be passing a CSS selector or XPath expression rather than a locator. " \
-            desc << "\nNote: It appears you may be passing a CSS selector or XPath expression rather than a locator. " \
+                  "Please see the documentation for acceptable locator values.\n\n"
-                    "Please see the documentation for acceptable locator values.\n\n"
-          end
         end
         desc
       end
@@ -653,7 +651,7 @@ module Capybara
 
           d = u.dot w
           e = v.dot w
-          cap_d = (a * c) - (b * b)
+          cap_d = (a * c) - (b**2)
           sD = tD = cap_d
 
           # compute the line parameters of the two closest points

lib/capybara/queries/sibling_query.rb

@@ -15,7 +15,7 @@ module Capybara
         end
       end
 
-      def description(applied = false)
+      def description(applied = false) # rubocop:disable Style/OptionalBooleanParameter
         desc = super
         sibling_query = @sibling_node&.instance_variable_get(:@query)
         desc += " that is a sibling of #{sibling_query.description}" if sibling_query

lib/capybara/rack_test/driver.rb

@@ -17,6 +17,7 @@ class Capybara::RackTest::Driver < Capybara::Driver::Base
   def initialize(app, **options)
     raise ArgumentError, 'rack-test requires a rack application, but none was given' unless app
 
+    super()
     @app = app
     @options = DEFAULT_OPTIONS.merge(options)
   end

lib/capybara/rack_test/form.rb

@@ -6,7 +6,7 @@ class Capybara::RackTest::Form < Capybara::RackTest::Node
   # That check should be based solely on the form element's 'enctype' attribute value,
   # which should probably be provided to Rack::Test in its non-GET request methods.
   class NilUploadedFile < Rack::Test::UploadedFile
-    def initialize
+    def initialize # rubocop:disable Lint/MissingSuper
       @empty_file = Tempfile.new('nil_uploaded_file')
       @empty_file.close
     end

lib/capybara/registration_container.rb

@@ -27,7 +27,7 @@ module Capybara
       super
     end
 
-    def respond_to_missing?(method_name, include_private = false)
+    def respond_to_missing?(method_name, include_all)
       @registered.respond_to?(method_name) || super
     end
 

lib/capybara/registrations/patches/puma_ssl.rb

@@ -4,12 +4,14 @@ module Puma
   module MiniSSL
     class Socket
       def read_nonblock(size, *_)
+        wait_states = %i[wait_readable wait_writable]
+
         loop do
           output = engine_read_all
           return output if output
 
           data = @socket.read_nonblock(size, exception: false)
-          raise IO::EAGAINWaitReadable if %i[wait_readable wait_writable].include? data
+          raise IO::EAGAINWaitReadable if wait_states.include? data
           return nil if data.nil?
 
           @engine.inject(data)

lib/capybara/result.rb

@@ -91,13 +91,9 @@ module Capybara
         return load_up_to(count + 1) <=> count
       end
 
-      if min && (min = Integer(min))
+      return -1 if min && (min = Integer(min)) && (load_up_to(min) < min)
-        return -1 if load_up_to(min) < min
-      end
 
-      if max && (max = Integer(max))
+      return 1 if max && (max = Integer(max)) && (load_up_to(max + 1) > max)
-        return 1 if load_up_to(max + 1) > max
-      end
 
       if between
         min, max = (between.begin && between.min) || 1, between.end

lib/capybara/selector/definition.rb

@@ -259,7 +259,8 @@ module Capybara
       end
 
       def parameter_names(block)
-        block.parameters.select { |(type, _name)| %i[key keyreq].include? type }.map { |(_type, name)| name }
+        key_types = %i[key keyreq]
+        block.parameters.select { |(type, _name)| key_types.include? type }.map { |(_type, name)| name }
       end
 
       def expression(type, allowed_filters, &block)

lib/capybara/selector/definition/element.rb

@@ -18,7 +18,8 @@ Capybara.add_selector(:element, locator_type: [String, Symbol]) do
   end
 
   describe_expression_filters do |**options|
-    booleans, values = options.partition { |_k, v| [true, false].include? v }.map(&:to_h)
+    boolean_values = [true, false]
+    booleans, values = options.partition { |_k, v| boolean_values.include? v }.map(&:to_h)
     desc = describe_all_expression_filters(**values)
     desc + booleans.map do |k, v|
       v ? " with #{k} attribute" : "without #{k} attribute"

lib/capybara/selenium/driver.rb

@@ -75,6 +75,7 @@ class Capybara::Selenium::Driver < Capybara::Driver::Base
   end
 
   def initialize(app, **options)
+    super()
     self.class.load_selenium
     @app = app
     @browser = nil

lib/capybara/selenium/node.rb

@@ -464,8 +464,8 @@ private
     end
   end
 
-  def each_key(keys)
+  def each_key(keys, &block)
-    normalize_keys(keys).each { |key| yield(key) }
+    normalize_keys(keys).each(&block)
   end
 
   def find_context

lib/capybara/selenium/nodes/chrome_node.rb

@@ -110,7 +110,7 @@ private
     end
   end
 
-  def browser_version(to_float = true)
+  def browser_version(to_float: true)
     caps = capabilities
     ver = (caps[:browser_version] || caps[:version])
     ver = ver.to_f if to_float

lib/capybara/server/animation_disabler.rb

@@ -40,7 +40,7 @@ module Capybara
       end
 
       def insert_disable(html)
-        html.sub(%r{(</head>)}, disable_markup + '\\1')
+        html.sub(%r{(</head>)}, "#{disable_markup}\\1")
       end
 
       DISABLE_MARKUP_TEMPLATE = <<~HTML

lib/capybara/session.rb

@@ -355,8 +355,8 @@ module Capybara
     #
     # @param [String] locator    Id or legend of the fieldset
     #
-    def within_fieldset(locator)
+    def within_fieldset(locator, &block)
-      within(:fieldset, locator) { yield }
+      within(:fieldset, locator, &block)
     end
 
     ##
@@ -365,8 +365,8 @@ module Capybara
     #
     # @param [String] locator    Id or caption of the table
     #
-    def within_table(locator)
+    def within_table(locator, &block)
-      within(:table, locator) { yield }
+      within(:table, locator, &block)
     end
 
     ##
@@ -398,8 +398,9 @@ module Capybara
         driver.switch_to_frame(:parent)
       when :top
         idx = scopes.index(:frame)
+        top_level_scopes = [:frame, nil]
         if idx
-          if scopes.slice(idx..-1).any? { |scope| ![:frame, nil].include?(scope) }
+          if scopes.slice(idx..-1).any? { |scope| !top_level_scopes.include?(scope) }
             raise Capybara::ScopeError, "`switch_to_frame(:top)` cannot be called from inside a descendant frame's "\
                                         '`within` block.'
           end
@@ -788,7 +789,7 @@ module Capybara
     #
     # Yield a block using a specific maximum wait time.
     #
-    def using_wait_time(seconds)
+    def using_wait_time(seconds, &block)
       if Capybara.threadsafe
         begin
           previous_wait_time = config.default_max_wait_time
@@ -798,7 +799,7 @@ module Capybara
           config.default_max_wait_time = previous_wait_time
         end
       else
-        Capybara.using_wait_time(seconds) { yield }
+        Capybara.using_wait_time(seconds, &block)
       end
     end
 

lib/capybara/spec/session/find_spec.rb

@@ -60,7 +60,7 @@ Capybara::SpecHelper.spec '#find' do
     end
   end
 
-  context 'with frozen time', requires: [:js] do
+  context 'with frozen time', requires: [:js] do # rubocop:disable RSpec/EmptyExampleGroup
     if defined?(Process::CLOCK_MONOTONIC)
       it 'will time out even if time is frozen' do
         @session.visit('/with_js')

lib/capybara/spec/session/node_spec.rb

@@ -388,8 +388,8 @@ Capybara::SpecHelper.spec 'node' do
 
   describe '#==' do
     it 'preserve object identity' do
-      expect(@session.find('//h1') == @session.find('//h1')).to be true # rubocop:disable Lint/UselessComparison
+      expect(@session.find('//h1') == @session.find('//h1')).to be true # rubocop:disable Lint/BinaryOperatorWithIdenticalOperands
-      expect(@session.find('//h1') === @session.find('//h1')).to be true # rubocop:disable Style/CaseEquality, Lint/UselessComparison
+      expect(@session.find('//h1') === @session.find('//h1')).to be true # rubocop:disable Style/CaseEquality, Lint/BinaryOperatorWithIdenticalOperands
       expect(@session.find('//h1').eql?(@session.find('//h1'))).to be false
     end
 

lib/capybara/spec/session/save_page_spec.rb

@@ -31,7 +31,7 @@ Capybara::SpecHelper.spec '#save_page' do
   it 'can store files in a specified directory' do
     Capybara.save_path = alternative_path
     @session.save_page
-    path = Dir.glob(alternative_path + '/capybara-*.html').first
+    path = Dir.glob("#{alternative_path}/capybara-*.html").first
     expect(File.read(path)).to include('Another World')
   end
 
@@ -43,14 +43,14 @@ Capybara::SpecHelper.spec '#save_page' do
   it 'can store files in a specified directory with a given filename' do
     Capybara.save_path = alternative_path
     @session.save_page('capybara-001133.html')
-    path = alternative_path + '/capybara-001133.html'
+    path = "#{alternative_path}/capybara-001133.html"
     expect(File.read(path)).to include('Another World')
   end
 
   it 'can store files in a specified directory with a given relative filename' do
     Capybara.save_path = alternative_path
     @session.save_page('tmp/capybara-001144.html')
-    path = alternative_path + '/tmp/capybara-001144.html'
+    path = "#{alternative_path}/tmp/capybara-001144.html"
     expect(File.read(path)).to include('Another World')
   end
 
@@ -63,7 +63,7 @@ Capybara::SpecHelper.spec '#save_page' do
   it 'returns an absolute path in given directory' do
     Capybara.save_path = alternative_path
     result = @session.save_page
-    path = File.expand_path(Dir.glob(alternative_path + '/capybara-*.html').first, alternative_path)
+    path = File.expand_path(Dir.glob("#{alternative_path}/capybara-*.html").first, alternative_path)
     expect(result).to eq(path)
   end
 

lib/capybara/spec/spec_helper.rb

@@ -58,7 +58,7 @@ module Capybara
 
       def run_specs(session, name, **options, &filter_block)
         specs = @specs
-        RSpec.describe Capybara::Session, name, options do # rubocop:disable RSpec/EmptyExampleGroup
+        RSpec.describe Capybara::Session, name, options do
           include Capybara::SpecHelper
           include Capybara::RSpecMatchers
 
@@ -72,11 +72,11 @@ module Capybara
           end
 
           before :each, psc: true do
-            SpecHelper.reset_threadsafe(true, session)
+            SpecHelper.reset_threadsafe(bool: true, session: session)
           end
 
           after psc: true do
-            SpecHelper.reset_threadsafe(false, session)
+            SpecHelper.reset_threadsafe(session: session)
           end
 
           before :each, :exact_false do
@@ -91,7 +91,7 @@ module Capybara
         end
       end
 
-      def reset_threadsafe(bool = false, session = nil)
+      def reset_threadsafe(bool: false, session: nil)
         # Work around limit on when threadsafe can be changed
         Capybara::Session.class_variable_set(:@@instance_created, false) # rubocop:disable Style/ClassVars
         Capybara.threadsafe = bool
@@ -109,11 +109,9 @@ module Capybara
       stream.reopen(old_stream)
     end
 
-    def quietly
+    def quietly(&block)
-      silence_stream(STDOUT) do
+      silence_stream($stdout) do
-        silence_stream(STDERR) do
+        silence_stream($stderr, &block)
-          yield
-        end
       end
     end
 
@@ -133,4 +131,4 @@ module Capybara
   end
 end
 
-Dir[File.dirname(__FILE__) + '/session/**/*.rb'].each { |file| require_relative file }
+Dir["#{File.dirname(__FILE__)}/session/**/*.rb"].each { |file| require_relative file }

lib/capybara/spec/test_app.rb

@@ -9,6 +9,7 @@ class TestApp < Sinatra::Base
   class TestAppError < Exception; end # rubocop:disable Lint/InheritException
   class TestAppOtherError < Exception # rubocop:disable Lint/InheritException
     def initialize(string1, msg)
+      super()
       @something = string1
       @message = msg
     end
@@ -89,11 +90,11 @@ class TestApp < Sinatra::Base
   end
 
   get '/form/get' do
-    '<pre id="results">' + params[:form].to_yaml + '</pre>'
+    %(<pre id="results">#{params[:form].to_yaml}</pre>)
   end
 
   post '/relative' do
-    '<pre id="results">' + params[:form].to_yaml + '</pre>'
+    %(<pre id="results">#{params[:form].to_yaml}</pre>)
   end
 
   get '/favicon.ico' do
@@ -180,7 +181,7 @@ class TestApp < Sinatra::Base
 
   post '/form' do
     self.class.form_post_count += 1
-    '<pre id="results">' + params[:form].merge('post_count' => self.class.form_post_count).to_yaml + '</pre>'
+    %(<pre id="results">#{params[:form].merge('post_count' => self.class.form_post_count).to_yaml}</pre>)
   end
 
   post '/upload_empty' do

spec/rspec/features_spec.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+# rubocop:disable RSpec/MultipleDescribes
+
 require 'spec_helper'
 require 'capybara/rspec'
 
@@ -96,4 +98,4 @@ ffeature 'if ffeature aliases focused tag then' do # rubocop:disable RSpec/Focus
     expect(example.metadata[:focus]).to eq true
   end
 end
-# rubocop:enable RSpec/RepeatedExample
+# rubocop:enable RSpec/RepeatedExample, RSpec/MultipleDescribes

spec/rspec/scenarios_spec.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+# rubocop:disable RSpec/MultipleDescribes
+
 require 'spec_helper'
 require 'capybara/rspec'
 
@@ -17,3 +19,5 @@ feature 'if xscenario aliases to pending then' do
   xscenario "this test should be 'temporarily disabled with xscenario'" do
   end
 end
+
+# rubocop:enable RSpec/MultipleDescribes

spec/rspec_spec.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+# rubocop:disable RSpec/MultipleDescribes
+
 require 'spec_helper'
 
 RSpec.describe 'capybara/rspec' do
@@ -143,3 +145,5 @@ feature 'Feature DSL' do
     expect(page.body).to include('Another World')
   end
 end
+
+# rubocop:enable RSpec/MultipleDescribes

spec/shared_selenium_session.rb

@@ -167,7 +167,7 @@ RSpec.shared_examples 'Capybara::Session' do |session, mode|
         long_string = (0...60).map { |i| ((i % 26) + 65).chr }.join
         session.visit('/form')
         session.fill_in('form_first_name', with: long_string, fill_options: { clear: :none })
-        expect(session.find(:fillable_field, 'form_first_name').value).to eq('John' + long_string)
+        expect(session.find(:fillable_field, 'form_first_name').value).to eq("John#{long_string}")
       end
     end