Skip to content

Commit

Permalink
RuboCop upgrade (#548)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
natesholland authored and ixti committed May 8, 2019
1 parent f37a10e commit fec85f6
Show file tree
Hide file tree
Showing 24 changed files with 47 additions and 36 deletions.
9 changes: 9 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ Metrics/ParameterLists:
Performance/RegexpMatch:
Enabled: false

Performance/UnfreezeString:
Enabled: false

## Style #######################################################################

Style/CollectionMethods:
Expand Down Expand Up @@ -110,3 +113,9 @@ Style/TrivialAccessors:

Style/YodaCondition:
Enabled: false

Style/FormatStringToken:
EnforcedStyle: unannotated

Style/RescueStandardError:
EnforcedStyle: implicit
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions http.gemspec
Original file line number Diff line number Diff line change
@@ -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"

Expand All @@ -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"

Expand Down
2 changes: 2 additions & 0 deletions lib/http/chainable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions lib/http/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions lib/http/headers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ def keys
# @return [Boolean]
def ==(other)
return false unless other.respond_to? :to_a

@pile == other.to_a
end

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/http/mime_type/adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/http/mime_type/json.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 3 additions & 6 deletions lib/http/options.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/http/redirector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions lib/http/request/body.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/http/request/writer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/http/response/body.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion lib/http/response/status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
4 changes: 1 addition & 3 deletions lib/http/timeout/global.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/http/timeout/per_operation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/http/client_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# frozen_string_literal: true
# coding: utf-8
# frozen_string_literal: true

require "support/http_handling_shared"
require "support/dummy_server"
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/http/request/writer_spec.rb
Original file line number Diff line number Diff line change
@@ -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 }
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/http/request_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# frozen_string_literal: true
# coding: utf-8
# frozen_string_literal: true

RSpec.describe HTTP::Request do
let(:proxy) { {} }
Expand Down
6 changes: 3 additions & 3 deletions spec/lib/http/response/status_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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} }
Expand Down Expand Up @@ -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} }
Expand Down Expand Up @@ -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}? }
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/http_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# frozen_string_literal: true
# encoding: utf-8
# frozen_string_literal: true

require "json"

Expand Down
2 changes: 1 addition & 1 deletion spec/support/black_hole.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module BlackHole
class << self
def method_missing(*) # rubocop: disable Style/MethodMissing
def method_missing(*) # rubocop:disable Style/MethodMissingSuper
self
end

Expand Down
7 changes: 3 additions & 4 deletions spec/support/dummy_server/servlet.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions spec/support/ssl_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit fec85f6

Please sign in to comment.