mirror of
https://github.com/Shopify/liquid.git
synced 2026-09-12 23:40:45 -07:00
Fix empty? semantics and string first/last for empty strings
- nil is NOT empty (but IS blank) - matches Shopify production - String first/last returns '' for empty strings, not nil - matches ActiveSupport - Add test for nil not being empty
This commit is contained in:
@@ -160,10 +160,9 @@ module Liquid
|
|||||||
end
|
end
|
||||||
|
|
||||||
# Implement empty? semantics
|
# Implement empty? semantics
|
||||||
|
# Note: nil is NOT empty (but IS blank). empty? checks if a collection has zero elements.
|
||||||
def liquid_empty?(value)
|
def liquid_empty?(value)
|
||||||
case value
|
case value
|
||||||
when NilClass
|
|
||||||
true
|
|
||||||
when String, Array, Hash
|
when String, Array, Hash
|
||||||
value.empty?
|
value.empty?
|
||||||
else
|
else
|
||||||
|
|||||||
@@ -768,7 +768,8 @@ module Liquid
|
|||||||
# @liquid_syntax array | first
|
# @liquid_syntax array | first
|
||||||
# @liquid_return [untyped]
|
# @liquid_return [untyped]
|
||||||
def first(array)
|
def first(array)
|
||||||
return array[0] if array.is_a?(String)
|
# ActiveSupport returns "" for empty strings, not nil
|
||||||
|
return array[0] || "" if array.is_a?(String)
|
||||||
array.first if array.respond_to?(:first)
|
array.first if array.respond_to?(:first)
|
||||||
end
|
end
|
||||||
|
|
||||||
@@ -780,7 +781,8 @@ module Liquid
|
|||||||
# @liquid_syntax array | last
|
# @liquid_syntax array | last
|
||||||
# @liquid_return [untyped]
|
# @liquid_return [untyped]
|
||||||
def last(array)
|
def last(array)
|
||||||
return array[-1] if array.is_a?(String)
|
# ActiveSupport returns "" for empty strings, not nil
|
||||||
|
return array[-1] || "" if array.is_a?(String)
|
||||||
array.last if array.respond_to?(:last)
|
array.last if array.respond_to?(:last)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@@ -71,8 +71,9 @@ module Liquid
|
|||||||
object = object.send(key).to_liquid
|
object = object.send(key).to_liquid
|
||||||
|
|
||||||
# Handle string first/last like ActiveSupport does (returns first/last character)
|
# Handle string first/last like ActiveSupport does (returns first/last character)
|
||||||
|
# ActiveSupport returns "" for empty strings, not nil
|
||||||
elsif lookup_command?(i) && object.is_a?(String) && (key == "first" || key == "last")
|
elsif lookup_command?(i) && object.is_a?(String) && (key == "first" || key == "last")
|
||||||
object = key == "first" ? object[0] : object[-1]
|
object = key == "first" ? (object[0] || "") : (object[-1] || "")
|
||||||
|
|
||||||
# No key was present with the desired value and it wasn't one of the directly supported
|
# No key was present with the desired value and it wasn't one of the directly supported
|
||||||
# keywords either. The only thing we got left is to return nil or
|
# keywords either. The only thing we got left is to return nil or
|
||||||
|
|||||||
@@ -635,10 +635,12 @@ class StandardFiltersTest < Minitest::Test
|
|||||||
# This enables template patterns like:
|
# This enables template patterns like:
|
||||||
# {{ product.title | first }} => "S" (for "Snowboard")
|
# {{ product.title | first }} => "S" (for "Snowboard")
|
||||||
# {{ customer.name | last }} => "h" (for "Smith")
|
# {{ customer.name | last }} => "h" (for "Smith")
|
||||||
|
#
|
||||||
|
# Note: ActiveSupport returns "" for empty strings, not nil.
|
||||||
assert_equal('f', @filters.first('foo'))
|
assert_equal('f', @filters.first('foo'))
|
||||||
assert_equal('o', @filters.last('foo'))
|
assert_equal('o', @filters.last('foo'))
|
||||||
assert_nil(@filters.first(''))
|
assert_equal('', @filters.first(''))
|
||||||
assert_nil(@filters.last(''))
|
assert_equal('', @filters.last(''))
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_first_last_on_unicode_strings
|
def test_first_last_on_unicode_strings
|
||||||
|
|||||||
@@ -353,6 +353,16 @@ class ConditionUnitTest < Minitest::Test
|
|||||||
assert_evaluates_true(VariableLookup.new('empty_hash'), '==', empty_literal)
|
assert_evaluates_true(VariableLookup.new('empty_hash'), '==', empty_literal)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_nil_is_not_empty
|
||||||
|
# nil is NOT empty - empty? checks if a collection has zero elements.
|
||||||
|
# nil is not a collection, so it cannot be empty.
|
||||||
|
# This differs from blank: nil IS blank, but nil is NOT empty.
|
||||||
|
@context['nil_value'] = nil
|
||||||
|
empty_literal = Condition.class_variable_get(:@@method_literals)['empty']
|
||||||
|
|
||||||
|
assert_evaluates_false(VariableLookup.new('nil_value'), '==', empty_literal)
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def assert_evaluates_true(left, op, right)
|
def assert_evaluates_true(left, op, right)
|
||||||
|
|||||||
Reference in New Issue
Block a user