Fix low hanging Rubocop TODOs

I wanted to see what fixing Rubocop TODOs was like, so I tried to
eliminate all the easy ones. Most of these were pretty easy, and the
changes required are relatively minimal.

Some of the stuff left is harder. Pretty much everything under
`Metrics/*` is going to be a pretty big yak shave. A few of the others
are just going to need a little more work (e.g. `Style/ClassVars` and
`Style/GuardClause`). Going to stop here for now.

.rubocop_todo.yml

@@ -6,74 +6,6 @@
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 1
-# Configuration parameters: Include.
-# Include: **/Gemfile, **/gems.rb
-Bundler/DuplicatedGem:
-  Exclude:
-    - 'Gemfile'
-
-# Offense count: 1
-Lint/AmbiguousRegexpLiteral:
-  Exclude:
-    - 'test/stripe/stripe_object_test.rb'
-
-# Offense count: 3
-# Configuration parameters: AllowSafeAssignment.
-Lint/AssignmentInCondition:
-  Exclude:
-    - 'lib/stripe/account.rb'
-    - 'lib/stripe/api_resource.rb'
-
-# Offense count: 1
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyleAlignWith, SupportedStylesAlignWith, AutoCorrect.
-# SupportedStylesAlignWith: keyword, variable, start_of_line
-Lint/EndAlignment:
-  Exclude:
-    - 'lib/stripe/stripe_client.rb'
-
-# Offense count: 2
-Lint/ImplicitStringConcatenation:
-  Exclude:
-    - 'test/stripe/webhook_test.rb'
-
-# Offense count: 3
-Lint/IneffectiveAccessModifier:
-  Exclude:
-    - 'lib/stripe.rb'
-    - 'lib/stripe/invoice.rb'
-    - 'lib/stripe/stripe_object.rb'
-
-# Offense count: 1
-Lint/LiteralInCondition:
-  Exclude:
-    - 'lib/stripe/stripe_object.rb'
-
-# Offense count: 2
-Lint/NonLocalExitFromIterator:
-  Exclude:
-    - 'lib/stripe/util.rb'
-
-# Offense count: 5
-Lint/RescueWithoutErrorClass:
-  Exclude:
-    - 'lib/stripe/stripe_client.rb'
-    - 'lib/stripe/util.rb'
-    - 'lib/stripe/webhook.rb'
-
-# Offense count: 2
-# Configuration parameters: ContextCreatingMethods, MethodCreatingMethods.
-Lint/UselessAccessModifier:
-  Exclude:
-    - 'lib/stripe.rb'
-    - 'lib/stripe/util.rb'
-
-# Offense count: 1
-Lint/UselessAssignment:
-  Exclude:
-    - 'stripe.gemspec'
-
 # Offense count: 18
 Metrics/AbcSize:
   Max: 49
@@ -116,33 +48,14 @@ Metrics/ParameterLists:
 # Offense count: 5
 Metrics/PerceivedComplexity:
   Max: 11
-
-# Offense count: 4
-Naming/AccessorMethodName:
-  Exclude:
-    - 'lib/stripe/stripe_client.rb'
-    - 'test/stripe/api_resource_test.rb'
-
-# Offense count: 3
-# Cop supports --auto-correct.
-# Configuration parameters: AutoCorrect.
-Performance/HashEachMethods:
   Exclude:
-    - 'lib/stripe/api_operations/save.rb'
     - 'lib/stripe/stripe_object.rb'
 
 # Offense count: 1
 Security/MarshalLoad:
   Exclude:
     - 'lib/stripe/stripe_object.rb'
 
-# Offense count: 1
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-# SupportedStyles: nested, compact
-Style/ClassAndModuleChildren:
-  Exclude:
-    - 'test/test_helper.rb'
-
 # Offense count: 2
 Style/ClassVars:
   Exclude:
@@ -153,11 +66,6 @@ Style/ClassVars:
 Style/Documentation:
   Enabled: false
 
-# Offense count: 7
-Style/DoubleNegation:
-  Exclude:
-    - 'test/stripe/stripe_client_test.rb'
-
 # Offense count: 5
 # Configuration parameters: MinBodyLength.
 Style/GuardClause:
@@ -166,17 +74,3 @@ Style/GuardClause:
     - 'lib/stripe/source.rb'
     - 'lib/stripe/stripe_client.rb'
     - 'lib/stripe/stripe_object.rb'
-
-# Offense count: 1
-Style/IfInsideElse:
-  Exclude:
-    - 'lib/stripe/stripe_object.rb'
-
-# Offense count: 2
-# Cop supports --auto-correct.
-# Configuration parameters: AutoCorrect, EnforcedStyle, SupportedStyles.
-# SupportedStyles: predicate, comparison
-Style/NumericPredicate:
-  Exclude:
-    - 'spec/**/*'
-    - 'lib/stripe/util.rb'

Gemfile

@@ -23,7 +23,7 @@ group :development do
   if RUBY_VERSION >= "2.2.2"
     gem "rack", ">= 1.5"
   else
-    gem "rack", ">= 1.5", "< 2.0"
+    gem "rack", ">= 1.5", "< 2.0" # rubocop:disable Bundler/DuplicatedGem
   end
 
   platforms :mri do

lib/stripe.rb

@@ -4,6 +4,7 @@
 require "logger"
 require "openssl"
 require "rbconfig"
+require "securerandom"
 require "set"
 require "socket"
 
@@ -212,12 +213,11 @@ def self.set_app_info(name, version: nil, url: nil)
     }
   end
 
-  private
-
   # DEPRECATED. Use `Util#encode_parameters` instead.
   def self.uri_encode(params)
     Util.encode_parameters(params)
   end
+  private_class_method :uri_encode
   class << self
     extend Gem::Deprecate
     deprecate :uri_encode, "Stripe::Util#encode_parameters", 2016, 1

lib/stripe/account.rb

@@ -71,8 +71,8 @@ def serialize_params(options = {})
     end
 
     def serialize_params_account(_obj, update_hash)
-      if entity = @values[:legal_entity]
-        if owners = entity[:additional_owners]
+      if (entity = @values[:legal_entity])
+        if (owners = entity[:additional_owners])
           entity_update = update_hash[:legal_entity] ||= {}
           entity_update[:additional_owners] =
             serialize_additional_owners(entity, owners)

lib/stripe/api_operations/save.rb

@@ -15,7 +15,7 @@ module ClassMethods
         #   idempotency_key to be passed in the request headers, or for the
         #   api_key to be overwritten. See {APIOperations::Request.request}.
         def update(id, params = {}, opts = {})
-          params.each do |k, _v|
+          params.each_key do |k|
             if protected_fields.include?(k)
               raise ArgumentError, "Cannot update protected field: #{k}"
             end

lib/stripe/api_resource.rb

@@ -45,7 +45,7 @@ def self.save_nested_resource(name)
     end
 
     def resource_url
-      unless id = self["id"]
+      unless (id = self["id"])
         raise InvalidRequestError.new("Could not determine which URL to request: #{self.class} instance has invalid ID: #{id.inspect}", "id")
       end
       "#{self.class.resource_url}/#{CGI.escape(id)}"

lib/stripe/invoice.rb

@@ -16,14 +16,14 @@ def pay(params = {}, opts = {})
       initialize_from(resp.data, opts)
     end
 
-    private
-
     def self.upcoming_url
       resource_url + "/upcoming"
     end
+    private_class_method :upcoming_url
 
     def pay_url
       resource_url + "/pay"
     end
+    private :pay_url
   end
 end

lib/stripe/stripe_client.rb

@@ -201,7 +201,7 @@ def execute_request_with_rescues(api_base, context)
       # We rescue all exceptions from a request so that we have an easy spot to
       # implement our retry logic across the board. We'll re-raise if it's a type
       # of exception that we didn't expect to handle.
-      rescue => e
+      rescue StandardError => e
         # If we modify context we copy it into a new variable so as not to
         # taint the original on a retry.
         error_context = context
@@ -420,7 +420,7 @@ def request_headers(api_key, method)
         headers.update(
           "X-Stripe-Client-User-Agent" => JSON.generate(user_agent)
         )
-      rescue => e
+      rescue StandardError => e
         headers.update(
           "X-Stripe-Client-Raw-User-Agent" => user_agent.inspect,
           :error => "#{e} (#{e.class})"
@@ -517,7 +517,7 @@ def dup_from_response(resp)
                     resp.headers
                   else
                     resp[:headers]
-        end
+                  end
 
         context = dup
         context.account = headers["Stripe-Account"]
@@ -532,30 +532,30 @@ def dup_from_response(resp)
     # in so that we can generate a rich user agent header to help debug
     # integrations.
     class SystemProfiler
-      def self.get_uname
+      def self.uname
         if File.exist?("/proc/version")
           File.read("/proc/version").strip
         else
           case RbConfig::CONFIG["host_os"]
           when /linux|darwin|bsd|sunos|solaris|cygwin/i
-            get_uname_from_system
+            uname_from_system
           when /mswin|mingw/i
-            get_uname_from_system_ver
+            uname_from_system_ver
           else
             "unknown platform"
           end
         end
       end
 
-      def self.get_uname_from_system
+      def self.uname_from_system
         (`uname -a 2>/dev/null` || "").strip
       rescue Errno::ENOENT
         "uname executable not found"
       rescue Errno::ENOMEM # couldn't create subprocess
         "uname lookup failed"
       end
 
-      def self.get_uname_from_system_ver
+      def self.uname_from_system_ver
         (`ver` || "").strip
       rescue Errno::ENOENT
         "ver executable not found"
@@ -564,7 +564,7 @@ def self.get_uname_from_system_ver
       end
 
       def initialize
-        @uname = self.class.get_uname
+        @uname = self.class.uname
       end
 
       def user_agent

lib/stripe/stripe_object.rb

@@ -153,7 +153,7 @@ def self._load(args)
     # values in a tenant array are also marked as dirty.
     def dirty!
       @unsaved_values = Set.new(@values.keys)
-      @values.each do |_k, v|
+      @values.each_value do |v|
         dirty_value!(v)
       end
     end
@@ -194,15 +194,15 @@ def serialize_params(obj, options = {})
       deprecate :serialize_params, "#serialize_params", 2016, 9
     end
 
-    protected
-
     # A protected field is one that doesn't get an accessor assigned to it
     # (i.e. `obj.public = ...`) and one which is not allowed to be updated via
     # the class level `Model.update(id, { ... })`.
     def self.protected_fields
       []
     end
 
+    protected
+
     def metaclass
       class << self; self; end
     end
@@ -271,8 +271,8 @@ def method_missing(name, *args)
           raise NoMethodError, "Cannot set #{attr} on this object. HINT: you can't set: #{@@permanent_attributes.to_a.join(', ')}"
         end
         return mth.call(args[0])
-      else
-        return @values[name] if @values.key?(name)
+      elsif @values.key?(name)
+        return @values[name]
       end
 
       begin
@@ -323,7 +323,7 @@ def initialize_from(values, opts, partial = false)
       end
 
       update_attributes(values, opts, dirty: false)
-      values.each do |k, _|
+      values.each_key do |k|
         @transient_values.delete(k)
         @unsaved_values.delete(k)
       end
@@ -332,8 +332,7 @@ def initialize_from(values, opts, partial = false)
     end
 
     def serialize_params_value(value, original, unsaved, force, key: nil)
-      case true
-      when value.nil?
+      if value.nil?
         ""
 
       # The logic here is that essentially any object embedded in another
@@ -358,7 +357,7 @@ def serialize_params_value(value, original, unsaved, force, key: nil)
       # We throw an error if a property was set explicitly but we can't do
       # anything with it because the integration is probably not working as the
       # user intended it to.
-      when value.is_a?(APIResource) && !value.save_with_parent
+      elsif value.is_a?(APIResource) && !value.save_with_parent
         if !unsaved
           nil
         elsif value.respond_to?(:id) && !value.id.nil?
@@ -369,7 +368,7 @@ def serialize_params_value(value, original, unsaved, force, key: nil)
             "not marked as `save_with_parent`."
         end
 
-      when value.is_a?(Array)
+      elsif value.is_a?(Array)
         update = value.map { |v| serialize_params_value(v, nil, true, force) }
 
         # This prevents an array that's unchanged from being resent.
@@ -385,10 +384,10 @@ def serialize_params_value(value, original, unsaved, force, key: nil)
       # existing array being held by a StripeObject. This could happen for
       # example by appending a new hash onto `additional_owners` for an
       # account.
-      when value.is_a?(Hash)
+      elsif value.is_a?(Hash)
         Util.convert_to_stripe_object(value, @opts).serialize_params
 
-      when value.is_a?(StripeObject)
+      elsif value.is_a?(StripeObject)
         update = value.serialize_params(force: force)
 
         # If the entire object was replaced, then we need blank each field of