Fix array filters to not support nested properties

History.md

@@ -2,9 +2,9 @@
 
 ## 5.8.0 (unreleased)
 
-## 5.7.2 2025-01-30
+## 5.7.2 2025-01-31
 
-- Fix the `sort` filter to handle nested properties gracefully when their types don't match
+* Fix array filters to not support nested properties
 
 ## 5.7.1 2025-01-24
 

lib/liquid/standardfilters.rb

@@ -387,23 +387,7 @@ module Liquid
         end
       elsif ary.all? { |el| el.respond_to?(:[]) }
         begin
-          ary.sort do |a, b|
-            a = fetch_property(a, property)
-            b = fetch_property(b, property)
-
-            ##
-            # We handle nested properties gracefully to avoid breaking backward
-            # compatibility.
-            #
-            # However, we raise errors for incompatible types when no nested
-            # properties are used to maintain strict type checking in simple
-            # cases.
-            if has_nested_property?(property)
-              type_safe_compare(a, b) { |a, b| nil_safe_compare(a, b) }
-            else
-              nil_safe_compare(a, b)
-            end
-          end
+          ary.sort { |a, b| nil_safe_compare(a[property], b[property]) }
         rescue TypeError
           raise_property_error(property)
         end
@@ -432,7 +416,7 @@ module Liquid
         end
       elsif ary.all? { |el| el.respond_to?(:[]) }
         begin
-          ary.sort { |a, b| nil_safe_casecmp(fetch_property(a, property), fetch_property(b, property)) }
+          ary.sort { |a, b| nil_safe_casecmp(a[property], b[property]) }
         rescue TypeError
           raise_property_error(property)
         end
@@ -520,7 +504,7 @@ module Liquid
         []
       else
         ary.uniq do |item|
-          fetch_property(item, property)
+          item[property]
         rescue TypeError
           raise_property_error(property)
         rescue NoMethodError
@@ -556,7 +540,7 @@ module Liquid
         if property == "to_liquid"
           e
         elsif e.respond_to?(:[])
-          r = fetch_property(e, property)
+          r = e[property]
           r.is_a?(Proc) ? r.call : r
         end
       end
@@ -580,7 +564,7 @@ module Liquid
         []
       else
         ary.reject do |item|
-          fetch_property(item, property).nil?
+          item[property].nil?
         rescue TypeError
           raise_property_error(property)
         rescue NoMethodError
@@ -966,7 +950,7 @@ module Liquid
         if property.nil?
           item
         elsif item.respond_to?(:[])
-          fetch_property(item, property)
+          item[property]
         else
           0
         end
@@ -992,9 +976,9 @@ module Liquid
 
       block.call(ary) do |item|
         if target_value.nil?
-          fetch_property(item, property)
+          item[property]
         else
-          fetch_property(item, property) == target_value
+          item[property] == target_value
         end
       rescue TypeError
         raise_property_error(property)
@@ -1004,35 +988,6 @@ module Liquid
       end
     end
 
-    def fetch_property(drop, property_or_keys)
-      ##
-      # This keeps backward compatibility by supporting properties containing
-      # dots. This is valid in Liquid syntax and used in some runtimes, such as
-      # Shopify with metafields.
-      #
-      # Using this approach, properties like 'price.value' can be accessed in
-      # both of the following examples:
-      #
-      # ```
-      # [
-      #   { 'name' => 'Item 1', 'price.price' => 40000 },
-      #   { 'name' => 'Item 2', 'price' => { 'value' => 39900 } }
-      # ]
-      # ```
-      value = drop[property_or_keys]
-
-      return value if !value.nil? || !has_nested_property?(property_or_keys)
-
-      keys = property_or_keys.split('.')
-      keys.reduce(drop) do |drop, key|
-        drop.respond_to?(:[]) ? drop[key] : drop
-      end
-    end
-
-    def has_nested_property?(property)
-      property.is_a?(String) && property.include?('.')
-    end
-
     def raise_property_error(property)
       raise Liquid::ArgumentError, "cannot select the property '#{property}'"
     end
@@ -1056,16 +1011,6 @@ module Liquid
       end
     end
 
-    def type_safe_compare(a, b)
-      klass_a = a.class
-      klass_b = b.class
-
-      # Converting classes to string to have a deterministic comparison.
-      return nil_safe_casecmp(klass_a, klass_b) if klass_a != klass_b
-
-      yield(a, b)
-    end
-
     def nil_safe_casecmp(a, b)
       if !a.nil? && !b.nil?
         a.to_s.casecmp(b.to_s)

test/integration/standard_filter_test.rb

@@ -54,30 +54,6 @@ class TestEnumerable < Liquid::Drop
   end
 end
 
-class TestDeepEnumerable < Liquid::Drop
-  include Enumerable
-
-  class Product < Liquid::Drop
-    attr_reader :title, :price, :premium
-
-    def initialize(title:, price:, premium: nil)
-      @title = { "content" => title, "language" => "en" }
-      @price = { "value" => price, "unit" => "USD" }
-      @premium = { "category" => premium } if premium
-    end
-  end
-
-  def each(&block)
-    [
-      Product.new(title: "Pro goggles",    price: 1299),
-      Product.new(title: "Thermal gloves", price: 1299),
-      Product.new(title: "Alpine jacket",  price: 3999, premium: 'Basic'),
-      Product.new(title: "Mountain boots", price: 3899, premium: 'Pro'),
-      Product.new(title: "Safety helmet",  price: 1999)
-    ].each(&block)
-  end
-end
-
 class NumberLikeThing < Liquid::Drop
   def initialize(amount)
     @amount = amount
@@ -438,15 +414,6 @@ class StandardFiltersTest < Minitest::Test
     end
   end
 
-  def test_sort_natural_with_deep_enumerables
-    template = <<~LIQUID
-      {{- products | sort_natural: 'title.content' | map: 'title.content' | join: ', ' -}}
-    LIQUID
-    expected_output = "Alpine jacket, Mountain boots, Pro goggles, Safety helmet, Thermal gloves"
-
-    assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new })
-  end
-
   def test_legacy_sort_hash
     assert_equal([{ a: 1, b: 2 }], @filters.sort(a: 1, b: 2))
   end
@@ -483,15 +450,6 @@ class StandardFiltersTest < Minitest::Test
     end
   end
 
-  def test_uniq_with_deep_enumerables
-    template = <<~LIQUID
-      {{- products | uniq: 'price.value' | map: "title.content" | join: ', ' -}}
-    LIQUID
-    expected_output = "Pro goggles, Alpine jacket, Mountain boots, Safety helmet"
-
-    assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new })
-  end
-
   def test_compact_empty_array
     assert_equal([], @filters.compact([], "a"))
   end
@@ -508,15 +466,6 @@ class StandardFiltersTest < Minitest::Test
     end
   end
 
-  def test_compact_with_deep_enumerables
-    template = <<~LIQUID
-      {{- products | compact: 'premium.category' | map: 'title.content' | join: ', ' -}}
-    LIQUID
-    expected_output = "Alpine jacket, Mountain boots"
-
-    assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new })
-  end
-
   def test_reverse
     assert_equal([4, 3, 2, 1], @filters.reverse([1, 2, 3, 4]))
   end
@@ -626,15 +575,6 @@ class StandardFiltersTest < Minitest::Test
     assert_template_result("213", '{{ foo | sort: "bar" | map: "foo" }}', { "foo" => TestEnumerable.new })
   end
 
-  def test_sort_with_deep_enumerables
-    template = <<~LIQUID
-      {{- products | sort: 'price.value' | map: 'title.content' | join: ', ' -}}
-    LIQUID
-    expected_output = "Pro goggles, Thermal gloves, Safety helmet, Mountain boots, Alpine jacket"
-
-    assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new })
-  end
-
   def test_first_and_last_call_to_liquid
     assert_template_result('foobar', '{{ foo | first }}', { 'foo' => [ThingWithToLiquid.new] })
     assert_template_result('foobar', '{{ foo | last }}', { 'foo' => [ThingWithToLiquid.new] })
@@ -951,15 +891,6 @@ class StandardFiltersTest < Minitest::Test
     assert_template_result(expected_output, template, { "array" => array })
   end
 
-  def test_reject_with_deep_enumerables
-    template = <<~LIQUID
-      {{- products | reject: 'title.content', 'Pro goggles' | map: 'price.value' | join: ', ' -}}
-    LIQUID
-    expected_output = "1299, 3999, 3899, 1999"
-
-    assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new })
-  end
-
   def test_has
     array = [
       { "handle" => "alpha", "ok" => true },
@@ -1028,16 +959,6 @@ class StandardFiltersTest < Minitest::Test
     assert_template_result(expected_output, template, { "array" => array })
   end
 
-  def test_has_with_deep_enumerables
-    template = <<~LIQUID
-      {{- products | has: 'title.content', 'Pro goggles' -}},
-      {{- products | has: 'title.content', 'foo' -}}
-    LIQUID
-    expected_output = "true,false"
-
-    assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new })
-  end
-
   def test_find_with_value
     products = [
       { "title" => "Pro goggles",    "price" => 1299 },
@@ -1056,16 +977,6 @@ class StandardFiltersTest < Minitest::Test
     assert_template_result(expected_output, template, { "products" => products })
   end
 
-  def test_find_with_deep_enumerables
-    template = <<~LIQUID
-      {%- assign product = products | find: 'title.content', 'Pro goggles' -%}
-      {{- product.title.content -}}
-    LIQUID
-    expected_output = "Pro goggles"
-
-    assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new })
-  end
-
   def test_find_with_empty_arrays
     template = <<~LIQUID
       {%- assign product = products | find: 'title.content', 'Not found' -%}
@@ -1096,16 +1007,6 @@ class StandardFiltersTest < Minitest::Test
     assert_template_result(expected_output, template, { "products" => products })
   end
 
-  def test_find_index_with_deep_enumerables
-    template = <<~LIQUID
-      {%- assign index = products | find_index: 'title.content', 'Alpine jacket' -%}
-      {{- index -}}
-    LIQUID
-    expected_output = "2"
-
-    assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new })
-  end
-
   def test_find_index_with_empty_arrays
     template = <<~LIQUID
       {%- assign index = products | find_index: 'title.content', 'Not found' -%}
@@ -1216,15 +1117,6 @@ class StandardFiltersTest < Minitest::Test
     assert_nil(@filters.where([nil], "ok"))
   end
 
-  def test_where_with_deep_enumerables
-    template = <<~LIQUID
-      {{- products | where: 'title.content', 'Pro goggles' | map: 'price.value' -}}
-    LIQUID
-    expected_output = "1299"
-
-    assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new })
-  end
-
   def test_all_filters_never_raise_non_liquid_exception
     test_drop = TestDrop.new(value: "test")
     test_drop.context = Context.new
@@ -1376,103 +1268,6 @@ class StandardFiltersTest < Minitest::Test
     assert_template_result("0", "{{ input | sum: 'subtotal' }}", { "input" => input })
   end
 
-  def test_sum_with_deep_enumerables
-    template = <<~LIQUID
-      {{- products | sum: 'price.value' -}}
-    LIQUID
-    expected_output = "12495"
-
-    assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new })
-  end
-
-  def test_sort_with_different_types
-    input = [
-      { "price" => 1000 },
-      { "price" => :none },
-      { "price" => 3000 }
-    ]
-
-    assert_raises(Liquid::ArgumentError) do
-      @filters.sort(input, "price")
-    end
-  end
-
-  def test_sort_with_nested_different_types
-    input = [
-      { "price" => { "value" => 1000, "unit" => "BRL" } },
-      { "price" => { "value" => 2000, "unit" => nil } },
-      { "price" => { "value" => 3000, "unit" => :none } }
-    ]
-    expected_output = "2000, 1000, 3000"
-    template = <<~LIQUID
-      {{- input | sort: 'price.unit' | map: 'price.value' | join: ', ' -}}
-    LIQUID
-
-    assert_template_result(expected_output, template, { "input" => input })
-  end
-
-  def test_sort_natural_with_nested_different_types
-    input = [
-      { "price" => { "value" => 1000, "unit" => "brl" } },
-      { "price" => { "value" => 2000, "unit" => "BRL" } },
-      { "price" => { "value" => 3000, "unit" => nil } },
-      { "price" => { "value" => 4000, "unit" => :brl } }
-    ]
-    expected_output = "1000, 2000, 4000, 3000"
-    template = <<~LIQUID
-      {{- input | sort_natural: 'price.unit' | map: 'price.value' | join: ', ' -}}
-    LIQUID
-
-    assert_template_result(expected_output, template, { "input" => input })
-  end
-
-  def test_uniq_with_nested_different_types
-    input = [
-      { "price" => { "value" => 1000, "unit" => "BRL" } },
-      { "price" => { "value" => 2000, "unit" => "BRL" } },
-      { "price" => { "value" => 3000, "unit" => :USD } },
-      { "price" => { "value" => 4000, "unit" => :BRL } },
-      { "price" => { "value" => 5000, "unit" => nil } }
-    ]
-
-    expected_output = "BRL, USD, BRL, " # Uniq handles different types uniqueness
-    template = <<~LIQUID
-      {{- input | uniq: 'price.unit' | map: 'price.unit' | join: ', ' -}}
-    LIQUID
-
-    assert_template_result(expected_output, template, { "input" => input })
-  end
-
-  def test_map_with_nested_different_types
-    input = [
-      { "price" => { "value" => 1000, "unit" => "brl" } },
-      { "price" => { "value" => 2000, "unit" => "BRL" } },
-      { "price" => { "value" => 3000, "unit" => nil } },
-      { "price" => { "value" => 4000, "unit" => :brl } }
-    ]
-    expected_output = "brl, BRL, , brl"
-    template = <<~LIQUID
-      {{- input | map: 'price.unit'| join: ', ' -}}
-    LIQUID
-
-    assert_template_result(expected_output, template, { "input" => input })
-  end
-
-  def test_sum_with_nested_different_types
-    input = [
-      { "price" => { "value" => 1000 } },
-      { "price" => { "value" => nil } },
-      { "price" => { "value" => :none } },
-      { "price" => { "value" => 3000 } }
-    ]
-    expected_output = "4000"
-    template = <<~LIQUID
-      {{- input | sum: 'price.value' -}}
-    LIQUID
-
-    assert_template_result(expected_output, template, { "input" => input })
-  end
-
   private
 
   def with_timezone(tz)