Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LiquidDoc] Parser support for optional parameters #733

Draft
wants to merge 3 commits into
base: jm/prevent_hover_nonexistent_snippets
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/smart-onions-pump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/liquid-html-parser': minor
---

Add parser support for optional liquiddoc parameters
3 changes: 2 additions & 1 deletion packages/liquid-html-parser/grammar/liquid-html.ohm
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,8 @@ LiquidDoc <: Helpers {
paramNode = "@param" strictSpace* paramType? strictSpace* paramName (strictSpace* "-")? strictSpace* paramDescription
paramType = "{" strictSpace* paramTypeContent strictSpace* "}"
paramTypeContent = anyExceptStar<("}"| strictSpace)>
paramName = identifierCharacter+
paramName = "["? strictSpace* paramNameContent strictSpace* "]"?
paramNameContent = identifierCharacter+
paramDescription = anyExceptStar<endOfParam>
endOfParam = strictSpace* (newline | end)
}
Expand Down
74 changes: 73 additions & 1 deletion packages/liquid-html-parser/src/stage-1-cst.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1011,11 +1011,12 @@ describe('Unit: Stage 1 (CST)', () => {
expectPath(cst, '0.children.0.value').to.equal('@param');
});

it('should parse @param with name', () => {
it('should parse required @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.required').to.equal(true);
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(
Expand All @@ -1028,6 +1029,77 @@ describe('Unit: Stage 1 (CST)', () => {
expectPath(cst, '0.children.0.paramDescription.value').to.equal('');
});

it('should parse an optional @param', () => {
const testStr = `{% doc %}
@param [paramWithNoDescription]
@param [ paramWithWhitespace ]
@param {String} [optionalParam] - The optional param
{% enddoc %}`;
cst = toCST(testStr);

expectPath(cst, '0.children.0.type').to.equal('LiquidDocParamNode');
expectPath(cst, '0.children.0.required').to.equal(false);
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('');

expectPath(cst, '0.children.1.type').to.equal('LiquidDocParamNode');
expectPath(cst, '0.children.1.required').to.equal(false);
expectPath(cst, '0.children.1.paramName.type').to.equal('TextNode');
expectPath(cst, '0.children.1.paramName.value').to.equal('paramWithWhitespace');
expectPath(cst, '0.children.1.paramName.locStart').to.equal(
testStr.indexOf('paramWithWhitespace'),
);
expectPath(cst, '0.children.1.paramName.locEnd').to.equal(
testStr.indexOf('paramWithWhitespace') + 'paramWithWhitespace'.length,
);

expectPath(cst, '0.children.2.type').to.equal('LiquidDocParamNode');
expectPath(cst, '0.children.2.required').to.equal(false);
expectPath(cst, '0.children.2.paramType.value').to.equal('String');
});

it('should parse @param with missing optional delimiters as required', () => {
const testStr = `{% doc %}
@param paramWithMissingHeadDelim]
@param [paramWithMissingTailDelim
{% enddoc %}`;
cst = toCST(testStr);

expectPath(cst, '0.children.0.type').to.equal('LiquidDocParamNode');
expectPath(cst, '0.children.0.required').to.equal(true);
expectPath(cst, '0.children.0.paramName.type').to.equal('TextNode');
expectPath(cst, '0.children.0.paramName.value').to.equal('paramWithMissingHeadDelim');
expectPath(cst, '0.children.0.paramName.locStart').to.equal(
testStr.indexOf('paramWithMissingHeadDelim'),
);
expectPath(cst, '0.children.0.paramName.locEnd').to.equal(
testStr.indexOf('paramWithMissingHeadDelim') + 'paramWithMissingHeadDelim'.length,
);
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('LiquidDocParamNode');
expectPath(cst, '0.children.1.required').to.equal(true);
expectPath(cst, '0.children.1.paramName.type').to.equal('TextNode');
expectPath(cst, '0.children.1.paramName.value').to.equal('paramWithMissingTailDelim');
expectPath(cst, '0.children.1.paramName.locStart').to.equal(
testStr.indexOf('paramWithMissingTailDelim'),
);
expectPath(cst, '0.children.1.paramName.locEnd').to.equal(
testStr.indexOf('paramWithMissingTailDelim') + 'paramWithMissingTailDelim'.length,
);
expectPath(cst, '0.children.1.paramDescription.type').to.equal('TextNode');
expectPath(cst, '0.children.1.paramDescription.value').to.equal('');
});

it('should parse @param with name and description', () => {
const testStr = `{% doc %} @param paramWithDescription param with description {% enddoc %}`;
cst = toCST(testStr);
Expand Down
10 changes: 9 additions & 1 deletion packages/liquid-html-parser/src/stage-1-cst.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export interface ConcreteLiquidDocParamNode
paramName: ConcreteTextNode;
paramDescription: ConcreteTextNode | null;
paramType: ConcreteTextNode | null;
required: boolean;
}

export interface ConcreteHtmlNodeBase<T> extends ConcreteBasicNode<T> {
Expand Down Expand Up @@ -1341,10 +1342,17 @@ function toLiquidDocAST(source: string, matchingSource: string, offset: number)
paramType: 2,
paramName: 4,
paramDescription: 8,
required: function (nodes: Node[]) {
const paramName = nodes[4];
return (
paramName.children[0].sourceString === '' || paramName.children[4].sourceString === ''
);
},
},
paramType: 2,
paramTypeContent: textNode,
paramName: textNode,
paramName: 2,
paramNameContent: textNode,
paramDescription: textNode,
fallbackNode: textNode,
};
Expand Down
29 changes: 22 additions & 7 deletions packages/liquid-html-parser/src/stage-2-ast.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,8 @@ describe('Unit: Stage 2 (AST)', () => {

ast = toLiquidAST(`
{% doc -%}
@param asdf
@param requiredParameter
@param [optionalParameter] - optional parameter description
@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 %}
Expand All @@ -1240,22 +1241,36 @@ describe('Unit: Stage 2 (AST)', () => {
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.required').to.eql(true);
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.paramName.value').to.eql('requiredParameter');
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.required').to.eql(false);
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.paramName.value').to.eql('optionalParameter');
expectPath(ast, 'children.0.body.nodes.1.paramDescription.type').to.eql('TextNode');
expectPath(ast, 'children.0.body.nodes.1.paramDescription.value').to.eql(
'optional parameter description',
);

expectPath(ast, 'children.0.body.nodes.2.type').to.eql('LiquidDocParamNode');
expectPath(ast, 'children.0.body.nodes.2.name').to.eql('param');
expectPath(ast, 'children.0.body.nodes.2.required').to.eql(true);
expectPath(ast, 'children.0.body.nodes.2.paramName.type').to.eql('TextNode');
expectPath(ast, 'children.0.body.nodes.2.paramName.value').to.eql('paramWithDescription');
expectPath(ast, 'children.0.body.nodes.2.paramDescription.type').to.eql('TextNode');
expectPath(ast, 'children.0.body.nodes.2.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(
expectPath(ast, 'children.0.body.nodes.2.paramType.type').to.eql('TextNode');
expectPath(ast, 'children.0.body.nodes.2.paramType.value').to.eql('String');

expectPath(ast, 'children.0.body.nodes.3.type').to.eql('TextNode');
expectPath(ast, 'children.0.body.nodes.3.value').to.eql(
'@unsupported this node falls back to a text node',
);
});
Expand Down
5 changes: 4 additions & 1 deletion packages/liquid-html-parser/src/stage-2-ast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ export interface TextNode extends ASTNode<NodeTypes.TextNode> {
value: string;
}

/** Represents a `@param` node in a LiquidDoc comment - `@param paramName {paramType} - paramDescription` */
/** Represents a `@param` node in a LiquidDoc comment - `@param {paramType} [paramName] - paramDescription` */
export interface LiquidDocParamNode extends ASTNode<NodeTypes.LiquidDocParamNode> {
name: 'param';
/** The name of the parameter (e.g. "product") */
Expand All @@ -764,6 +764,8 @@ export interface LiquidDocParamNode extends ASTNode<NodeTypes.LiquidDocParamNode
paramDescription: TextNode | null;
/** Optional type annotation for the parameter (e.g. "{string}", "{number}") */
paramType: TextNode | null;
/** Whether this parameter must be passed when using the snippet */
required: boolean;
}
export interface ASTNode<T> {
/**
Expand Down Expand Up @@ -1293,6 +1295,7 @@ function buildAst(
},
paramDescription: toNullableTextNode(node.paramDescription),
paramType: toNullableTextNode(node.paramType),
required: node.required,
});
break;
}
Expand Down
Loading