Skip to content

Commit b0d892b

Browse files
EvilGenius13EvilGenius13
committed
wip/new changes
1 parent cdee0cf commit b0d892b

File tree

2 files changed

+164
-38
lines changed

2 files changed

+164
-38
lines changed

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,29 @@ describe('Module: InvalidConditionalBooleanExpression', () => {
187187
expect(fixed).to.equal('{% if 1 == 2 %}hello{% endif %}');
188188
});
189189

190+
it('should report an offense for malformed comparisons like missing quotes', async () => {
191+
const testCases = [
192+
{
193+
source: "{% if 'wat' == 'squat > 2 %}hello{% endif %}",
194+
description: 'missing closing quote creates trailing comparison',
195+
},
196+
{
197+
source: "{% if 'wat' == 'squat' > 2 %}hello{% endif %}",
198+
description: 'extra comparison after valid comparison',
199+
},
200+
{
201+
source: "{% if price == 'test' != 5 %}hello{% endif %}",
202+
description: 'chained comparisons without logical operators',
203+
},
204+
];
205+
206+
for (const testCase of testCases) {
207+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase.source);
208+
expect(offenses, `Failed for: ${testCase.description}`).to.have.length(1);
209+
expect(offenses[0].message).to.contain('Trailing tokens ignored');
210+
}
211+
});
212+
190213
it('should report an offense for multiple trailing tokens', async () => {
191214
const source = '{% if 10 > 4 baz qux %}hello{% endif %}';
192215
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, source);
@@ -310,4 +333,41 @@ describe('Module: InvalidConditionalBooleanExpression', () => {
310333
expect(offenses).to.have.length(0);
311334
}
312335
});
336+
337+
it('should report an offense for misspelled logical operators', async () => {
338+
const testCases = [
339+
{
340+
source: '{% if "wat" == "squat" adn "wat" == "squat" %}hello{% endif %}',
341+
misspelled: 'adn',
342+
expectedFix: '"wat" == "squat"',
343+
},
344+
{
345+
source: '{% if variable > 5 andd other < 10 %}hello{% endif %}',
346+
misspelled: 'andd',
347+
expectedFix: 'variable > 5',
348+
},
349+
];
350+
351+
for (const testCase of testCases) {
352+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase.source);
353+
expect(offenses).to.have.length(1);
354+
expect(offenses[0].message).to.contain('Trailing tokens ignored after comparison');
355+
356+
const fixed = applyFix(testCase.source, offenses[0]);
357+
expect(fixed).to.contain(testCase.expectedFix);
358+
}
359+
});
360+
361+
it('should NOT report an offense for valid logical operators', async () => {
362+
const validCases = [
363+
'{% if price > 100 and discount < 50 %}hello{% endif %}',
364+
'{% if user.active or user.premium %}hello{% endif %}',
365+
'{% if x == 1 and y == 2 or z == 3 %}hello{% endif %}',
366+
];
367+
368+
for (const testCase of validCases) {
369+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase);
370+
expect(offenses).to.have.length(0);
371+
}
372+
});
313373
});

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

Lines changed: 104 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,34 @@
11
import { LiquidBranch, LiquidTag } from '@shopify/liquid-html-parser';
2-
import { Check, Context, SourceCodeType, Problem } from '../../..';
2+
import { SourceCodeType, Problem } from '../../..';
3+
import { getValuesInMarkup } from './utils';
34

4-
const LeftSideEvaluation =
5-
/^(['"][^'"]*['"]|\d+(?:\.\d+)?|true|false|nil|empty|blank)\s+((?![a-zA-Z_])\S.*)/;
6-
const TrailingJunk = /^(.+?(?:==|!=|>=|<=|>|<|contains)\s+\S+)\s+(.+?)(?:\s+(?:and|or)\s|$)/;
7-
const Comparison = /^\s*(==|!=|>=|<=|>|<|contains)\s/;
8-
const LogicalOperator = /^(?:and|or)\s/;
5+
type TokenType = 'variable' | keyof typeof TOKEN_PATTERNS;
6+
7+
interface Token {
8+
value: string;
9+
type: TokenType;
10+
}
11+
12+
interface ExpressionIssue {
13+
message: string;
14+
fix: string;
15+
}
16+
17+
const TOKEN_PATTERNS = {
18+
logical: /^(and|or)$/,
19+
comparison: /^(==|!=|>=|<=|>|<|contains)$/,
20+
invalid: /^[@#$&]$/,
21+
literal: /^(['"][^'"]*['"]|\d+(?:\.\d+)?|true|false|nil|empty|blank)$/,
22+
} as const;
23+
24+
function classifyToken(value: string): TokenType {
25+
for (const [type, pattern] of Object.entries(TOKEN_PATTERNS)) {
26+
if (pattern.test(value)) {
27+
return type as TokenType;
28+
}
29+
}
30+
return 'variable';
31+
}
932

1033
export function detectInvalidConditionalNode(
1134
node: LiquidBranch | LiquidTag,
@@ -16,7 +39,7 @@ export function detectInvalidConditionalNode(
1639
const markup = node.markup;
1740
if (typeof markup !== 'string' || !markup.trim()) return;
1841

19-
const issue = checkLaxParsing(markup);
42+
const issue = analyzeConditionalExpression(markup);
2043
if (!issue) return;
2144

2245
const openingTagRange = node.blockStartPosition || node.position;
@@ -28,7 +51,7 @@ export function detectInvalidConditionalNode(
2851
const endIndex = startIndex + markup.length;
2952

3053
return {
31-
message: `Liquid lax parsing issue: ${issue.issue}`,
54+
message: `Liquid lax parsing issue: ${issue.message}`,
3255
startIndex,
3356
endIndex,
3457
fix: (corrector) => {
@@ -37,45 +60,88 @@ export function detectInvalidConditionalNode(
3760
};
3861
}
3962

40-
function checkLaxParsing(markup: string): { issue: string; fix: string } | null {
41-
const trimmed = markup.trim();
63+
function isValueToken(token: Token): boolean {
64+
return token.type === 'literal' || token.type === 'variable';
65+
}
4266

43-
// Pattern 1: Left-side evaluation - parser stops at first complete value
44-
// EX: {% if 7 1 > 100 %}hello{% endif %} <-- Lax parsing will ignore everything after 7 so we will remove 1 > 100
45-
const leftSideMatch = LeftSideEvaluation.exec(trimmed);
46-
if (leftSideMatch) {
47-
const [, leftValue, rest] = leftSideMatch;
48-
// Check if the remaining string would create a valid comparison or logical operator
49-
if (!Comparison.test(rest) && !LogicalOperator.test(rest)) {
50-
return {
51-
issue: `Expression stops at truthy value '${leftValue}', ignoring: '${rest}'`,
52-
fix: leftValue,
53-
};
54-
}
55-
}
67+
function isOperatorToken(token: Token): boolean {
68+
return token.type === 'logical' || token.type === 'comparison';
69+
}
5670

57-
// Pattern 2: Malformed expression starting with invalid token
58-
// EX: {% if 2 %}hello{% endif %} is truthy but any special character such as @#! will evaluate to false
59-
const malformedStart = /^([^a-zA-Z_'"\d\s(][^\s]*)/.exec(trimmed);
60-
if (malformedStart) {
71+
function checkInvalidStartingToken(tokens: Token[]): ExpressionIssue | null {
72+
const firstToken = tokens[0];
73+
if (firstToken.type === 'invalid' || firstToken.type === 'comparison') {
6174
return {
62-
issue: `Malformed expression starting with invalid token '${malformedStart[1]}'`,
75+
message: `Malformed expression starting with invalid token '${firstToken.value}'`,
6376
fix: 'false',
6477
};
6578
}
79+
return null;
80+
}
81+
82+
function checkTrailingTokensAfterComparison(tokens: Token[]): ExpressionIssue | null {
83+
const COMPARISON_LENGTH = 3;
84+
const minTokensForTrailing = COMPARISON_LENGTH + 1;
6685

67-
// Pattern 3: Trailing junk after complete comparison
68-
// EX: {% if 1 > 100 foobar %}hello{% endif %} Lax parsing will ignore everything after 1 > 100 so we will remove foobar
69-
const trailingMatch = TrailingJunk.exec(trimmed);
70-
if (trailingMatch) {
71-
const [, comparison, trailing] = trailingMatch;
72-
if (!LogicalOperator.test(trailing)) {
73-
return {
74-
issue: `Trailing tokens ignored after comparison: '${trailing.trim()}'`,
75-
fix: comparison.trim(),
76-
};
86+
for (let i = 0; i <= tokens.length - minTokensForTrailing; i++) {
87+
const [v1, op, v2] = tokens.slice(i, i + 3);
88+
const remaining = tokens.slice(i + 3);
89+
90+
if (isValueToken(v1) && op.type === 'comparison' && isValueToken(v2)) {
91+
if (remaining.length > 0) {
92+
if (remaining[0].type !== 'logical') {
93+
const validExpr = tokens
94+
.slice(0, i + 3)
95+
.map((t) => t.value)
96+
.join(' ');
97+
const junk = remaining.map((t) => t.value).join(' ');
98+
return {
99+
message: `Trailing tokens ignored after comparison: '${junk.trim()}'`,
100+
fix: validExpr,
101+
};
102+
}
103+
}
77104
}
78105
}
106+
return null;
107+
}
108+
109+
function checkLaxParsingIssues(tokens: Token[]): ExpressionIssue | null {
110+
for (let i = 0; i < tokens.length - 1; i++) {
111+
const current = tokens[i];
112+
const next = tokens[i + 1];
79113

114+
if (current.type === 'literal' && !isOperatorToken(next)) {
115+
const remaining = tokens.slice(i + 1);
116+
const hasUnknownOperator =
117+
remaining[0]?.type === 'variable' && remaining.some(isOperatorToken);
118+
119+
if (!hasUnknownOperator) {
120+
const ignored = remaining.map((t) => t.value).join(' ');
121+
return {
122+
message: `Expression stops at truthy value '${current.value}', ignoring: '${ignored}'`,
123+
fix: current.value,
124+
};
125+
}
126+
}
127+
}
80128
return null;
81129
}
130+
131+
function analyzeConditionalExpression(markup: string): ExpressionIssue | null {
132+
const trimmed = markup.trim();
133+
if (!trimmed) return null;
134+
135+
const tokens: Token[] = getValuesInMarkup(trimmed).map(({ value }) => ({
136+
value,
137+
type: classifyToken(value),
138+
}));
139+
140+
if (tokens.length === 0) return null;
141+
142+
return (
143+
checkInvalidStartingToken(tokens) ||
144+
checkTrailingTokensAfterComparison(tokens) ||
145+
checkLaxParsingIssues(tokens)
146+
);
147+
}

0 commit comments

Comments
 (0)