Deprecate the `eager_load_paths` configuration

Since the default in Rails 4.0 is to run in 'threadsafe' mode we need
to eager load all of the paths in `autoload_paths` so we alias
`eager_load_paths` to it. This may have unintended consequences if
you have added 'lib' to `autoload_paths` such as loading unneeded
code or code intended only for development and/or test environments.
If this applies to your application you should thoroughly check what
is being eager loaded.

7 files changed, 91 insertions, 66 deletions

diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md
index 3b0f282143..ab30962aad 100644
--- a/railties/CHANGELOG.md
+++ b/railties/CHANGELOG.md
@@ -1,5 +1,14 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Deprecate the `eager_load_paths` configuration and alias it to `autoload_paths`.
+    Since the default in Rails 4.0 is to run in 'threadsafe' mode we need to eager
+    load all of the paths in `autoload_paths`. This may have unintended consequences
+    if you have added 'lib' to `autoload_paths` such as loading unneeded code or
+    code intended only for development and/or test environments. If this applies to
+    your application you should thoroughly check what is being eager loaded.
+
+    *Andrew White*
+
 *   Restore Rails::Engine::Railties#engines with deprecation to ensure
     compatibility with gems such as Thinking Sphinx
     Fix #8551
diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb
index 46a6485c44..8a6d1f34aa 100644
--- a/railties/lib/rails/engine.rb
+++ b/railties/lib/rails/engine.rb
@@ -34,9 +34,8 @@ module Rails
   # == Configuration
   #
   # Besides the +Railtie+ configuration which is shared across the application, in a
-  # <tt>Rails::Engine</tt> you can access <tt>autoload_paths</tt>, <tt>eager_load_paths</tt>
-  # and <tt>autoload_once_paths</tt>, which, differently from a <tt>Railtie</tt>, are scoped to
-  # the current engine.
+  # <tt>Rails::Engine</tt> you can access <tt>autoload_paths</tt> and <tt>autoload_once_paths</tt>,
+  # which, differently from a <tt>Railtie</tt>, are scoped to the current engine.
   #
   #   class MyEngine < Rails::Engine
   #     # Add a load path for this specific Engine
@@ -456,9 +455,9 @@ module Rails
     end
 
     # Eager load the application by loading all ruby
-    # files inside eager_load paths.
+    # files inside autoload_paths.
     def eager_load!
-      config.eager_load_paths.each do |load_path|
+      config.autoload_paths.each do |load_path|
         matcher = /\A#{Regexp.escape(load_path)}\/(.*)\.rb\Z/
         Dir.glob("#{load_path}/**/*.rb").sort.each do |file|
           require_dependency file.sub(matcher, '\1')
@@ -558,7 +557,6 @@ module Rails
 
       # Freeze so future modifications will fail rather than do nothing mysteriously
       config.autoload_paths.freeze
-      config.eager_load_paths.freeze
       config.autoload_once_paths.freeze
     end
 
@@ -671,7 +669,7 @@ module Rails
     end
 
     def _all_autoload_paths #:nodoc:
-      @_all_autoload_paths ||= (config.autoload_paths + config.eager_load_paths + config.autoload_once_paths).uniq
+      @_all_autoload_paths ||= (config.autoload_paths + config.autoload_once_paths).uniq
     end
 
     def _all_load_paths #:nodoc:
diff --git a/railties/lib/rails/engine/configuration.rb b/railties/lib/rails/engine/configuration.rb
index 10d1821709..2b23d8be38 100644
--- a/railties/lib/rails/engine/configuration.rb
+++ b/railties/lib/rails/engine/configuration.rb
@@ -4,7 +4,7 @@ module Rails
   class Engine
     class Configuration < ::Rails::Railtie::Configuration
       attr_reader :root
-      attr_writer :middleware, :eager_load_paths, :autoload_once_paths, :autoload_paths
+      attr_writer :middleware, :autoload_once_paths, :autoload_paths
 
       def initialize(root=nil)
         super()
@@ -39,16 +39,16 @@ module Rails
         @paths ||= begin
           paths = Rails::Paths::Root.new(@root)
 
-          paths.add "app",                 eager_load: true, glob: "*"
+          paths.add "app",                 autoload: true, glob: "*"
           paths.add "app/assets",          glob: "*"
-          paths.add "app/controllers",     eager_load: true
-          paths.add "app/helpers",         eager_load: true
-          paths.add "app/models",          eager_load: true
-          paths.add "app/mailers",         eager_load: true
+          paths.add "app/controllers",     autoload: true
+          paths.add "app/helpers",         autoload: true
+          paths.add "app/models",          autoload: true
+          paths.add "app/mailers",         autoload: true
           paths.add "app/views"
 
-          paths.add "app/controllers/concerns", eager_load: true
-          paths.add "app/models/concerns",      eager_load: true
+          paths.add "app/controllers/concerns", autoload: true
+          paths.add "app/models/concerns",      autoload: true
 
           paths.add "lib",                 load_path: true
           paths.add "lib/assets",          glob: "*"
@@ -76,7 +76,13 @@ module Rails
       end
 
       def eager_load_paths
-        @eager_load_paths ||= paths.eager_load
+        ActiveSupport::Deprecation.warn "eager_load_paths is deprecated and all autoload_paths are now eagerly loaded."
+        autoload_paths
+      end
+
+      def eager_load_paths=(paths)
+        ActiveSupport::Deprecation.warn "eager_load_paths is deprecated and all autoload_paths are now eagerly loaded."
+        self.autoload_paths = paths
       end
 
       def autoload_once_paths
diff --git a/railties/lib/rails/paths.rb b/railties/lib/rails/paths.rb
index de6795eda2..80ba144441 100644
--- a/railties/lib/rails/paths.rb
+++ b/railties/lib/rails/paths.rb
@@ -5,13 +5,13 @@ module Rails
     # paths by a Hash like API. It requires you to give a physical path on initialization.
     #
     #   root = Root.new "/rails"
-    #   root.add "app/controllers", eager_load: true
+    #   root.add "app/controllers", autoload: true
     #
     # The command above creates a new root object and add "app/controllers" as a path.
     # This means we can get a <tt>Rails::Paths::Path</tt> object back like below:
     #
     #   path = root["app/controllers"]
-    #   path.eager_load?               # => true
+    #   path.autoload?                 # => true
     #   path.is_a?(Rails::Paths::Path) # => true
     #
     # The +Path+ object is simply an enumerable and allows you to easily add extra paths:
@@ -30,7 +30,7 @@ module Rails
     #   root["config/routes"].inspect # => ["config/routes.rb"]
     #
     # The +add+ method accepts the following options as arguments:
-    # eager_load, autoload, autoload_once and glob.
+    # autoload, autoload_once and glob.
     #
     # Finally, the +Path+ object also provides a few helpers:
     #
@@ -85,7 +85,8 @@ module Rails
       end
 
       def eager_load
-        filter_by(:eager_load?)
+        ActiveSupport::Deprecation.warn "eager_load is deprecated and all autoload_paths are now eagerly loaded."
+        filter_by(:autoload?)
       end
 
       def autoload_paths
@@ -124,9 +125,13 @@ module Rails
         @glob     = options[:glob]
 
         options[:autoload_once] ? autoload_once! : skip_autoload_once!
-        options[:eager_load]    ? eager_load!    : skip_eager_load!
         options[:autoload]      ? autoload!      : skip_autoload!
         options[:load_path]     ? load_path!     : skip_load_path!
+
+        if !options.key?(:autoload) && options.key?(:eager_load)
+          ActiveSupport::Deprecation.warn "the :eager_load option is deprecated and all :autoload paths are now eagerly loaded."
+          options[:eager_load] ? autoload! : skip_autoload!
+        end
       end
 
       def children
@@ -143,22 +148,37 @@ module Rails
         expanded.last
       end
 
-      %w(autoload_once eager_load autoload load_path).each do |m|
+      %w(autoload_once autoload load_path).each do |m|
         class_eval <<-RUBY, __FILE__, __LINE__ + 1
-          def #{m}!        # def eager_load!
-            @#{m} = true   #   @eager_load = true
+          def #{m}!        # def autoload!
+            @#{m} = true   #   @autoload = true
           end              # end
                            #
-          def skip_#{m}!   # def skip_eager_load!
-            @#{m} = false  #   @eager_load = false
+          def skip_#{m}!   # def skip_autoload!
+            @#{m} = false  #   @autoload = false
           end              # end
                            #
-          def #{m}?        # def eager_load?
-            @#{m}          #   @eager_load
+          def #{m}?        # def autoload?
+            @#{m}          #   @autoload
           end              # end
         RUBY
       end
 
+      def eager_load!
+        ActiveSupport::Deprecation.warn "eager_load paths are deprecated and all autoload paths are now eagerly loaded."
+        autoload!
+      end
+
+      def skip_eager_load!
+        ActiveSupport::Deprecation.warn "eager_load paths are deprecated and all autoload paths are now eagerly loaded."
+        skip_autoload!
+      end
+
+      def eager_load?
+        ActiveSupport::Deprecation.warn "eager_load paths are deprecated and all autoload paths are now eagerly loaded."
+        autoload?
+      end
+
       def each(&block)
         @paths.each(&block)
       end
diff --git a/railties/test/application/initializers/load_path_test.rb b/railties/test/application/initializers/load_path_test.rb
index 31811e7f92..595f58bd91 100644
--- a/railties/test/application/initializers/load_path_test.rb
+++ b/railties/test/application/initializers/load_path_test.rb
@@ -64,7 +64,7 @@ module ApplicationTests
 
       add_to_config <<-RUBY
         config.root = "#{app_path}"
-        config.eager_load_paths << "#{app_path}/lib"
+        config.autoload_paths << "#{app_path}/lib"
       RUBY
 
       require "#{app_path}/config/environment"
diff --git a/railties/test/application/paths_test.rb b/railties/test/application/paths_test.rb
index 4029984ce9..f6a77a0d17 100644
--- a/railties/test/application/paths_test.rb
+++ b/railties/test/application/paths_test.rb
@@ -54,11 +54,11 @@ module ApplicationTests
       assert_equal root("app", "controllers"), @paths["app/controllers"].expanded.first
     end
 
-    test "booting up Rails yields a list of paths that are eager" do
-      eager_load = @paths.eager_load
-      assert eager_load.include?(root("app/controllers"))
-      assert eager_load.include?(root("app/helpers"))
-      assert eager_load.include?(root("app/models"))
+    test "booting up Rails yields a list of paths that will be eager loaded" do
+      autoload_paths = @paths.autoload_paths
+      assert autoload_paths.include?(root("app/controllers"))
+      assert autoload_paths.include?(root("app/helpers"))
+      assert autoload_paths.include?(root("app/models"))
     end
 
     test "environments has a glob equal to the current environment" do
diff --git a/railties/test/paths_test.rb b/railties/test/paths_test.rb
index 12f18b9dbf..6f860469fd 100644
--- a/railties/test/paths_test.rb
+++ b/railties/test/paths_test.rb
@@ -135,56 +135,48 @@ class PathsTest < ActiveSupport::TestCase
     assert_equal 2, @root.autoload_once.size
   end
 
-  test "it is possible to mark a path as eager loaded" do
+  test "marking a path as eager loaded is deprecated" do
     @root["app"] = "/app"
-    @root["app"].eager_load!
-    assert @root["app"].eager_load?
-    assert @root.eager_load.include?(@root["app"].to_a.first)
+    assert_deprecated{ @root["app"].eager_load! }
+    assert_deprecated{ assert @root["app"].eager_load? }
+    assert_deprecated{ assert @root.eager_load.include?(@root["app"].to_a.first) }
   end
 
-  test "it is possible to skip a path from eager loading" do
+  test "skipping a path from eager loading is deprecated" do
     @root["app"] = "/app"
-    @root["app"].eager_load!
-    assert @root["app"].eager_load?
+    assert_deprecated{ @root["app"].eager_load! }
+    assert_deprecated{ assert @root["app"].eager_load? }
 
-    @root["app"].skip_eager_load!
-    assert !@root["app"].eager_load?
-    assert !@root.eager_load.include?(@root["app"].to_a.first)
+    assert_deprecated{ @root["app"].skip_eager_load! }
+    assert_deprecated{ assert !@root["app"].eager_load? }
+    assert_deprecated{ assert !@root.eager_load.include?(@root["app"].to_a.first) }
   end
 
-  test "it is possible to add a path without assignment and mark it as eager" do
-    @root.add "app", with: "/app", eager_load: true
-    assert @root["app"].eager_load?
-    assert @root.eager_load.include?("/app")
+  test "adding a path with eager_load option is deprecated" do
+    assert_deprecated{ @root.add "app", with: "/app", eager_load: true }
+    assert_deprecated{ assert @root["app"].eager_load? }
+    assert_deprecated{ assert @root.eager_load.include?("/app") }
   end
 
-  test "it is possible to add multiple paths without assignment and mark them as eager" do
-    @root.add "app", with: ["/app", "/app2"], eager_load: true
-    assert @root["app"].eager_load?
-    assert @root.eager_load.include?("/app")
-    assert @root.eager_load.include?("/app2")
-  end
-
-  test "it is possible to create a path without assignment and mark it both as eager and load once" do
-    @root.add "app", with: "/app", eager_load: true, autoload_once: true
-    assert @root["app"].eager_load?
-    assert @root["app"].autoload_once?
-    assert @root.eager_load.include?("/app")
-    assert @root.autoload_once.include?("/app")
+  test "adding multiple paths with eager_load option is deprecated" do
+    assert_deprecated{ @root.add "app", with: ["/app", "/app2"], eager_load: true }
+    assert_deprecated{ assert @root["app"].eager_load? }
+    assert_deprecated{ assert @root.eager_load.include?("/app") }
+    assert_deprecated{ assert @root.eager_load.include?("/app2") }
   end
 
   test "making a path eager more than once only includes it once in @root.eager_paths" do
     @root["app"] = "/app"
-    @root["app"].eager_load!
-    @root["app"].eager_load!
-    assert_equal 1, @root.eager_load.select {|p| p == @root["app"].expanded.first }.size
+    assert_deprecated{ @root["app"].eager_load! }
+    assert_deprecated{ @root["app"].eager_load! }
+    assert_deprecated{ assert_equal 1, @root.eager_load.select {|p| p == @root["app"].expanded.first }.size }
   end
 
   test "paths added to a eager_load path should be added to the eager_load collection" do
     @root["app"] = "/app"
-    @root["app"].eager_load!
+    assert_deprecated{ @root["app"].eager_load! }
     @root["app"] << "/app2"
-    assert_equal 2, @root.eager_load.size
+    assert_deprecated{ assert_equal 2, @root.eager_load.size }
   end
 
   test "it should be possible to add a path's default glob" do