bind visitor working with collector object

7 files changed, 49 insertions, 19 deletions

diff --git a/lib/arel/nodes/node.rb b/lib/arel/nodes/node.rb
index 36e7628612..239c4fd766 100644
--- a/lib/arel/nodes/node.rb
+++ b/lib/arel/nodes/node.rb
@@ -1,3 +1,5 @@
+require 'arel/collectors/sql_string'
+
 module Arel
   module Nodes
     ###
@@ -42,7 +44,9 @@ module Arel
       #
       # Maybe we should just use `Table.engine`?  :'(
       def to_sql engine = Table.engine
-        engine.connection.visitor.accept self
+        collector = Arel::Collectors::SQLString.new
+        collector = engine.connection.visitor.accept self, collector
+        collector.value
       end
 
       # Iterate through AST, nodes will be yielded depth-first
diff --git a/lib/arel/tree_manager.rb b/lib/arel/tree_manager.rb
index 1adb230991..87887800d0 100644
--- a/lib/arel/tree_manager.rb
+++ b/lib/arel/tree_manager.rb
@@ -1,3 +1,5 @@
+require 'arel/collectors/sql_string'
+
 module Arel
   class TreeManager
     include Arel::FactoryMethods
@@ -21,7 +23,9 @@ module Arel
     end
 
     def to_sql
-      visitor.accept @ast
+      collector = Arel::Collectors::SQLString.new
+      collector = visitor.accept @ast, collector
+      collector.value
     end
 
     def initialize_copy other
diff --git a/lib/arel/visitors/bind_visitor.rb b/lib/arel/visitors/bind_visitor.rb
index 5cb251ffde..c316c8d702 100644
--- a/lib/arel/visitors/bind_visitor.rb
+++ b/lib/arel/visitors/bind_visitor.rb
@@ -6,24 +6,26 @@ module Arel
         super
       end
 
-      def accept node, &block
+      def accept node, collector, &block
         @block = block if block_given?
         super
       end
 
       private
 
-      def visit_Arel_Nodes_Assignment o
+      def visit_Arel_Nodes_Assignment o, collector
         if o.right.is_a? Arel::Nodes::BindParam
-          "#{visit o.left} = #{visit o.right}"
+          collector = visit o.left, collector
+          collector << " = "
+          visit o.right, collector
         else
           super
         end
       end
 
-      def visit_Arel_Nodes_BindParam o
+      def visit_Arel_Nodes_BindParam o, collector
         if @block
-          @block.call
+          @block.call(collector)
         else
           super
         end
diff --git a/lib/arel/visitors/to_sql.rb b/lib/arel/visitors/to_sql.rb
index 0c2e649995..8c90fd76af 100644
--- a/lib/arel/visitors/to_sql.rb
+++ b/lib/arel/visitors/to_sql.rb
@@ -533,8 +533,9 @@ module Arel
         "#{visit o.left} AS #{visit o.right}"
       end
 
-      def visit_Arel_Nodes_UnqualifiedColumn o
-        "#{quote_column_name o.name}"
+      def visit_Arel_Nodes_UnqualifiedColumn o, collector
+        collector << "#{quote_column_name o.name}"
+        collector
       end
 
       def visit_Arel_Attributes_Attribute o
diff --git a/lib/arel/visitors/visitor.rb b/lib/arel/visitors/visitor.rb
index 0540445088..2d4b472ea3 100644
--- a/lib/arel/visitors/visitor.rb
+++ b/lib/arel/visitors/visitor.rb
@@ -1,8 +1,8 @@
 module Arel
   module Visitors
     class Visitor
-      def accept object
-        visit object
+      def accept object, collector
+        visit object, collector
       end
 
       private
@@ -18,8 +18,8 @@ module Arel
         DISPATCH[self.class]
       end
 
-      def visit object
-        send dispatch[object.class], object 
+      def visit object, collector
+        send dispatch[object.class], object, collector
       rescue NoMethodError => e
         raise e if respond_to?(dispatch[object.class], true)
         superklass = object.class.ancestors.find { |klass|
diff --git a/test/helper.rb b/test/helper.rb
index 1292c09a08..6e8ac836fc 100644
--- a/test/helper.rb
+++ b/test/helper.rb
@@ -11,3 +11,12 @@ class Object
     gsub(/\s+/, ' ').strip.must_equal other.gsub(/\s+/, ' ').strip
   end
 end
+
+module Arel
+  class Test < MiniTest::Test
+    def assert_like expected, actual
+      assert_equal expected.gsub(/\s+/, ' ').strip,
+                   actual.gsub(/\s+/, ' ').strip
+    end
+  end
+end
diff --git a/test/visitors/test_bind_visitor.rb b/test/visitors/test_bind_visitor.rb
index 2bfd03c861..4563fddfd1 100644
--- a/test/visitors/test_bind_visitor.rb
+++ b/test/visitors/test_bind_visitor.rb
@@ -4,8 +4,14 @@ require 'support/fake_record'
 
 module Arel
   module Visitors
-    class TestBindVisitor < Minitest::Test
-      
+    class TestBindVisitor < Arel::Test
+      attr_reader :collector
+
+      def setup
+        @collector = Collectors::SQLString.new
+        super
+      end
+
       ##
       # Tests visit_Arel_Nodes_Assignment correctly
       # substitutes binds with values from block
@@ -19,8 +25,12 @@ module Arel
         }.new Table.engine.connection
 
         assignment = um.ast.values[0]
-        actual = visitor.accept(assignment) { "replace" } 
-        actual.must_be_like "\"name\" = replace"
+        actual = visitor.accept(assignment, collector) { |collector|
+          collector << "replace"
+        }
+        assert actual
+        value = actual.value
+        assert_like "\"name\" = replace", value
       end
 
       def test_visitor_yields_on_binds
@@ -33,7 +43,7 @@ module Arel
 
         bp = Nodes::BindParam.new 'omg'
         called = false
-        visitor.accept(bp) { called = true }
+        visitor.accept(bp, collector) { |collector| called = true }
         assert called
       end
 
@@ -49,7 +59,7 @@ module Arel
         called = false
 
         assert_raises(TypeError) {
-          visitor.accept(bp) { called = true }
+          visitor.accept(bp, collector) { called = true }
         }
         refute called
       end