RuboCop upgrade (#548)

* Update Rubocop and run auto-fixes

In an attempt to do some house keeping this pushed the rubocop version
up by ten and fixes everything that is correctable from an auto-fix
standpoint. These changes should be safe if we assume that rubocop's
auto-fix changes are safe.

* Disable cop around black hole

This black hole method should not call up to super. That is the purpose
of a black hole object.

* Do not take certain changes to maintain backwards compatibility

Do not take the Unfreeze string cop because it is only supported in Ruby
2.3 and above.

* Fix class_eval to be friendlier

This came up in Style::EvalWithLocation. It makes backtraces easier to
follow.

* Selectively disabled cops for methods and classes

Rubocop now requires that cops are scoped tighter. This reformats it so
that only the specific method or class has the cop disabled.

* Enforce implicit on Style/RescueStandardError

Per ixti's code review, the maintainers prefer this enforced style so I
am changing to the requested style.

* Simplify code to ternary

This code could easily be collapsed to a ternary per ixti's request so
that is what I am doing.

Author: natesholland

.rubocop.yml

@@ -61,6 +61,9 @@ Metrics/ParameterLists:
 Performance/RegexpMatch:
   Enabled: false
 
+Performance/UnfreezeString:
+  Enabled: false
+
 ## Style #######################################################################
 
 Style/CollectionMethods:
@@ -110,3 +113,9 @@ Style/TrivialAccessors:
 
 Style/YodaCondition:
   Enabled: false
+
+Style/FormatStringToken:
+  EnforcedStyle: unannotated
+
+Style/RescueStandardError:
+  EnforcedStyle: implicit

Gemfile

@@ -28,7 +28,7 @@ group :test do
   gem "rspec", "~> 3.0"
   gem "rspec-its"
 
-  gem "rubocop", "= 0.49.1"
+  gem "rubocop", "= 0.59.2"
 
   gem "yardstick"
 end

http.gemspec

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-lib = File.expand_path("../lib", __FILE__)
+lib = File.expand_path("lib", __dir__)
 $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 require "http/version"
 
@@ -27,10 +27,10 @@ Gem::Specification.new do |gem|
 
   gem.required_ruby_version = ">= 2.3"
 
-  gem.add_runtime_dependency "http-parser",    "~> 1.2.0"
-  gem.add_runtime_dependency "http-form_data", "~> 2.0"
-  gem.add_runtime_dependency "http-cookie",    "~> 1.0"
   gem.add_runtime_dependency "addressable",    "~> 2.3"
+  gem.add_runtime_dependency "http-cookie",    "~> 1.0"
+  gem.add_runtime_dependency "http-form_data", "~> 2.0"
+  gem.add_runtime_dependency "http-parser",    "~> 1.2.0"
 
   gem.add_development_dependency "bundler", "~> 2.0"
 

lib/http/chainable.rb

@@ -101,6 +101,7 @@ def timeout(options)
 
       %i[global read write connect].each do |k|
         next unless options.key? k
+
         options["#{k}_timeout".to_sym] = options.delete k
       end
 
@@ -144,6 +145,7 @@ def persistent(host, timeout: 5)
       options  = {:keep_alive_timeout => timeout}
       p_client = branch default_options.merge(options).with_persistent host
       return p_client unless block_given?
+
       yield p_client
     ensure
       p_client.close if p_client

lib/http/client.rb

@@ -128,15 +128,11 @@ def verify_connection!(uri)
     def make_request_uri(uri, opts)
       uri = uri.to_s
 
-      if default_options.persistent? && uri !~ HTTP_OR_HTTPS_RE
-        uri = "#{default_options.persistent}#{uri}"
-      end
+      uri = "#{default_options.persistent}#{uri}" if default_options.persistent? && uri !~ HTTP_OR_HTTPS_RE
 
       uri = HTTP::URI.parse uri
 
-      if opts.params && !opts.params.empty?
-        uri.query_values = uri.query_values(Array).to_a.concat(opts.params.to_a)
-      end
+      uri.query_values = uri.query_values(Array).to_a.concat(opts.params.to_a) if opts.params && !opts.params.empty?
 
       # Some proxies (seen on WEBRick) fail if URL has
       # empty path (e.g. `http://example.com`) while it's RFC-complaint:

lib/http/headers.rb

@@ -118,6 +118,7 @@ def keys
     # @return [Boolean]
     def ==(other)
       return false unless other.respond_to? :to_a
+
       @pile == other.to_a
     end
 
@@ -127,6 +128,7 @@ def ==(other)
     # @return [Headers] self-reference
     def each
       return to_enum(__method__) unless block_given?
+
       @pile.each { |arr| yield(arr) }
       self
     end
@@ -218,6 +220,7 @@ def normalize_header(name)
     def validate_value(value)
       v = value.to_s
       return v unless v.include?("\n")
+
       raise HeaderError, "Invalid HTTP header field value: #{v.inspect}"
     end
   end

lib/http/mime_type/adapter.rb

@@ -15,7 +15,7 @@ class << self
       end
 
       %w[encode decode].each do |operation|
-        class_eval <<-RUBY, __FILE__, __LINE__
+        class_eval <<-RUBY, __FILE__, __LINE__ + 1
           def #{operation}(*)
             fail Error, "\#{self.class} does not supports ##{operation}"
           end

lib/http/mime_type/json.rb

@@ -10,6 +10,7 @@ class JSON < Adapter
       # Encodes object to JSON
       def encode(obj)
         return obj.to_json if obj.respond_to?(:to_json)
+
         ::JSON.dump obj
       end
 

lib/http/options.rb

@@ -1,14 +1,12 @@
 # frozen_string_literal: true
 
-# rubocop:disable Metrics/ClassLength
-
 require "http/headers"
 require "openssl"
 require "socket"
 require "http/uri"
 
 module HTTP
-  class Options
+  class Options # rubocop:disable Metrics/ClassLength
     @default_socket_class     = TCPSocket
     @default_ssl_socket_class = OpenSSL::SSL::SSLSocket
     @default_timeout_class    = HTTP::Timeout::Null
@@ -18,9 +16,8 @@ class << self
       attr_accessor :default_socket_class, :default_ssl_socket_class, :default_timeout_class
       attr_reader :available_features
 
-      def new(options = {}) # rubocop:disable Style/OptionHash
-        return options if options.is_a?(self)
-        super
+      def new(options = {})
+        options.is_a?(self) ? options : super
       end
 
       def defined_options

lib/http/redirector.rb

@@ -89,6 +89,7 @@ def redirect_to(uri)
 
       if UNSAFE_VERBS.include?(verb) && STRICT_SENSITIVE_CODES.include?(code)
         raise StateError, "can't follow #{@response.status} redirect" if @strict
+
         verb = :get
       end
 

lib/http/request/body.rb

@@ -19,6 +19,7 @@ def size
           @source.bytesize
         elsif @source.respond_to?(:read)
           raise RequestError, "IO object must respond to #size" unless @source.respond_to?(:size)
+
           @source.size
         elsif @source.nil?
           0

lib/http/request/writer.rb

@@ -108,6 +108,7 @@ def write(data)
         until data.empty?
           length = @socket.write(data)
           break unless data.bytesize > length
+
           data = data.byteslice(length..-1)
         end
       rescue Errno::EPIPE

lib/http/response/body.rb

@@ -64,6 +64,7 @@ def to_s
       # Assert that the body is actively being streamed
       def stream!
         raise StateError, "body has already been consumed" if @streaming == false
+
         @streaming = true
       end
 

lib/http/response/status.rb

@@ -132,7 +132,7 @@ def inspect
       end
 
       SYMBOLS.each do |code, symbol|
-        class_eval <<-RUBY, __FILE__, __LINE__
+        class_eval <<-RUBY, __FILE__, __LINE__ + 1
           def #{symbol}?      # def bad_request?
             #{code} == code   #   400 == code
           end                 # end
@@ -141,6 +141,7 @@ def #{symbol}?      # def bad_request?
 
       def __setobj__(obj)
         raise TypeError, "Expected #{obj.inspect} to respond to #to_i" unless obj.respond_to? :to_i
+
         @code = obj.to_i
       end
 

lib/http/timeout/global.rb

@@ -121,9 +121,7 @@ def reset_timer
 
       def log_time
         @time_left -= (Time.now - @started)
-        if @time_left <= 0
-          raise TimeoutError, "Timed out after using the allocated #{@timeout} seconds"
-        end
+        raise TimeoutError, "Timed out after using the allocated #{@timeout} seconds" if @time_left <= 0
 
         reset_timer
       end

lib/http/timeout/per_operation.rb

@@ -66,6 +66,7 @@ def readpartial(size, buffer = nil)
             return result if result != :wait_readable
 
             raise TimeoutError, "Read timed out after #{@read_timeout} seconds" if timeout
+
             # marking the socket for timeout. Why is this not being raised immediately?
             # it seems there is some race-condition on the network level between calling
             # #read_nonblock and #wait_readable, in which #read_nonblock signalizes waiting

spec/lib/http/client_spec.rb

@@ -1,5 +1,5 @@
-# frozen_string_literal: true
 # coding: utf-8
+# frozen_string_literal: true
 
 require "support/http_handling_shared"
 require "support/dummy_server"

spec/lib/http/request/writer_spec.rb

@@ -1,5 +1,5 @@
-# frozen_string_literal: true
 # coding: utf-8
+# frozen_string_literal: true
 
 RSpec.describe HTTP::Request::Writer do
   let(:io)          { StringIO.new }

spec/lib/http/request_spec.rb

@@ -1,5 +1,5 @@
-# frozen_string_literal: true
 # coding: utf-8
+# frozen_string_literal: true
 
 RSpec.describe HTTP::Request do
   let(:proxy)       { {} }

spec/lib/http/response/status_spec.rb

@@ -26,7 +26,7 @@
     end
 
     described_class::REASONS.each do |code, reason|
-      class_eval <<-RUBY
+      class_eval <<-RUBY, __FILE__, __LINE__ + 1
         context 'with well-known code: #{code}' do
           let(:code) { #{code} }
           it { is_expected.to eq #{reason.inspect} }
@@ -165,7 +165,7 @@
     end
 
     described_class::SYMBOLS.each do |code, symbol|
-      class_eval <<-RUBY
+      class_eval <<-RUBY, __FILE__, __LINE__ + 1
         context 'with well-known code: #{code}' do
           let(:code) { #{code} }
           it { is_expected.to be #{symbol.inspect} }
@@ -193,7 +193,7 @@
   end
 
   described_class::SYMBOLS.each do |code, symbol|
-    class_eval <<-RUBY
+    class_eval <<-RUBY, __FILE__, __LINE__ + 1
       describe '##{symbol}?' do
         subject { status.#{symbol}? }
 

spec/lib/http_spec.rb

@@ -1,5 +1,5 @@
-# frozen_string_literal: true
 # encoding: utf-8
+# frozen_string_literal: true
 
 require "json"
 

spec/support/black_hole.rb

@@ -2,7 +2,7 @@
 
 module BlackHole
   class << self
-    def method_missing(*) # rubocop: disable Style/MethodMissing
+    def method_missing(*) # rubocop:disable Style/MethodMissingSuper
       self
     end
 

spec/support/dummy_server/servlet.rb

@@ -1,9 +1,8 @@
-# frozen_string_literal: true
 # encoding: UTF-8
+# frozen_string_literal: true
 
 class DummyServer < WEBrick::HTTPServer
-  # rubocop:disable Metrics/ClassLength
-  class Servlet < WEBrick::HTTPServlet::AbstractServlet
+  class Servlet < WEBrick::HTTPServlet::AbstractServlet # rubocop:disable Metrics/ClassLength
     def self.sockets
       @sockets ||= []
     end
@@ -18,7 +17,7 @@ def self.handlers
     end
 
     %w[get post head].each do |method|
-      class_eval <<-RUBY, __FILE__, __LINE__
+      class_eval <<-RUBY, __FILE__, __LINE__ + 1
         def self.#{method}(path, &block)
           handlers["#{method}:\#{path}"] = block
         end

spec/support/ssl_helper.rb

@@ -5,7 +5,7 @@
 require "certificate_authority"
 
 module SSLHelper
-  CERTS_PATH = Pathname.new File.expand_path("../../../tmp/certs", __FILE__)
+  CERTS_PATH = Pathname.new File.expand_path("../../tmp/certs", __dir__)
 
   class RootCertificate < ::CertificateAuthority::Certificate
     EXTENSIONS = {"keyUsage" => {"usage" => %w[critical keyCertSign]}}.freeze
@@ -90,7 +90,7 @@ def client_params
     end
 
     %w[server client].each do |side|
-      class_eval <<-RUBY, __FILE__, __LINE__
+      class_eval <<-RUBY, __FILE__, __LINE__ + 1
         def #{side}_cert
           @#{side}_cert ||= ChildCertificate.new ca
         end