Ordering of arguments changes the behaviour of binary expressions

[danielmpetrov]

Let's start with a minimal reproduction:

var parser = new FluidParser();
var template1 = parser.Parse("{{ '123' == 123 }}");
var template2 = parser.Parse("{{ 123 == '123' }}");
Console.WriteLine(template1.Render()); // false
Console.WriteLine(template2.Render()); // true

As a Liquid writer, I would expect a == b and b == a to evaluate to the same boolean, but the above example shows that evaluated varies based on argument order. This is not only applicable to the equals binary expression either. For example, less than shows an identical discrepancy:

var parser = new FluidParser();
var template1 = parser.Parse("{{ '1' < 2 }}");
var template2 = parser.Parse("{{ 1 < '2' }}");
Console.WriteLine(template1.Render()); // false
Console.WriteLine(template2.Render()); // true

Or a boolean example:

var parser = new FluidParser();
var template1 = parser.Parse("{{ 'hi' == true }}");
var template2 = parser.Parse("{{ true == 'hi' }}");
Console.WriteLine(template1.Render()); // false
Console.WriteLine(template2.Render()); // true

Contrast this with LiquidJS, where the outcome of these expressions is more sensible (https://liquidjs.com/playground.html#e3sgJzEyMycgPT0gMTIzIH19Cnt7IDEyMyA9PSAnMTIzJyB9fQp7eyAnMScgPCAyIH19Cnt7IDEgPCAnMicgfX0Ke3sgJ2hpJyA9PSB0cnVlIH19Cnt7IHRydWUgPT0gJ2hpJyB9fQo=,e30=):

{{ '123' == 123 }} // false
{{ 123 == '123' }} // false
{{ '1' < 2 }} // true
{{ 1 < '2' }} // true
{{ 'hi' == true }} // false
{{ true == 'hi' }} // false

It seems that runtime types are always respected first, but for numbers there's a dynamic conversion. I am not sure where the reference Shopify implementations stands.

[sebastienros]

Thanks for the repros. Note that your examples also show a related bug that needs to be fixed in Fluid, which is that comparison operators should return the compared value and not true/false when used in an output tag. Pretty sure I filed it (or at least I knew about it).

For reference here are the results in Shopify (the source of truth)

[drosenba]

RE: "results in Shopify"
How does one test there - is there a public "playground"? (or does one need a Spotify account?)

How does it evaluate these?
{% if "1" <= "1" %}true{% else %}false{% endif %} // ??
{% if "1" <= "2" %}true{% else %}false{% endif %} // ??

Does it return false for each, because I see in the Fluid.NET source code for LowerThanExpression.cs, that only types NumberValue and Nil are considered for comparison, else it returns Nil.

In these "playgrounds", it returns what I would expect: true (I.e., that one should be able to do 'less than' etc. on Strings too.)
https://geekplayers.com/run-liquild-online.html
https://liquidjs.com/playground.html

P.S. I commented on this closed issue a few days ago, because I did not see this issue
#313 (comment)
I hope you can answer my questions there, and also consider reopening the discussion of support for heterogeneous comparisons.

[sebastienros]

Use the blue boxes in this page: https://shopify.dev/docs/api/liquid#liquid_basics

Nothing more to open, this issue already is, and I totally acknowledged the difference by showing the different result in Shopify. Feel free to start trying to fix it as I won't be able to in the short term.

[drosenba]

Thanks for the "playground" link.

For completeness, in that link I tried:
{% if "1" <= "1" %}true{% else %}false{% endif %} // true
{% if "1" <= "2" %}true{% else %}false{% endif %} // true

The results there are
true // true
true // true

The results in Fluid.Net are
false // true
false // true

RE: " more to open"
I was referring to the other closed ticket, so that we could discuss heterogeneous comparisons.

[danielmpetrov]

Hey, @sebastienros apologies for the delayed follow up.

Thanks for the confirmation and the Shopify playground link. I wasn't aware of the related bug where operators shouldn't return their result, but the left operand instead. That's is a bit odd, but good to know! 😃

Perhaps I can give a closer look, but no promises. 👍