Merge pull request #238 from Shopify/fix_clashing_method_names_in_enumerable_drops

Fix clashing method names in enumerable drops

lib/liquid/drop.rb

@@ -54,7 +54,16 @@ module Liquid
 
     # Check for method existence without invoking respond_to?, which creates symbols
     def self.invokable?(method_name)
-      @invokable_methods ||= Set.new(["to_liquid"] + (public_instance_methods - Liquid::Drop.public_instance_methods).map(&:to_s))
+      unless @invokable_methods
+        # Ruby 1.8 compatibility: call to_s on method names (which are strings in 1.8, but already symbols in 1.9)
+        blacklist = (Liquid::Drop.public_instance_methods + [:each]).map(&:to_s)
+        if include?(Enumerable)
+          blacklist += Enumerable.public_instance_methods.map(&:to_s)
+          blacklist -= [:sort, :count, :first, :min, :max, :include?].map(&:to_s)
+        end
+        whitelist = [:to_liquid] + (public_instance_methods.map(&:to_s) - blacklist.map(&:to_s))
+        @invokable_methods = Set.new(whitelist.map(&:to_s))
+      end
       @invokable_methods.include?(method_name.to_s)
     end
   end

test/liquid/drop_test.rb

@@ -55,11 +55,44 @@ class ProductDrop < Liquid::Drop
 end
 
 class EnumerableDrop < Liquid::Drop
+  def before_method(method)
+    method
+  end
 
   def size
     3
   end
 
+  def first
+    1
+  end
+
+  def count
+    3
+  end
+
+  def min
+    1
+  end
+
+  def max
+    3
+  end
+
+  def each
+    yield 1
+    yield 2
+    yield 3
+  end
+end
+
+class RealEnumerableDrop < Liquid::Drop
+  include Enumerable
+
+  def before_method(method)
+    method
+  end
+
   def each
     yield 1
     yield 2
@@ -170,6 +203,33 @@ class DropsTest < Test::Unit::TestCase
     assert_equal '3', Liquid::Template.parse( '{{collection.size}}').render('collection' => EnumerableDrop.new)
   end
 
+  def test_enumerable_drop_will_invoke_before_method_for_clashing_method_names
+    ["select", "each", "map", "cycle"].each do |method|
+      assert_equal method.to_s, Liquid::Template.parse("{{collection.#{method}}}").render('collection' => EnumerableDrop.new)
+      assert_equal method.to_s, Liquid::Template.parse("{{collection[\"#{method}\"]}}").render('collection' => EnumerableDrop.new)
+      assert_equal method.to_s, Liquid::Template.parse("{{collection.#{method}}}").render('collection' => RealEnumerableDrop.new)
+      assert_equal method.to_s, Liquid::Template.parse("{{collection[\"#{method}\"]}}").render('collection' => RealEnumerableDrop.new)
+    end
+  end
+
+  def test_some_enumerable_methods_still_get_invoked
+    [ :count, :max ].each do |method|
+      assert_equal "3", Liquid::Template.parse("{{collection.#{method}}}").render('collection' => RealEnumerableDrop.new)
+      assert_equal "3", Liquid::Template.parse("{{collection[\"#{method}\"]}}").render('collection' => RealEnumerableDrop.new)
+      assert_equal "3", Liquid::Template.parse("{{collection.#{method}}}").render('collection' => EnumerableDrop.new)
+      assert_equal "3", Liquid::Template.parse("{{collection[\"#{method}\"]}}").render('collection' => EnumerableDrop.new)
+    end
+
+    assert_equal "yes", Liquid::Template.parse("{% if collection contains 3 %}yes{% endif %}").render('collection' => RealEnumerableDrop.new)
+
+    [ :min, :first ].each do |method|
+      assert_equal "1", Liquid::Template.parse("{{collection.#{method}}}").render('collection' => RealEnumerableDrop.new)
+      assert_equal "1", Liquid::Template.parse("{{collection[\"#{method}\"]}}").render('collection' => RealEnumerableDrop.new)
+      assert_equal "1", Liquid::Template.parse("{{collection.#{method}}}").render('collection' => EnumerableDrop.new)
+      assert_equal "1", Liquid::Template.parse("{{collection[\"#{method}\"]}}").render('collection' => EnumerableDrop.new)
+    end
+  end
+
   def test_empty_string_value_access
     assert_equal '', Liquid::Template.parse('{{ product[value] }}').render('product' => ProductDrop.new, 'value' => '')
   end