Consolidate contextualization of inputs via the context

History.md

@@ -4,6 +4,7 @@
 
 ### Fixes
 * StandardFilter: Fix missing @context on iterations (#1525) [Thierry Joyal]
+* Consolidate contextualization of inputs via the context (#XXX) [Thierry Joyal]
 
 ### Deprecation
 * Condition#evaluate to require mandatory context argument in Liquid 6.0.0 (#1527) [Thierry Joyal]

lib/liquid/context.rb

@@ -187,16 +187,11 @@ module Liquid
       # path and find_index() is optimized in MRI to reduce object allocation
       index = @scopes.find_index { |s| s.key?(key) }
 
-      variable = if index
+      if index
         lookup_and_evaluate(@scopes[index], key, raise_on_not_found: raise_on_not_found)
       else
         try_variable_find_in_environments(key, raise_on_not_found: raise_on_not_found)
       end
-
-      variable         = variable.to_liquid
-      variable.context = self if variable.respond_to?(:context=)
-
-      variable
     end
 
     def lookup_and_evaluate(obj, key, raise_on_not_found: true)
@@ -207,9 +202,9 @@ module Liquid
       value = obj[key]
 
       if value.is_a?(Proc) && obj.respond_to?(:[]=)
-        obj[key] = value.arity == 0 ? value.call : value.call(self)
+        obj[key] = contextualize(value)
       else
-        value
+        contextualize(value)
       end
     end
 
@@ -228,6 +223,20 @@ module Liquid
       @disabled_tags.fetch(tag_name, 0) > 0
     end
 
+    # Convert input objects into liquid aware representations
+    # Procs will be resolved
+    # Assigns the context (self) through context=
+    def contextualize(object)
+      if object.is_a?(Proc)
+        object = object.arity == 0 ? object.call : object.call(self)
+      end
+
+      object = object.to_liquid
+      object.context = self if object.respond_to?(:context=)
+
+      object
+    end
+
     protected
 
     attr_writer :base_scope_depth, :warnings, :errors, :strainer, :filters, :disabled_tags

lib/liquid/standardfilters.rb

@@ -582,8 +582,7 @@ module Liquid
 
       def each
         @input.each do |e|
-          e = e.respond_to?(:to_liquid) ? e.to_liquid : e
-          e.context = @context if e.respond_to?(:context=)
+          e = @context.contextualize(e)
           yield(e)
         end
       end

lib/liquid/variable_lookup.rb

@@ -49,15 +49,14 @@ module Liquid
             ((object.respond_to?(:key?) && object.key?(key)) ||
              (object.respond_to?(:fetch) && key.is_a?(Integer)))
 
-          # if its a proc we will replace the entry with the proc
-          res    = context.lookup_and_evaluate(object, key)
-          object = res.to_liquid
+          object = context.lookup_and_evaluate(object, key)
 
           # Some special cases. If the part wasn't in square brackets and
           # no key with the same name was found we interpret following calls
           # as commands and call them on the current object
         elsif @command_flags & (1 << i) != 0 && object.respond_to?(key)
-          object = object.send(key).to_liquid
+          object = object.send(key)
+          object = context.contextualize(object)
 
           # No key was present with the desired value and it wasn't one of the directly supported
           # keywords either. The only thing we got left is to return nil or
@@ -66,9 +65,6 @@ module Liquid
           return nil unless context.strict_variables
           raise Liquid::UndefinedVariable, "undefined variable #{key}"
         end
-
-        # If we are dealing with a drop here we have to
-        object.context = context if object.respond_to?(:context=)
       end
 
       object

test/integration/standard_filter_test.rb

@@ -34,6 +34,10 @@ class TestDrop < Liquid::Drop
   def registers
     @context.registers
   end
+
+  def to_s
+    "TestDrop(value:#{@value})"
+  end
 end
 
 class TestModel
@@ -467,10 +471,38 @@ class StandardFiltersTest < Minitest::Test
   end
 
   def test_map_over_proc
-    drop  = TestDrop.new(value: "testfoo")
+    drop  = TestDrop.new(value: "123")
     p     = proc { drop }
     templ = '{{ procs | map: "value" }}'
-    assert_template_result("testfoo", templ, "procs" => [p])
+    assert_template_result("123", templ, "procs" => [p])
+  end
+
+  def test_join_over_proc
+    drop  = TestDrop.new(value: "123")
+    p     = proc { drop }
+    templ = '{{ procs | join }}'
+    assert_template_result('TestDrop(value:123)', templ, "procs" => [p])
+  end
+
+  def test_sort_over_proc
+    drop  = TestDrop.new(value: "123")
+    p     = proc { drop }
+    templ = '{{ procs | sort: "value" }}'
+    assert_template_result("TestDrop(value:123)", templ, "procs" => [p])
+  end
+
+  def test_where_over_proc
+    drop  = TestDrop.new(value: "123")
+    p     = proc { drop }
+    templ = '{{ procs | where: "value", "123" }}'
+    assert_template_result("TestDrop(value:123)", templ, "procs" => [p])
+  end
+
+  def test_uniq_over_proc
+    drop  = TestDrop.new(value: "123")
+    p     = proc { drop }
+    templ = '{{ procs | uniq }}'
+    assert_template_result("TestDrop(value:123)", templ, "procs" => [p])
   end
 
   def test_map_over_drops_returning_procs
@@ -888,7 +920,7 @@ class StandardFiltersTest < Minitest::Test
       { foo: "bar" },
       [{ "foo" => "bar" }, { "foo" => 123 }, { "foo" => nil }, { "foo" => true }, { "foo" => ["foo", "bar"] }],
       { 1 => "bar" },
-      ["foo", 123, nil, true, false, Drop, ["foo"], { foo: "bar" }],
+      ["foo", 123, nil, true, false, ["foo"], { foo: "bar" }],
     ]
     StandardFilters.public_instance_methods(false).each do |method|
       arg_count = @filters.method(method).arity

test/integration/tags/standard_tag_test.rb

@@ -272,14 +272,36 @@ class StandardTagTest < Minitest::Test
       '{%cycle var1: "one", "two" %} {%cycle var2: "one", "two" %} {%cycle var1: "one", "two" %} {%cycle var2: "one", "two" %} {%cycle var1: "one", "two" %} {%cycle var2: "one", "two" %}', assigns)
   end
 
-  def test_size_of_array
-    assigns = { "array" => [1, 2, 3, 4] }
-    assert_template_result('array has 4 elements', "array has {{ array.size }} elements", assigns)
+  def test_command_methods_of_array
+    assigns = { "array" => [11, 22, 33] }
+
+    assert_template_result("3", "{{ array.size }}", assigns)
+
+    assert_template_result("11", "{{ array.first }}", assigns)
+
+    assert_template_result("33", "{{ array.last }}", assigns)
   end
 
-  def test_size_of_hash
-    assigns = { "hash" => { a: 1, b: 2, c: 3, d: 4 } }
-    assert_template_result('hash has 4 elements', "hash has {{ hash.size }} elements", assigns)
+  def test_command_methods_of_hash
+    assigns = { "hash" => { a: 11, b: 22, c: 33 } }
+
+    assert_template_result("3", "{{ hash.size }}", assigns)
+
+    assert_template_result("a11", "{{ hash.first }}", assigns)
+    assert_template_result("a", "{{ hash.first.first }}", assigns)
+    assert_template_result("11", "{{ hash.first.last }}", assigns)
+
+    assert_template_result("", "{{ hash.last }}", assigns)
+  end
+
+  def test_command_methods_with_proc
+    skip("Liquid-C does not properly resolve Procs in with command methods") if ENV['LIQUID_C'] == '1'
+
+    assigns = { "array" => [proc { "test" }] }
+    assert_template_result("test", "{{ array.first }}", assigns)
+
+    assigns = { "hash" => { a: proc { "test" } } }
+    assert_template_result("test", "{{ hash.first.last }}", assigns)
   end
 
   def test_illegal_symbols