Fix backward-compatibility issue with the 'sort' filter

History.md

@@ -2,6 +2,10 @@
 
 ## 5.8.0 (unreleased)
 
+## 5.7.2 2025-01-30
+
+- Fix the `sort` filter to handle nested properties gracefully when their types don't match
+
 ## 5.7.1 2025-01-24
 
 * Fix the `find` and `find_index`filters to return `nil` when filtering empty arrays

lib/liquid/standardfilters.rb

@@ -387,7 +387,23 @@ module Liquid
         end
       elsif ary.all? { |el| el.respond_to?(:[]) }
         begin
-          ary.sort { |a, b| nil_safe_compare(fetch_property(a, property), fetch_property(b, property)) }
+          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
         rescue TypeError
           raise_property_error(property)
         end
@@ -1005,7 +1021,7 @@ module Liquid
       # ```
       value = drop[property_or_keys]
 
-      return value if !value.nil? || !property_or_keys.is_a?(String)
+      return value if !value.nil? || !has_nested_property?(property_or_keys)
 
       keys = property_or_keys.split('.')
       keys.reduce(drop) do |drop, key|
@@ -1013,6 +1029,10 @@ module Liquid
       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
@@ -1036,6 +1056,16 @@ 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)

lib/liquid/version.rb

@@ -2,5 +2,5 @@
 # frozen_string_literal: true
 
 module Liquid
-  VERSION = "5.7.1"
+  VERSION = "5.7.2"
 end

test/integration/standard_filter_test.rb

@@ -1385,6 +1385,94 @@ class StandardFiltersTest < Minitest::Test
     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)