Merge pull request #665 from Shopify/jm/liquid_doc_param_description

Add parsing + prettier support for param name and description in doc tags

packages/liquid-html-parser/grammar/liquid-html.ohm

@@ -395,8 +395,9 @@ LiquidDoc <: Helpers {
     | fallbackNode
 
   fallbackNode = "@" anyExceptStar<newline>
-  paramNode = "@param" space* paramNodeName
-  paramNodeName = anyExceptStar<newline>
+  paramNode = "@param" space* (paramName)? (space+ (paramDescription))?
+  paramName = identifierCharacter+
+  paramDescription = (~newline identifierCharacter | space)+
 }
 
 LiquidHTML <: Liquid {

packages/liquid-html-parser/src/stage-1-cst.spec.ts

@@ -981,58 +981,97 @@ describe('Unit: Stage 1 (CST)', () => {
           expectPath(cst, '0.blockEndLocEnd').to.equal(testStr.length);
         }
       });
+    });
 
-      it('should parse doc tags', () => {
-        for (const { toCST, expectPath } of testCases) {
-          const testStr = `{% doc -%} 
-            @param asdf
-            @unsupported
-            {%- enddoc %}`;
-
+    describe('Case: LiquidDoc', () => {
+      for (const { toCST, expectPath } of testCases) {
+        it('should parse basic doc tag structure', () => {
+          const testStr = `{% doc -%} content {%- enddoc %}`;
           cst = toCST(testStr);
 
           expectPath(cst, '0.type').to.equal('LiquidRawTag');
           expectPath(cst, '0.name').to.equal('doc');
-          expectPath(cst, '0.body').to.include('@param asdf');
           expectPath(cst, '0.whitespaceStart').to.equal('');
           expectPath(cst, '0.whitespaceEnd').to.equal('-');
           expectPath(cst, '0.delimiterWhitespaceStart').to.equal('-');
           expectPath(cst, '0.delimiterWhitespaceEnd').to.equal('');
-          expectPath(cst, '0.blockStartLocStart').to.equal(0);
-          expectPath(cst, '0.blockStartLocEnd').to.equal(0 + '{% doc -%}'.length);
+          expectPath(cst, '0.blockStartLocStart').to.equal(testStr.indexOf('{% doc -%}'));
+          expectPath(cst, '0.blockStartLocEnd').to.equal(
+            testStr.indexOf('{% doc -%}') + '{% doc -%}'.length,
+          );
           expectPath(cst, '0.blockEndLocStart').to.equal(testStr.length - '{%- enddoc %}'.length);
           expectPath(cst, '0.blockEndLocEnd').to.equal(testStr.length);
+        });
+
+        it('should parse @param with no name or description', () => {
+          const testStr = `{% doc %} @param {% enddoc %}`;
+          cst = toCST(testStr);
 
           expectPath(cst, '0.children.0.type').to.equal('LiquidDocParamNode');
-          expectPath(cst, '0.children.0.locStart').to.equal(testStr.indexOf('@param'));
-          expectPath(cst, '0.children.0.locEnd').to.equal(testStr.indexOf('asdf') + 'asdf'.length);
+          expectPath(cst, '0.children.0.value').to.equal('@param');
+          expectPath(cst, '0.children.0.paramName.type').to.equal('TextNode');
+          expectPath(cst, '0.children.0.paramName.value').to.equal('');
+          expectPath(cst, '0.children.0.paramDescription.type').to.equal('TextNode');
+          expectPath(cst, '0.children.0.paramDescription.value').to.equal('');
+        });
 
-          expectPath(cst, '0.children.1.type').to.equal('TextNode');
-          expectPath(cst, '0.children.1.locStart').to.equal(testStr.indexOf('@unsupported'));
-          expectPath(cst, '0.children.1.locEnd').to.equal(
-            testStr.indexOf('@unsupported') + '@unsupported'.length,
+        it('should parse @param with name but no description', () => {
+          const testStr = `{% doc %} @param paramWithNoDescription {% enddoc %}`;
+          cst = toCST(testStr);
+
+          expectPath(cst, '0.children.0.type').to.equal('LiquidDocParamNode');
+          expectPath(cst, '0.children.0.paramName.type').to.equal('TextNode');
+          expectPath(cst, '0.children.0.paramName.value').to.equal('paramWithNoDescription');
+          expectPath(cst, '0.children.0.paramName.locStart').to.equal(
+            testStr.indexOf('paramWithNoDescription'),
           );
-        }
-      });
+          expectPath(cst, '0.children.0.paramName.locEnd').to.equal(
+            testStr.indexOf('paramWithNoDescription') + 'paramWithNoDescription'.length,
+          );
+          expectPath(cst, '0.children.0.paramDescription.type').to.equal('TextNode');
+          expectPath(cst, '0.children.0.paramDescription.value').to.equal('');
+        });
 
-      it('should parse tag open / close', () => {
-        BLOCKS.forEach((block: string) => {
-          for (const { toCST, expectPath } of testCases) {
-            cst = toCST(`{% ${block} args -%}{%- end${block} %}`);
-            expectPath(cst, '0.type').to.equal('LiquidTagOpen', block);
-            expectPath(cst, '0.name').to.equal(block);
-            expectPath(cst, '0.whitespaceStart').to.equal(null);
-            expectPath(cst, '0.whitespaceEnd').to.equal('-');
-            if (!NamedTags.hasOwnProperty(block)) {
-              expectPath(cst, '0.markup').to.equal('args');
-            }
-            expectPath(cst, '1.type').to.equal('LiquidTagClose');
-            expectPath(cst, '1.name').to.equal(block);
-            expectPath(cst, '1.whitespaceStart').to.equal('-');
-            expectPath(cst, '1.whitespaceEnd').to.equal(null);
-          }
+        it('should parse @param with name and description', () => {
+          const testStr = `{% doc %} @param paramWithDescription param with description {% enddoc %}`;
+          cst = toCST(testStr);
+
+          expectPath(cst, '0.children.0.type').to.equal('LiquidDocParamNode');
+          expectPath(cst, '0.children.0.paramName.type').to.equal('TextNode');
+          expectPath(cst, '0.children.0.paramName.value').to.equal('paramWithDescription');
+          expectPath(cst, '0.children.0.paramDescription.type').to.equal('TextNode');
+          expectPath(cst, '0.children.0.paramDescription.value').to.equal('param with description');
         });
-      });
+
+        it('should parse unsupported doc tags as text nodes', () => {
+          const testStr = `{% doc %} @unsupported this tag is not supported {% enddoc %}`;
+          cst = toCST(testStr);
+
+          expectPath(cst, '0.children.0.type').to.equal('TextNode');
+          expectPath(cst, '0.children.0.value').to.equal('@unsupported this tag is not supported');
+        });
+
+        it('should parse multiple doc tags in sequence', () => {
+          const testStr = `{% doc %}
+          @param param1 first parameter
+          @param param2 second parameter
+          @unsupported
+        {% enddoc %}`;
+
+          cst = toCST(testStr);
+
+          expectPath(cst, '0.children.0.type').to.equal('LiquidDocParamNode');
+          expectPath(cst, '0.children.0.paramName.value').to.equal('param1');
+          expectPath(cst, '0.children.0.paramDescription.value').to.equal('first parameter');
+
+          expectPath(cst, '0.children.1.type').to.equal('LiquidDocParamNode');
+          expectPath(cst, '0.children.1.paramName.value').to.equal('param2');
+          expectPath(cst, '0.children.1.paramDescription.value').to.equal('second parameter');
+
+          expectPath(cst, '0.children.2.type').to.equal('TextNode');
+          expectPath(cst, '0.children.2.value').to.equal('@unsupported');
+        });
+      }
     });
   });
 

packages/liquid-html-parser/src/stage-1-cst.ts

@@ -107,10 +107,13 @@ export interface ConcreteBasicNode<T> {
   locEnd: number;
 }
 
+// todo: change param and description to concrete nodes
 export interface ConcreteLiquidDocParamNode
   extends ConcreteBasicNode<ConcreteNodeTypes.LiquidDocParamNode> {
   name: string;
   value: string;
+  paramName: ConcreteTextNode;
+  paramDescription: ConcreteTextNode;
 }
 
 export interface ConcreteHtmlNodeBase<T> extends ConcreteBasicNode<T> {
@@ -1329,15 +1332,35 @@ function toLiquidDocAST(source: string, matchingSource: string, offset: number)
     paramNode: {
       type: ConcreteNodeTypes.LiquidDocParamNode,
       name: 0,
-      value: 2,
+      value: 0,
       locStart,
       locEnd,
       source,
+      paramName: function (nodes: Node[]) {
+        const nameNode = nodes[2];
+        return {
+          type: ConcreteNodeTypes.TextNode,
+          value: nameNode.sourceString.trim(),
+          source,
+          locStart: offset + nameNode.source.startIdx,
+          locEnd: offset + nameNode.source.endIdx,
+        };
+      },
+      paramDescription: function (nodes: Node[]) {
+        const descriptionNode = nodes[4];
+        return {
+          type: ConcreteNodeTypes.TextNode,
+          value: descriptionNode.sourceString.trim(),
+          source,
+          locStart: offset + descriptionNode.source.startIdx,
+          locEnd: offset + descriptionNode.source.endIdx,
+        };
+      },
     },
     fallbackNode: {
       type: ConcreteNodeTypes.TextNode,
       value: function () {
-        return (this as any).sourceString;
+        return (this as any).sourceString.trim();
       },
       locStart,
       locEnd,

packages/liquid-html-parser/src/stage-2-ast.spec.ts

@@ -1232,16 +1232,30 @@ describe('Unit: Stage 2 (AST)', () => {
       ast = toLiquidAST(`
         {% doc -%}
         @param asdf
+        @param paramWithDescription param with description
         @unsupported this node falls back to a text node
         {%- enddoc %}
       `);
       expectPath(ast, 'children.0.type').to.eql('LiquidRawTag');
       expectPath(ast, 'children.0.name').to.eql('doc');
       expectPath(ast, 'children.0.body.nodes.0.type').to.eql('LiquidDocParamNode');
       expectPath(ast, 'children.0.body.nodes.0.name').to.eql('@param');
+      expectPath(ast, 'children.0.body.nodes.0.paramName.type').to.eql('TextNode');
+      expectPath(ast, 'children.0.body.nodes.0.paramName.value').to.eql('asdf');
+      expectPath(ast, 'children.0.body.nodes.0.paramDescription.type').to.eql('TextNode');
+      expectPath(ast, 'children.0.body.nodes.0.paramDescription.value').to.eql('');
+
+      expectPath(ast, 'children.0.body.nodes.1.type').to.eql('LiquidDocParamNode');
+      expectPath(ast, 'children.0.body.nodes.1.name').to.eql('@param');
+      expectPath(ast, 'children.0.body.nodes.1.paramName.type').to.eql('TextNode');
+      expectPath(ast, 'children.0.body.nodes.1.paramName.value').to.eql('paramWithDescription');
+      expectPath(ast, 'children.0.body.nodes.1.paramDescription.type').to.eql('TextNode');
+      expectPath(ast, 'children.0.body.nodes.1.paramDescription.value').to.eql(
+        'param with description',
+      );
 
-      expectPath(ast, 'children.0.body.nodes.1.type').to.eql('TextNode');
-      expectPath(ast, 'children.0.body.nodes.1.value').to.eql(
+      expectPath(ast, 'children.0.body.nodes.2.type').to.eql('TextNode');
+      expectPath(ast, 'children.0.body.nodes.2.value').to.eql(
         '@unsupported this node falls back to a text node',
       );
     });

packages/liquid-html-parser/src/stage-2-ast.ts

@@ -758,6 +758,8 @@ export interface TextNode extends ASTNode<NodeTypes.TextNode> {
 export interface LiquidDocParamNode extends ASTNode<NodeTypes.LiquidDocParamNode> {
   name: string;
   value: string;
+  paramDescription: TextNode;
+  paramName: TextNode;
 }
 
 export interface ASTNode<T> {
@@ -1281,6 +1283,18 @@ function buildAst(
           position: position(node),
           source: node.source,
           value: node.value,
+          paramName: {
+            type: NodeTypes.TextNode,
+            value: node.paramName.value,
+            position: position(node.paramName),
+            source: node.paramName.source,
+          },
+          paramDescription: {
+            type: NodeTypes.TextNode,
+            value: node.paramDescription.value,
+            position: position(node.paramDescription),
+            source: node.paramDescription.source,
+          },
         });
         break;
       }

packages/prettier-plugin-liquid/src/printer/print/liquid.ts

@@ -1,4 +1,10 @@
-import { NodeTypes, NamedTags, isBranchedTag, RawMarkup } from '@shopify/liquid-html-parser';
+import {
+  NodeTypes,
+  NamedTags,
+  isBranchedTag,
+  RawMarkup,
+  LiquidDocParamNode,
+} from '@shopify/liquid-html-parser';
 import { Doc, doc } from 'prettier';
 
 import {
@@ -497,7 +503,22 @@ export function printLiquidDoc(
   _args: LiquidPrinterArgs,
 ) {
   const body = path.map((p: any) => print(p), 'nodes');
-  return [indent([hardline, body]), hardline];
+  return [indent([hardline, join(hardline, body)]), hardline];
+}
+
+export function printLiquidDocParam(
+  path: AstPath<LiquidDocParamNode>,
+  _options: LiquidParserOptions,
+  _print: LiquidPrinter,
+  _args: LiquidPrinterArgs,
+): Doc {
+  const node = path.getValue();
+  return [
+    node.name,
+    ' ',
+    node.paramName.value,
+    node.paramDescription.value ? ' ' + node.paramDescription.value : '',
+  ];
 }
 
 function innerLeadingWhitespace(node: LiquidTag | LiquidBranch) {

packages/prettier-plugin-liquid/src/printer/printer-liquid-html.ts

@@ -46,6 +46,7 @@ import {
   printLiquidRawTag,
   printLiquidTag,
   printLiquidVariableOutput,
+  printLiquidDocParam,
 } from './print/liquid';
 import { printClosingTagSuffix, printOpeningTagPrefix } from './print/tag';
 import { bodyLines, hasLineBreakInRange, isEmpty, isTextLikeNode, reindent } from './utils';
@@ -555,7 +556,7 @@ function printNode(
     }
 
     case NodeTypes.LiquidDocParamNode: {
-      return [node.name, ' ', node.value];
+      return printLiquidDocParam(path as AstPath<LiquidDocParamNode>, options, print, args);
     }
 
     default: {

packages/prettier-plugin-liquid/src/test/liquid-doc/fixed.liquid

@@ -1,9 +1,14 @@
 It should indent the body
 {% doc %}
-  @param body
+  @param paramName param with description
 {% enddoc %}
 
 It should not dedent to root
 {% doc %}
-  @param body
+  @param paramName param with description
+{% enddoc %}
+
+It should trim whitespace between nodes
+{% doc %}
+  @param paramName param with description
 {% enddoc %}

packages/prettier-plugin-liquid/src/test/liquid-doc/index.liquid

@@ -1,9 +1,14 @@
 It should indent the body
 {% doc %}
-@param body
+@param paramName param with description
 {% enddoc %}
 
 It should not dedent to root
 {% doc %}
-    @param body
+@param paramName param with description
+{% enddoc %}
+
+It should trim whitespace between nodes
+{% doc %}
+@param paramName       param with description
 {% enddoc %}