Handle edge cases and exceptions consistent with #map

lib/liquid/standardfilters.rb

@@ -879,20 +879,18 @@ def default(input, default_value = '', options = {})
     def sum(input, property = nil)
       ary = InputIterator.new(input, context)
 
-      return 0 if ary.empty?
-
-      if property.nil?
-        ary.sum { |el| Utils.to_number(el) }
-      elsif ary.all? { |el| el.respond_to?(:[]) }
-        begin
-          ary.sum { |el| Utils.to_number(el[property]) }
+      if ary.empty?
+        0
+      elsif property.nil?
+        ary.sum do |item|
+          Utils.to_number(item)
+        end
+      else
+        ary.sum do |item|
+          item.respond_to?(:[]) ? Utils.to_number(item[property]) : 0
         rescue TypeError
           raise_property_error(property)
         end
-      else
-        # If `property` is present, but not all elements respond to `:[]`,
-        # raise an ArgumentError
-        raise_property_error(property)
       end
     end
 

test/integration/standard_filter_test.rb

@@ -929,33 +929,44 @@ def test_where_no_target_value
   end
 
   def test_sum
-    src_w_prop = "{{ ary | sum: 'qty' }}"
-    src_wo_prop = "{{ ary | sum }}"
+    input = [1, 2]
+    assert_equal(3, @filters.sum(input))
+  end
 
-    # [Without property] Test empty array
-    assert_template_result("0", src_wo_prop, { "ary" => [] })
+  def test_sum_with_numeric_strings
+    input = [1, "2"]
+    assert_equal(3, @filters.sum(input))
+  end
 
-    # [Without property] Test array with only numeric elements
-    assert_template_result("6", src_wo_prop, { "ary" => [1, 2, 3] })
+  def test_sum_with_non_numeric_values
+    input = [3, "foo", { "foo" => "bar" }, true, nil]
+    assert_equal(3, @filters.sum(input))
+  end
 
-    # [Without property] Test array with some non-numeric elements
-    with_non_num = { "ary" => [1, 2, "a", "b", { "qty" => 2 }] }
-    exc = assert_raises(Liquid::ArgumentError) do
-      Template.parse(src_wo_prop).render!(with_non_num)
-    end
-    assert_equal(exc.message, "Liquid error: cannot sum non-numeric values")
-    assert_equal(exc.message, Template.parse(src_wo_prop).render(with_non_num))
+  def test_sum_with_property_on_indexable_map_values
+    input = [
+      { "quantity" => 1 },
+      { "quantity" => 2 },
+    ]
 
-    # [Without property] Test array with hashes, all of which have given property
-    assert_template_result("6", src_w_prop, { "ary" => [{ "qty" => 1 }, { "qty" => 2 }, { "qty" => 3 }] })
+    assert_equal(3, @filters.sum(input, "quantity"))
+    assert_equal(0, @filters.sum(input, "weight"))
+  end
+
+  def test_sum_with_property_on_indexable_non_map_values
+    input = [1, "foo", { "quantity" => 2 }]
 
-    # [Without property] Test array with hashes, some of which do not have given property
-    with_missing_prop = { "ary" => [{ "qty" => 1 }, { "qty" => 2 }, { "not_qty" => 3 }] }
-    exc = assert_raises(Liquid::ArgumentError) do
-      Template.parse(src_w_prop).render!(with_missing_prop)
+    assert_raises(Liquid::ArgumentError, "cannot select the property 'quantity'") do
+      @filters.sum(input, "quantity")
     end
-    assert_equal(exc.message, "Liquid error: cannot select the property 'qty'")
-    assert_equal(exc.message, Template.parse(src_w_prop).render(with_missing_prop))
+  end
+
+  def test_sum_with_property_on_unindexable_values
+    input = [true, nil, :foo]
+    assert_equal(0, @filters.sum(input, "quantity"))
+
+    mixed_input = [true, nil, :foo, { "quantity" => 2 }]
+    assert_equal(2, @filters.sum(mixed_input, "quantity"))
   end
 
   private