rubycdp / ferrum

Headless Chrome Ruby API
https://ferrum.rubycdp.com
MIT License
1.7k stars 122 forks source link

Some advice when run rubocop with modified config. #344

Closed zw963 closed 1 year ago

zw963 commented 1 year ago

Please check following patch file.

Following is zipped patch file.

diff.zip

Following is diff file content, all spec is passed as usual.

CLICK ME ```diff diff --git a/lib/ferrum/browser.rb b/lib/ferrum/browser.rb index 850cb59..587c1b3 100644 --- a/lib/ferrum/browser.rb +++ b/lib/ferrum/browser.rb @@ -164,8 +164,8 @@ module Ferrum if proxy options.parse_proxy(proxy) - params.merge!(proxyServer: "#{proxy[:host]}:#{proxy[:port]}") - params.merge!(proxyBypassList: proxy[:bypass]) if proxy[:bypass] + params[:proxyServer] = "#{proxy[:host]}:#{proxy[:port]}" + params[:proxyBypassList] = proxy[:bypass] if proxy[:bypass] end context = contexts.create(**params) diff --git a/lib/ferrum/browser/client.rb b/lib/ferrum/browser/client.rb index b5284e0..80de4a6 100644 --- a/lib/ferrum/browser/client.rb +++ b/lib/ferrum/browser/client.rb @@ -50,12 +50,12 @@ module Ferrum response end - def on(event, &block) + def on(event, &) case event when *INTERRUPTIONS - @interrupter.on(event, &block) + @interrupter.on(event, &) else - @subscriber.on(event, &block) + @subscriber.on(event, &) end end diff --git a/lib/ferrum/browser/options/base.rb b/lib/ferrum/browser/options/base.rb index 6ebc63b..aaa6d41 100644 --- a/lib/ferrum/browser/options/base.rb +++ b/lib/ferrum/browser/options/base.rb @@ -17,7 +17,7 @@ module Ferrum end def except(*keys) - to_h.reject { |n, _| keys.include?(n) } + to_h.except(*keys) end def detect_path diff --git a/lib/ferrum/browser/options/chrome.rb b/lib/ferrum/browser/options/chrome.rb index 1cce92d..2dd665c 100644 --- a/lib/ferrum/browser/options/chrome.rb +++ b/lib/ferrum/browser/options/chrome.rb @@ -66,8 +66,8 @@ module Ferrum "user-data-dir" => user_data_dir) if options.proxy - flags.merge!("proxy-server" => "#{options.proxy[:host]}:#{options.proxy[:port]}") - flags.merge!("proxy-bypass-list" => options.proxy[:bypass]) if options.proxy[:bypass] + flags["proxy-server"] = "#{options.proxy[:host]}:#{options.proxy[:port]}" + flags["proxy-bypass-list"] = options.proxy[:bypass] if options.proxy[:bypass] end flags diff --git a/lib/ferrum/browser/process.rb b/lib/ferrum/browser/process.rb index b3a2248..4c8d231 100644 --- a/lib/ferrum/browser/process.rb +++ b/lib/ferrum/browser/process.rb @@ -53,7 +53,7 @@ module Ferrum proc { begin FileUtils.remove_entry(path) - rescue StandardError + rescue Errno::ENOENT end } diff --git a/lib/ferrum/dialog.rb b/lib/ferrum/dialog.rb index 56fef60..1bb897a 100644 --- a/lib/ferrum/dialog.rb +++ b/lib/ferrum/dialog.rb @@ -29,7 +29,7 @@ module Ferrum def accept(prompt_text = nil) options = { accept: true } response = prompt_text || default_prompt - options.merge!(promptText: response) if response + options[:promptText] = response if response @page.command("Page.handleJavaScriptDialog", slowmoable: true, **options) end diff --git a/lib/ferrum/keyboard.rb b/lib/ferrum/keyboard.rb index e3ab9ac..aba987b 100644 --- a/lib/ferrum/keyboard.rb +++ b/lib/ferrum/keyboard.rb @@ -88,7 +88,7 @@ module Ferrum # @return [Integer] # def modifiers(keys) - keys.map { |k| MODIFIERS[k.to_s] }.compact.reduce(0, :|) + keys.filter_map { |k| MODIFIERS[k.to_s] }.reduce(0, :|) end private @@ -110,7 +110,7 @@ module Ferrum when Symbol key = keys.to_s.downcase - if MODIFIERS.keys.include?(key) + if MODIFIERS.key?(key) pressed_keys.last.push(key) nil else diff --git a/lib/ferrum/mouse.rb b/lib/ferrum/mouse.rb index 54083b1..8cc059b 100644 --- a/lib/ferrum/mouse.rb +++ b/lib/ferrum/mouse.rb @@ -141,7 +141,7 @@ module Ferrum def mouse_event(type:, button: :left, count: 1, modifiers: nil, wait: 0) button = validate_button(button) options = { x: @x, y: @y, type: type, button: button, clickCount: count } - options.merge!(modifiers: modifiers) if modifiers + options[:modifiers] = modifiers if modifiers @page.command("Input.dispatchMouseEvent", wait: wait, slowmoable: true, **options) end diff --git a/lib/ferrum/network.rb b/lib/ferrum/network.rb index ca7b9b9..deb3697 100644 --- a/lib/ferrum/network.rb +++ b/lib/ferrum/network.rb @@ -17,7 +17,7 @@ module Ferrum SignedExchange Ping CSPViolationReport Preflight Other].freeze AUTHORIZE_BLOCK_MISSING = "Block is missing, call `authorize(...) { |r| r.continue } " \ "or subscribe to `on(:request)` events before calling it" - AUTHORIZE_TYPE_WRONG = ":type should be in #{AUTHORIZE_TYPE}" + AUTHORIZE_TYPE_WRONG = ":type should be in #{AUTHORIZE_TYPE}".freeze ALLOWED_CONNECTION_TYPE = %w[none cellular2g cellular3g cellular4g bluetooth ethernet wifi wimax other].freeze # Network traffic. @@ -231,7 +231,7 @@ module Ferrum # def authorize(user:, password:, type: :server, &block) raise ArgumentError, AUTHORIZE_TYPE_WRONG unless AUTHORIZE_TYPE.include?(type) - raise ArgumentError, AUTHORIZE_BLOCK_MISSING if !block_given? && !@page.subscribed?("Fetch.requestPaused") + raise ArgumentError, AUTHORIZE_BLOCK_MISSING if !block && !@page.subscribed?("Fetch.requestPaused") @authorized_ids ||= {} @authorized_ids[type] ||= [] diff --git a/lib/ferrum/network/request.rb b/lib/ferrum/network/request.rb index 800c9b1..8e4f186 100644 --- a/lib/ferrum/network/request.rb +++ b/lib/ferrum/network/request.rb @@ -47,7 +47,7 @@ module Ferrum # @return [Boolean] # def type?(value) - type.downcase == value.to_s.downcase + type.casecmp(value.to_s).zero? end # diff --git a/lib/ferrum/network/response.rb b/lib/ferrum/network/response.rb index 2a02b38..796a4ac 100644 --- a/lib/ferrum/network/response.rb +++ b/lib/ferrum/network/response.rb @@ -107,7 +107,7 @@ module Ferrum # @return [String, nil] # def content_type - @content_type ||= headers.find { |k, _| k.downcase == "content-type" }&.last&.sub(/;.*\z/, "") + @content_type ||= headers.find { |k, _| k.casecmp("content-type").zero? }&.last&.sub(/;.*\z/, "") end # See https://crbug.com/883475 diff --git a/lib/ferrum/node.rb b/lib/ferrum/node.rb index 1c94456..87bcd8b 100644 --- a/lib/ferrum/node.rb +++ b/lib/ferrum/node.rb @@ -214,7 +214,7 @@ module Ferrum def computed_style page .command("CSS.getComputedStyleForNode", nodeId: node_id)["computedStyle"] - .each_with_object({}) { |style, memo| memo.merge!(style["name"] => style["value"]) } + .each_with_object({}) { |style, memo| memo[style["name"]] = style["value"] } end private diff --git a/lib/ferrum/page.rb b/lib/ferrum/page.rb index aa3f654..35937d4 100644 --- a/lib/ferrum/page.rb +++ b/lib/ferrum/page.rb @@ -112,14 +112,14 @@ module Ferrum # def go_to(url = nil) options = { url: combine_url!(url) } - options.merge!(referrer: referrer) if referrer + options[:referrer] = referrer if referrer response = command("Page.navigate", wait: GOTO_WAIT, **options) raise StatusError.new(options[:url], "Request to #{options[:url]} failed (#{response['errorText']})") if response['errorText'] response["frameId"] rescue TimeoutError if @browser.options.pending_connection_errors - pendings = network.traffic.select(&:pending?).map(&:url).compact + pendings = network.traffic.select(&:pending?).filter_map(&:url) raise PendingConnectionsError.new(options[:url], pendings) unless pendings.empty? end end diff --git a/lib/ferrum/page/screenshot.rb b/lib/ferrum/page/screenshot.rb index fdbe1a2..9538afb 100644 --- a/lib/ferrum/page/screenshot.rb +++ b/lib/ferrum/page/screenshot.rb @@ -178,22 +178,22 @@ module Ferrum end dimension = PAPER_FORMATS.fetch(format) - options.merge!(paper_width: dimension[:width], - paper_height: dimension[:height]) + options[:paper_width] = dimension[:width] + options[:paper_height] = dimension[:height] end options.transform_keys { |k| to_camel_case(k) } end - def screenshot_options(path = nil, format: nil, scale: 1.0, **options) + def screenshot_options(path=nil, format: nil, scale: 1.0, **options) screenshot_options = {} format, quality = format_options(format, path, options[:quality]) - screenshot_options.merge!(quality: quality) if quality - screenshot_options.merge!(format: format) + screenshot_options[:quality] = quality if quality + screenshot_options[:format] = format clip = area_options(options[:full], options[:selector], scale) - screenshot_options.merge!(clip: clip) if clip + screenshot_options[:clip] = clip if clip screenshot_options end @@ -201,7 +201,7 @@ module Ferrum def format_options(format, path, quality) format ||= path ? File.extname(path).delete(".") : "png" format = "jpeg" if format == "jpg" - raise "Not supported options `:format` #{format}. jpeg | png" if format !~ /jpeg|png/i + raise "Not supported options `:format` #{format}. jpeg | png" if !/jpeg|png/i.match?(format) quality ||= 75 if format == "jpeg" @@ -225,7 +225,7 @@ module Ferrum clip = { x: 0, y: 0, width: width, height: height } end - clip.merge!(scale: scale) + clip[:scale] = scale end clip diff --git a/lib/ferrum/page/stream.rb b/lib/ferrum/page/stream.rb index d69c2b6..e854436 100644 --- a/lib/ferrum/page/stream.rb +++ b/lib/ferrum/page/stream.rb @@ -19,7 +19,7 @@ module Ferrum end def stream_to_memory(encoding:, handle:) - data = String.new # Mutable string has << and compatible to File + data = +'' # Mutable string has << and compatible to File stream(output: data, handle: handle) encoding == :base64 ? Base64.encode64(data) : data end diff --git a/lib/ferrum/page/tracing.rb b/lib/ferrum/page/tracing.rb index 7f8a005..3468cf0 100644 --- a/lib/ferrum/page/tracing.rb +++ b/lib/ferrum/page/tracing.rb @@ -53,7 +53,7 @@ module Ferrum if screenshots included = trace_config.fetch(:includedCategories, []) - trace_config.merge!(includedCategories: included | SCREENSHOT_CATEGORIES) + trace_config[:includedCategories] = included | SCREENSHOT_CATEGORIES end subscribe_tracing_complete @@ -82,7 +82,7 @@ module Ferrum next if index.to_i != 0 @pending.set(stream_handle(event["stream"])) - rescue StandardError => e + rescue => e @pending.fail(e) end diff --git a/lib/ferrum/proxy.rb b/lib/ferrum/proxy.rb index 62974db..38b2f8e 100644 --- a/lib/ferrum/proxy.rb +++ b/lib/ferrum/proxy.rb @@ -35,7 +35,7 @@ module Ferrum authenticator = WEBrick::HTTPAuth::ProxyBasicAuth.new(Realm: "Proxy Realm", UserDB: htpasswd, Logger: Logger.new(IO::NULL)) - options.merge!(ProxyAuthProc: authenticator.method(:authenticate).to_proc) + options[:ProxyAuthProc] = authenticator.method(:authenticate).to_proc end @server = HTTPProxyServer.new(**options) diff --git a/lib/ferrum/utils/elapsed_time.rb b/lib/ferrum/utils/elapsed_time.rb index 0dc0699..ea2d6db 100644 --- a/lib/ferrum/utils/elapsed_time.rb +++ b/lib/ferrum/utils/elapsed_time.rb @@ -11,12 +11,12 @@ module Ferrum @start ||= monotonic_time end - def elapsed_time(start = nil) + def elapsed_time(start=nil) monotonic_time - (start || @start) end def monotonic_time - Concurrent.monotonic_time + Process.clock_gettime(Process::CLOCK_MONOTONIC) end def timeout?(start, timeout) diff --git a/lib/ferrum/utils/platform.rb b/lib/ferrum/utils/platform.rb index d9adcf5..cc28de0 100644 --- a/lib/ferrum/utils/platform.rb +++ b/lib/ferrum/utils/platform.rb @@ -17,7 +17,7 @@ module Ferrum end def mac? - RbConfig::CONFIG["host_os"] =~ /darwin/ + RbConfig::CONFIG["host_os"].include?('darwin') end def mri? diff --git a/spec/browser_spec.rb b/spec/browser_spec.rb index 2d4ab27..6d7ab92 100644 --- a/spec/browser_spec.rb +++ b/spec/browser_spec.rb @@ -416,7 +416,7 @@ describe Ferrum::Browser do # browser is fully alive. begin popup2.execute("window.close()") - rescue StandardError + rescue Ferrum::DeadBrowserError end diff --git a/spec/headers_spec.rb b/spec/headers_spec.rb index 2343937..c72751d 100644 --- a/spec/headers_spec.rb +++ b/spec/headers_spec.rb @@ -122,7 +122,7 @@ describe Ferrum::Headers do it "keeps temporary headers local to the current window" do browser.create_page - browser.headers.add("X-Custom-Header" => "1", permanent: false) + browser.headers.add("X-Custom-Header" => "1", :permanent => false) browser.switch_to_window browser.windows.last browser.go_to("/ferrum/headers") @@ -135,7 +135,7 @@ describe Ferrum::Headers do it "does not mix temporary headers with permanent ones when propagating to other windows" do browser.create_page - browser.headers.add("X-Custom-Header" => "1", permanent: false) + browser.headers.add("X-Custom-Header" => "1", :permanent => false) browser.headers.add("Host" => "foo.com") browser.switch_to_window browser.windows.last @@ -151,7 +151,7 @@ describe Ferrum::Headers do it "does not propagate temporary headers to new windows" do browser.go_to - browser.headers.add("X-Custom-Header" => "1", permanent: false) + browser.headers.add("X-Custom-Header" => "1", :permanent => false) browser.create_page browser.switch_to_window browser.windows.last diff --git a/spec/network_spec.rb b/spec/network_spec.rb index 5f65a8d..d36a07c 100644 --- a/spec/network_spec.rb +++ b/spec/network_spec.rb @@ -221,11 +221,11 @@ describe Ferrum::Network do it "gets cleared along with network traffic" do network.blacklist = /unwanted/ page.go_to("/ferrum/url_blacklist") - expect(traffic.select(&:blocked?).length).to eq(3) + expect(traffic.count(&:blocked?)).to eq(3) network.clear(:traffic) - expect(traffic.select(&:blocked?).length).to eq(0) + expect(traffic.count(&:blocked?)).to eq(0) end it "supports multiple pages each with their own blacklist" do diff --git a/spec/node_spec.rb b/spec/node_spec.rb index 14143d8..24ccd27 100644 --- a/spec/node_spec.rb +++ b/spec/node_spec.rb @@ -665,7 +665,7 @@ describe Ferrum::Node do it "clears the input" do keys = Ferrum::Utils::Platform.mac? ? %i[Alt Shift Left] : %i[Ctrl Shift Left] - change_me.type(2.times.map { keys }, :backspace) + change_me.type(Array.new(2) { keys }, :backspace) expect(change_me.value).to eq("") end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d000fc6..48034db 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -29,12 +29,12 @@ RSpec.configure do |config| config.before(:all) do base_url = Ferrum::Server.server.base_url options = { base_url: base_url } - options.merge!(headless: false) if ENV["HEADLESS"] == "false" - options.merge!(slowmo: ENV["SLOWMO"].to_f) if ENV["SLOWMO"].to_f > 0 + options[:headless] = false if ENV["HEADLESS"] == "false" + options[:slowmo] = ENV["SLOWMO"].to_f if ENV["SLOWMO"].to_f > 0 if ENV["CI"] ferrum_logger = StringIO.new - options.merge!(logger: ferrum_logger) + options[:logger] = ferrum_logger end @browser = Ferrum::Browser.new(**options) @@ -73,14 +73,14 @@ RSpec.configure do |config| screenshot_name = "screenshot-#{filename}-#{line_number}-#{timestamp}.png" screenshot_path = "/tmp/ferrum/#{screenshot_name}" browser.screenshot(path: screenshot_path, full: true) - rescue StandardError => e + rescue => e puts "#{e.class}: #{e.message}" end def save_exception_log(_browser, filename, line_number, timestamp, ferrum_logger) log_name = "logfile-#{filename}-#{line_number}-#{timestamp}.txt" File.binwrite("/tmp/ferrum/#{log_name}", ferrum_logger.string) - rescue StandardError => e + rescue => e puts "#{e.class}: #{e.message}" end end diff --git a/spec/support/application.rb b/spec/support/application.rb index d64c6f4..ab577f7 100644 --- a/spec/support/application.rb +++ b/spec/support/application.rb @@ -5,8 +5,8 @@ require "sinatra/base" module Ferrum class Application < Sinatra::Base configure { set :protection, except: :frame_options } - FERRUM_VIEWS = "#{File.dirname(__FILE__)}/views" - FERRUM_PUBLIC = "#{File.dirname(__FILE__)}/public" + FERRUM_VIEWS = "#{File.dirname(__FILE__)}/views".freeze + FERRUM_PUBLIC = "#{File.dirname(__FILE__)}/public".freeze class TestAppError < Exception; end # rubocop:disable Lint/InheritException @@ -54,7 +54,7 @@ module Ferrum end get "/referer_base" do - <<~HERE.gsub(/\n/, "") + <<~HERE.delete("\n") direct link link via redirect
@@ -209,7 +209,7 @@ module Ferrum buffer << "Content-type: #{params.dig(:form, :document, :type)}" buffer << "File content: #{params.dig(:form, :document, :tempfile).read}" buffer.join(" | ") - rescue StandardError + rescue "No file uploaded" end @@ -221,7 +221,7 @@ module Ferrum buffer << "File content: #{doc[:tempfile].read}" end buffer.join(" | ") - rescue StandardError + rescue "No files uploaded" end ```
zw963 commented 1 year ago

BTW: the "require 'base64'" in https://github.com/rubycdp/ferrum/blob/main/lib/ferrum/browser.rb#L3 is not necessary?

route commented 1 year ago

Sorry I'm not following... You didn't say what the issue is, how to reproduce it and what is the desired behavior. Please next time consider to properly describe the issue in the desc and title, and if it's a question open a discussion instead of issue.

It's not easy to follow that diff.

zw963 commented 1 year ago

All suggested by rubocop, forget to paste the content of suggestion ...