Merge pull request #1023 from Shopify/update-ohm-to-support-bool-expressions

aswamy · web-flow · commit 92ae40771bc4 · 2025-08-22T10:22:10.000-04:00
Update our parsing rules to support boolean expressions
diff --git a/.changeset/rotten-llamas-promise.md b/.changeset/rotten-llamas-promise.md
@@ -0,0 +1,8 @@
+---
+'@shopify/theme-language-server-common': minor
+'@shopify/prettier-plugin-liquid': minor
+'@shopify/liquid-html-parser': minor
+'@shopify/theme-check-common': minor
+---
+
+Detect and fix use of boolean expressions
diff --git a/packages/liquid-html-parser/grammar/liquid-html.ohm b/packages/liquid-html-parser/grammar/liquid-html.ohm
@@ -237,7 +237,7 @@ Liquid <: Helpers {
   // because we'd otherwise positively match the following string
   // instead of falling back to the other rule:
   // {{ 'string' | some_filter }}
-  liquidVariable<delim> = liquidExpression<delim> liquidFilter<delim>* space* &delim
+  liquidVariable<delim> = liquidComplexExpression<delim> liquidFilter<delim>* space* &delim
 
   liquidExpression<delim> =
     | liquidString<delim>
@@ -246,6 +246,18 @@ Liquid <: Helpers {
     | liquidRange<delim>
     | liquidVariableLookup<delim>
 
+  liquidComplexExpression<delim> =
+    | liquidBooleanExpression<delim>
+    | liquidExpression<delim>
+
+  liquidBooleanExpression<delim> = booleanExpressionCondition<delim> listOf<booleanExpressionSubsequentCondition<delim>, conditionSeparator>
+
+  // This might over-capture things since the conditions below can contain any `liquidExpression`s.
+  // We will need to clean this up in CST. If we restrict the conditions to only contain comparisons and
+  // variable lookups, we can't support "truthy" expressions like `{{ some_var and 'this' }}`
+  booleanExpressionSubsequentCondition<delim> = space* logicalOperator space* (comparison<delim> | liquidExpression<delim>) space*
+  booleanExpressionCondition<delim> = comparison<delim> | liquidExpression<delim>
+
   liquidString<delim> = liquidSingleQuotedString<delim> | liquidDoubleQuotedString<delim>
   liquidSingleQuotedString<delim> = "'" anyExceptStar<("'"| delim)> "'"
   liquidDoubleQuotedString<delim> = "\"" anyExceptStar<("\""| delim)> "\""
diff --git a/packages/liquid-html-parser/src/stage-1-cst.spec.ts b/packages/liquid-html-parser/src/stage-1-cst.spec.ts
@@ -39,6 +39,56 @@ describe('Unit: Stage 1 (CST)', () => {
         }
       });
 
+      it('should parse single conditions', () => {
+        [
+          { expression: `"1" == "1"`, comparator: '==' },
+          { expression: `1 == 1`, comparator: '==' },
+          { expression: `1 != 1`, comparator: '!=' },
+          { expression: `1 > 1`, comparator: '>' },
+          { expression: `1 < 1`, comparator: '<' },
+          { expression: `1 >= 1`, comparator: '>=' },
+          { expression: `1 <= 1`, comparator: '<=' },
+        ].forEach(({ expression, comparator }) => {
+          for (const { toCST, expectPath } of testCases) {
+            cst = toCST(`{{ ${expression} }}`);
+            expectPath(cst, '0.type').to.equal('LiquidVariableOutput');
+            expectPath(cst, '0.markup.type').to.equal('LiquidVariable');
+            expectPath(cst, '0.markup.expression.type').to.equal('BooleanExpression');
+            expectPath(cst, '0.markup.expression.conditions.0.type').to.equal('Condition');
+            expectPath(cst, '0.markup.expression.conditions.0.expression.type').to.equal(
+              'Comparison',
+            );
+            expectPath(cst, '0.markup.expression.conditions.0.expression.comparator').to.equal(
+              comparator,
+            );
+            expectPath(cst, '0.markup.expression.conditions.0.expression.left.value').to.equal('1');
+            expectPath(cst, '0.markup.expression.conditions.0.expression.right.value').to.equal(
+              '1',
+            );
+          }
+        });
+      });
+
+      it('should parse multiple conditions', () => {
+        [
+          { expression: `1 == 1 and 2 == 2` },
+          { expression: `1 == 1 or 2 == 2` },
+          { expression: `1 == 1 and 2 == 2 or 3 == 3` },
+          { expression: `1 == 1 and some_variable or 3 == 3` },
+          { expression: `some_var and 'raw string'` },
+        ].forEach(({ expression }) => {
+          for (const { toCST, expectPath } of testCases) {
+            cst = toCST(`{{ ${expression} }}`);
+            expectPath(cst, '0.type').to.equal('LiquidVariableOutput');
+            expectPath(cst, '0.markup.type').to.equal('LiquidVariable');
+            expectPath(cst, '0.markup.expression.type').to.equal('BooleanExpression');
+            expectPath(cst, '0.markup.expression.conditions.length').to.equal(
+              expression.split(/and|or/).length,
+            );
+          }
+        });
+      });
+
       it('should parse strings', () => {
         [
           { expression: `"string o' string"`, value: `string o' string`, single: false },
diff --git a/packages/liquid-html-parser/src/stage-1-cst.ts b/packages/liquid-html-parser/src/stage-1-cst.ts
@@ -69,6 +69,7 @@ export enum ConcreteNodeTypes {
   NamedArgument = 'NamedArgument',
   LiquidLiteral = 'LiquidLiteral',
   VariableLookup = 'VariableLookup',
+  BooleanExpression = 'BooleanExpression',
   String = 'String',
   Number = 'Number',
   Range = 'Range',
@@ -434,6 +435,15 @@ export type ConcreteLiquidExpression =
   | ConcreteLiquidRange
   | ConcreteLiquidVariableLookup;
 
+export type ConcreteComplexLiquidExpression =
+  | ConcreteLiquidBooleanExpression
+  | ConcreteLiquidExpression;
+
+export interface ConcreteLiquidBooleanExpression
+  extends ConcreteBasicNode<ConcreteNodeTypes.BooleanExpression> {
+  conditions: ConcreteLiquidCondition[];
+}
+
 export interface ConcreteStringLiteral extends ConcreteBasicNode<ConcreteNodeTypes.String> {
   value: string;
   single: boolean;
@@ -934,6 +944,7 @@ function toCST<T>(
 
     liquidDropCases: 0,
     liquidExpression: 0,
+    liquidComplexExpression: 0,
     liquidDropBaseCase: (sw: Node) => sw.sourceString.trimEnd(),
     liquidVariable: {
       type: ConcreteNodeTypes.LiquidVariable,
@@ -1004,6 +1015,50 @@ function toCST<T>(
       source,
     },
 
+    liquidBooleanExpression(initialCondition: Node, subsequentConditions: Node) {
+      const initialConditionAst = initialCondition.toAST(
+        (this as any).args.mapping,
+      ) as ConcreteLiquidCondition;
+      const subsequentConditionAsts = subsequentConditions.toAST(
+        (this as any).args.mapping,
+      ) as ConcreteLiquidCondition[];
+
+      // liquidBooleanExpression can capture too much. If there are no comparisons (e.g. `==`, `>`, etc.)
+      // and we only have a single condition (i.e. no `and` or `or` operators), we can return the expression directly.
+      if (
+        subsequentConditionAsts.length === 0 &&
+        initialConditionAst.expression.type !== ConcreteNodeTypes.Comparison
+      ) {
+        return initialConditionAst.expression;
+      }
+
+      const asts = [initialConditionAst, ...subsequentConditionAsts];
+
+      return {
+        type: ConcreteNodeTypes.BooleanExpression,
+        conditions: asts,
+        locStart: asts.at(0)!.locStart,
+        locEnd: asts.at(-1)!.locEnd,
+        source,
+      };
+    },
+    booleanExpressionCondition: {
+      type: ConcreteNodeTypes.Condition,
+      relation: null,
+      expression: 0,
+      locStart,
+      locEnd,
+      source,
+    },
+    booleanExpressionSubsequentCondition: {
+      type: ConcreteNodeTypes.Condition,
+      relation: 1,
+      expression: 3,
+      locStart,
+      locEnd,
+      source,
+    },
+
     liquidString: 0,
     liquidDoubleQuotedString: {
       type: ConcreteNodeTypes.String,
diff --git a/packages/liquid-html-parser/src/stage-2-ast.spec.ts b/packages/liquid-html-parser/src/stage-2-ast.spec.ts
@@ -30,6 +30,55 @@ describe('Unit: Stage 2 (AST)', () => {
         }
       });
 
+      it('should parse comparisons as LiquidVariable > BooleanExpression > Comparison', () => {
+        [
+          { expression: `1 == 1` },
+          { expression: `1 != 1` },
+          { expression: `1 > 1` },
+          { expression: `1 < 1` },
+          { expression: `1 >= 1` },
+          { expression: `1 <= 1` },
+        ].forEach(({ expression }) => {
+          for (const { toAST, expectPath, expectPosition } of testCases) {
+            ast = toAST(`{{ ${expression} }}`);
+            expectPath(ast, 'children.0').to.exist;
+            expectPath(ast, 'children.0.type').to.eql('LiquidVariableOutput');
+            expectPath(ast, 'children.0.markup.type').to.eql('LiquidVariable');
+            expectPath(ast, 'children.0.markup.rawSource').to.eql(expression);
+            expectPath(ast, 'children.0.markup.expression.type').to.eql('BooleanExpression');
+            expectPath(ast, 'children.0.markup.expression.condition.type').to.eql('Comparison');
+            expectPosition(ast, 'children.0');
+            expectPosition(ast, 'children.0.markup');
+            expectPosition(ast, 'children.0.markup.expression');
+          }
+        });
+      });
+
+      it('should parse logical operations as LiquidVariable > BooleanExpression > LogicalExpression', () => {
+        [
+          { expression: `1 == 1 and 2 == 2` },
+          { expression: `1 == 1 or 2 == 2` },
+          { expression: `1 == 1 and 2 == 2 or 3 == 3` },
+          { expression: `1 == 1 and some_variable or 3 == 3` },
+          { expression: `some_var and 'raw string'` },
+        ].forEach(({ expression }) => {
+          for (const { toAST, expectPath, expectPosition } of testCases) {
+            ast = toAST(`{{ ${expression} }}`);
+            expectPath(ast, 'children.0').to.exist;
+            expectPath(ast, 'children.0.type').to.eql('LiquidVariableOutput');
+            expectPath(ast, 'children.0.markup.type').to.eql('LiquidVariable');
+            expectPath(ast, 'children.0.markup.rawSource').to.eql(expression);
+            expectPath(ast, 'children.0.markup.expression.type').to.eql('BooleanExpression');
+            expectPath(ast, 'children.0.markup.expression.condition.type').to.eql(
+              'LogicalExpression',
+            );
+            expectPosition(ast, 'children.0');
+            expectPosition(ast, 'children.0.markup');
+            expectPosition(ast, 'children.0.markup.expression');
+          }
+        });
+      });
+
       it('should parse strings as LiquidVariable > String', () => {
         [
           { expression: `"string o' string"`, value: `string o' string`, single: false },
diff --git a/packages/liquid-html-parser/src/stage-2-ast.ts b/packages/liquid-html-parser/src/stage-2-ast.ts
@@ -74,6 +74,7 @@ import {
   LiquidHtmlConcreteNode,
   ConcreteLiquidTagBaseCase,
   ConcreteLiquidTagContentForMarkup,
+  ConcreteComplexLiquidExpression,
 } from './stage-1-cst';
 import { Comparators, NamedTags, NodeTypes, nonTraversableProperties, Position } from './types';
 import { assertNever, deepGet, dropLast } from './utils';
@@ -95,7 +96,7 @@ export type LiquidHtmlNode =
   | HtmlNode
   | AttributeNode
   | LiquidVariable
-  | LiquidExpression
+  | ComplexLiquidExpression
   | LiquidFilter
   | LiquidNamedArgument
   | AssignMarkup
@@ -508,7 +509,7 @@ export interface LiquidVariableOutput extends ASTNode<NodeTypes.LiquidVariableOu
 /** Represents an expression and filters, e.g. expression | filter1 | filter2 */
 export interface LiquidVariable extends ASTNode<NodeTypes.LiquidVariable> {
   /** expression | filter1 | filter2 */
-  expression: LiquidExpression;
+  expression: ComplexLiquidExpression;
 
   /** expression | filter1 | filter2 */
   filters: LiquidFilter[];
@@ -525,6 +526,8 @@ export type LiquidExpression =
   | LiquidRange
   | LiquidVariableLookup;
 
+export type ComplexLiquidExpression = LiquidBooleanExpression | LiquidExpression;
+
 /** https://shopify.dev/docs/api/liquid/filters */
 export interface LiquidFilter extends ASTNode<NodeTypes.LiquidFilter> {
   /** name: arg1, arg2, namedArg3: value3 */
@@ -545,6 +548,10 @@ export interface LiquidNamedArgument extends ASTNode<NodeTypes.NamedArgument> {
   value: LiquidExpression;
 }
 
+export interface LiquidBooleanExpression extends ASTNode<NodeTypes.BooleanExpression> {
+  condition: LiquidConditionalExpression;
+}
+
 /** https://shopify.dev/docs/api/liquid/basics#string */
 export interface LiquidString extends ASTNode<NodeTypes.String> {
   /** single or double quote? */
@@ -1941,14 +1948,30 @@ function toLiquidVariableOutput(node: ConcreteLiquidVariableOutput): LiquidVaria
 function toLiquidVariable(node: ConcreteLiquidVariable): LiquidVariable {
   return {
     type: NodeTypes.LiquidVariable,
-    expression: toExpression(node.expression),
+    expression: toComplexExpression(node.expression),
     filters: node.filters.map(toFilter),
     position: position(node),
     rawSource: node.rawSource,
     source: node.source,
   };
 }
 
+function toComplexExpression(node: ConcreteComplexLiquidExpression): ComplexLiquidExpression {
+  switch (node.type) {
+    case ConcreteNodeTypes.BooleanExpression: {
+      return {
+        type: NodeTypes.BooleanExpression,
+        position: position(node),
+        condition: toConditionalExpression(node.conditions),
+        source: node.source,
+      };
+    }
+    default: {
+      return toExpression(node);
+    }
+  }
+}
+
 function toExpression(node: ConcreteLiquidExpression): LiquidExpression {
   switch (node.type) {
     case ConcreteNodeTypes.String: {
diff --git a/packages/liquid-html-parser/src/types.ts b/packages/liquid-html-parser/src/types.ts
@@ -29,6 +29,7 @@ export enum NodeTypes {
   LiquidFilter = 'LiquidFilter',
   NamedArgument = 'NamedArgument',
   LiquidLiteral = 'LiquidLiteral',
+  BooleanExpression = 'BooleanExpression',
   String = 'String',
   Number = 'Number',
   Range = 'Range',
diff --git a/packages/prettier-plugin-liquid/src/printer/preprocess/augment-with-css-properties.ts b/packages/prettier-plugin-liquid/src/printer/preprocess/augment-with-css-properties.ts
@@ -115,6 +115,7 @@ function getCssDisplay(node: AugmentedNode<WithSiblings>, options: LiquidParserO
     case NodeTypes.LiquidFilter:
     case NodeTypes.NamedArgument:
     case NodeTypes.LiquidLiteral:
+    case NodeTypes.BooleanExpression:
     case NodeTypes.String:
     case NodeTypes.Number:
     case NodeTypes.Range:
@@ -225,6 +226,7 @@ function getNodeCssStyleWhiteSpace(
     case NodeTypes.LiquidFilter:
     case NodeTypes.NamedArgument:
     case NodeTypes.LiquidLiteral:
+    case NodeTypes.BooleanExpression:
     case NodeTypes.String:
     case NodeTypes.Number:
     case NodeTypes.Range:
diff --git a/packages/prettier-plugin-liquid/src/printer/printer-liquid-html.ts b/packages/prettier-plugin-liquid/src/printer/printer-liquid-html.ts
@@ -506,6 +506,10 @@ function printNode(
       return ['---', hardline, node.body, '---'];
     }
 
+    case NodeTypes.BooleanExpression: {
+      return path.call((p: any) => print(p), 'condition');
+    }
+
     case NodeTypes.String: {
       const preferredQuote = options.liquidSingleQuote ? `'` : `"`;
       const valueHasQuotes = node.value.includes(preferredQuote);
diff --git a/packages/theme-check-common/src/checks/liquid-html-syntax-error/index.ts b/packages/theme-check-common/src/checks/liquid-html-syntax-error/index.ts
diff --git a/packages/theme-language-server-common/src/TypeSystem.ts b/packages/theme-language-server-common/src/TypeSystem.ts
diff --git a/packages/theme-language-server-common/src/completions/params/LiquidCompletionParams.ts b/packages/theme-language-server-common/src/completions/params/LiquidCompletionParams.ts
