class_attribute is not a direct replacement of class_inheritable_*.

If you are setting a hash or an array in class_attribute or you need
to freeze it, to ensure people won't modify it in place or you need
to dup it on inheritance.

5 files changed, 29 insertions, 21 deletions

diff --git a/actionpack/lib/abstract_controller/view_paths.rb b/actionpack/lib/abstract_controller/view_paths.rb
index b331eb51b6..b552a649d1 100644
--- a/actionpack/lib/abstract_controller/view_paths.rb
+++ b/actionpack/lib/abstract_controller/view_paths.rb
@@ -5,6 +5,7 @@ module AbstractController
     included do
       class_attribute :_view_paths
       self._view_paths = ActionView::PathSet.new
+      self._view_paths.freeze
     end
 
     delegate :find_template, :template_exists?, :view_paths, :formats, :formats=,
@@ -61,7 +62,7 @@ module AbstractController
       # paths<ViewPathSet, Object>:: If a ViewPathSet is provided, use that;
       #   otherwise, process the parameter into a ViewPathSet.
       def view_paths=(paths)
-        self._view_paths = paths.is_a?(ActionView::PathSet) ? paths : ActionView::Base.process_view_paths(paths)
+        self._view_paths = ActionView::Base.process_view_paths(paths)
         self._view_paths.freeze
       end
     end
diff --git a/actionpack/lib/action_controller/metal/hide_actions.rb b/actionpack/lib/action_controller/metal/hide_actions.rb
index 3358d80c35..32d7a96701 100644
--- a/actionpack/lib/action_controller/metal/hide_actions.rb
+++ b/actionpack/lib/action_controller/metal/hide_actions.rb
@@ -8,7 +8,7 @@ module ActionController
 
     included do
       class_attribute :hidden_actions
-      self.hidden_actions = Set.new
+      self.hidden_actions = Set.new.freeze
     end
 
   private
@@ -25,7 +25,7 @@ module ActionController
       # ==== Parameters
       # *args<#to_s>:: A list of actions
       def hide_action(*args)
-        self.hidden_actions = hidden_actions.dup.merge(args.map(&:to_s))
+        self.hidden_actions = hidden_actions.dup.merge(args.map(&:to_s)).freeze
       end
 
       def inherited(klass)
@@ -41,7 +41,7 @@ module ActionController
       # Overrides AbstractController::Base#action_methods to remove any methods
       # that are listed as hidden methods.
       def action_methods
-        @action_methods ||= Set.new(super.reject {|name| hidden_actions.include?(name)})
+        @action_methods ||= Set.new(super.reject { |name| hidden_actions.include?(name) })
       end
     end
   end
diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb
index 7dd9dea358..2efc575081 100644
--- a/actionpack/lib/action_view/base.rb
+++ b/actionpack/lib/action_view/base.rb
@@ -158,7 +158,6 @@ module ActionView #:nodoc:
     end
 
     include Helpers, Rendering, Partials, Layouts, ::ERB::Util, Context
-    extend  ActiveSupport::Memoizable
 
     # Specify whether RJS responses should be wrapped in a try/catch block
     # that alert()s the caught exception (and then re-raises it).
@@ -170,9 +169,6 @@ module ActionView #:nodoc:
     @@field_error_proc = Proc.new{ |html_tag, instance| "<div class=\"field_with_errors\">#{html_tag}</div>".html_safe }
 
     class_attribute :helpers
-    remove_method :helpers
-    attr_reader :helpers
-
     class_attribute :_router
 
     class << self
@@ -201,20 +197,21 @@ module ActionView #:nodoc:
     end
 
     def self.process_view_paths(value)
-      return value.dup if value.is_a?(PathSet)
-      ActionView::PathSet.new(Array.wrap(value))
+      value.is_a?(PathSet) ?
+        value.dup : ActionView::PathSet.new(Array.wrap(value))
     end
 
     def initialize(lookup_context = nil, assigns_for_first_render = {}, controller = nil, formats = nil) #:nodoc:
-      @config  = nil
-      @assigns = assigns_for_first_render.each { |key, value| instance_variable_set("@#{key}", value) }
-      @helpers = self.class.helpers || Module.new
+      self.assigns = assigns_for_first_render.each { |key, value| instance_variable_set("@#{key}", value) }
+      self.helpers = self.class.helpers || Module.new
 
       if @_controller = controller
         @_request = controller.request if controller.respond_to?(:request)
       end
 
-      @_config       = ActiveSupport::InheritableOptions.new(controller.config) if controller && controller.respond_to?(:config)
+      config = controller && controller.respond_to?(:config) ? controller.config : {}
+      @_config = ActiveSupport::InheritableOptions.new(config)
+
       @_content_for  = Hash.new { |h,k| h[k] = ActiveSupport::SafeBuffer.new }
       @_virtual_path = nil
       @output_buffer = nil
diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb
index 7520e0331d..d7e3544849 100644
--- a/activemodel/lib/active_model/validations.rb
+++ b/activemodel/lib/active_model/validations.rb
@@ -50,11 +50,10 @@ module ActiveModel
       extend  HelperMethods
       include HelperMethods
 
-      define_callbacks :validate, :scope => :name
-
       attr_accessor :validation_context
+      define_callbacks :validate, :scope => :name
 
-      class_inheritable_accessor :_validators
+      class_attribute :_validators
       self._validators = Hash.new { |h,k| h[k] = [] }
     end
 
@@ -128,8 +127,7 @@ module ActiveModel
         set_callback(:validate, *args, &block)
       end
 
-      # List all validators that being used to validate the model using +validates_with+
-      # method.
+      # List all validators that being used to validate the model using +validates_with+ method.
       def validators
         _validators.values.flatten.uniq
       end
@@ -139,9 +137,17 @@ module ActiveModel
         _validators[attribute.to_sym]
       end
 
+      # Check if method is an attribute method or not.
       def attribute_method?(attribute)
         method_defined?(attribute)
       end
+
+      # Copy validators on inheritance.
+      def inherited(base)
+        dup = _validators.dup
+        base._validators = dup.each { |k, v| dup[k] = v.dup }
+        super
+      end
     end
 
     # Returns the Errors object that holds all information about attribute error messages.
diff --git a/activerecord/lib/active_record/observer.rb b/activerecord/lib/active_record/observer.rb
index ed0f039597..0ea7fe7365 100644
--- a/activerecord/lib/active_record/observer.rb
+++ b/activerecord/lib/active_record/observer.rb
@@ -88,7 +88,7 @@ module ActiveRecord
   #
   class Observer < ActiveModel::Observer
     class_attribute :observed_methods
-    self.observed_methods = []
+    self.observed_methods = [].freeze
 
     def initialize
       super
@@ -97,7 +97,11 @@ module ActiveRecord
 
     def self.method_added(method)
       method = method.to_sym
-      observed_methods << method if ActiveRecord::Callbacks::CALLBACKS.include?(method)
+
+      if ActiveRecord::Callbacks::CALLBACKS.include?(method)
+        self.observed_methods += [method]
+        self.observed_methods.freeze
+      end
     end
 
     protected