Skip to content

Commit

Permalink
Add content_for tag parser support + ValidContentForArguments check
Browse files Browse the repository at this point in the history
Fixes #466
  • Loading branch information
charlespwd committed Nov 7, 2024
1 parent 6af0983 commit 3030bdc
Show file tree
Hide file tree
Showing 29 changed files with 489 additions and 62 deletions.
7 changes: 7 additions & 0 deletions .changeset/purple-pears-report.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@shopify/theme-check-browser': minor
'@shopify/theme-check-common': minor
'@shopify/theme-check-node': minor
---

Add the `ValidContentForArguments` check
7 changes: 7 additions & 0 deletions .changeset/tasty-mugs-behave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@shopify/liquid-html-parser': minor
'@shopify/prettier-plugin-liquid': minor
'@shopify/theme-language-server-common': minor
---

Add support for the `content_for` Liquid tag
6 changes: 6 additions & 0 deletions packages/liquid-html-parser/grammar/liquid-html.ohm
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ Liquid <: Helpers {
| liquidTagBreak
| liquidTagContinue
| liquidTagCycle
| liquidTagContentFor
| liquidTagDecrement
| liquidTagEcho
| liquidTagElse
Expand Down Expand Up @@ -125,6 +126,11 @@ Liquid <: Helpers {
liquidTagLiquid = liquidTagRule<"liquid", liquidTagLiquidMarkup>
liquidTagLiquidMarkup = tagMarkup

liquidTagContentFor = liquidTagRule<"content_for", liquidTagContentForMarkup>
liquidTagContentForMarkup =
contentForType (argumentSeparatorOptionalComma tagArguments) (space* ",")? space*
contentForType = liquidString<delimTag>

liquidTagInclude = liquidTagRule<"include", liquidTagRenderMarkup>
liquidTagRender = liquidTagRule<"render", liquidTagRenderMarkup>
liquidTagRenderMarkup =
Expand Down
39 changes: 39 additions & 0 deletions packages/liquid-html-parser/src/stage-1-cst.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,45 @@ describe('Unit: Stage 1 (CST)', () => {
expectPath(cst, '0.markup.expression.lookups').to.eql([]);
}
});

it('should parse content_for "blocks"', () => {
for (const { toCST, expectPath } of testCases) {
cst = toCST(`{% content_for "blocks" -%}`);
expectPath(cst, '0.type').to.equal('LiquidTag');
expectPath(cst, '0.name').to.equal('content_for');
expectPath(cst, '0.markup.type').to.equal('ContentForMarkup');
expectPath(cst, '0.markup.contentForType.type').to.equal('String');
expectPath(cst, '0.markup.contentForType.value').to.equal('blocks');
expectPath(cst, '0.markup.contentForType.single').to.equal(false);
expectPath(cst, '0.markup.args').to.have.lengthOf(0);
expectPath(cst, '0.whitespaceStart').to.equal(null);
expectPath(cst, '0.whitespaceEnd').to.equal('-');
}
});

it('should parse content_for "block", id: "my-id", type: "my-block"', () => {
for (const { toCST, expectPath } of testCases) {
cst = toCST(`{% content_for "block", id: "block-id", type: "block-type" -%}`);
expectPath(cst, '0.type').to.equal('LiquidTag');
expectPath(cst, '0.name').to.equal('content_for');
expectPath(cst, '0.markup.type').to.equal('ContentForMarkup');
expectPath(cst, '0.markup.contentForType.type').to.equal('String');
expectPath(cst, '0.markup.contentForType.value').to.equal('block');
expectPath(cst, '0.markup.contentForType.single').to.equal(false);
expectPath(cst, '0.markup.args').to.have.lengthOf(2);
const namedArguments = [
{ name: 'id', valueType: 'String' },
{ name: 'type', valueType: 'String' },
];
namedArguments.forEach(({ name, valueType }, i) => {
expectPath(cst, `0.markup.args.${i}.type`).to.equal('NamedArgument');
expectPath(cst, `0.markup.args.${i}.name`).to.equal(name);
expectPath(cst, `0.markup.args.${i}.value.type`).to.equal(valueType);
});
expectPath(cst, '0.whitespaceStart').to.equal(null);
expectPath(cst, '0.whitespaceEnd').to.equal('-');
}
});
});

describe('Case: LiquidTagOpen', () => {
Expand Down
22 changes: 22 additions & 0 deletions packages/liquid-html-parser/src/stage-1-cst.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export enum ConcreteNodeTypes {
Condition = 'Condition',

AssignMarkup = 'AssignMarkup',
ContentForMarkup = 'ContentForMarkup',
CycleMarkup = 'CycleMarkup',
ForMarkup = 'ForMarkup',
RenderMarkup = 'RenderMarkup',
Expand Down Expand Up @@ -265,6 +266,7 @@ export type ConcreteLiquidTag = ConcreteLiquidTagNamed | ConcreteLiquidTagBaseCa
export type ConcreteLiquidTagNamed =
| ConcreteLiquidTagAssign
| ConcreteLiquidTagCycle
| ConcreteLiquidTagContentFor
| ConcreteLiquidTagEcho
| ConcreteLiquidTagIncrement
| ConcreteLiquidTagDecrement
Expand Down Expand Up @@ -321,11 +323,20 @@ export interface ConcreteLiquidTagCycleMarkup
args: ConcreteLiquidExpression[];
}

export interface ConcreteLiquidTagContentFor
extends ConcreteLiquidTagNode<NamedTags.content_for, ConcreteLiquidTagContentForMarkup> {}

export interface ConcreteLiquidTagRender
extends ConcreteLiquidTagNode<NamedTags.render, ConcreteLiquidTagRenderMarkup> {}
export interface ConcreteLiquidTagInclude
extends ConcreteLiquidTagNode<NamedTags.include, ConcreteLiquidTagRenderMarkup> {}

export interface ConcreteLiquidTagContentForMarkup
extends ConcreteBasicNode<ConcreteNodeTypes.ContentForMarkup> {
contentForType: ConcreteStringLiteral;
args: ConcreteLiquidNamedArgument[];
}

export interface ConcreteLiquidTagRenderMarkup
extends ConcreteBasicNode<ConcreteNodeTypes.RenderMarkup> {
snippet: ConcreteStringLiteral | ConcreteLiquidVariableLookup;
Expand Down Expand Up @@ -715,6 +726,7 @@ function toCST<T>(
liquidTagBaseCase: 0,
liquidTagAssign: 0,
liquidTagEcho: 0,
liquidTagContentFor: 0,
liquidTagCycle: 0,
liquidTagIncrement: 0,
liquidTagDecrement: 0,
Expand Down Expand Up @@ -775,6 +787,16 @@ function toCST<T>(
source,
},

liquidTagContentForMarkup: {
type: ConcreteNodeTypes.ContentForMarkup,
contentForType: 0,
args: 2,
locStart,
locEnd,
source,
},
contentForType: 0,

liquidTagRenderMarkup: {
type: ConcreteNodeTypes.RenderMarkup,
snippet: 0,
Expand Down
41 changes: 41 additions & 0 deletions packages/liquid-html-parser/src/stage-2-ast.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,47 @@ describe('Unit: Stage 2 (AST)', () => {
});
});
});

describe('Case: content_for', () => {
it('should parse content_for tags with no arguments', () => {
for (const { toAST, expectPath, expectPosition } of testCases) {
ast = toAST(`{% content_for "blocks" %}`);
expectPath(ast, 'children.0.type').to.equal('LiquidTag');
expectPath(ast, 'children.0.name').to.equal('content_for');
expectPath(ast, 'children.0.markup.type').to.equal('ContentForMarkup');
expectPath(ast, 'children.0.markup.contentForType.type').to.equal('String');
expectPath(ast, 'children.0.markup.contentForType.value').to.equal('blocks');
expectPosition(ast, 'children.0');
expectPosition(ast, 'children.0.markup');
}
});

it('should parse content_for named expression arguments', () => {
for (const { toAST, expectPath, expectPosition } of testCases) {
ast = toAST(`{% content_for "snippet", s: 'string', n: 10, r: (1..2), v: variable %}`);
expectPath(ast, 'children.0.type').to.equal('LiquidTag');
expectPath(ast, 'children.0.name').to.equal('content_for');
expectPath(ast, 'children.0.markup.type').to.equal('ContentForMarkup');
expectPath(ast, 'children.0.markup.contentForType.type').to.equal('String');
expectPath(ast, 'children.0.markup.contentForType.value').to.equal('snippet');
expectPath(ast, 'children.0.markup.args').to.have.lengthOf(4);
expectPath(ast, 'children.0.markup.args.0.type').to.equal('NamedArgument');
expectPath(ast, 'children.0.markup.args.0.name').to.equal('s');
expectPath(ast, 'children.0.markup.args.0.value.type').to.equal('String');
expectPath(ast, 'children.0.markup.args.1.type').to.equal('NamedArgument');
expectPath(ast, 'children.0.markup.args.1.name').to.equal('n');
expectPath(ast, 'children.0.markup.args.1.value.type').to.equal('Number');
expectPath(ast, 'children.0.markup.args.2.type').to.equal('NamedArgument');
expectPath(ast, 'children.0.markup.args.2.name').to.equal('r');
expectPath(ast, 'children.0.markup.args.2.value.type').to.equal('Range');
expectPath(ast, 'children.0.markup.args.3.type').to.equal('NamedArgument');
expectPath(ast, 'children.0.markup.args.3.name').to.equal('v');
expectPath(ast, 'children.0.markup.args.3.value.type').to.equal('VariableLookup');
expectPosition(ast, 'children.0');
expectPosition(ast, 'children.0.markup');
}
});
});
});

it(`should parse liquid inline comments`, () => {
Expand Down
33 changes: 33 additions & 0 deletions packages/liquid-html-parser/src/stage-2-ast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ import {
ConcreteLiquidRawTag,
LiquidHtmlConcreteNode,
ConcreteLiquidTagBaseCase,
ConcreteLiquidTagContentForMarkup,
} from './stage-1-cst';
import { Comparators, NamedTags, NodeTypes, nonTraversableProperties, Position } from './types';
import { assertNever, deepGet, dropLast } from './utils';
Expand All @@ -97,6 +98,7 @@ export type LiquidHtmlNode =
| LiquidFilter
| LiquidNamedArgument
| AssignMarkup
| ContentForMarkup
| CycleMarkup
| ForMarkup
| RenderMarkup
Expand Down Expand Up @@ -188,6 +190,7 @@ export type LiquidTagNamed =
| LiquidTagAssign
| LiquidTagCase
| LiquidTagCapture
| LiquidTagContentFor
| LiquidTagCycle
| LiquidTagDecrement
| LiquidTagEcho
Expand Down Expand Up @@ -359,6 +362,10 @@ export interface PaginateMarkup extends ASTNode<NodeTypes.PaginateMarkup> {
args: LiquidNamedArgument[];
}

/** https://shopify.dev/docs/api/liquid/tags#content_for */
export interface LiquidTagContentFor
extends LiquidTagNode<NamedTags.content_for, ContentForMarkup> {}

/** https://shopify.dev/docs/api/liquid/tags#render */
export interface LiquidTagRender extends LiquidTagNode<NamedTags.render, RenderMarkup> {}

Expand All @@ -377,6 +384,14 @@ export interface LiquidTagLayout extends LiquidTagNode<NamedTags.layout, LiquidE
/** https://shopify.dev/docs/api/liquid/tags#liquid */
export interface LiquidTagLiquid extends LiquidTagNode<NamedTags.liquid, LiquidStatement[]> {}

/** {% content_for 'contentForType' [...namedArguments] %} */
export interface ContentForMarkup extends ASTNode<NodeTypes.ContentForMarkup> {
/** {% content_for 'contentForType' %} */
contentForType: LiquidString;
/** {% content_for 'contentForType', arg1: value1, arg2: value2 %} */
args: LiquidNamedArgument[];
}

/** {% render 'snippet' [(with|for) variable [as alias]], [...namedArguments] %} */
export interface RenderMarkup extends ASTNode<NodeTypes.RenderMarkup> {
/** {% render snippet %} */
Expand Down Expand Up @@ -1408,6 +1423,14 @@ function toNamedLiquidTag(
};
}

case NamedTags.content_for: {
return {
...liquidTagBaseAttributes(node),
name: node.name,
markup: toContentForMarkup(node.markup),
};
}

case NamedTags.include:
case NamedTags.render: {
return {
Expand Down Expand Up @@ -1684,6 +1707,16 @@ function toRawMarkupKindFromLiquidNode(node: ConcreteLiquidRawTag): RawMarkupKin
}
}

function toContentForMarkup(node: ConcreteLiquidTagContentForMarkup): ContentForMarkup {
return {
type: NodeTypes.ContentForMarkup,
contentForType: toExpression(node.contentForType) as LiquidString,
args: node.args.map(toNamedArgument),
position: position(node),
source: node.source,
};
}

function toRenderMarkup(node: ConcreteLiquidTagRenderMarkup): RenderMarkup {
return {
type: NodeTypes.RenderMarkup,
Expand Down
2 changes: 2 additions & 0 deletions packages/liquid-html-parser/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export enum NodeTypes {
LogicalExpression = 'LogicalExpression',

AssignMarkup = 'AssignMarkup',
ContentForMarkup = 'ContentForMarkup',
CycleMarkup = 'CycleMarkup',
ForMarkup = 'ForMarkup',
PaginateMarkup = 'PaginateMarkup',
Expand All @@ -50,6 +51,7 @@ export enum NamedTags {
assign = 'assign',
capture = 'capture',
case = 'case',
content_for = 'content_for',
cycle = 'cycle',
decrement = 'decrement',
echo = 'echo',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ function getCssDisplay(node: AugmentedNode<WithSiblings>, options: LiquidParserO
case NodeTypes.VariableLookup:
case NodeTypes.AssignMarkup:
case NodeTypes.CycleMarkup:
case NodeTypes.ContentForMarkup:
case NodeTypes.ForMarkup:
case NodeTypes.PaginateMarkup:
case NodeTypes.RenderMarkup:
Expand Down Expand Up @@ -225,6 +226,7 @@ function getNodeCssStyleWhiteSpace(
case NodeTypes.VariableLookup:
case NodeTypes.AssignMarkup:
case NodeTypes.CycleMarkup:
case NodeTypes.ContentForMarkup:
case NodeTypes.ForMarkup:
case NodeTypes.PaginateMarkup:
case NodeTypes.RenderMarkup:
Expand Down
6 changes: 6 additions & 0 deletions packages/prettier-plugin-liquid/src/printer/print/liquid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,12 @@ function printNamedLiquidBlockStart(
]);
}

case NamedTags.content_for: {
const markup = node.markup;
const trailingWhitespace = markup.args.length > 0 ? line : ' ';
return tag(trailingWhitespace);
}

case NamedTags.include:
case NamedTags.render: {
const markup = node.markup;
Expand Down
17 changes: 17 additions & 0 deletions packages/prettier-plugin-liquid/src/printer/printer-liquid-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,23 @@ function printNode(
return doc;
}

case NodeTypes.ContentForMarkup: {
const contentForType = path.call((p: any) => print(p), 'contentForType');
const doc: Doc = [contentForType];
if (node.args.length > 0) {
doc.push(
',',
line,
join(
[',', line],
path.map((p) => print(p), 'args'),
),
);
}

return doc;
}

case NodeTypes.RenderMarkup: {
const snippet = path.call((p: any) => print(p), 'snippet');
const doc: Doc = [snippet];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
It should never break a name
printWidth: 1
{% content_for 'block' %}
{%- content_for 'blocks' -%}

It should break on named args
printWidth: 1
{% content_for 'block',
key1: val1,
key2: (0..1)
%}

It should not require commas (as per syntax) but add them when pretty printed
{% content_for 'block', key1: val1, key2: (0..1) %}

It should not require spaces (as per syntax) but add them when pretty printed
{% content_for 'block', key1: val1, key2: (0..1) %}

It should strip trailing commas by default
{% content_for 'foo' %}
{% content_for 'foo', product: product %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
It should never break a name
printWidth: 1
{% content_for "block" %}
{%- content_for "blocks" -%}

It should break on named args
printWidth: 1
{% content_for "block", key1: val1, key2: (0..1) %}

It should not require commas (as per syntax) but add them when pretty printed
{% content_for "block" key1: val1 key2: (0..1) %}

It should not require spaces (as per syntax) but add them when pretty printed
{%content_for "block",key1:val1,key2:(0..1)%}

It should strip trailing commas by default
{% content_for "foo", %}
{% content_for "foo",product:product, %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { test } from 'vitest';
import { assertFormattedEqualsFixed } from '../test-helpers';

test('Unit: liquid-tag-content-for', async () => {
await assertFormattedEqualsFixed(__dirname);
});
Loading

0 comments on commit 3030bdc

Please sign in to comment.