From e3fdeae708afcf467b10ed20aa62843184a0e18c Mon Sep 17 00:00:00 2001
From: James <72664870+jg-rp@users.noreply.github.com>
Date: Fri, 9 Feb 2024 16:24:18 +0000
Subject: [PATCH] fix: compare strings with all comparison operators (#19)
---
CHANGELOG.md | 7 ++
package.json | 2 +-
src/expression.ts | 182 ++++++++++++++++++++------------
src/types.ts | 9 ++
tests/golden/golden_liquid.json | 74 ++++++++++++-
5 files changed, 202 insertions(+), 72 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 82a60d99..52d0a37b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,12 @@
# LiquidScript Change Log
+## Version 1.8.2 (unreleased)
+
+**Fixes**
+
+- Fixed comparison of strings in logical expressions. Previously we only supported comparing strings for equality with `==` and `!=`, now we support `<`, `>`, `<=` and `>=` too.
+
+
## Version 1.8.1
**Fixes**
diff --git a/package.json b/package.json
index 0c17ac54..7fda8dee 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
{
"name": "liquidscript",
- "version": "1.8.1",
+ "version": "1.8.2",
"author": "James",
"license": "MIT",
"homepage": "https://jg-rp.github.io/liquidscript/",
diff --git a/src/expression.ts b/src/expression.ts
index 3a62be17..b11a2e9b 100644
--- a/src/expression.ts
+++ b/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,91 +889,86 @@ 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
@@ -979,11 +976,56 @@ function compare(left: unknown, operator: string, right: unknown): boolean {
*/
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("");
}
diff --git a/src/types.ts b/src/types.ts
index 329cbe67..800459f5 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -2,6 +2,15 @@ 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
diff --git a/tests/golden/golden_liquid.json b/tests/golden/golden_liquid.json
index d8a36c62..6ad69b36 100644
--- a/tests/golden/golden_liquid.json
+++ b/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 %}",