Merge branch 'master' into adequaterecord

* master:
  Fix eager load of Serializers on Active Model
  Consistence in the block style
  Remove warnings on Ruby 2.1
  Only some dynamic finders are deprecated.
  clean up security guide: his => their [ci skip]
  Remove warning
  Extract a method to simplify setup code
  Use minitest's skip rather than conditionals + early returns
  Move AR test classes inside the test case
  Do not set up a variable used only in two tests
  No need for instance vars on single tests
  Cleanup duplicated setup of callbacks in transactions tests
  No need to use #send with public methods
  Update deprecation warning to give more information about caller
  Update changelog
  Avoid scanning multiple render calls as a single match.
  Improve ERB dependency detection.

12 files changed, 360 insertions, 220 deletions

diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md
index a57da72d8c..19877ca8cb 100644
--- a/actionview/CHANGELOG.md
+++ b/actionview/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   Improved ERB dependency detection. New argument types and formattings for the `render`
+    calls can be matched.
+
+    Fixes #13074 and #13116
+
+    *João Britto*
+
 *   Use `display:none` instead of `display:inline` for hidden fields
 
     Fixes #6403
diff --git a/actionview/lib/action_view/dependency_tracker.rb b/actionview/lib/action_view/dependency_tracker.rb
index b2e8334077..0ccf2515c5 100644
--- a/actionview/lib/action_view/dependency_tracker.rb
+++ b/actionview/lib/action_view/dependency_tracker.rb
@@ -1,7 +1,7 @@
 require 'thread_safe'
 
 module ActionView
-  class DependencyTracker
+  class DependencyTracker # :nodoc:
     @trackers = ThreadSafe::Cache.new
 
     def self.find_dependencies(name, template)
@@ -23,24 +23,52 @@ module ActionView
       @trackers.delete(handler)
     end
 
-    class ERBTracker
+    class ERBTracker # :nodoc:
       EXPLICIT_DEPENDENCY = /# Template Dependency: (\S+)/
 
+      # A valid ruby identifier - suitable for class, method and specially variable names
+      IDENTIFIER = /
+        [[:alpha:]_] # at least one uppercase letter, lowercase letter or underscore
+        [[:word:]]*  # followed by optional letters, numbers or underscores
+      /x
+
+      # Any kind of variable name. e.g. @instance, @@class, $global or local.
+      # Possibly following a method call chain
+      VARIABLE_OR_METHOD_CHAIN = /
+        (?:\$|@{1,2})?            # optional global, instance or class variable indicator
+        (?:#{IDENTIFIER}\.)*      # followed by an optional chain of zero-argument method calls
+        (?<dynamic>#{IDENTIFIER}) # and a final valid identifier, captured as DYNAMIC
+      /x
+
+      # A simple string literal. e.g. "School's out!"
+      STRING = /
+        (?<quote>['"]) # an opening quote
+        (?<static>.*?) # with anything inside, captured as STATIC
+        \k<quote>      # and a matching closing quote
+      /x
+
+      # Part of any hash containing the :partial key
+      PARTIAL_HASH_KEY = /
+        (?:\bpartial:|:partial\s*=>) # partial key in either old or new style hash syntax
+        \s*                          # followed by optional spaces
+      /x
+
       # Matches:
-      #   render partial: "comments/comment", collection: commentable.comments
-      #   render "comments/comments"
-      #   render 'comments/comments'
-      #   render('comments/comments')
+      #   partial: "comments/comment", collection: @all_comments => "comments/comment"
+      #   (object: @single_comment, partial: "comments/comment") => "comments/comment"
       #
-      #   render(@topic)         => render("topics/topic")
-      #   render(topics)         => render("topics/topic")
-      #   render(message.topics) => render("topics/topic")
-      RENDER_DEPENDENCY = /
-        render\s*                     # render, followed by optional whitespace
-        \(?                           # start an optional parenthesis for the render call
-        (partial:|:partial\s+=>)?\s*  # naming the partial, used with collection -- 1st capture
-        ([@a-z"'][@\w\/\."']+)        # the template name itself -- 2nd capture
-      /x
+      #   "comments/comments"
+      #   'comments/comments'
+      #   ('comments/comments')
+      #
+      #   (@topic)         => "topics/topic"
+      #    topics          => "topics/topic"
+      #   (message.topics) => "topics/topic"
+      RENDER_ARGUMENTS = /\A
+        (?:\s*\(?\s*)                             # optional opening paren surrounded by spaces
+        (?:.*?#{PARTIAL_HASH_KEY})?               # optional hash, up to the partial key declaration
+        (?:#{STRING}|#{VARIABLE_OR_METHOD_CHAIN}) # finally, the dependency name of interest
+      /xm
 
       def self.call(name, template)
         new(name, template).dependencies
@@ -68,19 +96,33 @@ module ActionView
         end
 
         def render_dependencies
-          source.scan(RENDER_DEPENDENCY).
-            collect(&:second).uniq.
+          render_dependencies = []
+          render_calls = source.split(/\brender\b/).drop(1)
 
-            # render(@topic)         => render("topics/topic")
-            # render(topics)         => render("topics/topic")
-            # render(message.topics) => render("topics/topic")
-            collect { |name| name.sub(/\A@?([a-z_]+\.)*([a-z_]+)\z/) { "#{$2.pluralize}/#{$2.singularize}" } }.
+          render_calls.each do |arguments|
+            arguments.scan(RENDER_ARGUMENTS) do
+              add_dynamic_dependency(render_dependencies, Regexp.last_match[:dynamic])
+              add_static_dependency(render_dependencies, Regexp.last_match[:static])
+            end
+          end
 
-            # render("headline") => render("message/headline")
-            collect { |name| name.include?("/") ? name : "#{directory}/#{name}" }.
+          render_dependencies.uniq
+        end
+
+        def add_dynamic_dependency(dependencies, dependency)
+          if dependency
+            dependencies << "#{dependency.pluralize}/#{dependency.singularize}"
+          end
+        end
 
-            # replace quotes from string renders
-            collect { |name| name.gsub(/["']/, "") }
+        def add_static_dependency(dependencies, dependency)
+          if dependency
+            if dependency.include?('/')
+              dependencies << dependency
+            else
+              dependencies << "#{directory}/#{dependency}"
+            end
+          end
         end
 
         def explicit_dependencies
diff --git a/actionview/test/template/dependency_tracker_test.rb b/actionview/test/template/dependency_tracker_test.rb
index 7a9b4b26ac..df3a0602d1 100644
--- a/actionview/test/template/dependency_tracker_test.rb
+++ b/actionview/test/template/dependency_tracker_test.rb
@@ -1,3 +1,5 @@
+# encoding: utf-8
+
 require 'abstract_unit'
 require 'action_view/dependency_tracker'
 
@@ -52,23 +54,127 @@ class ERBTrackerTest < Minitest::Test
 
   def test_dependency_of_erb_template_with_number_in_filename
     template = FakeTemplate.new("<%# render 'messages/message123' %>", :erb)
-    tracker = make_tracker('messages/_message123', template)
+    tracker = make_tracker("messages/_message123", template)
 
     assert_equal ["messages/message123"], tracker.dependencies
   end
 
   def test_finds_dependency_in_correct_directory
     template = FakeTemplate.new("<%# render(message.topic) %>", :erb)
-    tracker = make_tracker('messages/_message', template)
+    tracker = make_tracker("messages/_message", template)
 
     assert_equal ["topics/topic"], tracker.dependencies
   end
 
   def test_finds_dependency_in_correct_directory_with_underscore
     template = FakeTemplate.new("<%# render(message_type.messages) %>", :erb)
-    tracker = make_tracker('message_types/_message_type', template)
+    tracker = make_tracker("message_types/_message_type", template)
 
     assert_equal ["messages/message"], tracker.dependencies
   end
-end
 
+  def test_dependency_of_erb_template_with_no_spaces_after_render
+    template = FakeTemplate.new("<%# render'messages/message' %>", :erb)
+    tracker = make_tracker("messages/_message", template)
+
+    assert_equal ["messages/message"], tracker.dependencies
+  end
+
+  def test_finds_no_dependency_when_render_begins_the_name_of_an_identifier
+    template = FakeTemplate.new("<%# rendering 'it useless' %>", :erb)
+    tracker = make_tracker("resources/_resource", template)
+
+    assert_equal [], tracker.dependencies
+  end
+
+  def test_finds_no_dependency_when_render_ends_the_name_of_another_method
+    template = FakeTemplate.new("<%# surrender 'to reason' %>", :erb)
+    tracker = make_tracker("resources/_resource", template)
+
+    assert_equal [], tracker.dependencies
+  end
+
+  def test_finds_dependency_on_multiline_render_calls
+    template = FakeTemplate.new("<%#
+      render :object => @all_posts,
+             :partial => 'posts' %>", :erb)
+
+    tracker = make_tracker("some/_little_posts", template)
+
+    assert_equal ["some/posts"], tracker.dependencies
+  end
+
+  def test_finds_multiple_unrelated_odd_dependencies
+    template = FakeTemplate.new("
+      <%# render('shared/header', title: 'Title') %>
+      <h2>Section title</h2>
+      <%# render@section %>
+    ", :erb)
+
+    tracker = make_tracker("multiple/_dependencies", template)
+
+    assert_equal ["shared/header", "sections/section"], tracker.dependencies
+  end
+
+  def test_finds_dependencies_for_all_kinds_of_identifiers
+    template = FakeTemplate.new("
+      <%# render $globals %>
+      <%# render @instance_variables %>
+      <%# render @@class_variables %>
+    ", :erb)
+
+    tracker = make_tracker("identifiers/_all", template)
+
+    assert_equal [
+      "globals/global",
+      "instance_variables/instance_variable",
+      "class_variables/class_variable"
+    ], tracker.dependencies
+  end
+
+  def test_finds_dependencies_on_method_chains
+    template = FakeTemplate.new("<%# render @parent.child.grandchildren %>", :erb)
+    tracker = make_tracker("method/_chains", template)
+
+    assert_equal ["grandchildren/grandchild"], tracker.dependencies
+  end
+
+  def test_finds_dependencies_with_special_characters
+    template = FakeTemplate.new("<%# render @pokémon, partial: 'ピカチュウ' %>", :erb)
+    tracker = make_tracker("special/_characters", template)
+
+    assert_equal ["special/ピカチュウ"], tracker.dependencies
+  end
+
+  def test_finds_dependencies_with_quotes_within
+    template = FakeTemplate.new(%{
+      <%# render "single/quote's" %>
+      <%# render 'double/quote"s' %>
+    }, :erb)
+
+    tracker = make_tracker("quotes/_single_and_double", template)
+
+    assert_equal ["single/quote's", 'double/quote"s'], tracker.dependencies
+  end
+
+  def test_finds_dependencies_with_extra_spaces
+    template = FakeTemplate.new(%{
+      <%= render              "header" %>
+      <%= render    partial:  "form" %>
+      <%= render              @message %>
+      <%= render ( @message.events ) %>
+      <%= render    :collection => @message.comments,
+                    :partial =>    "comments/comment" %>
+    }, :erb)
+
+    tracker = make_tracker("spaces/_extra", template)
+
+    assert_equal [
+      "spaces/header",
+      "spaces/form",
+      "messages/message",
+      "events/event",
+      "comments/comment"
+    ], tracker.dependencies
+  end
+end
diff --git a/activemodel/lib/active_model.rb b/activemodel/lib/active_model.rb
index 469f137d03..3e49c34182 100644
--- a/activemodel/lib/active_model.rb
+++ b/activemodel/lib/active_model.rb
@@ -59,9 +59,9 @@ module ActiveModel
     end
   end
 
-  def eager_load!
+  def self.eager_load!
     super
-    ActiveModel::Serializer.eager_load!
+    ActiveModel::Serializers.eager_load!
   end
 end
 
diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index fee3f51d9e..297792aeec 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -462,7 +462,7 @@ module ActiveRecord
         @class_names.delete_if { |k,klass|
           unless klass.is_a? Class
             klass = klass.safe_constantize
-            ActiveSupport::Deprecation.warn("The ability to pass in strings as a class name will be removed in Rails 4.2, consider using the class itself instead.")
+            ActiveSupport::Deprecation.warn("The ability to pass in strings as a class name to `set_fixture_class` will be removed in Rails 4.2. Use the class itself instead.")
           end
           !insert_class(@class_names, k, klass)
         }
@@ -568,7 +568,7 @@ module ActiveRecord
       @model_class = nil
 
       if class_name.is_a?(String)
-        ActiveSupport::Deprecation.warn("The ability to pass in strings as a class name will be removed in Rails 4.2, consider using the class itself instead.")
+        ActiveSupport::Deprecation.warn("The ability to pass in strings as a class name to `FixtureSet.new` will be removed in Rails 4.2. Use the class itself instead.")
       end
 
       if class_name.is_a?(Class) # TODO: Should be an AR::Base type class, or any?
diff --git a/activerecord/test/cases/associations/eager_singularization_test.rb b/activerecord/test/cases/associations/eager_singularization_test.rb
index 634f6b63ba..669569a774 100644
--- a/activerecord/test/cases/associations/eager_singularization_test.rb
+++ b/activerecord/test/cases/associations/eager_singularization_test.rb
@@ -1,128 +1,133 @@
 require "cases/helper"
 
-class Virus < ActiveRecord::Base
-  belongs_to :octopus
-end
-class Octopus < ActiveRecord::Base
-  has_one :virus
-end
-class Pass < ActiveRecord::Base
-  belongs_to :bus
-end
-class Bus < ActiveRecord::Base
-  has_many :passes
-end
-class Mess < ActiveRecord::Base
-  has_and_belongs_to_many :crises
-end
-class Crisis < ActiveRecord::Base
-  has_and_belongs_to_many :messes
-  has_many :analyses, :dependent => :destroy
-  has_many :successes, :through => :analyses
-  has_many :dresses, :dependent => :destroy
-  has_many :compresses, :through => :dresses
-end
-class Analysis < ActiveRecord::Base
-  belongs_to :crisis
-  belongs_to :success
-end
-class Success < ActiveRecord::Base
-  has_many :analyses, :dependent => :destroy
-  has_many :crises, :through => :analyses
-end
-class Dress < ActiveRecord::Base
-  belongs_to :crisis
-  has_many :compresses
-end
-class Compress < ActiveRecord::Base
-  belongs_to :dress
-end
-
 
 class EagerSingularizationTest < ActiveRecord::TestCase
+  class Virus < ActiveRecord::Base
+    belongs_to :octopus
+  end
+
+  class Octopus < ActiveRecord::Base
+    has_one :virus
+  end
+
+  class Pass < ActiveRecord::Base
+    belongs_to :bus
+  end
+
+  class Bus < ActiveRecord::Base
+    has_many :passes
+  end
+
+  class Mess < ActiveRecord::Base
+    has_and_belongs_to_many :crises
+  end
+
+  class Crisis < ActiveRecord::Base
+    has_and_belongs_to_many :messes
+    has_many :analyses, :dependent => :destroy
+    has_many :successes, :through => :analyses
+    has_many :dresses, :dependent => :destroy
+    has_many :compresses, :through => :dresses
+  end
+
+  class Analysis < ActiveRecord::Base
+    belongs_to :crisis
+    belongs_to :success
+  end
+
+  class Success < ActiveRecord::Base
+    has_many :analyses, :dependent => :destroy
+    has_many :crises, :through => :analyses
+  end
+
+  class Dress < ActiveRecord::Base
+    belongs_to :crisis
+    has_many :compresses
+  end
+
+  class Compress < ActiveRecord::Base
+    belongs_to :dress
+  end
 
   def setup
-    if ActiveRecord::Base.connection.supports_migrations?
-      ActiveRecord::Base.connection.create_table :viri do |t|
-        t.column :octopus_id, :integer
-        t.column :species, :string
-      end
-      ActiveRecord::Base.connection.create_table :octopi do |t|
-        t.column :species, :string
-      end
-      ActiveRecord::Base.connection.create_table :passes do |t|
-        t.column :bus_id, :integer
-        t.column :rides, :integer
-      end
-      ActiveRecord::Base.connection.create_table :buses do |t|
-        t.column :name, :string
-      end
-      ActiveRecord::Base.connection.create_table :crises_messes, :id => false do |t|
-        t.column :crisis_id, :integer
-        t.column :mess_id, :integer
-      end
-      ActiveRecord::Base.connection.create_table :messes do |t|
-        t.column :name, :string
-      end
-      ActiveRecord::Base.connection.create_table :crises do |t|
-        t.column :name, :string
-      end
-      ActiveRecord::Base.connection.create_table :successes do |t|
-        t.column :name, :string
-      end
-      ActiveRecord::Base.connection.create_table :analyses do |t|
-        t.column :crisis_id, :integer
-        t.column :success_id, :integer
-      end
-      ActiveRecord::Base.connection.create_table :dresses do |t|
-        t.column :crisis_id, :integer
-      end
-      ActiveRecord::Base.connection.create_table :compresses do |t|
-        t.column :dress_id, :integer
-      end
-      @have_tables = true
-    else
-      @have_tables = false
+    skip 'Does not support migrations' unless connection.supports_migrations?
+
+    connection.create_table :viri do |t|
+      t.column :octopus_id, :integer
+      t.column :species, :string
+    end
+    connection.create_table :octopi do |t|
+      t.column :species, :string
+    end
+    connection.create_table :passes do |t|
+      t.column :bus_id, :integer
+      t.column :rides, :integer
+    end
+    connection.create_table :buses do |t|
+      t.column :name, :string
+    end
+    connection.create_table :crises_messes, :id => false do |t|
+      t.column :crisis_id, :integer
+      t.column :mess_id, :integer
+    end
+    connection.create_table :messes do |t|
+      t.column :name, :string
+    end
+    connection.create_table :crises do |t|
+      t.column :name, :string
+    end
+    connection.create_table :successes do |t|
+      t.column :name, :string
+    end
+    connection.create_table :analyses do |t|
+      t.column :crisis_id, :integer
+      t.column :success_id, :integer
+    end
+    connection.create_table :dresses do |t|
+      t.column :crisis_id, :integer
+    end
+    connection.create_table :compresses do |t|
+      t.column :dress_id, :integer
     end
   end
 
   def teardown
-    ActiveRecord::Base.connection.drop_table :viri
-    ActiveRecord::Base.connection.drop_table :octopi
-    ActiveRecord::Base.connection.drop_table :passes
-    ActiveRecord::Base.connection.drop_table :buses
-    ActiveRecord::Base.connection.drop_table :crises_messes
-    ActiveRecord::Base.connection.drop_table :messes
-    ActiveRecord::Base.connection.drop_table :crises
-    ActiveRecord::Base.connection.drop_table :successes
-    ActiveRecord::Base.connection.drop_table :analyses
-    ActiveRecord::Base.connection.drop_table :dresses
-    ActiveRecord::Base.connection.drop_table :compresses
+    connection.drop_table :viri
+    connection.drop_table :octopi
+    connection.drop_table :passes
+    connection.drop_table :buses
+    connection.drop_table :crises_messes
+    connection.drop_table :messes
+    connection.drop_table :crises
+    connection.drop_table :successes
+    connection.drop_table :analyses
+    connection.drop_table :dresses
+    connection.drop_table :compresses
+  end
+
+  def connection
+    ActiveRecord::Base.connection
   end
 
   def test_eager_no_extra_singularization_belongs_to
-    return unless @have_tables
     assert_nothing_raised do
       Virus.all.merge!(:includes => :octopus).to_a
     end
   end
 
   def test_eager_no_extra_singularization_has_one
-    return unless @have_tables
     assert_nothing_raised do
       Octopus.all.merge!(:includes => :virus).to_a
     end
   end
 
   def test_eager_no_extra_singularization_has_many
-    return unless @have_tables
     assert_nothing_raised do
       Bus.all.merge!(:includes => :passes).to_a
     end
   end
 
   def test_eager_no_extra_singularization_has_and_belongs_to_many
-    return unless @have_tables
     assert_nothing_raised do
       Crisis.all.merge!(:includes => :messes).to_a
       Mess.all.merge!(:includes => :crises).to_a
@@ -130,14 +135,12 @@ class EagerSingularizationTest < ActiveRecord::TestCase
   end
 
   def test_eager_no_extra_singularization_has_many_through_belongs_to
-    return unless @have_tables
     assert_nothing_raised do
       Crisis.all.merge!(:includes => :successes).to_a
     end
   end
 
   def test_eager_no_extra_singularization_has_many_through_has_many
-    return unless @have_tables
     assert_nothing_raised do
       Crisis.all.merge!(:includes => :compresses).to_a
     end
diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb
index fe0781b7ec..7e7d95841b 100644
--- a/activerecord/test/cases/transaction_callbacks_test.rb
+++ b/activerecord/test/cases/transaction_callbacks_test.rb
@@ -30,14 +30,14 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
 
     has_many :replies, class_name: "ReplyWithCallbacks", foreign_key: "parent_id"
 
-    after_commit{|record| record.send(:do_after_commit, nil)}
-    after_commit(:on => :create){|record| record.send(:do_after_commit, :create)}
-    after_commit(:on => :update){|record| record.send(:do_after_commit, :update)}
-    after_commit(:on => :destroy){|record| record.send(:do_after_commit, :destroy)}
-    after_rollback{|record| record.send(:do_after_rollback, nil)}
-    after_rollback(:on => :create){|record| record.send(:do_after_rollback, :create)}
-    after_rollback(:on => :update){|record| record.send(:do_after_rollback, :update)}
-    after_rollback(:on => :destroy){|record| record.send(:do_after_rollback, :destroy)}
+    after_commit { |record| record.do_after_commit(nil) }
+    after_commit(on: :create) { |record| record.do_after_commit(:create) }
+    after_commit(on: :update) { |record| record.do_after_commit(:update) }
+    after_commit(on: :destroy) { |record| record.do_after_commit(:destroy) }
+    after_rollback { |record| record.do_after_rollback(nil) }
+    after_rollback(on: :create) { |record| record.do_after_rollback(:create) }
+    after_rollback(on: :update) { |record| record.do_after_rollback(:update) }
+    after_rollback(on: :destroy) { |record| record.do_after_rollback(:destroy) }
 
     def history
       @history ||= []
@@ -67,7 +67,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def setup
-    @first, @second = TopicWithCallbacks.find(1, 3).sort_by { |t| t.id }
+    @first = TopicWithCallbacks.find(1)
   end
 
   def test_call_after_commit_after_transaction_commits
@@ -79,40 +79,25 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def test_only_call_after_commit_on_update_after_transaction_commits_for_existing_record
-    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    add_transaction_execution_blocks @first
 
     @first.save!
     assert_equal [:commit_on_update], @first.history
   end
 
   def test_only_call_after_commit_on_destroy_after_transaction_commits_for_destroyed_record
-    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    add_transaction_execution_blocks @first
 
     @first.destroy
     assert_equal [:commit_on_destroy], @first.history
   end
 
   def test_only_call_after_commit_on_create_after_transaction_commits_for_new_record
-    @new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today)
-    @new_record.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @new_record.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @new_record.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @new_record.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @new_record.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @new_record.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
-
-    @new_record.save!
-    assert_equal [:commit_on_create], @new_record.history
+    new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today)
+    add_transaction_execution_blocks new_record
+
+    new_record.save!
+    assert_equal [:commit_on_create], new_record.history
   end
 
   def test_only_call_after_commit_on_create_after_transaction_commits_for_new_record_if_create_succeeds_creating_through_association
@@ -123,12 +108,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def test_only_call_after_commit_on_update_after_transaction_commits_for_existing_record_on_touch
-    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    add_transaction_execution_blocks @first
 
     @first.touch
     assert_equal [:commit_on_update], @first.history
@@ -147,12 +127,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def test_only_call_after_rollback_on_update_after_transaction_rollsback_for_existing_record
-    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    add_transaction_execution_blocks @first
 
     Topic.transaction do
       @first.save!
@@ -163,12 +138,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def test_only_call_after_rollback_on_update_after_transaction_rollsback_for_existing_record_on_touch
-    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    add_transaction_execution_blocks @first
 
     Topic.transaction do
       @first.touch
@@ -179,12 +149,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def test_only_call_after_rollback_on_destroy_after_transaction_rollsback_for_destroyed_record
-    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @first.after_commit_block(:destroy){|r| r.history << :commit_on_update}
-    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    add_transaction_execution_blocks @first
 
     Topic.transaction do
       @first.destroy
@@ -195,20 +160,15 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def test_only_call_after_rollback_on_create_after_transaction_rollsback_for_new_record
-    @new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today)
-    @new_record.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @new_record.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @new_record.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @new_record.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @new_record.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @new_record.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today)
+    add_transaction_execution_blocks new_record
 
     Topic.transaction do
-      @new_record.save!
+      new_record.save!
       raise ActiveRecord::Rollback
     end
 
-    assert_equal [:rollback_on_create], @new_record.history
+    assert_equal [:rollback_on_create], new_record.history
   end
 
   def test_call_after_rollback_when_commit_fails
@@ -235,23 +195,24 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
     @first.after_rollback_block{|r| r.rollbacks(1)}
     @first.after_commit_block{|r| r.commits(1)}
 
-    def @second.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end
-    def @second.commits(i=0); @commits ||= 0; @commits += i if i; end
-    @second.after_rollback_block{|r| r.rollbacks(1)}
-    @second.after_commit_block{|r| r.commits(1)}
+    second = TopicWithCallbacks.find(3)
+    def second.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end
+    def second.commits(i=0); @commits ||= 0; @commits += i if i; end
+    second.after_rollback_block{|r| r.rollbacks(1)}
+    second.after_commit_block{|r| r.commits(1)}
 
     Topic.transaction do
       @first.save!
       Topic.transaction(:requires_new => true) do
-        @second.save!
+        second.save!
         raise ActiveRecord::Rollback
       end
     end
 
     assert_equal 1, @first.commits
     assert_equal 0, @first.rollbacks
-    assert_equal 0, @second.commits
-    assert_equal 1, @second.rollbacks
+    assert_equal 0, second.commits
+    assert_equal 1, second.rollbacks
   end
 
   def test_only_call_after_rollback_on_records_rolled_back_to_a_savepoint_when_release_savepoint_fails
@@ -282,32 +243,34 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
     def @first.last_after_transaction_error; @last_transaction_error; end
     @first.after_commit_block{|r| r.last_after_transaction_error = :commit; raise "fail!";}
     @first.after_rollback_block{|r| r.last_after_transaction_error = :rollback; raise "fail!";}
-    @second.after_commit_block{|r| r.history << :after_commit}
-    @second.after_rollback_block{|r| r.history << :after_rollback}
+
+    second = TopicWithCallbacks.find(3)
+    second.after_commit_block{|r| r.history << :after_commit}
+    second.after_rollback_block{|r| r.history << :after_rollback}
 
     Topic.transaction do
       @first.save!
-      @second.save!
+      second.save!
     end
     assert_equal :commit, @first.last_after_transaction_error
-    assert_equal [:after_commit], @second.history
+    assert_equal [:after_commit], second.history
 
-    @second.history.clear
+    second.history.clear
     Topic.transaction do
       @first.save!
-      @second.save!
+      second.save!
       raise ActiveRecord::Rollback
     end
     assert_equal :rollback, @first.last_after_transaction_error
-    assert_equal [:after_rollback], @second.history
+    assert_equal [:after_rollback], second.history
   end
 
   def test_after_rollback_callbacks_should_validate_on_condition
-    assert_raise(ArgumentError) { Topic.send(:after_rollback, :on => :save) }
+    assert_raise(ArgumentError) { Topic.after_rollback(on: :save) }
   end
 
   def test_after_commit_callbacks_should_validate_on_condition
-    assert_raise(ArgumentError) { Topic.send(:after_commit, :on => :save) }
+    assert_raise(ArgumentError) { Topic.after_commit(on: :save) }
   end
 
   def test_saving_a_record_with_a_belongs_to_that_specifies_touching_the_parent_should_call_callbacks_on_the_parent_object
@@ -324,6 +287,17 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
 
     assert flag
   end
+
+  private
+
+    def add_transaction_execution_blocks(record)
+      record.after_commit_block(:create) { |r| r.history << :commit_on_create }
+      record.after_commit_block(:update) { |r| r.history << :commit_on_update }
+      record.after_commit_block(:destroy) { |r| r.history << :commit_on_destroy }
+      record.after_rollback_block(:create) { |r| r.history << :rollback_on_create }
+      record.after_rollback_block(:update) { |r| r.history << :rollback_on_update }
+      record.after_rollback_block(:destroy) { |r| r.history << :rollback_on_destroy }
+    end
 end
 
 class CallbacksOnMultipleActionsTest < ActiveRecord::TestCase
diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb
index f690eab604..594a4ca938 100644
--- a/activesupport/lib/active_support/hash_with_indifferent_access.rb
+++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb
@@ -159,7 +159,7 @@ module ActiveSupport
     #
     #   counters.fetch('foo')          # => 1
     #   counters.fetch(:bar, 0)        # => 0
-    #   counters.fetch(:bar) {|key| 0} # => 0
+    #   counters.fetch(:bar) { |key| 0 } # => 0
     #   counters.fetch(:zoo)           # => KeyError: key not found: "zoo"
     def fetch(key, *extras)
       super(convert_key(key), *extras)
@@ -172,7 +172,7 @@ module ActiveSupport
     #   hash[:b] = 'y'
     #   hash.values_at('a', 'b') # => ["x", "y"]
     def values_at(*indices)
-      indices.collect {|key| self[convert_key(key)]}
+      indices.collect { |key| self[convert_key(key)] }
     end
 
     # Returns an exact copy of the hash.
@@ -228,7 +228,11 @@ module ActiveSupport
     def to_options!; self end
 
     def select(*args, &block)
-      dup.tap {|hash| hash.select!(*args, &block)}
+      dup.tap { |hash| hash.select!(*args, &block) }
+    end
+
+    def reject(*args, &block)
+      dup.tap { |hash| hash.reject!(*args, &block) }
     end
 
     # Convert to a regular hash with string keys.
diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb
index 1a3693f766..58a2ce2105 100644
--- a/activesupport/lib/active_support/ordered_hash.rb
+++ b/activesupport/lib/active_support/ordered_hash.rb
@@ -28,6 +28,10 @@ module ActiveSupport
       coder.represent_seq '!omap', map { |k,v| { k => v } }
     end
 
+    def reject(*args, &block)
+      dup.tap { |hash| hash.reject!(*args, &block) }
+    end
+
     def nested_under_indifferent_access
       self
     end
diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb
index 4680fe5dfe..ff6e21854e 100644
--- a/activesupport/test/core_ext/module_test.rb
+++ b/activesupport/test/core_ext/module_test.rb
@@ -246,12 +246,12 @@ class ModuleTest < ActiveSupport::TestCase
   end
 
   def test_delegation_line_number
-    file, line = Someone.instance_method(:foo).source_location
+    _, line = Someone.instance_method(:foo).source_location
     assert_equal Someone::FAILED_DELEGATE_LINE, line
   end
 
   def test_delegate_line_with_nil
-    file, line = Someone.instance_method(:bar).source_location
+    _, line = Someone.instance_method(:bar).source_location
     assert_equal Someone::FAILED_DELEGATE_LINE_2, line
   end
 
diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md
index 4725e2c8a2..3783be50c0 100644
--- a/guides/source/active_record_querying.md
+++ b/guides/source/active_record_querying.md
@@ -1338,11 +1338,6 @@ Client.unscoped {
 Dynamic Finders
 ---------------
 
-NOTE: Dynamic finders have been deprecated in Rails 4.0 and will be
-removed in Rails 4.1. The best practice is to use Active Record scopes
-instead. You can find the deprecation gem at
-https://github.com/rails/activerecord-deprecated_finders
-
 For every field (also known as an attribute) you define in your table, Active Record provides a finder method. If you have a field called `first_name` on your `Client` model for example, you get `find_by_first_name` for free from Active Record. If you have a `locked` field on the `Client` model, you also get `find_by_locked` and methods.
 
 You can specify an exclamation point (`!`) on the end of the dynamic finders to get them to raise an `ActiveRecord::RecordNotFound` error if they do not return any records, like `Client.find_by_name!("Ryan")`
@@ -1352,6 +1347,11 @@ If you want to find both by name and locked, you can chain these finders togethe
 Find or Build a New Object
 --------------------------
 
+NOTE: Some dynamic finders have been deprecated in Rails 4.0 and will be
+removed in Rails 4.1. The best practice is to use Active Record scopes
+instead. You can find the deprecation gem at
+https://github.com/rails/activerecord-deprecated_finders
+
 It's common that you need to find a record or create it if it doesn't exist. You can do that with the `find_or_create_by` and `find_or_create_by!` methods.
 
 ### `find_or_create_by`
diff --git a/guides/source/security.md b/guides/source/security.md
index c367604d6f..cffe7c85f1 100644
--- a/guides/source/security.md
+++ b/guides/source/security.md
@@ -81,7 +81,7 @@ Here are some general guidelines on sessions.
 * _Do not store large objects in a session_. Instead you should store them in the database and save their id in the session. This will eliminate synchronization headaches and it won't fill up your session storage space (depending on what session storage you chose, see below).
 This will also be a good idea, if you modify the structure of an object and old versions of it are still in some user's cookies. With server-side session storages you can clear out the sessions, but with client-side storages, this is hard to mitigate.
 
-* _Critical data should not be stored in session_. If the user clears his cookies or closes the browser, they will be lost. And with a client-side session storage, the user can read the data.
+* _Critical data should not be stored in session_. If the user clears their cookies or closes the browser, they will be lost. And with a client-side session storage, the user can read the data.
 
 ### Session Storage