mirror of
https://github.com/Shopify/liquid.git
synced 2026-09-20 11:20:41 -07:00
Fix "sort" filter on empty array to return empty array
When sorting an empty array with the "sort" filter, it returns nil instead of []. This confuses subsequent filters in the chain that expect an array. For example, when followed by the "map" filter, it produces an array containing one nil element: [nil]. I could special-case the nil return value, but that would be more cumbersome than making sure "sort" always returns an array. Add a case to the "sort" method to return [] if the array is empty, before performing any checks on ary.first that assume a non-empty array. There is still a danger of returning nil if the first item in the array is nil and it is non-empty, but I'm not sure how better to handle that case. Apply a similar fix to sort_natural, uniq, and compact filters.
This commit is contained in:
@@ -116,6 +116,8 @@ module Liquid
|
|||||||
ary = InputIterator.new(input)
|
ary = InputIterator.new(input)
|
||||||
if property.nil?
|
if property.nil?
|
||||||
ary.sort
|
ary.sort
|
||||||
|
elsif ary.empty? # The next two cases assume a non-empty array.
|
||||||
|
[]
|
||||||
elsif ary.first.respond_to?(:[]) && !ary.first[property].nil?
|
elsif ary.first.respond_to?(:[]) && !ary.first[property].nil?
|
||||||
ary.sort { |a, b| a[property] <=> b[property] }
|
ary.sort { |a, b| a[property] <=> b[property] }
|
||||||
elsif ary.first.respond_to?(property)
|
elsif ary.first.respond_to?(property)
|
||||||
@@ -130,6 +132,8 @@ module Liquid
|
|||||||
|
|
||||||
if property.nil?
|
if property.nil?
|
||||||
ary.sort { |a, b| a.casecmp(b) }
|
ary.sort { |a, b| a.casecmp(b) }
|
||||||
|
elsif ary.empty? # The next two cases assume a non-empty array.
|
||||||
|
[]
|
||||||
elsif ary.first.respond_to?(:[]) && !ary.first[property].nil?
|
elsif ary.first.respond_to?(:[]) && !ary.first[property].nil?
|
||||||
ary.sort { |a, b| a[property].casecmp(b[property]) }
|
ary.sort { |a, b| a[property].casecmp(b[property]) }
|
||||||
elsif ary.first.respond_to?(property)
|
elsif ary.first.respond_to?(property)
|
||||||
@@ -144,6 +148,8 @@ module Liquid
|
|||||||
|
|
||||||
if property.nil?
|
if property.nil?
|
||||||
ary.uniq
|
ary.uniq
|
||||||
|
elsif ary.empty? # The next two cases assume a non-empty array.
|
||||||
|
[]
|
||||||
elsif ary.first.respond_to?(:[])
|
elsif ary.first.respond_to?(:[])
|
||||||
ary.uniq{ |a| a[property] }
|
ary.uniq{ |a| a[property] }
|
||||||
end
|
end
|
||||||
@@ -175,6 +181,8 @@ module Liquid
|
|||||||
|
|
||||||
if property.nil?
|
if property.nil?
|
||||||
ary.compact
|
ary.compact
|
||||||
|
elsif ary.empty? # The next two cases assume a non-empty array.
|
||||||
|
[]
|
||||||
elsif ary.first.respond_to?(:[])
|
elsif ary.first.respond_to?(:[])
|
||||||
ary.reject{ |a| a[property].nil? }
|
ary.reject{ |a| a[property].nil? }
|
||||||
elsif ary.first.respond_to?(property)
|
elsif ary.first.respond_to?(property)
|
||||||
@@ -374,6 +382,11 @@ module Liquid
|
|||||||
to_a.compact
|
to_a.compact
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def empty?
|
||||||
|
@input.each { return false }
|
||||||
|
true
|
||||||
|
end
|
||||||
|
|
||||||
def each
|
def each
|
||||||
@input.each do |e|
|
@input.each do |e|
|
||||||
yield(e.respond_to?(:to_liquid) ? e.to_liquid : e)
|
yield(e.respond_to?(:to_liquid) ? e.to_liquid : e)
|
||||||
|
|||||||
@@ -158,6 +158,14 @@ 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")
|
assert_equal [{ "a" => 1 }, { "a" => 2 }, { "a" => 3 }, { "a" => 4 }], @filters.sort([{ "a" => 4 }, { "a" => 3 }, { "a" => 1 }, { "a" => 2 }], "a")
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_sort_empty_array
|
||||||
|
assert_equal [], @filters.sort([], "a")
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_sort_natural_empty_array
|
||||||
|
assert_equal [], @filters.sort_natural([], "a")
|
||||||
|
end
|
||||||
|
|
||||||
def test_legacy_sort_hash
|
def test_legacy_sort_hash
|
||||||
assert_equal [{ a: 1, b: 2 }], @filters.sort({ a: 1, b: 2 })
|
assert_equal [{ a: 1, b: 2 }], @filters.sort({ a: 1, b: 2 })
|
||||||
end
|
end
|
||||||
@@ -177,6 +185,14 @@ class StandardFiltersTest < Minitest::Test
|
|||||||
assert_equal [testdrop], @filters.uniq([testdrop, TestDrop.new], 'test')
|
assert_equal [testdrop], @filters.uniq([testdrop, TestDrop.new], 'test')
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_uniq_empty_array
|
||||||
|
assert_equal [], @filters.uniq([], "a")
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_compact_empty_array
|
||||||
|
assert_equal [], @filters.compact([], "a")
|
||||||
|
end
|
||||||
|
|
||||||
def test_reverse
|
def test_reverse
|
||||||
assert_equal [4, 3, 2, 1], @filters.reverse([1, 2, 3, 4])
|
assert_equal [4, 3, 2, 1], @filters.reverse([1, 2, 3, 4])
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user