Skip to content

Commit a05e072

Browse files
committed
changed error message for trailing pipes, updated regex and added comments for invalidfilter regex
1 parent e4d9c1a commit a05e072

File tree

3 files changed

+27
-22
lines changed

3 files changed

+27
-22
lines changed

packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidFilterName.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,13 @@ async function detectInvalidFilterNameInMarkup(
5757
const filterEndIndex = match.index! + match[0].length;
5858
const afterFilter = trimmedMarkup.slice(filterEndIndex);
5959

60+
// This regex finds invalid trailing characters after a filter name using lookaheads:
61+
// 1. Skip valid syntax like ": parameter" or "| nextfilter"
62+
// 2. Capture any junk characters that shouldn't be there
63+
// 3. Stop before valid boundaries like colons or pipes
64+
// e.g. "upcase xyz" finds "xyz", but "upcase | downcase" is ignored as valid
6065
const invalidSegment = afterFilter.match(
61-
/^(?!\s*(?::|$|\|\s*[a-zA-Z]|\|\s*\||\s*\|\s*(?:[}%]|$|[\r\n])))([^:|]+?)(?=\s*(?::|$|\|))/,
66+
/^(?!\s*(?::|$|\|\s*[a-zA-Z]|\|\s*\||\s*\|\s*(?:[}%]|$)))([^:|]+?)(?=\s*(?::|$|\|))/,
6267
)?.[1];
6368
if (!invalidSegment) {
6469
continue;

packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidPipeSyntax.spec.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ describe('Module: InvalidPipeSyntax', () => {
88
const sourceCode = `{{ 'hello' | upcase | | downcase }}`;
99
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode);
1010
expect(offenses).toHaveLength(1);
11-
expect(offenses[0].message).toContain('Double pipe detected');
11+
expect(offenses[0].message).toContain('Remove extra `|` character(s)');
1212
expect(offenses[0].fix).toBeDefined();
1313

1414
const fixedCode = applyFix(sourceCode, offenses[0]);
@@ -19,7 +19,7 @@ describe('Module: InvalidPipeSyntax', () => {
1919
const sourceCode = `{{ 'hello' | upcase | | downcase }}`;
2020
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode);
2121
expect(offenses).toHaveLength(1);
22-
expect(offenses[0].message).toContain('Double pipe detected');
22+
expect(offenses[0].message).toContain('Remove extra `|` character(s)');
2323
expect(offenses[0].fix).toBeDefined();
2424

2525
const fixedCode = applyFix(sourceCode, offenses[0]);
@@ -30,7 +30,7 @@ describe('Module: InvalidPipeSyntax', () => {
3030
const sourceCode = `{% assign result = 'hello' | upcase | | downcase %}`;
3131
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode);
3232
expect(offenses).toHaveLength(1);
33-
expect(offenses[0].message).toContain('Double pipe detected');
33+
expect(offenses[0].message).toContain('Remove extra `|` character(s)');
3434
expect(offenses[0].fix).toBeDefined();
3535

3636
const fixedCode = applyFix(sourceCode, offenses[0]);
@@ -41,7 +41,7 @@ describe('Module: InvalidPipeSyntax', () => {
4141
const sourceCode = `{% echo 'hello' | upcase | | downcase %}`;
4242
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode);
4343
expect(offenses).toHaveLength(1);
44-
expect(offenses[0].message).toContain('Double pipe detected');
44+
expect(offenses[0].message).toContain('Remove extra `|` character(s)');
4545
expect(offenses[0].fix).toBeDefined();
4646

4747
const fixedCode = applyFix(sourceCode, offenses[0]);
@@ -53,8 +53,8 @@ describe('Module: InvalidPipeSyntax', () => {
5353
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode);
5454
expect(offenses).toHaveLength(2);
5555

56-
expect(offenses[0].message).toContain('Double pipe detected');
57-
expect(offenses[1].message).toContain('Double pipe detected');
56+
expect(offenses[0].message).toContain('Remove extra `|` character(s)');
57+
expect(offenses[1].message).toContain('Remove extra `|` character(s)');
5858

5959
expect(offenses[0].fix).toBeDefined();
6060
expect(offenses[1].fix).toBeDefined();
@@ -66,7 +66,7 @@ describe('Module: InvalidPipeSyntax', () => {
6666
const sourceCode = `{{ 'hello' | upcase | }}`;
6767
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode);
6868
expect(offenses).toHaveLength(1);
69-
expect(offenses[0].message).toContain('Trailing pipe detected');
69+
expect(offenses[0].message).toContain('Remove the trailing `|` character');
7070
expect(offenses[0].fix).toBeDefined();
7171

7272
const fixedCode = applyFix(sourceCode, offenses[0]);
@@ -77,7 +77,7 @@ describe('Module: InvalidPipeSyntax', () => {
7777
const sourceCode = `{% assign result = 'hello' | upcase | %}`;
7878
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode);
7979
expect(offenses).toHaveLength(1);
80-
expect(offenses[0].message).toContain('Trailing pipe detected');
80+
expect(offenses[0].message).toContain('Remove the trailing `|` character');
8181
expect(offenses[0].fix).toBeDefined();
8282

8383
const fixedCode = applyFix(sourceCode, offenses[0]);
@@ -88,7 +88,7 @@ describe('Module: InvalidPipeSyntax', () => {
8888
const sourceCode = `{% echo 'hello' | upcase | %}`;
8989
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode);
9090
expect(offenses).toHaveLength(1);
91-
expect(offenses[0].message).toContain('Trailing pipe detected');
91+
expect(offenses[0].message).toContain('Remove the trailing `|` character');
9292
expect(offenses[0].fix).toBeDefined();
9393

9494
const fixedCode = applyFix(sourceCode, offenses[0]);
@@ -99,8 +99,8 @@ describe('Module: InvalidPipeSyntax', () => {
9999
const sourceCode = `{{ 'hello' | upcase | | }}`;
100100
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode);
101101
expect(offenses).toHaveLength(2); // Double pipe AND trailing pipe
102-
expect(offenses[0].message).toContain('Double pipe detected');
103-
expect(offenses[1].message).toContain('Trailing pipe detected');
102+
expect(offenses[0].message).toContain('Remove extra `|` character(s)');
103+
expect(offenses[1].message).toContain('Remove the trailing `|` character');
104104
expect(offenses[0].fix).toBeDefined();
105105
expect(offenses[1].fix).toBeDefined();
106106
});
@@ -113,9 +113,9 @@ describe('Module: InvalidPipeSyntax', () => {
113113
expect(offenses).toHaveLength(2);
114114

115115
// First should be double pipe
116-
expect(offenses[0].message).toContain('Double pipe detected');
116+
expect(offenses[0].message).toContain('Remove extra `|` character(s)');
117117
// Second should be trailing pipe
118-
expect(offenses[1].message).toContain('Trailing pipe detected');
118+
expect(offenses[1].message).toContain('Remove the trailing `|` character');
119119
});
120120

121121
it('should not interfere with valid filter parameter syntax', async () => {
@@ -131,8 +131,8 @@ describe('Module: InvalidPipeSyntax', () => {
131131
%}`;
132132
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, sourceCode);
133133
expect(offenses).toHaveLength(2);
134-
expect(offenses[0].message).toContain('Double pipe detected');
135-
expect(offenses[1].message).toContain('Trailing pipe detected');
134+
expect(offenses[0].message).toContain('Remove extra `|` character(s)');
135+
expect(offenses[1].message).toContain('Remove the trailing `|` character');
136136
});
137137
});
138138

packages/theme-check-common/src/checks/liquid-html-syntax-error/checks/InvalidPipeSyntax.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ async function detectPipeSyntaxInMarkup(
3737
const problems: Problem<SourceCodeType.LiquidHtml>[] = [];
3838
const trimmedMarkup = markup.trim();
3939

40-
// Check for double pipes
41-
const doublePipePattern = /\|\s*\|\s*/g;
42-
const doublePipeMatches = Array.from(trimmedMarkup.matchAll(doublePipePattern));
40+
// Check for multiple consecutive pipes
41+
const extraPipesPattern = /(\|\s*){2,}/g;
42+
const extraPipesMatches = Array.from(trimmedMarkup.matchAll(extraPipesPattern));
4343

44-
for (const match of doublePipeMatches) {
44+
for (const match of extraPipesMatches) {
4545
const markupStartInSource = node.source.indexOf(markup, node.position.start);
4646
if (markupStartInSource === -1) {
4747
continue;
@@ -50,7 +50,7 @@ async function detectPipeSyntaxInMarkup(
5050
const problemEndInSource = problemStartInSource + match[0].length;
5151

5252
problems.push({
53-
message: `${INVALID_SYNTAX_MESSAGE} Double pipe detected. Remove the extra pipe.`,
53+
message: `${INVALID_SYNTAX_MESSAGE}. Remove extra \`|\` character(s).`,
5454
startIndex: problemStartInSource,
5555
endIndex: problemEndInSource,
5656
fix: (corrector) => {
@@ -73,7 +73,7 @@ async function detectPipeSyntaxInMarkup(
7373
const problemEndInSource = problemStartInSource + trailingPipeMatch[0].length;
7474

7575
problems.push({
76-
message: `${INVALID_SYNTAX_MESSAGE} Trailing pipe detected. Remove the trailing pipe.`,
76+
message: `${INVALID_SYNTAX_MESSAGE}. Remove the trailing \`|\` character.`,
7777
startIndex: problemStartInSource,
7878
endIndex: problemEndInSource,
7979
fix: (corrector) => {

0 commit comments

Comments
 (0)