Fix rubocop warnings

.rubocop.yml

@@ -1,7 +1,7 @@
 inherit_from: .rubocop_todo.yml
 
 AllCops:
-  TargetRubyVersion: 2.4
+  TargetRubyVersion: 2.5
   NewCops: enable
 
 Layout/FirstArrayElementIndentation:
@@ -13,13 +13,15 @@ Style/StringLiterals:
 Metrics/BlockLength:
   Exclude:
     - spec/**/*
+    - "*.gemspec"
 
 Naming/MethodParameterName:
   AllowedNames:
     - x
     - y
     - id
     - to
+    - on
 
 Naming/FileName:
   Exclude:

Gemfile

@@ -2,6 +2,6 @@
 
 source "https://rubygems.org"
 
-gem "byebug", "~> 11.0", platforms: [:mri, :mingw, :x64_mingw]
+gem "byebug", "~> 11.0", platforms: %i[mri mingw x64_mingw]
 
 gemspec

Rakefile

@@ -4,7 +4,7 @@ require "bundler/gem_tasks"
 require "rspec/core/rake_task"
 
 RSpec::Core::RakeTask.new("test") do |t|
-  t.ruby_opts="-w"
+  t.ruby_opts = "-w"
 end
 
 task default: :test

bin/console

@@ -1,7 +1,8 @@
 #!/usr/bin/env ruby
+# frozen_string_literal: true
 
 lib = File.expand_path("../lib", __dir__)
-$:.unshift(lib) unless $:.include?(lib)
+$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 
 require "irb"
 require "irb/completion"

ferrum.gemspec

@@ -1,5 +1,7 @@
+# frozen_string_literal: true
+
 lib = File.expand_path("lib", __dir__)
-$:.unshift(lib) unless $:.include?(lib)
+$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 
 require "ferrum/version"
 
@@ -20,22 +22,22 @@ Gem::Specification.new do |s|
     "bug_tracker_uri" => "https://github.com/rubycdp/ferrum/issues",
     "documentation_uri" => "https://github.com/rubycdp/ferrum/blob/master/README.md",
     "changelog_uri" => "https://github.com/rubycdp/ferrum/blob/master/CHANGELOG.md",
-    "source_code_uri" => "https://github.com/rubycdp/ferrum",
+    "source_code_uri" => "https://github.com/rubycdp/ferrum"
   }
 
   s.required_ruby_version = ">= 2.5.0"
 
-  s.add_runtime_dependency "websocket-driver", ">= 0.6", "< 0.8"
+  s.add_runtime_dependency "addressable",      "~> 2.5"
   s.add_runtime_dependency "cliver",           "~> 0.3"
   s.add_runtime_dependency "concurrent-ruby",  "~> 1.1"
-  s.add_runtime_dependency "addressable",      "~> 2.5"
   s.add_runtime_dependency "webrick",          "~> 1.7"
+  s.add_runtime_dependency "websocket-driver", ">= 0.6", "< 0.8"
 
+  s.add_development_dependency "chunky_png",   "~> 1.3"
+  s.add_development_dependency "image_size",   "~> 2.0"
+  s.add_development_dependency "pdf-reader",   "~> 2.2"
+  s.add_development_dependency "puma",         "~> 4.1"
   s.add_development_dependency "rake",         "~> 13.0"
   s.add_development_dependency "rspec",        "~> 3.8"
   s.add_development_dependency "sinatra",      "~> 2.0"
-  s.add_development_dependency "puma",         "~> 4.1"
-  s.add_development_dependency "image_size",   "~> 2.0"
-  s.add_development_dependency "pdf-reader",   "~> 2.2"
-  s.add_development_dependency "chunky_png",   "~> 1.3"
 end

lib/ferrum.rb

@@ -5,8 +5,11 @@
 
 module Ferrum
   class Error               < StandardError; end
+
   class NoSuchPageError     < Error; end
+
   class NoSuchTargetError   < Error; end
+
   class NotImplementedError < Error; end
 
   class StatusError < Error
@@ -59,7 +62,9 @@ def initialize(message = "Browser is dead or given window is closed")
 
   class NodeMovingError < Error
     def initialize(node, prev, current)
-      @node, @prev, @current = node, prev, current
+      @node = node
+      @prev = prev
+      @current = current
       super(message)
     end
 
@@ -146,6 +151,7 @@ def with_attempts(errors:, max:, wait:)
       yield
     rescue *Array(errors)
       raise if attempts >= max
+
       attempts += 1
       sleep(wait)
       retry

lib/ferrum/browser.rb

@@ -58,21 +58,18 @@ def initialize(options = nil)
 
         @options[:browser_options] ||= {}
         address = "#{@proxy_options[:host]}:#{@proxy_options[:port]}"
-        bypass = @proxy_options[:bypass]
         @options[:browser_options].merge!("proxy-server" => address)
-        @options[:browser_options].merge!("proxy-bypass-list" => bypass) if bypass
+        @options[:browser_options].merge!("proxy-bypass-list" => @proxy_options[:bypass]) if @proxy_options[:bypass]
       end
 
       @pending_connection_errors = @options.fetch(:pending_connection_errors, true)
       @slowmo = @options[:slowmo].to_f
 
-      if @options.key?(:base_url)
-        self.base_url = @options[:base_url]
-      end
+      self.base_url = @options[:base_url] if @options.key?(:base_url)
 
       if ENV["FERRUM_DEBUG"] && !@logger
-        STDOUT.sync = true
-        @logger = STDOUT
+        $stdout.sync = true
+        @logger = $stdout
         @options[:logger] = @logger
       end
 

lib/ferrum/browser/client.rb

@@ -18,9 +18,7 @@ def initialize(browser, ws_url, id_starts_with: 0)
 
         @thread = Thread.new do
           Thread.current.abort_on_exception = true
-          if Thread.current.respond_to?(:report_on_exception=)
-            Thread.current.report_on_exception = true
-          end
+          Thread.current.report_on_exception = true if Thread.current.respond_to?(:report_on_exception=)
 
           while message = @ws.messages.pop
             if INTERRUPTIONS.include?(message["method"])
@@ -44,6 +42,7 @@ def command(method, params = {})
 
         raise DeadBrowserError if data.nil? && @ws.messages.closed?
         raise TimeoutError unless data
+
         error, response = data.values_at("error", "result")
         raise_browser_error(error) if error
         response

lib/ferrum/browser/command.rb

@@ -5,7 +5,7 @@ class Browser
     class Command
       NOT_FOUND = "Could not find an executable for the browser. Try to make " \
                   "it available on the PATH or set environment variable for " \
-                  "example BROWSER_PATH=\"/usr/bin/chrome\"".freeze
+                  "example BROWSER_PATH=\"/usr/bin/chrome\""
 
       # Currently only these browsers support CDP:
       # https://github.com/cyrus-and/chrome-remote-interface#implementations
@@ -27,9 +27,11 @@ def self.build(options, user_data_dir)
       def initialize(defaults, options, user_data_dir)
         @flags = {}
         @defaults = defaults
-        @options, @user_data_dir = options, user_data_dir
+        @options = options
+        @user_data_dir = user_data_dir
         @path = options[:browser_path] || ENV["BROWSER_PATH"] || defaults.detect_path
         raise Cliver::Dependency::NotFound.new(NOT_FOUND) unless @path
+
         merge_options
       end
 
@@ -45,10 +47,7 @@ def to_a
 
       def merge_options
         @flags = defaults.merge_required(@flags, options, @user_data_dir)
-
-        unless options[:ignore_default_browser_options]
-          @flags = defaults.merge_default(@flags, options)
-        end
+        @flags = defaults.merge_default(@flags, options) unless options[:ignore_default_browser_options]
 
         @flags.merge!(options.fetch(:browser_options, {}))
       end

lib/ferrum/browser/options/chrome.rb

@@ -36,8 +36,8 @@ class Chrome < Base
           "metrics-recording-only" => nil,
           "safebrowsing-disable-auto-update" => nil,
           "password-store" => "basic",
-          "no-startup-window" => nil,
-          # Note: --no-sandbox is not needed if you properly setup a user in the container.
+          "no-startup-window" => nil
+          # NOTE: --no-sandbox is not needed if you properly setup a user in the container.
           # https://github.com/ebidel/lighthouse-ci/blob/master/builder/Dockerfile#L35-L40
           # "no-sandbox" => nil,
         }.freeze
@@ -65,9 +65,7 @@ def merge_required(flags, options, user_data_dir)
         end
 
         def merge_default(flags, options)
-          unless options.fetch(:headless, true)
-            defaults = except("headless", "disable-gpu")
-          end
+          defaults = except("headless", "disable-gpu") unless options.fetch(:headless, true)
 
           defaults ||= DEFAULT_OPTIONS
           defaults.merge(flags)

lib/ferrum/browser/options/firefox.rb

@@ -5,7 +5,7 @@ class Browser
     module Options
       class Firefox < Base
         DEFAULT_OPTIONS = {
-          "headless" => nil,
+          "headless" => nil
         }.freeze
 
         MAC_BIN_PATH = [
@@ -25,9 +25,7 @@ def merge_required(flags, options, user_data_dir)
         end
 
         def merge_default(flags, options)
-          unless options.fetch(:headless, true)
-            defaults = except("headless")
-          end
+          defaults = except("headless") unless options.fetch(:headless, true)
 
           defaults ||= DEFAULT_OPTIONS
           defaults.merge(flags)

lib/ferrum/browser/process.rb

@@ -22,7 +22,6 @@ class Process
                   :default_user_agent, :browser_version, :protocol_version,
                   :v8_version, :webkit_version, :xvfb
 
-
       extend Forwardable
       delegate path: :command
 
@@ -32,28 +31,33 @@ def self.start(*args)
 
       def self.process_killer(pid)
         proc do
-          begin
-            if Ferrum.windows?
-              # Process.kill is unreliable on Windows
-              ::Process.kill("KILL", pid) unless system("taskkill /f /t /pid #{pid} >NUL 2>NUL")
-            else
-              ::Process.kill("USR1", pid)
-              start = Ferrum.monotonic_time
-              while ::Process.wait(pid, ::Process::WNOHANG).nil?
-                sleep(WAIT_KILLED)
-                next unless Ferrum.timeout?(start, KILL_TIMEOUT)
-                ::Process.kill("KILL", pid)
-                ::Process.wait(pid)
-                break
-              end
+          if Ferrum.windows?
+            # Process.kill is unreliable on Windows
+            ::Process.kill("KILL", pid) unless system("taskkill /f /t /pid #{pid} >NUL 2>NUL")
+          else
+            ::Process.kill("USR1", pid)
+            start = Ferrum.monotonic_time
+            while ::Process.wait(pid, ::Process::WNOHANG).nil?
+              sleep(WAIT_KILLED)
+              next unless Ferrum.timeout?(start, KILL_TIMEOUT)
+
+              ::Process.kill("KILL", pid)
+              ::Process.wait(pid)
+              break
             end
-          rescue Errno::ESRCH, Errno::ECHILD
           end
+        rescue Errno::ESRCH, Errno::ECHILD
         end
       end
 
       def self.directory_remover(path)
-        proc { FileUtils.remove_entry(path) rescue Errno::ENOENT }
+        proc {
+          begin
+            FileUtils.remove_entry(path)
+          rescue StandardError
+            Errno::ENOENT
+          end
+        }
       end
 
       def initialize(options)
@@ -131,7 +135,7 @@ def parse_ws_url(read_io, timeout)
         output = ""
         start = Ferrum.monotonic_time
         max_time = start + timeout
-        regexp = /DevTools listening on (ws:\/\/.*)/
+        regexp = %r{DevTools listening on (ws://.*)}
         while (now = Ferrum.monotonic_time) < max_time
           begin
             output += read_io.read_nonblock(512)
@@ -146,7 +150,7 @@ def parse_ws_url(read_io, timeout)
         end
 
         unless ws_url
-          @logger.puts(output) if @logger
+          @logger&.puts(output)
           raise ProcessTimeoutError.new(timeout, output)
         end
       end
@@ -172,11 +176,9 @@ def parse_browser_versions
 
       def close_io(*ios)
         ios.each do |io|
-          begin
-            io.close unless io.closed?
-          rescue IOError
-            raise unless RUBY_ENGINE == "jruby"
-          end
+          io.close unless io.closed?
+        rescue IOError
+          raise unless RUBY_ENGINE == "jruby"
         end
       end
     end

lib/ferrum/browser/web_socket.rb

@@ -21,19 +21,15 @@ def initialize(url, max_receive_size, logger)
         @driver   = ::WebSocket::Driver.client(self, max_length: max_receive_size)
         @messages = Queue.new
 
-        if SKIP_LOGGING_SCREENSHOTS
-          @screenshot_commands = Concurrent::Hash.new
-        end
+        @screenshot_commands = Concurrent::Hash.new if SKIP_LOGGING_SCREENSHOTS
 
         @driver.on(:open,    &method(:on_open))
         @driver.on(:message, &method(:on_message))
         @driver.on(:close,   &method(:on_close))
 
         @thread = Thread.new do
           Thread.current.abort_on_exception = true
-          if Thread.current.respond_to?(:report_on_exception=)
-            Thread.current.report_on_exception = true
-          end
+          Thread.current.report_on_exception = true if Thread.current.respond_to?(:report_on_exception=)
 
           begin
             while data = @sock.readpartial(512)
@@ -71,9 +67,7 @@ def on_close(_event)
       end
 
       def send_message(data)
-        if SKIP_LOGGING_SCREENSHOTS
-          @screenshot_commands[data[:id]] = true
-        end
+        @screenshot_commands[data[:id]] = true if SKIP_LOGGING_SCREENSHOTS
 
         json = data.to_json
         @driver.text(json)

lib/ferrum/browser/xvfb.rb

@@ -4,7 +4,7 @@ module Ferrum
   class Browser
     class Xvfb
       NOT_FOUND = "Could not find an executable for the Xvfb. Try to install " \
-                  "it with your package manager".freeze
+                  "it with your package manager"
 
       def self.start(*args)
         new(*args).tap(&:start)
@@ -20,7 +20,7 @@ def initialize(options)
         @path = self.class.xvfb_path
         raise Cliver::Dependency::NotFound.new(NOT_FOUND) unless @path
 
-        @screen_size = options.fetch(:window_size, [1024, 768]).join("x") + "x24"
+        @screen_size = "#{options.fetch(:window_size, [1024, 768]).join('x')}x24"
         @display_id = (Time.now.to_f * 1000).to_i % 100_000_000
       end