Merge pull request #930 from er1/fix-sort-natural-on-nil

Fix sort and sort_natural on sorting with non-string and nil values

lib/liquid/standardfilters.rb

@@ -121,17 +121,23 @@ module Liquid
     def sort(input, property = nil)
       ary = InputIterator.new(input)
       if property.nil?
-        ary.sort
+        ary.sort do |a, b|
+          if !a.nil? && !b.nil?
+            a <=> b
+          else
+            a.nil? ? 1 : -1
+          end
+        end
       elsif ary.empty? # The next two cases assume a non-empty array.
         []
-      elsif ary.first.respond_to?(:[]) && !ary.first[property].nil?
+      elsif ary.all? { |el| el.respond_to?(:[]) }
         ary.sort do |a, b|
           a = a[property]
           b = b[property]
-          if a && b
+          if !a.nil? && !b.nil?
             a <=> b
           else
-            a ? -1 : 1
+            a.nil? ? 1 : -1
           end
         end
       end
@@ -143,11 +149,25 @@ module Liquid
       ary = InputIterator.new(input)
 
       if property.nil?
-        ary.sort { |a, b| a.casecmp(b) }
+        ary.sort do |a, b|
+          if !a.nil? && !b.nil?
+            a.to_s.casecmp(b.to_s)
+          else
+            a.nil? ? 1 : -1
+          end
+        end
       elsif ary.empty? # The next two cases assume a non-empty array.
         []
-      elsif ary.first.respond_to?(:[]) && !ary.first[property].nil?
-        ary.sort { |a, b| a[property].casecmp(b[property]) }
+      elsif ary.all? { |el| el.respond_to?(:[]) }
+        ary.sort do |a, b|
+          a = a[property]
+          b = b[property]
+          if !a.nil? && !b.nil?
+            a.to_s.casecmp(b.to_s)
+          else
+            a.nil? ? 1 : -1
+          end
+        end
       end
     end
 

test/integration/standard_filter_test.rb

@@ -190,6 +190,11 @@ class StandardFiltersTest < Minitest::Test
     assert_equal [{ "a" => 1 }, { "a" => 2 }, { "a" => 3 }, { "a" => 4 }], @filters.sort([{ "a" => 4 }, { "a" => 3 }, { "a" => 1 }, { "a" => 2 }], "a")
   end
 
+  def test_sort_with_nils
+    assert_equal [1, 2, 3, 4, nil], @filters.sort([nil, 4, 3, 2, 1])
+    assert_equal [{ "a" => 1 }, { "a" => 2 }, { "a" => 3 }, { "a" => 4 }, {}], @filters.sort([{ "a" => 4 }, { "a" => 3 }, {}, { "a" => 1 }, { "a" => 2 }], "a")
+  end
+
   def test_sort_when_property_is_sometimes_missing_puts_nils_last
     input = [
       { "price" => 4, "handle" => "alpha" },
@@ -208,6 +213,57 @@ class StandardFiltersTest < Minitest::Test
     assert_equal expectation, @filters.sort(input, "price")
   end
 
+  def test_sort_natural
+    assert_equal ["a", "B", "c", "D"], @filters.sort_natural(["c", "D", "a", "B"])
+    assert_equal [{ "a" => "a" }, { "a" => "B" }, { "a" => "c" }, { "a" => "D" }], @filters.sort_natural([{ "a" => "D" }, { "a" => "c" }, { "a" => "a" }, { "a" => "B" }], "a")
+  end
+
+  def test_sort_natural_with_nils
+    assert_equal ["a", "B", "c", "D", nil], @filters.sort_natural([nil, "c", "D", "a", "B"])
+    assert_equal [{ "a" => "a" }, { "a" => "B" }, { "a" => "c" }, { "a" => "D" }, {}], @filters.sort_natural([{ "a" => "D" }, { "a" => "c" }, {}, { "a" => "a" }, { "a" => "B" }], "a")
+  end
+
+  def test_sort_natural_when_property_is_sometimes_missing_puts_nils_last
+    input = [
+      { "price" => "4", "handle" => "alpha" },
+      { "handle" => "beta" },
+      { "price" => "1", "handle" => "gamma" },
+      { "handle" => "delta" },
+      { "price" => 2, "handle" => "epsilon" }
+    ]
+    expectation = [
+      { "price" => "1", "handle" => "gamma" },
+      { "price" => 2, "handle" => "epsilon" },
+      { "price" => "4", "handle" => "alpha" },
+      { "handle" => "delta" },
+      { "handle" => "beta" }
+    ]
+    assert_equal expectation, @filters.sort_natural(input, "price")
+  end
+
+  def test_sort_natural_case_check
+    input = [
+      { "key" => "X" },
+      { "key" => "Y" },
+      { "key" => "Z" },
+      { "fake" => "t" },
+      { "key" => "a" },
+      { "key" => "b" },
+      { "key" => "c" }
+    ]
+    expectation = [
+      { "key" => "a" },
+      { "key" => "b" },
+      { "key" => "c" },
+      { "key" => "X" },
+      { "key" => "Y" },
+      { "key" => "Z" },
+      { "fake" => "t" }
+    ]
+    assert_equal expectation, @filters.sort_natural(input, "key")
+    assert_equal ["a", "b", "c", "X", "Y", "Z"], @filters.sort_natural(["X", "Y", "Z", "a", "b", "c"])
+  end
+
   def test_sort_empty_array
     assert_equal [], @filters.sort([], "a")
   end