Merge pull request #585 from stripe/brandur-guard-clause

Remove Rubocop TODO around guard clauses

.rubocop_todo.yml

@@ -65,12 +65,3 @@ Style/ClassVars:
 # Offense count: 51
 Style/Documentation:
   Enabled: false
-
-# Offense count: 5
-# Configuration parameters: MinBodyLength.
-Style/GuardClause:
-  Exclude:
-    - 'lib/stripe/alipay_account.rb'
-    - 'lib/stripe/source.rb'
-    - 'lib/stripe/stripe_client.rb'
-    - 'lib/stripe/stripe_object.rb'

lib/stripe/alipay_account.rb

@@ -6,9 +6,12 @@ module Stripe
     OBJECT_NAME = "alipay_account".freeze
 
     def resource_url
-      if respond_to?(:customer) && !customer.nil?
-        "#{Customer.resource_url}/#{CGI.escape(customer)}/sources/#{CGI.escape(id)}"
+      if !respond_to?(:customer) || customer.nil?
+        raise NotImplementedError,
+              "Alipay accounts cannot be accessed without a customer ID."
       end
+
+      "#{Customer.resource_url}/#{CGI.escape(customer)}/sources/#{CGI.escape(id)}"
     end
 
     def self.update(_id, _params = nil, _opts = nil)

lib/stripe/source.rb

@@ -6,13 +6,16 @@ module Stripe
     OBJECT_NAME = "source".freeze
 
     def delete(params = {}, opts = {})
-      if respond_to?(:customer) && !customer.nil? && !customer.empty?
-        url = "#{Customer.resource_url}/#{CGI.escape(customer)}/sources/#{CGI.escape(id)}"
-        resp, opts = request(:delete, url, params, Util.normalize_opts(opts))
-        initialize_from(resp.data, opts)
-      else
-        raise NotImplementedError, "Source objects cannot be deleted, they can only be detached from customer objects. This source object does not appear to be currently attached to a customer object."
+      if !respond_to?(:customer) || customer.nil? || customer.empty?
+        raise NotImplementedError,
+              "Source objects cannot be deleted, they can only be detached " \
+              "from customer objects. This source object does not appear to " \
+              "be currently attached to a customer object."
       end
+
+      url = "#{Customer.resource_url}/#{CGI.escape(customer)}/sources/#{CGI.escape(id)}"
+      resp, opts = request(:delete, url, params, Util.normalize_opts(opts))
+      initialize_from(resp.data, opts)
     end
 
     def verify(params = {}, opts = {})

lib/stripe/stripe_client.rb

@@ -181,12 +181,12 @@ module Stripe
           "if you have any questions."
       end
 
-      if api_key =~ /\s/
-        raise AuthenticationError, "Your API key is invalid, as it contains " \
-          "whitespace. (HINT: You can double-check your API key from the " \
-          "Stripe web interface. See https://stripe.com/api for details, or " \
-          "email support@stripe.com if you have any questions.)"
-      end
+      return unless api_key =~ /\s/
+
+      raise AuthenticationError, "Your API key is invalid, as it contains " \
+        "whitespace. (HINT: You can double-check your API key from the " \
+        "Stripe web interface. See https://stripe.com/api for details, or " \
+        "email support@stripe.com if you have any questions.)"
     end
 
     def execute_request_with_rescues(api_base, context)
@@ -458,12 +458,13 @@ module Stripe
                      body: body,
                      idempotency_key: context.idempotency_key,
                      request_id: context.request_id)
-      if context.request_id
-        Util.log_debug("Dashboard link for request",
-                       idempotency_key: context.idempotency_key,
-                       request_id: context.request_id,
-                       url: Util.request_id_dashboard_url(context.request_id, context.api_key))
-      end
+
+      return unless context.request_id
+
+      Util.log_debug("Dashboard link for request",
+                     idempotency_key: context.idempotency_key,
+                     request_id: context.request_id,
+                     url: Util.request_id_dashboard_url(context.request_id, context.api_key))
     end
     private :log_response
 

lib/stripe/stripe_object.rb

@@ -278,11 +278,12 @@ module Stripe
       begin
         super
       rescue NoMethodError => e
-        if @transient_values.include?(name)
-          raise NoMethodError, e.message + ".  HINT: The '#{name}' attribute was set in the past, however.  It was then wiped when refreshing the object with the result returned by Stripe's API, probably as a result of a save().  The attributes currently available on this object are: #{@values.keys.join(', ')}"
-        else
-          raise
-        end
+        # If we notice the accessed name if our set of transient values we can
+        # give the user a slightly more helpful error message. If not, just
+        # raise right away.
+        raise unless @transient_values.include?(name)
+
+        raise NoMethodError, e.message + ".  HINT: The '#{name}' attribute was set in the past, however.  It was then wiped when refreshing the object with the result returned by Stripe's API, probably as a result of a save().  The attributes currently available on this object are: #{@values.keys.join(', ')}"
       end
     end
 

test/stripe/alipay_account_test.rb

@@ -2,6 +2,24 @@ require File.expand_path("../../test_helper", __FILE__)
 
 module Stripe
   class AlipayAccountTest < Test::Unit::TestCase
+    context "#resource_url" do
+      should "return a resource URL" do
+        alipay_account = Stripe::AlipayAccount.construct_from(
+          id: "aliacc_123",
+          customer: "cus_123"
+        )
+        assert_equal "/v1/customers/cus_123/sources/aliacc_123",
+                     alipay_account.resource_url
+      end
+
+      should "raise without a customer" do
+        alipay_account = Stripe::AlipayAccount.construct_from(id: "aliacc_123")
+        assert_raises NotImplementedError do
+          alipay_account.resource_url
+        end
+      end
+    end
+
     should "raise on #retrieve" do
       assert_raises NotImplementedError do
         Stripe::AlipayAccount.retrieve("aliacc_123")