Goodbye Hash rockets... (#769)

Enforce ruby19 Hash syntax for Hashes with Symbol keys only, and enforce
hash-rockets in case of mixed usage:

    { a: 1, b: 2 }        # good
    { a: 1, "b" => 2 }    # bad
    { :a => 1, "b" => 2 } # good (no mixed syntax)

In future we will swap all options hashes with keyword args.

Author: ixti

.rubocop.yml

@@ -7,6 +7,7 @@ inherit_from:
   - .rubocop_todo.yml
   - .rubocop/layout.yml
   - .rubocop/metrics.yml
+  - .rubocop/rspec.yml
   - .rubocop/style.yml
 
 AllCops:

.rubocop/layout.yml

@@ -2,6 +2,10 @@ Layout/DotPosition:
   Enabled: true
   EnforcedStyle: leading
 
+Layout/FirstHashElementIndentation:
+  Enabled: true
+  EnforcedStyle: consistent
+
 Layout/HashAlignment:
   Enabled: true
   EnforcedColonStyle: table

.rubocop/rspec.yml

@@ -0,0 +1,5 @@
+RSpec/ExampleLength:
+  CountAsOne:
+    - array
+    - heredoc
+    - method_call

.rubocop/style.yml

@@ -12,7 +12,7 @@ Style/FormatStringToken:
 
 Style/HashSyntax:
   Enabled: true
-  EnforcedStyle: hash_rockets
+  EnforcedStyle: ruby19_no_mixed_keys
 
 Style/OptionHash:
   Enabled: true

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config --auto-gen-only-exclude --exclude-limit 100`
-# on 2023-10-17 15:09:44 UTC using RuboCop version 1.57.1.
+# on 2023-10-18 14:33:19 UTC using RuboCop version 1.57.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -208,20 +208,16 @@ RSpec/DescribedClass:
     - 'spec/lib/http/uri/normalizer_spec.rb'
     - 'spec/lib/http_spec.rb'
 
-# Offense count: 55
+# Offense count: 41
 # Configuration parameters: Max, CountAsOne.
 RSpec/ExampleLength:
   Exclude:
     - 'spec/lib/http/client_spec.rb'
     - 'spec/lib/http/connection_spec.rb'
     - 'spec/lib/http/features/auto_deflate_spec.rb'
-    - 'spec/lib/http/features/logging_spec.rb'
-    - 'spec/lib/http/options/features_spec.rb'
     - 'spec/lib/http/options/headers_spec.rb'
-    - 'spec/lib/http/options/merge_spec.rb'
     - 'spec/lib/http/redirector_spec.rb'
     - 'spec/lib/http/request/body_spec.rb'
-    - 'spec/lib/http/request/writer_spec.rb'
     - 'spec/lib/http/response/body_spec.rb'
     - 'spec/lib/http/uri_spec.rb'
     - 'spec/lib/http_spec.rb'

Gemfile

@@ -10,20 +10,20 @@ gem "rake"
 gem "webrick"
 
 group :development do
-  gem "guard-rspec", :require => false
-  gem "nokogiri",    :require => false
-  gem "pry",         :require => false
+  gem "guard-rspec", require: false
+  gem "nokogiri",    require: false
+  gem "pry",         require: false
 
   # RSpec formatter
-  gem "fuubar", :require => false
+  gem "fuubar", require: false
 
   platform :mri do
     gem "pry-byebug"
   end
 end
 
 group :test do
-  gem "certificate_authority", "~> 1.0", :require => false
+  gem "certificate_authority", "~> 1.0", require: false
 
   gem "backports"
 
@@ -32,8 +32,8 @@ group :test do
   gem "rubocop-rake", "~> 0.6.0"
   gem "rubocop-rspec", "~> 2.24.1"
 
-  gem "simplecov",      :require => false
-  gem "simplecov-lcov", :require => false
+  gem "simplecov",      require: false
+  gem "simplecov-lcov", require: false
 
   gem "rspec", "~> 3.10"
   gem "rspec-its"

Guardfile

@@ -2,7 +2,7 @@
 
 # More info at https://github.com/guard/guard#readme
 
-guard :rspec, :cmd => "GUARD_RSPEC=1 bundle exec rspec --no-profile" do
+guard :rspec, cmd: "GUARD_RSPEC=1 bundle exec rspec --no-profile" do
   require "guard/rspec/dsl"
   dsl = Guard::RSpec::Dsl.new(self)
 

Rakefile

@@ -61,4 +61,4 @@ task :generate_status_codes do
   end
 end
 
-task :default => %i[spec rubocop verify_measurements]
+task default: %i[spec rubocop verify_measurements]

lib/http/chainable.rb

@@ -92,7 +92,7 @@ def build_request(*args)
     #   @param [Numeric] global_timeout
     def timeout(options)
       klass, options = case options
-                       when Numeric then [HTTP::Timeout::Global, {:global => options}]
+                       when Numeric then [HTTP::Timeout::Global, {global: options}]
                        when Hash    then [HTTP::Timeout::PerOperation, options.dup]
                        when :null   then [HTTP::Timeout::Null, {}]
                        else raise ArgumentError, "Use `.timeout(global_timeout_in_seconds)` or `.timeout(connect: x, write: y, read: z)`."
@@ -106,8 +106,8 @@ def timeout(options)
       end
 
       branch default_options.merge(
-        :timeout_class   => klass,
-        :timeout_options => options
+        timeout_class:   klass,
+        timeout_options: options
       )
     end
 
@@ -142,7 +142,7 @@ def timeout(options)
     #   @yieldparam [HTTP::Client] client Persistent client
     #   @return [Object] result of last expression in the block
     def persistent(host, timeout: 5)
-      options  = {:keep_alive_timeout => timeout}
+      options  = {keep_alive_timeout: timeout}
       p_client = branch default_options.merge(options).with_persistent host
       return p_client unless block_given?
 

lib/http/client.rb

@@ -43,14 +43,14 @@ def build_request(verb, uri, opts = {})
       headers = make_request_headers(opts)
       body    = make_request_body(opts, headers)
 
-      req = HTTP::Request.new(
-        :verb           => verb,
-        :uri            => uri,
-        :uri_normalizer => opts.feature(:normalize_uri)&.normalizer,
-        :proxy          => opts.proxy,
-        :headers        => headers,
-        :body           => body
-      )
+      req = HTTP::Request.new({
+        verb:           verb,
+        uri:            uri,
+        uri_normalizer: opts.feature(:normalize_uri)&.normalizer,
+        proxy:          opts.proxy,
+        headers:        headers,
+        body:           body
+      })
 
       wrap_request(req, opts)
     end
@@ -110,13 +110,13 @@ def wrap_request(req, opts)
 
     def build_response(req, options)
       Response.new(
-        :status        => @connection.status_code,
-        :version       => @connection.http_version,
-        :headers       => @connection.headers,
-        :proxy_headers => @connection.proxy_response_headers,
-        :connection    => @connection,
-        :encoding      => options.encoding,
-        :request       => req
+        status:        @connection.status_code,
+        version:       @connection.http_version,
+        headers:       @connection.headers,
+        proxy_headers: @connection.proxy_response_headers,
+        connection:    @connection,
+        encoding:      options.encoding,
+        request:       req
       )
     end
 

lib/http/feature.rb

@@ -2,10 +2,6 @@
 
 module HTTP
   class Feature
-    def initialize(opts = {})
-      @opts = opts
-    end
-
     def wrap_request(request)
       request
     end
@@ -14,7 +10,7 @@ def wrap_response(response)
       response
     end
 
-    def on_error(request, error); end
+    def on_error(_request, _error); end
   end
 end
 

lib/http/features/auto_deflate.rb

@@ -1,21 +1,24 @@
 # frozen_string_literal: true
 
-require "zlib"
+require "set"
 require "tempfile"
+require "zlib"
 
 require "http/request/body"
 
 module HTTP
   module Features
     class AutoDeflate < Feature
+      VALID_METHODS = Set.new(%w[gzip deflate]).freeze
+
       attr_reader :method
 
-      def initialize(**)
-        super
+      def initialize(method: "gzip")
+        super()
 
-        @method = @opts.key?(:method) ? @opts[:method].to_s : "gzip"
+        @method = method.to_s
 
-        raise Error, "Only gzip and deflate methods are supported" unless %w[gzip deflate].include?(@method)
+        raise Error, "Only gzip and deflate methods are supported" unless VALID_METHODS.include?(@method)
       end
 
       def wrap_request(request)
@@ -27,13 +30,13 @@ def wrap_request(request)
         request.headers[Headers::CONTENT_ENCODING] = method
 
         Request.new(
-          :version        => request.version,
-          :verb           => request.verb,
-          :uri            => request.uri,
-          :headers        => request.headers,
-          :proxy          => request.proxy,
-          :body           => deflated_body(request.body),
-          :uri_normalizer => request.uri_normalizer
+          version:        request.version,
+          verb:           request.verb,
+          uri:            request.uri,
+          headers:        request.headers,
+          proxy:          request.proxy,
+          body:           deflated_body(request.body),
+          uri_normalizer: request.uri_normalizer
         )
       end
 
@@ -82,7 +85,7 @@ def compressed_each
         end
 
         def compress_all!
-          @compressed = Tempfile.new("http-compressed_body", :binmode => true)
+          @compressed = Tempfile.new("http-compressed_body", binmode: true)
           compress { |data| @compressed.write(data) }
           @compressed.rewind
         end

lib/http/features/auto_inflate.rb

@@ -12,13 +12,13 @@ def wrap_response(response)
         return response unless supported_encoding?(response)
 
         options = {
-          :status        => response.status,
-          :version       => response.version,
-          :headers       => response.headers,
-          :proxy_headers => response.proxy_headers,
-          :connection    => response.connection,
-          :body          => stream_for(response.connection),
-          :request       => response.request
+          status:        response.status,
+          version:       response.version,
+          headers:       response.headers,
+          proxy_headers: response.proxy_headers,
+          connection:    response.connection,
+          body:          stream_for(response.connection),
+          request:       response.request
         }
 
         Response.new(options)

lib/http/features/instrumentation.rb

@@ -22,6 +22,7 @@ class Instrumentation < Feature
       attr_reader :instrumenter, :name, :error_name
 
       def initialize(instrumenter: NullInstrumenter.new, namespace: "http")
+        super()
         @instrumenter = instrumenter
         @name = "request.#{namespace}"
         @error_name = "error.#{namespace}"
@@ -30,18 +31,18 @@ def initialize(instrumenter: NullInstrumenter.new, namespace: "http")
       def wrap_request(request)
         # Emit a separate "start" event, so a logger can print the request
         # being run without waiting for a response
-        instrumenter.instrument("start_#{name}", :request => request)
-        instrumenter.start(name, :request => request)
+        instrumenter.instrument("start_#{name}", request: request)
+        instrumenter.start(name, request: request)
         request
       end
 
       def wrap_response(response)
-        instrumenter.finish(name, :response => response)
+        instrumenter.finish(name, response: response)
         response
       end
 
       def on_error(request, error)
-        instrumenter.instrument(error_name, :request => request, :error => error)
+        instrumenter.instrument(error_name, request: request, error: error)
       end
 
       HTTP::Options.register_feature(:instrumentation, self)

lib/http/features/logging.rb

@@ -26,6 +26,7 @@ class NullLogger
       attr_reader :logger
 
       def initialize(logger: NullLogger.new)
+        super()
         @logger = logger
       end
 

lib/http/features/normalize_uri.rb

@@ -8,6 +8,7 @@ class NormalizeUri < Feature
       attr_reader :normalizer
 
       def initialize(normalizer: HTTP::URI::NORMALIZER)
+        super()
         @normalizer = normalizer
       end
 

lib/http/options.rb

@@ -49,19 +49,19 @@ def def_option(name, reader_only: false, &interpreter)
 
     def initialize(options = {})
       defaults = {
-        :response           => :auto,
-        :proxy              => {},
-        :timeout_class      => self.class.default_timeout_class,
-        :timeout_options    => {},
-        :socket_class       => self.class.default_socket_class,
-        :nodelay            => false,
-        :ssl_socket_class   => self.class.default_ssl_socket_class,
-        :ssl                => {},
-        :keep_alive_timeout => 5,
-        :headers            => {},
-        :cookies            => {},
-        :encoding           => nil,
-        :features           => {}
+        response:           :auto,
+        proxy:              {},
+        timeout_class:      self.class.default_timeout_class,
+        timeout_options:    {},
+        socket_class:       self.class.default_socket_class,
+        nodelay:            false,
+        ssl_socket_class:   self.class.default_ssl_socket_class,
+        ssl:                {},
+        keep_alive_timeout: 5,
+        headers:            {},
+        cookies:            {},
+        encoding:           nil,
+        features:           {}
       }
 
       opts_w_defaults = defaults.merge(options)
@@ -84,7 +84,7 @@ def initialize(options = {})
       self.encoding = Encoding.find(encoding)
     end
 
-    def_option :features, :reader_only => true do |new_features|
+    def_option :features, reader_only: true do |new_features|
       # Normalize features from:
       #
       #     [{feature_one: {opt: 'val'}}, :feature_two]
@@ -124,7 +124,7 @@ def features=(features)
       def_option method_name
     end
 
-    def_option :follow, :reader_only => true
+    def_option :follow, reader_only: true
 
     def follow=(value)
       @follow =
@@ -136,7 +136,7 @@ def follow=(value)
         end
     end
 
-    def_option :persistent, :reader_only => true
+    def_option :persistent, reader_only: true
 
     def persistent=(value)
       @persistent = value ? HTTP::URI.parse(value).origin : nil