ruby constant syntax is not supported as routing `:controller` option.

The current implementation only works correctly if you supply the `:controller`
with directory notation (eg. `:controller => 'admin/posts'`).

The ruby constant notation (eg. `:controller => 'Admin::Posts`) leads to unexpected problems with `url_for`.

This patch prints a warning for every non supported `:controller` option. I also added documentation how
to work with namespaced controllers. The warning links to that documentation in the rails guide.

3 files changed, 59 insertions, 9 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index fec35a36bd..c1181466f9 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,5 +1,18 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   We don't support the `:controller` option for route definitions
+    with the ruby constant notation. This will now result in an
+    `ArgumentError`.
+
+    Example:
+            # This raises an ArgumentError:
+            resources :posts, :controller => "Admin::Posts"
+
+            # Use directory notation instead:
+            resources :posts, :controller => "admin/posts"
+
+    *Yves Senn*
+
 *   `assert_template` can be used to verify the locals of partials,
     which live inside a directory.
     Fixes #8516.
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 82ef1d0333..34f5f80d4d 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -246,6 +246,12 @@ module ActionDispatch
                 raise ArgumentError, "missing :action"
               end
 
+              if controller.is_a?(String) && controller !~ /\A[a-z_\/]+\z/
+                message = "'#{controller}' is not a supported controller name. This can lead to potential routing problems."
+                message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use"
+                raise ArgumentError, message
+              end
+
               hash = {}
               hash[:controller] = controller unless controller.blank?
               hash[:action]     = action unless action.blank?
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 9f31ce8127..fb1b8526d0 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -2833,21 +2833,52 @@ class TestNamespaceWithControllerOption < ActionDispatch::IntegrationTest
     end
   end
 
-  DefaultScopeRoutes = ActionDispatch::Routing::RouteSet.new
-  DefaultScopeRoutes.draw do
-    namespace :admin do
-      resources :storage_files, :controller => "StorageFiles"
-    end
+  def draw(&block)
+    @app = ActionDispatch::Routing::RouteSet.new
+    @app.draw(&block)
   end
 
-  def app
-    DefaultScopeRoutes
-  end
+  def test_valid_controller_options_inside_namespace
+    draw do
+      namespace :admin do
+        resources :storage_files, :controller => "storage_files"
+      end
+    end
 
-  def test_controller_options
     get '/admin/storage_files'
     assert_equal "admin/storage_files#index", @response.body
   end
+
+  def test_resources_with_valid_namespaced_controller_option
+    draw do
+      resources :storage_files, :controller => 'admin/storage_files'
+    end
+
+    get 'storage_files'
+    assert_equal "admin/storage_files#index", @response.body
+  end
+
+  def test_warn_with_ruby_constant_syntax_controller_option
+    e = assert_raise(ArgumentError) do
+      draw do
+        namespace :admin do
+          resources :storage_files, :controller => "StorageFiles"
+        end
+      end
+    end
+
+    assert_match "'admin/StorageFiles' is not a supported controller name", e.message
+  end
+
+  def test_warn_with_ruby_constant_syntax_namespaced_controller_option
+    e = assert_raise(ArgumentError) do
+      draw do
+        resources :storage_files, :controller => 'Admin::StorageFiles'
+      end
+    end
+
+    assert_match "'Admin::StorageFiles' is not a supported controller name", e.message
+  end
 end
 
 class TestDefaultScope < ActionDispatch::IntegrationTest