Skip to content

Commit bb1578f

Browse files
committed
Fix nil comparisons to match Shopify Liquid behavior
- Handle nil comparisons gracefully in comparison operators (<, >, <=, >=) - All nil comparisons now evaluate to false instead of throwing errors - Matches Shopify Liquid reference implementation behavior - Fixes issue where templates fail with 'comparison of nil with int failed'
1 parent 9fa7a8e commit bb1578f

File tree

3 files changed

+192
-1
lines changed

3 files changed

+192
-1
lines changed

integration/template_test.go

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -679,3 +679,104 @@ func TestConditionBlogPostContains(t *testing.T) {
679679
})
680680
}
681681
}
682+
683+
// TestNilComparisons tests nil comparisons matching Shopify Liquid behavior
684+
// Reference: reference-liquid/test/integration/tags/if_else_tag_test.rb:122-132
685+
func TestNilComparisons(t *testing.T) {
686+
env := liquid.NewEnvironment()
687+
tags.RegisterStandardTags(env)
688+
689+
tests := []struct {
690+
name string
691+
template string
692+
expected string
693+
data map[string]interface{}
694+
}{
695+
{
696+
name: "null less than number",
697+
template: "{% if null < 10 %} NO {% endif %}",
698+
expected: "",
699+
data: nil,
700+
},
701+
{
702+
name: "null less than or equal number",
703+
template: "{% if null <= 10 %} NO {% endif %}",
704+
expected: "",
705+
data: nil,
706+
},
707+
{
708+
name: "null greater than or equal number",
709+
template: "{% if null >= 10 %} NO {% endif %}",
710+
expected: "",
711+
data: nil,
712+
},
713+
{
714+
name: "null greater than number",
715+
template: "{% if null > 10 %} NO {% endif %}",
716+
expected: "",
717+
data: nil,
718+
},
719+
{
720+
name: "number less than null",
721+
template: "{% if 10 < null %} NO {% endif %}",
722+
expected: "",
723+
data: nil,
724+
},
725+
{
726+
name: "number less than or equal null",
727+
template: "{% if 10 <= null %} NO {% endif %}",
728+
expected: "",
729+
data: nil,
730+
},
731+
{
732+
name: "number greater than or equal null",
733+
template: "{% if 10 >= null %} NO {% endif %}",
734+
expected: "",
735+
data: nil,
736+
},
737+
{
738+
name: "number greater than null",
739+
template: "{% if 10 > null %} NO {% endif %}",
740+
expected: "",
741+
data: nil,
742+
},
743+
{
744+
name: "null less than or equal zero",
745+
template: "{% if null <= 0 %} true {% else %} false {% endif %}",
746+
expected: " false ",
747+
data: nil,
748+
},
749+
{
750+
name: "zero less than or equal null",
751+
template: "{% if 0 <= null %} true {% else %} false {% endif %}",
752+
expected: " false ",
753+
data: nil,
754+
},
755+
{
756+
name: "nil variable comparison",
757+
template: "{% if pagination.total_pages > 1 %} YES {% else %} NO {% endif %}",
758+
expected: " NO ",
759+
data: map[string]interface{}{"pagination": nil},
760+
},
761+
{
762+
name: "nil nested property comparison",
763+
template: "{% if pagination.total_pages > 1 %} YES {% else %} NO {% endif %}",
764+
expected: " NO ",
765+
data: map[string]interface{}{},
766+
},
767+
}
768+
769+
for _, tt := range tests {
770+
t.Run(tt.name, func(t *testing.T) {
771+
tmpl, err := liquid.ParseTemplate(tt.template, &liquid.TemplateOptions{Environment: env})
772+
if err != nil {
773+
t.Fatalf("ParseTemplate() error = %v", err)
774+
}
775+
776+
result := tmpl.Render(tt.data, &liquid.RenderOptions{})
777+
if result != tt.expected {
778+
t.Errorf("Expected %q, got %q", tt.expected, result)
779+
}
780+
})
781+
}
782+
}

liquid/condition.go

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,10 @@ func getConditionOperator(op string) ConditionOperator {
161161
}
162162
case "<":
163163
return func(_ *Condition, left, right interface{}) (bool, error) {
164+
// Handle nil comparisons: all comparisons with nil evaluate to false (matching Shopify Liquid)
165+
if left == nil || right == nil {
166+
return false, nil
167+
}
164168
res, err := compareValues(left, right)
165169
if err != nil {
166170
return false, err
@@ -169,6 +173,10 @@ func getConditionOperator(op string) ConditionOperator {
169173
}
170174
case ">":
171175
return func(_ *Condition, left, right interface{}) (bool, error) {
176+
// Handle nil comparisons: all comparisons with nil evaluate to false (matching Shopify Liquid)
177+
if left == nil || right == nil {
178+
return false, nil
179+
}
172180
res, err := compareValues(left, right)
173181
if err != nil {
174182
return false, err
@@ -177,6 +185,10 @@ func getConditionOperator(op string) ConditionOperator {
177185
}
178186
case ">=":
179187
return func(_ *Condition, left, right interface{}) (bool, error) {
188+
// Handle nil comparisons: all comparisons with nil evaluate to false (matching Shopify Liquid)
189+
if left == nil || right == nil {
190+
return false, nil
191+
}
180192
res, err := compareValues(left, right)
181193
if err != nil {
182194
return false, err
@@ -185,6 +197,10 @@ func getConditionOperator(op string) ConditionOperator {
185197
}
186198
case "<=":
187199
return func(_ *Condition, left, right interface{}) (bool, error) {
200+
// Handle nil comparisons: all comparisons with nil evaluate to false (matching Shopify Liquid)
201+
if left == nil || right == nil {
202+
return false, nil
203+
}
188204
res, err := compareValues(left, right)
189205
if err != nil {
190206
return false, err
@@ -245,6 +261,32 @@ func checkMethodLiteral(ml *MethodLiteral, obj interface{}) bool {
245261
}
246262

247263
func compareValues(left, right interface{}) (int, error) {
264+
// Handle nil comparisons gracefully (matching Shopify Liquid behavior)
265+
// When comparing nil with a number, the comparison should evaluate to false
266+
// To achieve this, we return -1 for nil comparisons, which makes:
267+
// - "nil > number" → res > 0 → -1 > 0 → false ✓
268+
// - "number > nil" → res > 0 → -1 > 0 → false ✓
269+
// - "nil < number" → res < 0 → -1 < 0 → true (but condition will be false due to nil falsiness)
270+
// - "number < nil" → res < 0 → -1 < 0 → true (but condition will be false due to nil falsiness)
271+
if left == nil && right != nil {
272+
// Check if right is a number
273+
_, rightOk := toNumber(right)
274+
if rightOk {
275+
return -1, nil
276+
}
277+
}
278+
if right == nil && left != nil {
279+
// Check if left is a number
280+
_, leftOk := toNumber(left)
281+
if leftOk {
282+
return -1, nil
283+
}
284+
}
285+
if left == nil && right == nil {
286+
// Both are nil, they're equal
287+
return 0, nil
288+
}
289+
248290
// Simple numeric comparison
249291
leftNum, leftOk := toNumber(left)
250292
rightNum, rightOk := toNumber(right)
@@ -257,7 +299,7 @@ func compareValues(left, right interface{}) (int, error) {
257299
return 0, nil
258300
}
259301

260-
// If one is a number and the other is not, it's a type mismatch for comparison
302+
// If one is a number and the other is not (and neither is nil), it's a type mismatch for comparison
261303
if leftOk || rightOk {
262304
// Format types nicely for error message
263305
leftType := "nil"

liquid/condition_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,54 @@ func TestConditionCompareValuesEdgeCases(t *testing.T) {
440440
}
441441
}
442442

443+
// TestConditionCompareValuesNilComparisons tests nil comparisons (matching Shopify Liquid behavior)
444+
func TestConditionCompareValuesNilComparisons(t *testing.T) {
445+
// Test nil < number (nil is less than any number)
446+
result, err := compareValues(nil, 10)
447+
if err != nil {
448+
t.Errorf("compareValues(nil, 10) returned error: %v", err)
449+
}
450+
if result >= 0 {
451+
t.Errorf("Expected compareValues(nil, 10) to return negative, got %d", result)
452+
}
453+
454+
// Test number > nil (should return negative to make condition false)
455+
result2, err := compareValues(10, nil)
456+
if err != nil {
457+
t.Errorf("compareValues(10, nil) returned error: %v", err)
458+
}
459+
if result2 >= 0 {
460+
t.Errorf("Expected compareValues(10, nil) to return negative, got %d", result2)
461+
}
462+
463+
// Test nil == nil (both are equal)
464+
result3, err := compareValues(nil, nil)
465+
if err != nil {
466+
t.Errorf("compareValues(nil, nil) returned error: %v", err)
467+
}
468+
if result3 != 0 {
469+
t.Errorf("Expected compareValues(nil, nil) to return 0, got %d", result3)
470+
}
471+
472+
// Test nil with float
473+
result4, err := compareValues(nil, 3.14)
474+
if err != nil {
475+
t.Errorf("compareValues(nil, 3.14) returned error: %v", err)
476+
}
477+
if result4 >= 0 {
478+
t.Errorf("Expected compareValues(nil, 3.14) to return negative, got %d", result4)
479+
}
480+
481+
// Test float with nil
482+
result5, err := compareValues(3.14, nil)
483+
if err != nil {
484+
t.Errorf("compareValues(3.14, nil) returned error: %v", err)
485+
}
486+
if result5 >= 0 {
487+
t.Errorf("Expected compareValues(3.14, nil) to return negative, got %d", result5)
488+
}
489+
}
490+
443491
// TestConditionToNumberEdgeCases tests toNumber with various edge case inputs
444492
func TestConditionToNumberEdgeCases(t *testing.T) {
445493
// Test with int

0 commit comments

Comments
 (0)