Update grammar to strip whitespace using Ohm matcher

Shopify · Jan 14, 2025 · ba370d4 · ba370d4
1 parent 9edbe0a
commit ba370d4
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 61 deletions.
diff --git a/packages/liquid-html-parser/grammar/liquid-html.ohm b/packages/liquid-html-parser/grammar/liquid-html.ohm
@@ -394,11 +394,18 @@ LiquidDoc <: Helpers {
     | paramNode
     | fallbackNode
 
-  fallbackNode = "@" anyExceptStar<newline>
-  paramNode = "@param" space* paramType? space* paramName (space* "-")? paramDescription
-  paramType = "{" anyExceptStar<"}"> "}"
+  //By default, space matches new lines as well. We override it here to make writing rules easier.
+  strictSpace = " " | "\t"
+  // We use this as an escape hatch to stop matching TextNode and try again when one of these characters is encountered 
+  openControl:= "@" | end
+
+  fallbackNode = "@" anyExceptStar<endOfParam>
+  paramNode = "@param" strictSpace* paramType? strictSpace* paramName (strictSpace* "-")? strictSpace* paramDescription
+  paramType = "{" strictSpace* paramTypeContent strictSpace* "}"
+  paramTypeContent = anyExceptStar<("}"| strictSpace)>
   paramName = identifierCharacter+
-  paramDescription = anyExceptStar<(newline | end)>
+  paramDescription = anyExceptStar<endOfParam>
+  endOfParam = strictSpace* (newline | end)
 }
 
 LiquidHTML <: Liquid {

diff --git a/packages/liquid-html-parser/src/stage-1-cst.spec.ts b/packages/liquid-html-parser/src/stage-1-cst.spec.ts
@@ -1048,13 +1048,13 @@ describe('Unit: Stage 1 (CST)', () => {
 
           expectPath(cst, '0.children.0.paramType.type').to.equal('TextNode');
           expectPath(cst, '0.children.0.paramType.value').to.equal('String');
-          expectPath(cst, '0.children.0.paramType.locStart').to.equal(testStr.indexOf('{String}'));
+          expectPath(cst, '0.children.0.paramType.locStart').to.equal(testStr.indexOf('String'));
           expectPath(cst, '0.children.0.paramType.locEnd').to.equal(
-            testStr.indexOf('{String}') + '{String}'.length,
+            testStr.indexOf('String') + 'String'.length,
           );
         });
 
-        it('should parse @param with type with space inside', () => {
+        it('should strip whitespace around param type for @param annotation', () => {
           const testStr = `{% doc %} @param { String } paramWithType {% enddoc %}`;
           cst = toCST(testStr);
 
@@ -1063,11 +1063,9 @@ describe('Unit: Stage 1 (CST)', () => {
 
           expectPath(cst, '0.children.0.paramType.type').to.equal('TextNode');
           expectPath(cst, '0.children.0.paramType.value').to.equal('String');
-          expectPath(cst, '0.children.0.paramType.locStart').to.equal(
-            testStr.indexOf('{ String }'),
-          );
+          expectPath(cst, '0.children.0.paramType.locStart').to.equal(testStr.indexOf('String'));
           expectPath(cst, '0.children.0.paramType.locEnd').to.equal(
-            testStr.indexOf('{ String }') + '{ String }'.length,
+            testStr.indexOf('String') + 'String'.length,
           );
         });
 

diff --git a/packages/liquid-html-parser/src/stage-1-cst.ts b/packages/liquid-html-parser/src/stage-1-cst.ts
@@ -441,15 +441,14 @@ export interface ConcreteYamlFrontmatterNode
 
 export type LiquidHtmlConcreteNode =
   | ConcreteHtmlNode
-  | ConcreteLiquidNode
-  | ConcreteTextNode
   | ConcreteYamlFrontmatterNode
-  | LiquidDocConcreteNode;
+  | LiquidConcreteNode;
 
 export type LiquidConcreteNode =
   | ConcreteLiquidNode
   | ConcreteTextNode
-  | ConcreteYamlFrontmatterNode;
+  | ConcreteYamlFrontmatterNode
+  | LiquidDocConcreteNode;
 
 export type LiquidHtmlCST = LiquidHtmlConcreteNode[];
 
@@ -1317,17 +1316,22 @@ function toLiquidDocAST(source: string, matchingSource: string, offset: number)
     throw new LiquidHTMLCSTParsingError(res);
   }
 
+  /**
+   * Reusable text node type
+   */
+  const textNode = {
+    type: ConcreteNodeTypes.TextNode,
+    value: function () {
+      return (this as any).sourceString;
+    },
+    locStart,
+    locEnd,
+    source,
+  };
+
   const LiquidDocMappings: Mapping = {
     Node: 0,
-    TextNode: {
-      type: ConcreteNodeTypes.TextNode,
-      value: function () {
-        return (this as any).sourceString;
-      },
-      locStart,
-      locEnd,
-      source,
-    },
+    TextNode: textNode,
     paramNode: {
       type: ConcreteNodeTypes.LiquidDocParamNode,
       name: 'param',
@@ -1336,44 +1340,13 @@ function toLiquidDocAST(source: string, matchingSource: string, offset: number)
       source,
       paramType: 2,
       paramName: 4,
-      paramDescription: 7,
-    },
-    paramType: {
-      type: ConcreteNodeTypes.TextNode,
-      value: function (nodes: Node[]) {
-        return nodes[1].sourceString.trim();
-      },
-      source,
-      locStart,
-      locEnd,
-    },
-    paramName: {
-      type: ConcreteNodeTypes.TextNode,
-      value: function (nodes: Node[]) {
-        return nodes[0].sourceString.trim();
-      },
-      source,
-      locStart,
-      locEnd,
-    },
-    paramDescription: {
-      type: ConcreteNodeTypes.TextNode,
-      value: function (nodes: Node[]) {
-        return nodes[0].sourceString.trim();
-      },
-      source,
-      locStart,
-      locEnd,
-    },
-    fallbackNode: {
-      type: ConcreteNodeTypes.TextNode,
-      value: function () {
-        return (this as any).sourceString.trim();
-      },
-      locStart,
-      locEnd,
-      source,
+      paramDescription: 8,
     },
+    paramType: 2,
+    paramTypeContent: textNode,
+    paramName: textNode,
+    paramDescription: textNode,
+    fallbackNode: textNode,
   };
 
   return toAST(res, LiquidDocMappings);