Fix rubocop warnings

.rubocop.yml

@@ -2,5 +2,30 @@ inherit_from:
   - https://shopify.github.io/ruby-style-guide/rubocop.yml
   - .rubocop_todo.yml
 
+# The entries below are not included automatically in the TODO file, they should be fixed and removed as well
+Layout/SpaceInsideHashLiteralBraces:
+  Enabled: false
+Layout/SpaceBeforeBlockBraces:
+  Enabled: false
+Layout/SpaceAroundEqualsInParameterDefault:
+  Enabled: false
+Style/RaiseArgs:
+  Enabled: false
+
 AllCops:
   TargetRubyVersion: 2.4
+
+Lint/SuppressedException:
+  Exclude:
+    # Warning on test setup, the exception is expected
+    - 'test/test_helper.rb'
+
+Lint/MissingSuper:
+  Exclude:
+    # We explicitly do not want to call super here
+    - 'lib/shopify_api/graphql/http_client.rb'
+
+Lint/UnderscorePrefixedVariableName:
+  Exclude:
+    # This is an internal attribute and we want to make sure it's called _headers
+    - 'lib/shopify_api/resources/base.rb'

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2020-10-05 17:42:41 UTC using RuboCop version 0.92.0.
+# on 2020-10-05 18:45:45 UTC using RuboCop version 0.92.0.
 # 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
@@ -106,6 +106,13 @@ Layout/IndentationWidth:
   Exclude:
     - 'test/fulfillment_service_test.rb'
 
+# Offense count: 83
+# Cop supports --auto-correct.
+# Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
+# URISchemes: http, https
+Layout/LineLength:
+  Max: 320
+
 # Offense count: 2
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle.
@@ -143,7 +150,6 @@ Layout/SpaceAfterComma:
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: space, no_space
 Layout/SpaceAroundEqualsInParameterDefault:
-  Enabled: false
   Exclude:
     - 'lib/shopify_api/limits.rb'
 
@@ -165,8 +171,7 @@ Layout/SpaceAroundOperators:
 # SupportedStyles: space, no_space
 # SupportedStylesForEmptyBraces: space, no_space
 Layout/SpaceBeforeBlockBraces:
-  # EnforcedStyle: space
-  Enabled: false
+  EnforcedStyle: space
 
 # Offense count: 4
 # Cop supports --auto-correct.
@@ -181,9 +186,8 @@ Layout/SpaceBeforeComma:
 # SupportedStyles: space, no_space, compact
 # SupportedStylesForEmptyBrackets: space, no_space
 Layout/SpaceInsideArrayLiteralBrackets:
-  Enabled: false
-  # Exclude:
-  #   - 'test/resource_feedback_test.rb'
+  Exclude:
+    - 'test/resource_feedback_test.rb'
 
 # Offense count: 5
 # Cop supports --auto-correct.
@@ -202,8 +206,7 @@ Layout/SpaceInsideBlockBraces:
 # SupportedStyles: space, no_space, compact
 # SupportedStylesForEmptyBraces: space, no_space
 Layout/SpaceInsideHashLiteralBraces:
-  Enabled: false
-  # EnforcedStyle: space
+  EnforcedStyle: space
 
 # Offense count: 1
 # Cop supports --auto-correct.
@@ -213,7 +216,7 @@ Layout/SpaceInsideParens:
   Exclude:
     - 'lib/shopify_api/resources/recurring_application_charge.rb'
 
-# Offense count: 5
+# Offense count: 4
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: final_newline, final_blank_line
@@ -223,7 +226,6 @@ Layout/TrailingEmptyLines:
     - 'lib/shopify_api/resources/announcement.rb'
     - 'lib/shopify_api/resources/cart.rb'
     - 'test/cart_test.rb'
-    - 'test/metafield_test.rb'
 
 # Offense count: 8
 # Cop supports --auto-correct.
@@ -236,75 +238,6 @@ Layout/TrailingWhitespace:
     - 'test/storefront_access_token_test.rb'
     - 'test/user_test.rb'
 
-# Offense count: 1
-# Cop supports --auto-correct.
-Lint/AmbiguousOperator:
-  Exclude:
-    - 'test/price_rule_test.rb'
-
-# Offense count: 3
-# Configuration parameters: AllowSafeAssignment.
-Lint/AssignmentInCondition:
-  Exclude:
-    - 'lib/shopify_api/session.rb'
-
-# Offense count: 2
-# Cop supports --auto-correct.
-Lint/DeprecatedOpenSSLConstant:
-  Exclude:
-    - 'lib/shopify_api/session.rb'
-    - 'test/session_test.rb'
-
-# Offense count: 2
-Lint/MissingSuper:
-  Exclude:
-    - 'lib/shopify_api/graphql/http_client.rb'
-    - 'test/api_version_test.rb'
-
-# Offense count: 1
-# Configuration parameters: AllowComments.
-Lint/SuppressedException:
-  Exclude:
-    - 'test/test_helper.rb'
-
-# Offense count: 1
-# Configuration parameters: AllowKeywordBlockArguments.
-Lint/UnderscorePrefixedVariableName:
-  Exclude:
-    - 'lib/shopify_api/resources/base.rb'
-
-# Offense count: 12
-# Cop supports --auto-correct.
-# Configuration parameters: IgnoreEmptyBlocks, AllowUnusedKeywordArguments.
-Lint/UnusedBlockArgument:
-  Exclude:
-    - 'test/graphql_test.rb'
-    - 'test/lib/webmock_extensions/last_request.rb'
-
-# Offense count: 2
-# Cop supports --auto-correct.
-# Configuration parameters: AllowUnusedKeywordArguments, IgnoreEmptyMethods, IgnoreNotImplementedMethods.
-Lint/UnusedMethodArgument:
-  Exclude:
-    - 'lib/shopify_api/resources/asset.rb'
-    - 'test/test_helper.rb'
-
-# Offense count: 9
-Lint/UselessAssignment:
-  Exclude:
-    - 'lib/shopify_api/resources/line_item.rb'
-    - 'test/asset_test.rb'
-    - 'test/blog_test.rb'
-    - 'test/metafield_test.rb'
-    - 'test/pagination_test.rb'
-    - 'test/tax_service_test.rb'
-
-# Offense count: 1
-# Configuration parameters: CheckForMethodsWithNoSideEffects.
-Lint/Void:
-  Exclude:
-    - 'lib/shopify_api/pagination_link_headers.rb'
-
 # Offense count: 1
 Naming/AccessorMethodName:
   Exclude:
@@ -351,20 +284,13 @@ Style/IfInsideElse:
   Exclude:
     - 'lib/shopify_api/graphql.rb'
 
-# Offense count: 480
+# Offense count: 479
 # Cop supports --auto-correct.
 # Configuration parameters: IgnoreMacros, IgnoredMethods, IgnoredPatterns, IncludedMacros, AllowParenthesesInMultilineCall, AllowParenthesesInChaining, AllowParenthesesInCamelCaseMethod, EnforcedStyle.
 # SupportedStyles: require_parentheses, omit_parentheses
 Style/MethodCallWithArgsParentheses:
   Enabled: false
 
-# Offense count: 1
-# Cop supports --auto-correct.
-# Configuration parameters: IgnoredMethods.
-Style/MethodCallWithoutArgsParentheses:
-  Exclude:
-    - 'lib/shopify_api/session.rb'
-
 # Offense count: 1
 Style/MissingRespondToMissing:
   Exclude:
@@ -383,7 +309,6 @@ Style/PreferredHashMethods:
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: compact, exploded
 Style/RaiseArgs:
-  Enabled: false
   Exclude:
     - 'lib/shopify_api/pagination_link_headers.rb'
 
@@ -399,6 +324,13 @@ Style/RedundantPercentQ:
   Exclude:
     - 'shopify_api.gemspec'
 
+# Offense count: 1
+# Cop supports --auto-correct.
+# Configuration parameters: AllowMultipleReturnValues.
+Style/RedundantReturn:
+  Exclude:
+    - 'lib/shopify_api/resources/base.rb'
+
 # Offense count: 11
 # Cop supports --auto-correct.
 Style/RedundantSelf:
@@ -483,10 +415,3 @@ Style/TrailingCommaInHashLiteral:
 Style/TrivialAccessors:
   Exclude:
     - 'lib/shopify_api/graphql.rb'
-
-# Offense count: 83
-# Cop supports --auto-correct.
-# Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
-# URISchemes: http, https
-Layout/LineLength:
-  Max: 320

lib/shopify_api/pagination_link_headers.rb

@@ -9,8 +9,6 @@ module ShopifyAPI
       links = parse_link_header(link_header)
       @previous_link = links.find { |link| link.rel == :previous }
       @next_link = links.find { |link| link.rel == :next }
-
-      self
     end
 
     private

lib/shopify_api/resources/asset.rb

@@ -38,7 +38,7 @@ module ShopifyAPI
 
     conditional_prefix :theme
 
-    def self.element_path(id, prefix_options = {}, query_options = nil) #:nodoc:
+    def self.element_path(_id, prefix_options = {}, query_options = nil) #:nodoc:
       prefix_options, query_options = split_options(prefix_options) if query_options.nil?
       "#{prefix(prefix_options)}#{collection_name}.#{format.extension}#{query_string(query_options)}"
     end

lib/shopify_api/resources/line_item.rb

@@ -2,11 +2,11 @@ module ShopifyAPI
   class LineItem < Base
     class Property < Base
       def initialize(*args)
-        attributes = args[0] || {}
-        persisted = args[1] || false
+        self.attributes = args[0] || {}
+        self.persisted = args[1] || false
         super
       rescue NameError
-        attributes = attributes.to_hash
+        self.attributes = attributes.to_hash
         self
       end
     end

lib/shopify_api/session.rb

@@ -56,7 +56,7 @@ module ShopifyAPI
         # extract host, removing any username, password or path
         shop = URI.parse("https://#{domain}").host
         # extract subdomain of .myshopify.com
-        if idx = shop.index(".")
+        if (idx = shop.index("."))
           shop = shop.slice(0, idx)
         end
         return nil if shop.empty?
@@ -67,9 +67,9 @@ module ShopifyAPI
 
       def validate_signature(params)
         params = (params.respond_to?(:to_unsafe_hash) ? params.to_unsafe_hash : params).with_indifferent_access
-        return false unless signature = params[:hmac]
+        return false unless (signature = params[:hmac])
 
-        calculated_signature = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::SHA256.new(), secret, encoded_params_for_signature(params))
+        calculated_signature = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('SHA256'), secret, encoded_params_for_signature(params))
 
         Rack::Utils.secure_compare(calculated_signature, signature)
       end
@@ -114,7 +114,7 @@ module ShopifyAPI
         self.extra = JSON.parse(response.body)
         self.token = extra.delete('access_token')
 
-        if expires_in = extra.delete('expires_in')
+        if (expires_in = extra.delete('expires_in'))
           extra['expires_at'] = Time.now.utc.to_i + expires_in
         end
         token

test/api_version_test.rb

@@ -151,6 +151,7 @@ class ApiVersionTest < Test::Unit::TestCase
 
   class TestApiVersion < ShopifyAPI::ApiVersion
     def initialize(name)
+      super(name)
       @version_name = name
     end
   end

test/asset_test.rb

@@ -3,16 +3,16 @@ require 'test_helper'
 class AssetTest < Test::Unit::TestCase
   def test_get_assetss
     fake "themes/1/assets", :method => :get, :body => load_fixture('assets')
-    v = ShopifyAPI::Asset.find(:all, :params => {:theme_id => 1})
+    ShopifyAPI::Asset.find(:all, :params => {:theme_id => 1})
   end
 
   def test_get_asset_namespaced
     fake "themes/1/assets.json?asset%5Bkey%5D=templates%2Findex.liquid&theme_id=1", :extension=> false, :method => :get, :body => load_fixture('asset')
-    v = ShopifyAPI::Asset.find('templates/index.liquid', :params => {:theme_id => 1})
+    ShopifyAPI::Asset.find('templates/index.liquid', :params => {:theme_id => 1})
   end
 
   def test_get_asset
     fake "assets.json?asset%5Bkey%5D=templates%2Findex.liquid", :extension=> false, :method => :get, :body => load_fixture('asset')
-    v = ShopifyAPI::Asset.find('templates/index.liquid')
+    ShopifyAPI::Asset.find('templates/index.liquid')
   end
 end

test/blog_test.rb

@@ -2,7 +2,7 @@ require 'test_helper'
 class BlogTest < Test::Unit::TestCase
   test "blog creation" do
     fake "blogs", :method => :post, :status => 202, :body => load_fixture('blog')
-    blog = ShopifyAPI::Blog.create(:title => "Test Blog")
+    ShopifyAPI::Blog.create(:title => "Test Blog")
     assert_equal '{"blog":{"title":"Test Blog"}}', WebMock.last_request.body
   end
 end

test/graphql_test.rb

@@ -15,7 +15,7 @@ class GraphQLTest < Test::Unit::TestCase
   end
 
   test '#initialize_clients creates a GraphQL::Client from local schema file' do
-    version_fixtures('unstable') do |dir|
+    version_fixtures('unstable') do |_dir|
       ShopifyAPI::GraphQL.initialize_clients
 
       assert ShopifyAPI::GraphQL.client('unstable')
@@ -23,7 +23,7 @@ class GraphQLTest < Test::Unit::TestCase
   end
 
   test '#initialize_clients handles multiple schema files' do
-    version_fixtures('unstable', '2019-10') do |dir|
+    version_fixtures('unstable', '2019-10') do |_dir|
       ShopifyAPI::GraphQL.initialize_clients
 
       assert ShopifyAPI::GraphQL.client('unstable')
@@ -32,7 +32,7 @@ class GraphQLTest < Test::Unit::TestCase
   end
 
   test '#initialize_clients ignores non JSON schema files' do
-    version_fixtures('unstable', '2019-10') do |dir|
+    version_fixtures('unstable', '2019-10') do |_dir|
       FileUtils.touch(ShopifyAPI::GraphQL.schema_location.join('nope.txt'))
 
       ShopifyAPI::GraphQL.initialize_clients
@@ -65,7 +65,7 @@ class GraphQLTest < Test::Unit::TestCase
   end
 
   test '#client returns default schema if only one exists' do
-    version_fixtures('unstable') do |dir|
+    version_fixtures('unstable') do |_dir|
       ShopifyAPI::Base.api_version = 'unstable'
 
       ShopifyAPI::GraphQL.initialize_clients
@@ -75,7 +75,7 @@ class GraphQLTest < Test::Unit::TestCase
   end
 
   test '#client accepts optional api_version parameter' do
-    version_fixtures('unstable') do |dir|
+    version_fixtures('unstable') do |_dir|
       ShopifyAPI::Base.api_version = 'unstable'
 
       ShopifyAPI::GraphQL.initialize_clients
@@ -85,7 +85,7 @@ class GraphQLTest < Test::Unit::TestCase
   end
 
   test '#client executes queries on specified API version' do
-    version_fixtures('unstable', '2019-10') do |dir|
+    version_fixtures('unstable', '2019-10') do |_dir|
       ShopifyAPI::Base.api_version = 'unstable'
 
       ShopifyAPI::GraphQL.initialize_clients
@@ -111,7 +111,7 @@ class GraphQLTest < Test::Unit::TestCase
   end
 
   test '#client raises exception for version that does not exist' do
-    version_fixtures('unstable') do |dir|
+    version_fixtures('unstable') do |_dir|
       ShopifyAPI::Base.api_version = '2019-10'
 
       ShopifyAPI::GraphQL.initialize_clients
@@ -123,7 +123,7 @@ class GraphQLTest < Test::Unit::TestCase
   end
 
   test '#client lazily initializes clients' do
-    version_fixtures('unstable') do |dir|
+    version_fixtures('unstable') do |_dir|
       ShopifyAPI::Base.api_version = 'unstable'
 
       assert_raises ShopifyAPI::GraphQL::InvalidClient do
@@ -133,7 +133,7 @@ class GraphQLTest < Test::Unit::TestCase
   end
 
   test '#client caches lookups' do
-    version_fixtures('unstable') do |dir|
+    version_fixtures('unstable') do |_dir|
       ShopifyAPI::Base.api_version = 'unstable'
 
       client1 = ShopifyAPI::GraphQL.client
@@ -148,7 +148,7 @@ class GraphQLTest < Test::Unit::TestCase
     end
 
     ShopifyAPI::GraphQL.execution_adapter = SuperDuperExecutionAdapter
-    version_fixtures('unstable') do |dir|
+    version_fixtures('unstable') do |_dir|
       ShopifyAPI::Base.api_version = 'unstable'
 
       ShopifyAPI::GraphQL.initialize_clients
@@ -163,7 +163,7 @@ class GraphQLTest < Test::Unit::TestCase
     end
 
     ShopifyAPI::GraphQL.graphql_client = SuperDuperClient
-    version_fixtures('unstable') do |dir|
+    version_fixtures('unstable') do |_dir|
       ShopifyAPI::Base.api_version = 'unstable'
 
       ShopifyAPI::GraphQL.initialize_clients

test/lib/webmock_extensions/last_request.rb

@@ -11,6 +11,6 @@ module LastRequest
 end
 
 WebMock.extend(LastRequest)
-WebMock.after_request do |request_signature, response|
+WebMock.after_request do |request_signature, _response|
   WebMock.last_request = request_signature
 end

test/metafield_test.rb

@@ -3,7 +3,7 @@ require 'test_helper'
 class MetafieldTest < Test::Unit::TestCase
   def test_get_metafields
     fake "metafields", :method => :get, :body => load_fixture('metafields')
-    metafields = ShopifyAPI::Metafield.find(:all)
+    ShopifyAPI::Metafield.find(:all)
   end
 
   def test_get_metafield
@@ -43,4 +43,3 @@ class MetafieldTest < Test::Unit::TestCase
     assert metafield.destroy
   end
 end
-

test/pagination_test.rb

@@ -227,7 +227,7 @@ class PaginationTest < Test::Unit::TestCase
       body: load_fixture('orders'),
       link: "<https://this-is-my-test-shop.myshopify.com/admin/api/2019-10/orders.json?#{first_request_params}>; rel=\"next\""
     )
-    orders = ShopifyAPI::Order.where(limit: 5)
+    ShopifyAPI::Order.where(limit: 5)
 
     second_request_params = "page_info=#{@next_page_info}&limit=5"
     fake(
@@ -239,8 +239,7 @@ class PaginationTest < Test::Unit::TestCase
       body: load_fixture('orders'),
       link: "<https://this-is-my-test-shop.myshopify.com/admin/api/2019-10/orders.json?#{second_request_params}>; rel=\"next\""
     )
-
-    orders2 = ShopifyAPI::Order.where(limit: 10)
+    orders = ShopifyAPI::Order.where(limit: 10)
 
     fake(
       'orders',

test/price_rule_test.rb

@@ -43,7 +43,7 @@ class PriceRuleTest < Test::Unit::TestCase
     )
 
     assert_equal '{"price_rule":{"target_type":"line_item","allocation_method":"across","value_type":"fixed_amount","value":-10.0,"customer_selection":"all","starts_at":"2017-01-19T00:00:00Z"}}', WebMock.last_request.body
-    assert_equal -10, price_rule.value
+    assert_equal(-10, price_rule.value)
   end
 
   def test_update_price_rule

test/session_test.rb

@@ -519,7 +519,7 @@ class SessionTest < Test::Unit::TestCase
 
   def generate_signature(params)
     params = make_sorted_params(params) if params.is_a?(Hash)
-    OpenSSL::HMAC.hexdigest(OpenSSL::Digest::SHA256.new, ShopifyAPI::Session.secret, params)
+    OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('SHA256'), ShopifyAPI::Session.secret, params)
   end
 
   def any_api_version

test/tax_service_test.rb

@@ -2,7 +2,7 @@ require 'test_helper'
 class TaxServiceTest < Test::Unit::TestCase
   test "tax service creation" do
     fake "tax_services", :method => :post, :status => 202, :body => load_fixture('tax_service')
-    tax_service = ShopifyAPI::TaxService.create(:name => "My Tax Service", :url => "https://mytaxservice.com")
+    ShopifyAPI::TaxService.create(:name => "My Tax Service", :url => "https://mytaxservice.com")
     assert_equal '{"tax_service":{"name":"My Tax Service","url":"https://mytaxservice.com"}}', WebMock.last_request.body
 
   end

test/test_helper.rb

@@ -23,7 +23,7 @@ module Test
         self.test("should_#{string}", &block)
       end
 
-      def self.context(string)
+      def self.context(_string)
         yield
       end
 
@@ -34,6 +34,7 @@ module Test
             const = mod.const_get(const)
             const.format = :json if const.respond_to?(:format=)
           rescue NameError
+            # Do nothing
           end
         end