fix: compare strings with all comparison operators

src/expression.ts

@@ -12,12 +12,14 @@ import { Float, Integer, isInteger, parseNumberT } from "./number";
 import { range, Range } from "./range";
 import {
   isArray,
+  isBoolean,
   isIterable,
   isNumber,
   isObject,
   isPrimitiveInteger,
   isPropertyKey,
   isString,
+  isUndefined,
 } from "./types";
 import { Undefined } from "./undefined";
 
@@ -887,103 +889,143 @@ function isExpression(obj: unknown): obj is Expression {
 }
 
 // eslint-disable-next-line sonarjs/cognitive-complexity
-function compare(left: unknown, operator: string, right: unknown): boolean {
-  switch (operator) {
-    case "and":
-      return isLiquidTruthy(left) && isLiquidTruthy(right);
-    case "or":
-      return isLiquidTruthy(left) || isLiquidTruthy(right);
+function compare(left: unknown, op: string, right: unknown): boolean {
+  if (op === "and") {
+    return isLiquidTruthy(left) && isLiquidTruthy(right);
+  } else if (op === "or") {
+    return isLiquidTruthy(left) || isLiquidTruthy(right);
   }
 
   if (isLiquidPrimitive(left)) left = left[toLiquidPrimitive]();
   if (isLiquidPrimitive(right)) right = right[toLiquidPrimitive]();
 
-  if (isNumber(left) && isNumber(right)) {
-    const _left = parseNumberT(left);
-    switch (operator) {
-      case "==":
-        return _left.eq(right);
-      case "!=":
-      case "<>":
-        return !_left.eq(right);
-      case "<":
-        return _left.lt(right);
-      case "<=":
-        return _left.lte(right);
-      case ">":
-        return _left.gt(right);
-      case ">=":
-        return _left.gte(right);
-    }
-    throw new InternalTypeError(
-      `invalid operator '${left} ${operator} ${right}'`,
-    );
-  }
+  switch (op) {
+    case "==":
+      return eq(left, right);
+    case "!=":
+    case "<>":
+      return !eq(left, right);
+    case "<":
+      try {
+        return lt(left, right);
+      } catch {
+        throw new InternalTypeError(
+          `invalid operator '${left} ${op} ${right}'`,
+        );
+      }
+    case ">":
+      try {
+        return lt(right, left);
+      } catch {
+        throw new InternalTypeError(
+          `invalid operator '${left} ${op} ${right}'`,
+        );
+      }
+    case ">=":
+      try {
+        return lt(right, left) || eq(left, right);
+      } catch {
+        throw new InternalTypeError(
+          `invalid operator '${left} ${op} ${right}'`,
+        );
+      }
+    case "<=":
+      try {
+        return lt(left, right) || eq(left, right);
+      } catch {
+        throw new InternalTypeError(
+          `invalid operator '${left} ${op} ${right}'`,
+        );
+      }
+    case "contains":
+      if (isString(left)) {
+        return left.indexOf(String(right)) !== -1;
+      }
 
-  if (operator === "contains" && isNumber(right)) {
-    if (isString(left)) return left.indexOf(String(right)) !== -1;
-    if (isArray(left)) {
-      const n = parseNumberT(right);
-      for (const item of left) {
-        if (isNumber(item) && n.eq(item)) {
-          return true;
+      if (isArray(left)) {
+        if (isNumber(right)) {
+          const n = parseNumberT(right);
+          for (const item of left) {
+            if (isNumber(item) && n.eq(item)) {
+              return true;
+            }
+          }
+          return false;
         }
+        return left.indexOf(right) !== -1;
       }
-    }
-    return false;
-  }
-
-  if (
-    right instanceof Empty ||
-    right instanceof Blank ||
-    right instanceof Nil ||
-    right instanceof Range
-  )
-    [left, right] = [right, left];
-
-  if (left instanceof Range) return left.equals(right);
 
-  if (isArray(left) && isArray(right)) {
-    const _right = right; // for odd typescript bug?
-    return (
-      left.length === _right.length && left.every((v, i) => v === _right[i])
-    );
-  }
+      if (isUndefined(left)) {
+        return false;
+      }
 
-  switch (operator) {
-    case "==": {
-      if (left instanceof Undefined && right instanceof Undefined) return true;
-      return isExpression(left) ? left.equals(right) : left === right;
-    }
-    case "!=":
-    case "<>":
-      return isExpression(left) ? !left.equals(right) : left !== right;
-    case "contains":
-      if (isString(left)) return left.indexOf(String(right)) !== -1;
-      if (isArray(left)) return left.indexOf(right) !== -1;
       if (isObject(left) && isPropertyKey(right)) {
         return Object.propertyIsEnumerable.call(left, right);
       }
   }
 
-  if (left instanceof Undefined || right instanceof Undefined) return false;
-
   throw new InternalTypeError(
-    `invalid comparison operator '${left} ${operator} ${right}'`,
+    `invalid comparison operator '${left} ${op} ${right}'`,
   );
 }
+
 /**
  * Check a value for Liquid truthiness.
  * @param value - Any value
  * @returns `true` if the value is Liquid truthy, `false` otherwise.
  */
 export function isLiquidTruthy(value: unknown): boolean {
   if (isLiquidPrimitive(value)) value = value[toLiquidPrimitive]();
-  return value === false ||
+  return !(
+    value === false ||
     FALSE.equals(value) ||
     value === undefined ||
     value === null ||
     value instanceof Undefined
-    ? false
-    : true;
+  );
+}
+
+function eq(left: unknown, right: unknown): boolean {
+  if (
+    right instanceof Empty ||
+    right instanceof Blank ||
+    right instanceof Nil ||
+    right instanceof Range
+  )
+    [left, right] = [right, left];
+
+  if (left instanceof Undefined && right instanceof Undefined) {
+    return true;
+  }
+
+  if (isNumber(left) && isNumber(right)) {
+    return parseNumberT(left).eq(right);
+  }
+
+  if (isArray(left) && isArray(right)) {
+    const _right = right; // for odd typescript bug?
+    return (
+      left.length === _right.length && left.every((v, i) => v === _right[i])
+    );
+  }
+
+  return isExpression(left) || left instanceof Range
+    ? left.equals(right)
+    : left === right;
+}
+
+function lt(left: unknown, right: unknown): boolean {
+  if (isString(left) && isString(right)) {
+    return left < right;
+  }
+
+  if (isBoolean(left) || isBoolean(right)) {
+    return false;
+  }
+
+  if (isNumber(left) && isNumber(right)) {
+    return parseNumberT(left) < right;
+  }
+
+  throw new InternalTypeError("");
 }

src/types.ts

@@ -2,6 +2,17 @@ import { isLiquidStringable, toLiquidString } from "./drop";
 import { isNumberT, NumberT } from "./number";
 import { Undefined } from "./undefined";
 
+
+/**
+ * A type predicate for the primitive boolean.
+ * @param value - Any value
+ * @returns `true` if the value is a primitive boolean.
+ */
+export function isBoolean(value: unknown): value is boolean {
+  return typeof value == "boolean";
+}
+
+
 /**
  * A type predicate for the primitive string.
  * @param value - Any value

tests/golden/golden_liquid.json

@@ -1,5 +1,5 @@
 {
-    "version": "0.20.0",
+    "version": "0.22.0",
     "test_groups": [
         {
             "name": "liquid.golden.abs_filter",
@@ -4000,6 +4000,78 @@
                     "error": true,
                     "strict": false
                 },
+                {
+                    "name": "string is greater than or equal to string",
+                    "template": "{% if 'abc' >= 'acb' %}true{% else %}false{% endif %}",
+                    "want": "false",
+                    "context": {},
+                    "partials": {},
+                    "error": false,
+                    "strict": false
+                },
+                {
+                    "name": "string is greater than string",
+                    "template": "{% if 'abc' > 'acb' %}true{% else %}false{% endif %}",
+                    "want": "false",
+                    "context": {},
+                    "partials": {},
+                    "error": false,
+                    "strict": false
+                },
+                {
+                    "name": "string is less than or equal to string",
+                    "template": "{% if 'abc' <= 'acb' %}true{% else %}false{% endif %}",
+                    "want": "true",
+                    "context": {},
+                    "partials": {},
+                    "error": false,
+                    "strict": false
+                },
+                {
+                    "name": "string is less than string",
+                    "template": "{% if 'abc' < 'acb' %}true{% else %}false{% endif %}",
+                    "want": "true",
+                    "context": {},
+                    "partials": {},
+                    "error": false,
+                    "strict": false
+                },
+                {
+                    "name": "string is not greater than or equal to string",
+                    "template": "{% if 'bbb' >= 'aaa' %}true{% else %}false{% endif %}",
+                    "want": "true",
+                    "context": {},
+                    "partials": {},
+                    "error": false,
+                    "strict": false
+                },
+                {
+                    "name": "string is not greater than string",
+                    "template": "{% if 'bbb' > 'aaa' %}true{% else %}false{% endif %}",
+                    "want": "true",
+                    "context": {},
+                    "partials": {},
+                    "error": false,
+                    "strict": false
+                },
+                {
+                    "name": "string is not less than or equal to string",
+                    "template": "{% if 'bbb' <= 'aaa' %}true{% else %}false{% endif %}",
+                    "want": "false",
+                    "context": {},
+                    "partials": {},
+                    "error": false,
+                    "strict": false
+                },
+                {
+                    "name": "string is not less than string",
+                    "template": "{% if 'bbb' < 'aaa' %}true{% else %}false{% endif %}",
+                    "want": "false",
+                    "context": {},
+                    "partials": {},
+                    "error": false,
+                    "strict": false
+                },
                 {
                     "name": "undefined variables are falsy",
                     "template": "{% if nosuchthing %}bar{% else %}foo{% endif %}",