Revert "Rewrite journey routes formatter for performance"

This reverts commit 5c224de9e110763ec7a0f01f5b604bcf81f40bfb.

Conflicts:
	actionpack/lib/action_dispatch/journey/visitors.rb

5c224de9e110763ec7a0f01f5b604bcf81f40bfb introduced a bug in the
formatter.  This commit includes a regression test.

2 files changed, 43 insertions, 28 deletions

diff --git a/actionpack/lib/action_dispatch/journey/visitors.rb b/actionpack/lib/action_dispatch/journey/visitors.rb
index d9f634623d..3bee8ec5e0 100644
--- a/actionpack/lib/action_dispatch/journey/visitors.rb
+++ b/actionpack/lib/action_dispatch/journey/visitors.rb
@@ -107,10 +107,11 @@ module ActionDispatch
 
       # Used for formatting urls (url_for)
       class Formatter < Visitor # :nodoc:
-        attr_reader :options
+        attr_reader :options, :consumed
 
         def initialize(options)
           @options  = options
+          @consumed = {}
         end
 
         private
@@ -122,41 +123,35 @@ module ActionDispatch
             Router::Utils.escape_segment(value)
           end
 
-          def visit(node, optional = false)
-            case node.type
-            when :LITERAL, :SLASH, :DOT
-              node.left
-            when :STAR
-              visit_STAR(node.left)
-            when :GROUP
-              visit(node.left, true)
-            when :CAT
-              visit_CAT(node, optional)
-            when :SYMBOL
-              visit_SYMBOL(node, node.to_sym)
+          def visit_GROUP(node)
+            if consumed == options
+              nil
+            else
+              route = visit(node.left)
+              route.include?("\0") ? nil : route
             end
           end
 
-          def visit_CAT(node, optional)
-            left = visit(node.left, optional)
-            right = visit(node.right, optional)
+          def terminal(node)
+            node.left
+          end
 
-            if optional && !(right && left)
-              ""
-            else
-              [left, right].join
-            end
+          def binary(node)
+            [visit(node.left), visit(node.right)].join
           end
 
-          def visit_STAR(node)
-            if value = options[node.to_sym]
-              escape_path(value)
-            end
+          def nary(node)
+            node.children.map { |c| visit(c) }.join
           end
 
-          def visit_SYMBOL(node, name)
-            if value = options[name]
-              name == :controller ? escape_path(value) : escape_segment(value)
+          def visit_SYMBOL(node)
+            key = node.to_sym
+
+            if value = options[key]
+              consumed[key] = value
+              Router::Utils.escape_path(value)
+            else
+              "\0"
             end
           end
       end
diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb
index 0c6df16325..f52f8be101 100644
--- a/actionpack/test/controller/url_for_test.rb
+++ b/actionpack/test/controller/url_for_test.rb
@@ -11,6 +11,26 @@ module AbstractController
         W.default_url_options.clear
       end
 
+      def test_nested_optional
+        klass = Class.new {
+          include ActionDispatch::Routing::RouteSet.new.tap { |r|
+            r.draw {
+              get "/foo/(:bar/(:baz))/:zot", :as         => 'fun',
+                                             :controller => :articles,
+                                             :action     => :index
+            }
+          }.url_helpers
+          self.default_url_options[:host] = 'example.com'
+        }
+
+        path = klass.new.fun_path({:controller => :articles,
+                                   :baz        => "baz",
+                                   :zot        => "zot",
+                                   :only_path  => true })
+        # :bar key isn't provided
+        assert_equal '/foo/zot', path
+      end
+
       def add_host!
         W.default_url_options[:host] = 'www.basecamphq.com'
       end