Skip to content

Commit ef13b2d

Browse files
committed
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
1 parent b0fb0ad commit ef13b2d

File tree

5 files changed

+21
-7
lines changed

5 files changed

+21
-7
lines changed

lib/liquid/condition.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,9 @@ def liquid_blank?(value)
160160
end
161161

162162
# Implement empty? semantics
163+
# Note: nil is NOT empty (but IS blank). empty? checks if a collection has zero elements.
163164
def liquid_empty?(value)
164165
case value
165-
when NilClass
166-
true
167166
when String, Array, Hash
168167
value.empty?
169168
else

lib/liquid/standardfilters.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,8 @@ def date(input, format)
768768
# @liquid_syntax array | first
769769
# @liquid_return [untyped]
770770
def first(array)
771-
return array[0] if array.is_a?(String)
771+
# ActiveSupport returns "" for empty strings, not nil
772+
return array[0] || "" if array.is_a?(String)
772773
array.first if array.respond_to?(:first)
773774
end
774775

@@ -780,7 +781,8 @@ def first(array)
780781
# @liquid_syntax array | last
781782
# @liquid_return [untyped]
782783
def last(array)
783-
return array[-1] if array.is_a?(String)
784+
# ActiveSupport returns "" for empty strings, not nil
785+
return array[-1] || "" if array.is_a?(String)
784786
array.last if array.respond_to?(:last)
785787
end
786788

lib/liquid/variable_lookup.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,9 @@ def evaluate(context)
7171
object = object.send(key).to_liquid
7272

7373
# Handle string first/last like ActiveSupport does (returns first/last character)
74+
# ActiveSupport returns "" for empty strings, not nil
7475
elsif lookup_command?(i) && object.is_a?(String) && (key == "first" || key == "last")
75-
object = key == "first" ? object[0] : object[-1]
76+
object = key == "first" ? (object[0] || "") : (object[-1] || "")
7677

7778
# No key was present with the desired value and it wasn't one of the directly supported
7879
# keywords either. The only thing we got left is to return nil or

test/integration/standard_filter_test.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -635,10 +635,12 @@ def test_first_last_on_strings
635635
# This enables template patterns like:
636636
# {{ product.title | first }} => "S" (for "Snowboard")
637637
# {{ customer.name | last }} => "h" (for "Smith")
638+
#
639+
# Note: ActiveSupport returns "" for empty strings, not nil.
638640
assert_equal('f', @filters.first('foo'))
639641
assert_equal('o', @filters.last('foo'))
640-
assert_nil(@filters.first(''))
641-
assert_nil(@filters.last(''))
642+
assert_equal('', @filters.first(''))
643+
assert_equal('', @filters.last(''))
642644
end
643645

644646
def test_first_last_on_unicode_strings

test/unit/condition_unit_test.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,16 @@ def test_empty_with_empty_hash
353353
assert_evaluates_true(VariableLookup.new('empty_hash'), '==', empty_literal)
354354
end
355355

356+
def test_nil_is_not_empty
357+
# nil is NOT empty - empty? checks if a collection has zero elements.
358+
# nil is not a collection, so it cannot be empty.
359+
# This differs from blank: nil IS blank, but nil is NOT empty.
360+
@context['nil_value'] = nil
361+
empty_literal = Condition.class_variable_get(:@@method_literals)['empty']
362+
363+
assert_evaluates_false(VariableLookup.new('nil_value'), '==', empty_literal)
364+
end
365+
356366
private
357367

358368
def assert_evaluates_true(left, op, right)

0 commit comments

Comments
 (0)