Skip to content

Commit

Permalink
Add parsing + prettier support for @param in {% doc %} tags (#646)
Browse files Browse the repository at this point in the history
## What are you adding in this PR?

Closes Shopify/develop-advanced-edits#441
https://share.descript.com/view/sZkEWbd5kAr

Added support for parsing and formatting `@param` tags within Liquid doc blocks. 
- Added a configurable option to prettier `liquidDocParamDash` to format all descriptions with `-` (or not)

## What's next? Any followup issues?

- I'm moving on to completions while Josh is going to add parsing + prettier support for the `@example` tag
- I want to add support for optional params, default values, etc afterwards

## What did you learn?

Creating ohm syntax rules is an art form

## Testing
https://github.com/user-attachments/assets/3a902774-8088-4a00-9f8c-c915aec75fe3

## Before you deploy

- [x] I included a minor bump `changeset`
- [x] My feature is backward compatible
  • Loading branch information
jamesmengo authored Jan 16, 2025
2 parents 8091c60 + ac55577 commit 6ab6856
Show file tree
Hide file tree
Showing 16 changed files with 379 additions and 61 deletions.
7 changes: 7 additions & 0 deletions .changeset/slow-years-float.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@shopify/theme-language-server-common': minor
'@shopify/prettier-plugin-liquid': minor
'@shopify/liquid-html-parser': minor
---

Add prettier support for LiquidDoc {% doc %} tag and @param annotation
18 changes: 17 additions & 1 deletion packages/liquid-html-parser/grammar/liquid-html.ohm
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,23 @@ LiquidStatement <: Liquid {
}

LiquidDoc <: Helpers {
Node := (TextNode)*
Node := (LiquidDocNode | TextNode)*
LiquidDocNode =
| paramNode
| fallbackNode

// 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<endOfParam>
endOfParam = strictSpace* (newline | end)
}

LiquidHTML <: Liquid {
Expand Down
151 changes: 116 additions & 35 deletions packages/liquid-html-parser/src/stage-1-cst.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -981,53 +981,134 @@ 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 -%} Renders loading-spinner. {%- 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('Renders loading-spinner');
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);
expectPath(cst, '0.children').to.deep.equal([
{
locEnd: 35,
locStart: 11,
source: '{% doc -%} Renders loading-spinner. {%- enddoc %}',
type: 'TextNode',
value: 'Renders loading-spinner.',
},
]);
}
});
});

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 not parse @param without a name', () => {
const testStr = `{% doc %} @param {% enddoc %}`;
cst = toCST(testStr);

expectPath(cst, '0.children.0.type').to.equal('TextNode');
expectPath(cst, '0.children.0.value').to.equal('@param');
});
});

it('should parse @param with name', () => {
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 @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 @param with type', () => {
const testStr = `{% doc %} @param {String} paramWithType {% enddoc %}`;
cst = toCST(testStr);

expectPath(cst, '0.children.0.type').to.equal('LiquidDocParamNode');
expectPath(cst, '0.children.0.paramName.value').to.equal('paramWithType');

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.locEnd').to.equal(
testStr.indexOf('String') + 'String'.length,
);
});

it('should strip whitespace around param type for @param annotation', () => {
const testStr = `{% doc %} @param { String } paramWithType {% enddoc %}`;
cst = toCST(testStr);

expectPath(cst, '0.children.0.type').to.equal('LiquidDocParamNode');
expectPath(cst, '0.children.0.paramName.value').to.equal('paramWithType');

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.locEnd').to.equal(
testStr.indexOf('String') + 'String'.length,
);
});

it('should accept punctation inside the param description body', () => {
const testStr = `{% doc %} @param paramName paramDescription - asdf . \`should\` work {% enddoc %}`;
cst = toCST(testStr);

expectPath(cst, '0.children.0.paramDescription.value').to.equal(
'paramDescription - asdf . `should` work',
);
});

it('should parse fallback nodes as text nodes', () => {
const testStr = `{% doc %} @unsupported this should get matched as a fallback node and translated into a text node {% enddoc %}`;
cst = toCST(testStr);

expectPath(cst, '0.children.0.type').to.equal('TextNode');
expectPath(cst, '0.children.0.value').to.equal(
'@unsupported this should get matched as a fallback node and translated into a text node',
);
});

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');
});
}
});
});

Expand Down
50 changes: 41 additions & 9 deletions packages/liquid-html-parser/src/stage-1-cst.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ export enum ConcreteNodeTypes {
PaginateMarkup = 'PaginateMarkup',
RenderVariableExpression = 'RenderVariableExpression',
ContentForNamedArgument = 'ContentForNamedArgument',

LiquidDocParamNode = 'LiquidDocParamNode',
}

export const LiquidLiteralValues = {
Expand All @@ -105,6 +107,14 @@ export interface ConcreteBasicNode<T> {
locEnd: number;
}

export interface ConcreteLiquidDocParamNode
extends ConcreteBasicNode<ConcreteNodeTypes.LiquidDocParamNode> {
name: 'param';
paramName: ConcreteTextNode;
paramDescription: ConcreteTextNode | null;
paramType: ConcreteTextNode | null;
}

export interface ConcreteHtmlNodeBase<T> extends ConcreteBasicNode<T> {
attrList?: ConcreteAttributeNode[];
}
Expand Down Expand Up @@ -431,19 +441,21 @@ export interface ConcreteYamlFrontmatterNode

export type LiquidHtmlConcreteNode =
| ConcreteHtmlNode
| ConcreteLiquidNode
| ConcreteTextNode
| ConcreteYamlFrontmatterNode;
| ConcreteYamlFrontmatterNode
| LiquidConcreteNode;

export type LiquidConcreteNode =
| ConcreteLiquidNode
| ConcreteTextNode
| ConcreteYamlFrontmatterNode;
| ConcreteYamlFrontmatterNode
| LiquidDocConcreteNode;

export type LiquidHtmlCST = LiquidHtmlConcreteNode[];

export type LiquidCST = LiquidConcreteNode[];

export type LiquidDocConcreteNode = ConcreteLiquidDocParamNode;

interface Mapping {
[k: string]: number | TemplateMapping | TopLevelFunctionMapping;
}
Expand Down Expand Up @@ -1304,17 +1316,37 @@ 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;
},
TextNode: textNode,
paramNode: {
type: ConcreteNodeTypes.LiquidDocParamNode,
name: 'param',
locStart,
locEnd,
source,
paramType: 2,
paramName: 4,
paramDescription: 8,
},
paramType: 2,
paramTypeContent: textNode,
paramName: textNode,
paramDescription: textNode,
fallbackNode: textNode,
};

return toAST(res, LiquidDocMappings);
Expand Down
39 changes: 26 additions & 13 deletions packages/liquid-html-parser/src/stage-2-ast.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1229,22 +1229,35 @@ describe('Unit: Stage 2 (AST)', () => {
expectPath(ast, 'children.0.body.type').toEqual('RawMarkup');
expectPath(ast, 'children.0.body.nodes').toEqual([]);

ast = toLiquidAST(`{% doc -%} single line doc {%- enddoc %}`);
expectPath(ast, 'children.0.type').to.eql('LiquidRawTag');
expectPath(ast, 'children.0.name').to.eql('doc');
expectPath(ast, 'children.0.body.value').to.eql(' single line doc ');
expectPath(ast, 'children.0.body.nodes.0.type').toEqual('TextNode');

ast = toLiquidAST(`{% doc -%}
multi line doc
multi line doc
{%- enddoc %}`);
ast = toLiquidAST(`
{% doc -%}
@param asdf
@param {String} paramWithDescription - param with description and \`punctation\`. This is still a valid param 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.value').to.eql(
`multi line doc\n multi line 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 and `punctation`. This is still a valid param description.',
);
expectPath(ast, 'children.0.body.nodes.1.paramType.type').to.eql('TextNode');
expectPath(ast, 'children.0.body.nodes.1.paramType.value').to.eql('String');
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',
);
expectPath(ast, 'children.0.body.nodes.0.type').toEqual('TextNode');
});

it('should parse unclosed tables with assignments', () => {
Expand Down
Loading

0 comments on commit 6ab6856

Please sign in to comment.