From 47f857567c785f7876b04507c25b7cb0513a8a6c Mon Sep 17 00:00:00 2001 From: James Prior Date: Fri, 9 Feb 2024 16:11:37 +0000 Subject: [PATCH] fix: compare strings with all comparison operators --- src/expression.ts | 182 ++++++++++++++++++++------------ src/types.ts | 11 ++ tests/golden/golden_liquid.json | 74 ++++++++++++- 3 files changed, 196 insertions(+), 71 deletions(-) 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..c8208c66 100644 --- a/src/types.ts +++ b/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 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 %}",