Skip to content
Merged
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
7 changes: 7 additions & 0 deletions .changeset/liquiddoc-string-enums-editor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@shopify/theme-language-server-common': minor
---

Support LiquidDoc string enums in hovers and completions

Allowed values are kept through assignments, and `render`, `content_for` and `block` parameter completions insert the first one. The `default` filter widens enum values to `string`.
7 changes: 7 additions & 0 deletions .changeset/liquiddoc-string-enums.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@shopify/theme-check-common': minor
---

Support string enums in LiquidDoc parameter types

`@param` accepts unions of string literals, such as `{'heading' | 'small'}`, and literal arguments to `render`, `content_for` and `block` are checked against them.
5 changes: 5 additions & 0 deletions .changeset/quiet-doc-delimiters.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/liquid-html-parser': patch
---

Preserve quoted braces in LiquidDoc parameter types and leave incomplete string annotations unparsed so they cannot consume subsequent parameters.
17 changes: 17 additions & 0 deletions packages/liquid-html-parser/src/ast.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1825,6 +1825,23 @@ describe('Unit: Stage 2 (AST)', () => {
expectPath(ast, 'children.0.markup.1.children.0.children.1.markup.name').to.eql('var3');
});

it('should preserve string enum doc types in Liquid and HTML documents', () => {
const paramType = "'Heading' | 'a|b' | '}' | ''";
const source = `{% doc %}\n @param {${paramType}} [variant] - Shared text style\n{% enddoc %}`;
for (const toAST of [toLiquidAST, toLiquidHtmlAST]) {
const expectPosition = makeExpectPosition(toAST.name);
ast = toAST(source);
expectPath(ast, 'children.0.body.nodes.0.paramType.type').toEqual('TextNode');
expectPath(ast, 'children.0.body.nodes.0.paramType.value').toEqual(paramType);
expectPosition(ast, 'children.0.body.nodes.0.paramType').toEqual(paramType);
expectPosition(ast, 'children.0.body.nodes.0.paramName').toEqual('variant');
expectPath(ast, 'children.0.body.nodes.0.required').toEqual(false);
expectPath(ast, 'children.0.body.nodes.0.paramDescription.value').toEqual(
'Shared text style',
);
}
});

it(`should parse doc tags`, () => {
ast = toLiquidAST(`{% doc %}{% enddoc %}`);
expectPath(ast, 'children.0.type').to.eql('LiquidRawTag');
Expand Down
74 changes: 74 additions & 0 deletions packages/liquid-html-parser/src/liquid-doc/parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,80 @@ describe('Unit: liquid-doc parser', () => {
expect(param.required).toBe(false);
});

it.each([
"'heading' | 'small'",
'"heading"|"small"',
'\'Heading\' | "small"',
"'' | ' ' | 'two spaces'",
"'a|b' | 'small'",
"'}' | '{' | '{}'",
"'a}b' | 'small'",
"'a} [variant] - b'",
"'a} [variant] Users'",
"'a} variant - Users'",
'"it\'s" | \'"quoted"\'',
String.raw`'backslash\' | 'small'`,
" 'heading' \t|\t 'small' ",
])('preserves string enum type %s and its source positions', (paramType) => {
const nodes = parseDoc(`\n @param {${paramType}} [variant] - Shared text style\n`);
expect(nodes).toHaveLength(1);
const param = asParam(nodes[0]);
expect(param.paramType!.value).toBe(paramType);
expect(param.paramName.value).toBe('variant');
expect(param.required).toBe(false);
expect(param.paramDescription!.value).toBe('Shared text style');
for (const text of [param.paramType!, param.paramName, param.paramDescription!]) {
expect(param.source.slice(text.position.start, text.position.end)).toBe(text.value);
}
expect(param.source[param.paramType!.position.start - 1]).toBe('{');
expect(param.source[param.paramType!.position.end]).toBe('}');
});

it('parses required enum parameters with multiline descriptions and CRLF', () => {
const nodes = parseDoc(
"\r\n@param {'heading' | 'small'} variant - Shared style\r\n for text.\r\n@param {string} next\r\n",
);
expect(nodes).toHaveLength(2);
const param = asParam(nodes[0]);
expect(param.paramType!.value).toBe("'heading' | 'small'");
expect(param.paramName.value).toBe('variant');
expect(param.required).toBe(true);
expect(param.paramDescription!.value).toContain('for text.');
expect(asParam(nodes[1]).paramName.value).toBe('next');
});

it.each([
"'heading' |",
"| 'heading'",
"'heading' || 'small'",
"'heading' 'small'",
"'a}b' trailing",
])('preserves malformed enum type %s for semantic validation', (paramType) => {
const description = "It's {the shared style}.";
const nodes = parseDoc(
`\n@param {${paramType}} [variant] - ${description}\n@param {number} next\n`,
);
expect(nodes).toHaveLength(2);
const param = asParam(nodes[0]);
expect(param.paramType!.value).toBe(paramType);
expect(param.paramName.value).toBe('variant');
expect(param.required).toBe(false);
expect(param.paramDescription!.value).toBe(description);
expect(asParam(nodes[1]).paramName.value).toBe('next');
});

it.each(["'heading", "'a}b'", "'a}b' | 'small'"])(
'keeps an unclosed type %s from consuming the next parameter',
(paramType) => {
const nodes = parseDoc(`\n@param {${paramType}\n@param {number} next\n`);
expect(nodes).toHaveLength(2);
const param = asParam(nodes[0]);
expect(param.paramType).toBeNull();
expect(param.paramName.value).toBe('');
expect(asParam(nodes[1]).paramName.value).toBe('next');
},
);

it('parses param with description after dash', () => {
const nodes = parseDoc('\n@param product - The product to display\n');
expect(nodes.length).toBe(1);
Expand Down
35 changes: 28 additions & 7 deletions packages/liquid-html-parser/src/liquid-doc/tokenizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,16 +170,15 @@ export function tokenizeParamContent(text: string, startOffset: number): ParamTo
}

// Type: {type}
TYPE_RE.lastIndex = pos;
if (TYPE_RE.test(text)) {
const end = TYPE_RE.lastIndex;
const typeEnd = readTypeEnd(text, pos);
if (typeEnd !== undefined) {
tokens.push({
type: ParamTokenType.Type,
value: text.slice(pos + 1, end - 1),
value: text.slice(pos + 1, typeEnd - 1),
start: pos + startOffset,
end: end + startOffset,
end: typeEnd + startOffset,
});
pos = end;
pos = typeEnd;
continue;
}

Expand Down Expand Up @@ -247,12 +246,34 @@ export function tokenizeParamContent(text: string, startOffset: number): ParamTo
return tokens;
}

/** Returns the exclusive end of the `{type}` annotation at `start`. */
function readTypeEnd(text: string, start: number): number | undefined {
if (text[start] !== '{') return undefined;

for (let pos = start + 1; pos < text.length; pos++) {
const ch = text[pos];

if (ch === "'" || ch === '"') {
const closeQuote = text.indexOf(ch, pos + 1);
if (closeQuote === -1) {
const typeEnd = text.indexOf('}', pos + 1);
return typeEnd === -1 ? undefined : typeEnd + 1;
}
pos = closeQuote;
continue;
}

if (ch === '}') return pos + 1;
}

return undefined;
}

const ANNOTATION_RE = /@(\w+)/y;
const WHITESPACE_RE = /[ \t]+/y;
const NEWLINE_RE = /\r?\n/y;
/** Matches `@word` appearing after at least one character within a line (mid-line). */
const MIDLINE_ANNOTATION_RE = /.@\w+/g;
const TYPE_RE = /\{([^}]*)\}/y;
const OPTIONAL_NAME_RE = /\[([^\]]*)\]/y;
const WORD_RE = /[\w][\w-]*/y;
const PARAM_WHITESPACE_RE = /[ \t]+/y;
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,17 @@ It should format the param description with a dash separator
@param paramName - param with description
{% enddoc %}

It should preserve string enum types and literal contents
{% doc %}
@param {'Heading' | "small" | '' | 'two spaces' | 'a|b' | '}'} [variant] - Shared text style
{% enddoc %}

It should preserve malformed enum types for linting
{% doc %}
@param {'heading} [variant] - It's {the shared style}.
@param {'heading' |} variant - Shared text style
{% enddoc %}

It should respect the liquidDocParamDash option liquidDocParamDash: false
liquidDocParamDash: false
{% doc %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,17 @@ It should format the param description with a dash separator
@param paramName - param with description
{% enddoc %}

It should preserve string enum types and literal contents
{% doc %}
@param { 'Heading' | "small" | '' | 'two spaces' | 'a|b' | '}' } [variant] - Shared text style
{% enddoc %}

It should preserve malformed enum types for linting
{% doc %}
@param {'heading} [variant] - It's {the shared style}.
@param {'heading' |} variant - Shared text style
{% enddoc %}

It should respect the liquidDocParamDash option liquidDocParamDash: false
{% doc %}
@param paramName param with description
Expand Down
15 changes: 9 additions & 6 deletions packages/theme-check-common/src/block-parameters.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { DocDefinition, LiquidDocParameter } from './liquid-doc/liquidDoc';
import { parseParamTypeSyntax } from './liquid-doc/utils';
import { normalizeNamedParamType, parseStringLiterals } from './liquid-doc/utils';
import { schemaSettingLiquidType } from './schema-settings';
import { hasNoSchemaTag } from './to-schema';
import type { Dependencies, Setting, ThemeBlock } from './types';
Expand All @@ -13,8 +13,8 @@ export const BLOCK_CONTENT_PARAMETER = 'content';
export interface BlockParameter {
name: string;
/**
* The LiquidDoc-syntax type the caller passes, such as `string` or
* `product[]`. Undefined when the declaration is untyped or unmapped.
* The LiquidDoc-syntax type the caller passes, such as `string`, `product[]`
* or `'heading' | 'small'`. Undefined when the declaration is untyped or unmapped.
*/
type?: string;
/**
Expand Down Expand Up @@ -97,8 +97,7 @@ export function resolveBlockParameters(
* omitted or malformed type so callers do not report speculative mismatches.
*/
export function liquidDocType(type: string | null | undefined): string | undefined {
const normalizedType = type?.toLowerCase();
return normalizedType && parseParamTypeSyntax(normalizedType) ? normalizedType : undefined;
return type ? normalizeNamedParamType(type) : undefined;
}

function withLiquidDoc(
Expand All @@ -109,7 +108,11 @@ function withLiquidDoc(

return {
name: liquidDoc.name,
type: liquidDocType(liquidDoc.type),
// String literal types keep their spelling, since arguments must match them exactly.
type:
liquidDoc.type && parseStringLiterals(liquidDoc.type)
? liquidDoc.type.trim()
: liquidDocType(liquidDoc.type),
required: liquidDoc.required,
liquidDoc,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,19 @@ describe('ValidBlockArgumentTypes', () => {
expect(offenses).toEqual([]);
});

it('keeps the schema type of a setting that LiquidDoc declares as a string enum', async () => {
const block = blockSource(
[{ id: 'variant', type: 'text' }],
["@param {'heading' | 'small'} [variant] - Variant"],
);

expect(await definitions(block)).toEqual([]);
expect(await run("{% block 'card', variant: 'body' %}{% endblock %}", block)).toEqual([]);
expect(await run("{% block 'card', variant: 42 %}{% endblock %}", block)).toMatchObject([
{ message: "Type mismatch for argument 'variant': expected string, got number" },
]);
});

it('does not speculate about omitted LiquidDoc or unmapped schema types', async () => {
const offenses = await definitions(
blockSource([{ id: 'item', type: 'metaobject' }], ['@param [item] - Item']),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,15 @@ import {
type BlockParameters,
liquidDocType,
} from '../../block-parameters';
import { BasicParamTypes, inferArgumentType, isTypeCompatible } from '../../liquid-doc/utils';
import { generateTypeMismatchSuggestions } from '../../liquid-doc/arguments';
import {
BasicParamTypes,
checkArgumentType,
getArgumentTypeMismatchMessage,
inferArgumentType,
isTypeCompatible,
parseStringLiterals,
} from '../../liquid-doc/utils';
import * as path from '../../path';
import { isBlock } from '../../to-schema';
import { Severity, SourceCodeType, type Context, type LiquidCheckDefinition } from '../../types';
Expand Down Expand Up @@ -50,6 +58,11 @@ export const ValidBlockArgumentTypes: LiquidCheckDefinition = {

for (const argument of markup.args) {
const expectedType = parameters.get(argument.name)?.type;
if (expectedType && parseStringLiterals(expectedType)) {
reportStringLiteralMismatch(context, argument, expectedType);
continue;
}

const actualType = literalType(argument.value);
if (!expectedType || !actualType || isCallTypeCompatible(expectedType, actualType)) {
continue;
Expand Down Expand Up @@ -103,6 +116,23 @@ function isCallTypeCompatible(expectedType: string, actualType: string): boolean
return isTypeCompatible(expectedType, actualType as BasicParamTypes);
}

/** String literal types accept only their own values, so literals are compared by value. */
function reportStringLiteralMismatch(
context: Context<SourceCodeType.LiquidHtml>,
argument: BlockMarkup['args'][number],
expectedType: string,
): void {
if (checkArgumentType(expectedType, argument.value).kind !== 'incompatible') return;

const { start, end } = argument.value.position;
context.report({
message: getArgumentTypeMismatchMessage(argument.name, expectedType, argument.value),
startIndex: start,
endIndex: end,
suggest: generateTypeMismatchSuggestions(expectedType, start, end),
});
}

function reportLiquidDocTypeMismatch(
context: Context<SourceCodeType.LiquidHtml>,
node: LiquidDocParamNode,
Expand Down
Loading
Loading