Merge pull request #5289 from nebulab/kennyadsl/deprecation-updates

Deprecate `Spree::Deprecation` in favor of `Spree.deprecator`

Author: elia

.rubocop.yml

@@ -337,7 +337,7 @@ Lint/SuppressedException:
 
 Lint/MissingSuper:
   Exclude:
-    - 'core/lib/spree/deprecation.rb' # this is a known class that doesn't require super
+    - 'core/lib/spree/deprecated_instance_variable_proxy.rb' # this is a known class that doesn't require super
     - 'core/lib/spree/preferences/configuration.rb' # this class has no superclass defining `self.inherited`
 
 Rails/FindEach:

backend/app/controllers/spree/admin/stock_items_controller.rb

@@ -45,7 +45,7 @@ def load_stock_management_data
           view_context,
           :@stock_locations,
           :stock_item_stock_locations,
-          Spree::Deprecation,
+          Spree.deprecator,
           "Please, do not use @stock_item_stock_locations anymore in the views, use @stock_locations",
         )
         @variant_display_attributes = self.class.variant_display_attributes

backend/app/helpers/spree/admin/navigation_helper.rb

@@ -50,12 +50,12 @@ def tab(*args, &block)
         css_classes = Array(options[:css_class])
 
         if options.key?(:route)
-          Spree::Deprecation.warn "Passing a route to #tab is deprecated. Please pass a url instead."
+          Spree.deprecator.warn "Passing a route to #tab is deprecated. Please pass a url instead."
           options[:url] ||= spree.send("#{options[:route]}_path")
         end
 
         if args.any?
-          Spree::Deprecation.warn "Passing resources to #tab is deprecated. Please use the `label:` and `match_path:` options instead."
+          Spree.deprecator.warn "Passing resources to #tab is deprecated. Please use the `label:` and `match_path:` options instead."
           options[:label] ||= args.first
           options[:url] ||= spree.send("admin_#{args.first}_path")
           options[:selected] = args.include?(controller.controller_name.to_sym)

backend/app/views/spree/admin/shared/_product_sub_menu.html.erb

@@ -1,4 +1,4 @@
-<% Spree::Deprecation.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>
+<% Spree.deprecator.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>
 
 <ul class="admin-subnav" data-hook="admin_product_sub_tabs">
   <% if can? :admin, Spree::Product %>

backend/app/views/spree/admin/shared/_promotion_sub_menu.html.erb

@@ -1,4 +1,4 @@
-<% Spree::Deprecation.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>
+<% Spree.deprecator.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>
 
 <ul class="admin-subnav" data-hook="admin_promotion_sub_tabs">
   <% if can? :admin, Spree::Promotion %>

backend/app/views/spree/admin/shared/_settings_sub_menu.html.erb

@@ -1,4 +1,4 @@
-<% Spree::Deprecation.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>
+<% Spree.deprecator.warn "Using the #{@virtual_path.inspect} partial is deprecated, please use MenuItem#children instead." %>
 
 <ul class="admin-subnav" data-hook="admin_settings_sub_tabs">
   <% if can?(:admin, Spree::Store) %>

backend/lib/spree/backend_configuration/deprecated_tab_constants.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-Spree::Deprecation.warn(
+Spree.deprecator.warn(
   "Spree::BackendConfiguration::*_TABS is deprecated. Please use Spree::BackendConfiguration::MenuItem(match_path:) instead."
 )
 

backend/lib/spree/backend_configuration/menu_item.rb

@@ -9,11 +9,11 @@ class MenuItem
       def sections # rubocop:disable Style/TrivialAccessors
         @sections
       end
-      deprecate sections: :label, deprecator: Spree::Deprecation
+      deprecate sections: :label, deprecator: Spree.deprecator
 
       attr_accessor :position # rubocop:disable Layout/EmptyLinesAroundAttributeAccessor
-      deprecate position: nil, deprecator: Spree::Deprecation
-      deprecate "position=": nil, deprecator: Spree::Deprecation
+      deprecate position: nil, deprecator: Spree.deprecator
+      deprecate "position=": nil, deprecator: Spree.deprecator
 
       # @param icon [String] The icon to draw for this menu item
       # @param condition [Proc] A proc which returns true if this menu item
@@ -41,16 +41,16 @@ def initialize(
         if args.length == 2
           sections, icon = args
           label ||= sections.first.to_s
-          Spree::Deprecation.warn "Passing sections to #{self.class.name} is deprecated. Please pass a label instead."
-          Spree::Deprecation.warn "Passing icon to #{self.class.name} is deprecated. Please use the keyword argument instead."
+          Spree.deprecator.warn "Passing sections to #{self.class.name} is deprecated. Please pass a label instead."
+          Spree.deprecator.warn "Passing icon to #{self.class.name} is deprecated. Please use the keyword argument instead."
         elsif args.any?
           raise ArgumentError, "wrong number of arguments (given #{args.length}, expected 0..2)"
         end
 
         if partial.present? && children.blank?
           # We only show the deprecation if there are no children, because if there are children,
           # then the menu item is already future-proofed.
-          Spree::Deprecation.warn "Passing a partial to #{self.class.name} is deprecated. Please use the children keyword argument instead."
+          Spree.deprecator.warn "Passing a partial to #{self.class.name} is deprecated. Please use the children keyword argument instead."
         end
 
         @condition = condition || -> { true }

backend/spec/helpers/admin/navigation_helper_spec.rb

@@ -28,7 +28,7 @@
         without_partial_double_verification do
           allow(helper).to receive(:admin_orders_path).and_return("/admin/orders")
         end
-        expect(Spree::Deprecation).to receive(:warn)
+        expect(Spree.deprecator).to receive(:warn)
           .with("Passing a route to #tab is deprecated. Please pass a url instead.")
         expect(helper.tab(label: :orders, route: :admin_orders)).to include('href="/admin/orders"')
       end

backend/spec/lib/spree/backend_configuration/menu_item_spec.rb

@@ -5,7 +5,7 @@
 RSpec.describe Spree::BackendConfiguration::MenuItem do
   describe '#children' do
     it 'is the replacement for the deprecated #partial method' do
-      expect(Spree::Deprecation).to receive(:warn).with(a_string_matching(/partial/))
+      expect(Spree.deprecator).to receive(:warn).with(a_string_matching(/partial/))
 
       described_class.new(partial: 'foo')
     end
@@ -67,8 +67,8 @@
   describe "deprecated behavior" do
     describe "passing `sections` and `icon` sequentially" do
       it "warns about the deprecation" do
-        expect(Spree::Deprecation).to receive(:warn).with(a_string_matching(/sections/))
-        expect(Spree::Deprecation).to receive(:warn).with(a_string_matching(/icon/))
+        expect(Spree.deprecator).to receive(:warn).with(a_string_matching(/sections/))
+        expect(Spree.deprecator).to receive(:warn).with(a_string_matching(/icon/))
 
         described_class.new([:foo, :bar], 'icon')
       end

backend/spec/lib/spree/backend_configuration_spec.rb

@@ -118,7 +118,7 @@ def product_path(product)
   describe "deprecated behavior" do
     describe "loading *_TABS constants" do
       it "warns about the deprecation" do
-        expect(Spree::Deprecation).to receive(:warn).with(a_string_matching(/Spree::BackendConfiguration::\*_TABS is deprecated\./))
+        expect(Spree.deprecator).to receive(:warn).with(a_string_matching(/Spree::BackendConfiguration::\*_TABS is deprecated\./))
 
         described_class::ORDER_TABS
       end

core/app/models/spree/adjustment.rb

@@ -48,7 +48,7 @@ class Adjustment < Spree::Base
     scope :is_included, -> { where(included: true) }
     scope :additional, -> { where(included: false) }
 
-    singleton_class.deprecate :return_authorization, deprecator: Spree::Deprecation
+    singleton_class.deprecate :return_authorization, deprecator: Spree.deprecator
 
     extend DisplayMoney
     money_methods :amount

core/app/models/spree/order.rb

@@ -264,7 +264,7 @@ def recalculator
       @recalculator ||= Spree::Config.order_recalculator_class.new(self)
     end
     alias_method :updater, :recalculator
-    deprecate updater: :recalculator, deprecator: Spree::Deprecation
+    deprecate updater: :recalculator, deprecator: Spree.deprecator
 
     def assign_billing_to_shipping_address
       self.ship_address = bill_address if bill_address
@@ -513,7 +513,7 @@ def check_shipments_and_restart_checkout
     end
 
     alias_method :ensure_updated_shipments, :check_shipments_and_restart_checkout
-    deprecate ensure_updated_shipments: :check_shipments_and_restart_checkout, deprecator: Spree::Deprecation
+    deprecate ensure_updated_shipments: :check_shipments_and_restart_checkout, deprecator: Spree.deprecator
 
     def restart_checkout_flow
       return if state == 'cart'

core/app/models/spree/order_updater.rb

@@ -31,7 +31,7 @@ def recalculate
       end
     end
     alias_method :update, :recalculate
-    deprecate update: :recalculate, deprecator: Spree::Deprecation
+    deprecate update: :recalculate, deprecator: Spree.deprecator
 
     # Updates the +shipment_state+ attribute according to the following logic:
     #

core/lib/generators/spree/dummy/templates/rails/test.rb

@@ -33,6 +33,6 @@
 
   # Raise on deprecation warnings
   if ENV['SOLIDUS_RAISE_DEPRECATIONS'].present?
-    Spree::Deprecation.behavior = :raise
+    Spree.deprecator.behavior = :raise
   end
 end

core/lib/spree/core.rb

@@ -9,6 +9,8 @@
 require "active_storage/engine"
 require "sprockets/railtie"
 
+require 'active_support/deprecation'
+require 'spree/deprecated_instance_variable_proxy'
 require 'acts_as_list'
 require 'awesome_nested_set'
 require 'cancan'
@@ -19,13 +21,17 @@
 require 'ransack'
 require 'state_machines-activerecord'
 
-require 'spree/deprecation'
-
 # This is required because ActiveModel::Validations#invalid? conflicts with the
 # invalid state of a Payment. In the future this should be removed.
 StateMachines::Machine.ignore_method_conflicts = true
 
 module Spree
+  def self.deprecator
+    @deprecator ||= ActiveSupport::Deprecation.new('5.0', 'Solidus')
+  end
+
+  autoload :Deprecation, 'spree/deprecation'
+
   mattr_accessor :user_class, default: 'Spree::LegacyUser'
 
   def self.user_class

core/lib/spree/core/engine.rb

@@ -75,6 +75,12 @@ class Engine < ::Rails::Engine
         ActionMailer::Base.preview_path = app.config.action_mailer.preview_path
       end
 
+      initializer "spree.deprecator" do |app|
+        if app.respond_to?(:deprecators)
+          app.deprecators[:spree] = Spree.deprecator
+        end
+      end
+
       config.after_initialize do
         Spree::Config.check_load_defaults_called('Spree::Config')
         Spree::Config.static_model_preferences.validate!

core/lib/spree/deprecated_instance_variable_proxy.rb

@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'active_support/deprecation'
+
+module Spree
+  # This DeprecatedInstanceVariableProxy transforms instance variable to
+  # deprecated instance variable.
+  #
+  # It differs from ActiveSupport::DeprecatedInstanceVariableProxy since
+  # it allows to define a custom message.
+  #
+  #   class Example
+  #     def initialize(deprecator)
+  #       @request = Spree::DeprecatedInstanceVariableProxy.new(self, :request, :@request, deprecator, "Please, do not use this thing.")
+  #       @_request = :a_request
+  #     end
+  #
+  #     def request
+  #       @_request
+  #     end
+  #
+  #     def old_request
+  #       @request
+  #     end
+  #   end
+  #
+  # When someone execute any method on @request variable this will trigger
+  # +warn+ method on +deprecator_instance+ and will fetch <tt>@_request</tt>
+  # variable via +request+ method and execute the same method on non-proxy
+  # instance variable.
+  #
+  # Default deprecator is <tt>Spree.deprecator</tt>.
+  class DeprecatedInstanceVariableProxy < ActiveSupport::Deprecation::DeprecationProxy
+    def initialize(instance, method_or_var, var = "@#{method}", deprecator = Spree.deprecator, message = nil)
+      @instance = instance
+      @method_or_var = method_or_var
+      @var = var
+      @deprecator = deprecator
+      @message = message
+    end
+
+    private
+
+    def target
+      return @instance.instance_variable_get(@method_or_var) if @instance.instance_variable_defined?(@method_or_var)
+
+      @instance.__send__(@method_or_var)
+    end
+
+    def warn(callstack, called, args)
+      message = @message || "#{@var} is deprecated! Call #{@method_or_var}.#{called} instead of #{@var}.#{called}."
+      message = [message, "Args: #{args.inspect}"].join(" ") unless args.empty?
+
+      @deprecator.warn(message, callstack)
+    end
+  end
+end

core/lib/spree/deprecation.rb

@@ -1,59 +1,9 @@
 # frozen_string_literal: true
 
-require 'active_support/deprecation'
+require 'spree/core'
 
 module Spree
-  Deprecation = ActiveSupport::Deprecation.new('5.0', 'Solidus')
+  Deprecation = Spree.deprecator
 
-  # This DeprecatedInstanceVariableProxy transforms instance variable to
-  # deprecated instance variable.
-  #
-  # It differs from ActiveSupport::DeprecatedInstanceVariableProxy since
-  # it allows to define a custom message.
-  #
-  #   class Example
-  #     def initialize(deprecator)
-  #       @request = Spree::DeprecatedInstanceVariableProxy.new(self, :request, :@request, deprecator, "Please, do not use this thing.")
-  #       @_request = :a_request
-  #     end
-  #
-  #     def request
-  #       @_request
-  #     end
-  #
-  #     def old_request
-  #       @request
-  #     end
-  #   end
-  #
-  # When someone execute any method on @request variable this will trigger
-  # +warn+ method on +deprecator_instance+ and will fetch <tt>@_request</tt>
-  # variable via +request+ method and execute the same method on non-proxy
-  # instance variable.
-  #
-  # Default deprecator is <tt>Spree::Deprecation</tt>.
-  class DeprecatedInstanceVariableProxy < ActiveSupport::Deprecation::DeprecationProxy
-    def initialize(instance, method_or_var, var = "@#{method}", deprecator = Spree::Deprecation, message = nil)
-      @instance = instance
-      @method_or_var = method_or_var
-      @var = var
-      @deprecator = deprecator
-      @message = message
-    end
-
-    private
-
-    def target
-      return @instance.instance_variable_get(@method_or_var) if @instance.instance_variable_defined?(@method_or_var)
-
-      @instance.__send__(@method_or_var)
-    end
-
-    def warn(callstack, called, args)
-      message = @message || "#{@var} is deprecated! Call #{@method_or_var}.#{called} instead of #{@var}.#{called}."
-      message = [message, "Args: #{args.inspect}"].join(" ") unless args.empty?
-
-      @deprecator.warn(message, callstack)
-    end
-  end
+  Spree.deprecator.warn "Spree::Deprecation is deprecated. Please use Spree.deprecator instead.", caller(2)
 end

core/lib/spree/preferences/configuration.rb

@@ -59,7 +59,7 @@ def check_load_defaults_called(instance_constant_name = nil)
       return if load_defaults_called || !Spree::Core.has_install_generator_been_run?
 
       target_name = instance_constant_name || "#{self.class.name}.new"
-      Spree::Deprecation.warn <<~MSG
+      Spree.deprecator.warn <<~MSG
         It's recommended that you explicitly load the default configuration for
         your current Solidus version. You can do it by adding the following call
         to your Solidus initializer within the #{target_name} block:

core/lib/spree/preferences/preferable_class_methods.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-require 'spree/deprecation'
 require 'spree/encryptor'
 
 module Spree::Preferences

core/lib/spree/testing_support/dummy_app.rb

@@ -142,5 +142,5 @@ class Application < ::Rails::Application
 
 # Raise on deprecation warnings
 if ENV['SOLIDUS_RAISE_DEPRECATIONS'].present?
-  Spree::Deprecation.behavior = :raise
+  Spree.deprecator.behavior = :raise
 end

core/lib/spree/testing_support/factory_bot.rb

@@ -30,7 +30,7 @@ def self.check_version
           MSG
         end
       end
-      deprecate :check_version, deprecator: Spree::Deprecation
+      deprecate :check_version, deprecator: Spree.deprecator
 
       def self.add_definitions!
         ::FactoryBot.definition_file_paths.unshift(*definition_file_paths).uniq!

core/lib/spree/testing_support/silence_deprecations.rb

@@ -2,7 +2,7 @@
 
 RSpec.configure do |config|
   config.around(:each, silence_deprecations: true) do |example|
-    Spree::Deprecation.silence do
+    Spree.deprecator.silence do
       example.run
     end
   end

core/spec/lib/spree/migration_helpers_spec.rb

@@ -17,7 +17,7 @@
     context "when the column exists" do
       context "and the index does" do
         it "passes compatible arguments to index_exists?" do
-          expect { subject }.to_not raise_error(ArgumentError)
+          expect { subject }.to raise_error(NotImplementedError) # not ArgumentError
         end
       end
 
@@ -27,7 +27,7 @@
         end
 
         it "passes compatible arguments to add_index" do
-          expect { subject }.to_not raise_error(ArgumentError)
+          expect { subject }.to raise_error(TypeError) # not ArgumentError
         end
       end
     end