Refactor to remove rubocop:disable comments

Author: sferik

.rubocop.yml

@@ -4,7 +4,6 @@ plugins:
   - rubocop-minitest
 
 inherit_from:
-  - .rubocop_todo.yml
   - .rubocop/layout.yml
   - .rubocop/metrics.yml
   - .rubocop/minitest.yml
@@ -15,3 +14,9 @@ AllCops:
   DisplayStyleGuide: true
   NewCops: enable
   TargetRubyVersion: 3.0
+
+# Feature has no initialize, so super() is a no-op in subclasses
+Lint/MissingSuper:
+  AllowedParentClasses:
+    - BasicObject
+    - Feature

.rubocop/metrics.yml

@@ -1,3 +1,7 @@
+Metrics/AbcSize:
+  Exclude:
+    - 'test/support/**/*.rb'
+
 Metrics/BlockLength:
   CountAsOne:
     - array
@@ -18,6 +22,8 @@ Metrics/MethodLength:
     - array
     - heredoc
     - method_call
+  Exclude:
+    - 'test/support/**/*.rb'
 
 Metrics/ModuleLength:
   CountAsOne:

.rubocop/style.yml

@@ -1,34 +1,46 @@
-Style/Documentation:
-  Enabled: false
-
-# Hash[] is used instead of {} to satisfy Steep strict mode (UnannotatedEmptyCollection)
-Style/EmptyLiteral:
-  Exclude:
-    - 'lib/http/options.rb'
-
-Style/HashConversion:
-  Exclude:
-    - 'lib/http/options.rb'
-
-Style/OneClassPerFile:
-  Exclude:
-    - 'test/**/*.rb'
+Style/AccessorGrouping:
+  Enabled: true
+  EnforcedStyle: separated
 
 Style/DocumentDynamicEvalDefinition:
   Enabled: true
   Exclude:
     - 'test/**/*.rb'
 
+# Hash[] is used instead of {} to satisfy Steep strict mode (UnannotatedEmptyCollection)
+Style/EmptyLiteral:
+  Exclude:
+    - 'lib/http/options.rb'
+    - 'lib/http/options/definitions.rb'
+
 Style/FormatStringToken:
   Enabled: true
   EnforcedStyle: unannotated
 
+Style/HashConversion:
+  Exclude:
+    - 'lib/http/options.rb'
+    - 'lib/http/options/definitions.rb'
+
 Style/HashSyntax:
   Enabled: true
   EnforcedStyle: ruby19_no_mixed_keys
 
+Style/OneClassPerFile:
+  Exclude:
+    - 'test/**/*.rb'
+
+# Configuration parameters: SuspiciousParamNames, Allowlist.
+# SuspiciousParamNames: options, opts, args, params, parameters
 Style/OptionHash:
   Enabled: true
+  Exclude:
+    - 'lib/http/chainable.rb'
+    - 'lib/http/client.rb'
+    - 'lib/http/options.rb'
+    - 'lib/http/redirector.rb'
+    - 'lib/http/timeout/null.rb'
+    - 'test/support/dummy_server.rb'
 
 # Using explicit `./` makes code more consistent
 Style/RedundantCurrentDirectoryInPath:

.rubocop_todo.yml

@@ -30,17 +30,19 @@ task :steep do
   exit Steep::CLI.new(argv: ["check", "--log-level=fatal"], stdout: $stdout, stderr: $stderr, stdin: $stdin).run
 end
 
+desc "Generate HTTP status codes from IANA registry"
 task :generate_status_codes do
   require "http"
   require "nokogiri"
 
   url = "http://www.iana.org/assignments/http-status-codes/http-status-codes.xml"
   xml = Nokogiri::XML HTTP.get url
+  excluded_descriptions = %w[Unassigned (Unused)]
   arr = xml.xpath("//xmlns:record").reduce([]) do |a, e|
     code = e.xpath("xmlns:value").text.to_s
     desc = e.xpath("xmlns:description").text.to_s
 
-    next a if %w[Unassigned (Unused)].include?(desc)
+    next a if excluded_descriptions.include?(desc)
 
     a << "#{code} => #{desc.inspect}"
   end

Rakefile

@@ -32,9 +32,6 @@ Gem::Specification.new do |spec|
     end
   end
 
-  spec.bindir        = "exe"
-  spec.executables   = spec.files.filter_map { |f| File.basename(f) if f.start_with?("exe/") }
-  spec.test_files    = spec.files.select { |f| f.start_with?("test/") }
   spec.require_paths = ["lib"]
 
   spec.required_ruby_version = ">= 3.0"

http.gemspec

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "http/base64"
+require "http/chainable/helpers"
 require "http/headers"
 
 module HTTP
@@ -169,15 +170,12 @@ def timeout(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)`."
-
+                       else raise ArgumentError,
+                                  "Use `.timeout(global_timeout_in_seconds)` " \
+                                  "or `.timeout(connect: x, write: y, read: z)`."
                        end
 
-      %i[global read write connect].each do |k|
-        next unless options.key? k
-
-        options[:"#{k}_timeout"] = options.delete k
-      end
+      normalize_timeout_keys!(options)
 
       branch default_options.merge(
         timeout_class:   klass,
@@ -223,8 +221,7 @@ def timeout(options)
     # @return [HTTP::Client, Object]
     # @api public
     def persistent(host, timeout: 5)
-      options  = { keep_alive_timeout: timeout }
-      p_client = branch default_options.merge(options).with_persistent host
+      p_client = branch default_options.merge(keep_alive_timeout: timeout).with_persistent(host)
       return p_client unless block_given?
 
       yield p_client
@@ -242,13 +239,7 @@ def persistent(host, timeout: 5)
     # @return [HTTP::Client]
     # @api public
     def via(*proxy)
-      proxy_hash = {} #: Hash[Symbol, untyped]
-      proxy_hash[:proxy_address]  = proxy[0] if proxy[0].is_a?(String)
-      proxy_hash[:proxy_port]     = proxy[1] if proxy[1].is_a?(Integer)
-      proxy_hash[:proxy_username] = proxy[2] if proxy[2].is_a?(String)
-      proxy_hash[:proxy_password] = proxy[3] if proxy[3].is_a?(String)
-      proxy_hash[:proxy_headers]  = proxy[2] if proxy[2].is_a?(Hash)
-      proxy_hash[:proxy_headers]  = proxy[4] if proxy[4].is_a?(Hash)
+      proxy_hash = build_proxy_hash(proxy)
 
       raise(RequestError, "invalid HTTP proxy: #{proxy_hash}") unless (2..5).cover?(proxy_hash.keys.size)
 

lib/http/chainable.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+module HTTP
+  module Chainable
+    # Mapping of proxy argument positions to hash keys and expected types
+    PROXY_ARG_MAP = [
+      [:proxy_address,  0, String],
+      [:proxy_port,     1, Integer],
+      [:proxy_username, 2, String],
+      [:proxy_password, 3, String],
+      [:proxy_headers,  2, Hash],
+      [:proxy_headers,  4, Hash]
+    ].freeze
+
+    private
+
+    # Build proxy configuration hash from positional arguments
+    #
+    # @param [Array] proxy positional proxy arguments
+    # @return [Hash] proxy configuration
+    # @api private
+    def build_proxy_hash(proxy)
+      result = {} #: Hash[Symbol, untyped]
+      PROXY_ARG_MAP.each do |key, index, type|
+        result[key] = proxy[index] if proxy[index].is_a?(type)
+      end
+      result
+    end
+
+    # Normalize timeout option keys to include _timeout suffix
+    #
+    # @param [Hash] options timeout options to normalize
+    # @return [void]
+    # @api private
+    def normalize_timeout_keys!(options)
+      %i[global read write connect].each do |k|
+        next unless options.key? k
+
+        options[:"#{k}_timeout"] = options.delete k
+      end
+    end
+  end
+end