Skip to content

Commit 61545e3

Browse files
authored
Merge pull request #1036 from Shopify/conditional-boolean-check
Conditional boolean check
2 parents 155c81c + b0d892b commit 61545e3

File tree

3 files changed

+534
-1
lines changed

3 files changed

+534
-1
lines changed
Lines changed: 373 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,373 @@
1+
import { expect, describe, it } from 'vitest';
2+
import { runLiquidCheck, applyFix } from '../../../test';
3+
import { LiquidHTMLSyntaxError } from '..';
4+
5+
describe('Module: InvalidConditionalBooleanExpression', () => {
6+
it('should not report an offense for valid boolean expressions', async () => {
7+
const testCases = [
8+
'{% if 1 > 2 %}hello{% endif %}',
9+
'{% if variable == 5 %}hello{% endif %}',
10+
"{% if 'abc' contains 'a' %}hello{% endif %}",
11+
"{% if product.title != '' %}hello{% endif %}",
12+
'{% if 1 and 2 %}hello{% endif %}',
13+
'{% if true or false %}hello{% endif %}',
14+
'{% if 10 > 5 and user.active %}hello{% endif %}',
15+
'{% if price >= 100 or discount %}hello{% endif %}',
16+
'{% if (1 > 0) and (2 < 3) %}hello{% endif %}',
17+
"{% if user.name contains 'admin' or user.role == 'owner' %}hello{% endif %}",
18+
];
19+
20+
for (const testCase of testCases) {
21+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase);
22+
expect(offenses).to.have.length(0);
23+
}
24+
});
25+
26+
it('should not report an offense for valid single values', async () => {
27+
const testCases = [
28+
'{% if variable %}hello{% endif %}',
29+
'{% if user.active %}hello{% endif %}',
30+
'{% if true %}hello{% endif %}',
31+
'{% if false %}hello{% endif %}',
32+
'{% if 1 %}hello{% endif %}',
33+
'{% if 0 %}hello{% endif %}',
34+
"{% if 'string' %}hello{% endif %}",
35+
'{% if contains %}hello{% endif %}',
36+
];
37+
38+
for (const testCase of testCases) {
39+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase);
40+
expect(offenses).to.have.length(0);
41+
}
42+
});
43+
44+
it('should report an offense when parser stops at numbers', async () => {
45+
const source = '{% if 7 1 > 100 %}hello{% endif %}';
46+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, source);
47+
48+
expect(offenses).to.have.length(1);
49+
expect(offenses[0].message).to.equal(
50+
"Liquid lax parsing issue: Expression stops at truthy value '7', ignoring: '1 > 100'",
51+
);
52+
53+
const fixed = applyFix(source, offenses[0]);
54+
expect(fixed).to.equal('{% if 7 %}hello{% endif %}');
55+
});
56+
57+
it('should report an offense when parser stops at strings', async () => {
58+
const source = "{% if 'hello' 1 > 100 %}world{% endif %}";
59+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, source);
60+
61+
expect(offenses).to.have.length(1);
62+
expect(offenses[0].message).to.equal(
63+
"Liquid lax parsing issue: Expression stops at truthy value ''hello'', ignoring: '1 > 100'",
64+
);
65+
66+
const fixed = applyFix(source, offenses[0]);
67+
expect(fixed).to.equal("{% if 'hello' %}world{% endif %}");
68+
});
69+
70+
it('should report an offense when parser stops at liquid literals', async () => {
71+
const testCases = [
72+
{ source: '{% if true 1 > 0 %}hello{% endif %}', value: 'true' },
73+
{ source: '{% if false 1 > 0 %}hello{% endif %}', value: 'false' },
74+
{ source: '{% if nil 6 > 5 %}hello{% endif %}', value: 'nil' },
75+
{ source: '{% if empty 123 456 %}hello{% endif %}', value: 'empty' },
76+
{ source: '{% if blank 789 %}hello{% endif %}', value: 'blank' },
77+
];
78+
79+
for (const testCase of testCases) {
80+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase.source);
81+
expect(offenses).to.have.length(1);
82+
expect(offenses[0].message).to.contain(
83+
`Expression stops at truthy value '${testCase.value}'`,
84+
);
85+
86+
const fixed = applyFix(testCase.source, offenses[0]);
87+
expect(fixed).to.equal(`{% if ${testCase.value} %}hello{% endif %}`);
88+
}
89+
});
90+
91+
it('should report offenses in different liquid tag types', async () => {
92+
const testCases = [
93+
'{% if 7 1 > 100 %}hello{% endif %}',
94+
"{% unless 'test' 42 > 0 %}hello{% endunless %}",
95+
'{% if false %}no{% elsif 7 1 > 100 %}hello{% endif %}',
96+
];
97+
98+
for (const testCase of testCases) {
99+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase);
100+
expect(offenses).to.have.length(1);
101+
expect(offenses[0].message).to.contain('Expression stops at truthy value');
102+
}
103+
});
104+
105+
it('should report an offense for malformed expression starting with invalid token', async () => {
106+
const source = '{% if > 2 %}hello{% endif %}';
107+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, source);
108+
109+
expect(offenses).to.have.length(1);
110+
expect(offenses[0].message).to.equal(
111+
"Liquid lax parsing issue: Malformed expression starting with invalid token '>'",
112+
);
113+
114+
const fixed = applyFix(source, offenses[0]);
115+
expect(fixed).to.equal('{% if false %}hello{% endif %}');
116+
});
117+
118+
it('should report an offense for bare operators with no operands', async () => {
119+
const testCases = [
120+
{ source: '{% if > %}hello{% endif %}', token: '>' },
121+
{ source: '{% if == %}hello{% endif %}', token: '==' },
122+
{ source: '{% if < %}hello{% endif %}', token: '<' },
123+
{ source: '{% if != %}hello{% endif %}', token: '!=' },
124+
{ source: '{% if >= %}hello{% endif %}', token: '>=' },
125+
{ source: '{% if <= %}hello{% endif %}', token: '<=' },
126+
];
127+
128+
for (const testCase of testCases) {
129+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase.source);
130+
expect(offenses).to.have.length(1);
131+
expect(offenses[0].message).to.contain(
132+
`Malformed expression starting with invalid token '${testCase.token}'`,
133+
);
134+
135+
const fixed = applyFix(testCase.source, offenses[0]);
136+
expect(fixed).to.equal('{% if false %}hello{% endif %}');
137+
}
138+
});
139+
140+
it('should report an offense for other invalid starting characters', async () => {
141+
const testCases = [
142+
{ source: '{% if @ %}hello{% endif %}', token: '@' },
143+
{ source: '{% if # %}hello{% endif %}', token: '#' },
144+
{ source: '{% if $ %}hello{% endif %}', token: '$' },
145+
{ source: '{% if & %}hello{% endif %}', token: '&' },
146+
];
147+
148+
for (const testCase of testCases) {
149+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase.source);
150+
expect(offenses).to.have.length(1);
151+
expect(offenses[0].message).to.contain(
152+
`Malformed expression starting with invalid token '${testCase.token}'`,
153+
);
154+
155+
const fixed = applyFix(testCase.source, offenses[0]);
156+
expect(fixed).to.equal('{% if false %}hello{% endif %}');
157+
}
158+
});
159+
160+
it('should report an offense for malformed expressions in complex expressions', async () => {
161+
const testCases = [
162+
'{% if > 5 and true %}hello{% endif %}',
163+
'{% if == 2 or false %}hello{% endif %}',
164+
'{% if < 10 and variable %}hello{% endif %}',
165+
];
166+
167+
for (const testCase of testCases) {
168+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase);
169+
expect(offenses).to.have.length(1);
170+
expect(offenses[0].message).to.contain('Malformed expression starting with invalid token');
171+
172+
const fixed = applyFix(testCase, offenses[0]);
173+
expect(fixed).to.equal('{% if false %}hello{% endif %}');
174+
}
175+
});
176+
177+
it('should report an offense for trailing tokens after comparison', async () => {
178+
const source = '{% if 1 == 2 foobar %}hello{% endif %}';
179+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, source);
180+
181+
expect(offenses).to.have.length(1);
182+
expect(offenses[0].message).to.equal(
183+
"Liquid lax parsing issue: Trailing tokens ignored after comparison: 'foobar'",
184+
);
185+
186+
const fixed = applyFix(source, offenses[0]);
187+
expect(fixed).to.equal('{% if 1 == 2 %}hello{% endif %}');
188+
});
189+
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+
213+
it('should report an offense for multiple trailing tokens', async () => {
214+
const source = '{% if 10 > 4 baz qux %}hello{% endif %}';
215+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, source);
216+
217+
expect(offenses).to.have.length(1);
218+
expect(offenses[0].message).to.contain('Trailing tokens ignored');
219+
220+
const fixed = applyFix(source, offenses[0]);
221+
expect(fixed).to.equal('{% if 10 > 4 %}hello{% endif %}');
222+
});
223+
224+
it('should report an offense for trailing junk with different operators', async () => {
225+
const testCases = [
226+
"{% if 'abc' contains 'a' noise %}hello{% endif %}",
227+
'{% if price <= 50 extra %}hello{% endif %}',
228+
'{% if count != 0 junk %}hello{% endif %}',
229+
];
230+
231+
for (const testCase of testCases) {
232+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase);
233+
expect(offenses).to.have.length(1);
234+
expect(offenses[0].message).to.contain('Trailing tokens ignored');
235+
}
236+
});
237+
238+
it('should not report an offense for valid logical continuations', async () => {
239+
const testCases = [
240+
'{% if 1 > 0 and 2 < 3 %}hello{% endif %}',
241+
'{% if x == 5 or y != 10 %}hello{% endif %}',
242+
'{% if price >= 100 and discount %}hello{% endif %}',
243+
];
244+
245+
for (const testCase of testCases) {
246+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase);
247+
expect(offenses).to.have.length(0);
248+
}
249+
});
250+
251+
it('should not report an offense for truthy values followed by logical operators', async () => {
252+
const testCases = [
253+
'{% if true and variable %}hello{% endif %}',
254+
'{% if false or variable %}hello{% endif %}',
255+
'{% if 1 and user.active %}hello{% endif %}',
256+
'{% if 0 or fallback %}hello{% endif %}',
257+
"{% if 'string' and condition %}hello{% endif %}",
258+
"{% if 'value' or default %}hello{% endif %}",
259+
'{% if 42 and check %}hello{% endif %}',
260+
'{% if 3.14 or backup %}hello{% endif %}',
261+
'{% if nil and something %}hello{% endif %}',
262+
'{% if empty or alternative %}hello{% endif %}',
263+
'{% if blank and other %}hello{% endif %}',
264+
];
265+
266+
for (const testCase of testCases) {
267+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase);
268+
expect(offenses).to.have.length(0);
269+
}
270+
});
271+
272+
it('should not report an offense for complex expressions with truthy values and logical operators', async () => {
273+
const testCases = [
274+
'{% if true and variable > 5 %}hello{% endif %}',
275+
"{% if 'test' or user.name == 'admin' %}hello{% endif %}",
276+
'{% if 42 and price <= 100 %}hello{% endif %}',
277+
'{% if false or count != 0 %}hello{% endif %}',
278+
"{% if empty and product.title contains 'sale' %}hello{% endif %}",
279+
];
280+
281+
for (const testCase of testCases) {
282+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase);
283+
expect(offenses).to.have.length(0);
284+
}
285+
});
286+
287+
it('should not report an offense for unknown operators errors after values (Liquid catches these)', async () => {
288+
const testCases = [
289+
'{% if my_var word > 5 %}hello{% endif %}',
290+
'{% if jake johnson > 5 %}hello{% endif %}',
291+
"{% if 'test' invalid > thing %}hello{% endif %}",
292+
"{% if user.name custom 'admin' %}hello{% endif %}",
293+
];
294+
295+
for (const testCase of testCases) {
296+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase);
297+
expect(offenses).to.have.length(0);
298+
}
299+
});
300+
301+
it('should not report an offense for unknown operators after variables', async () => {
302+
const testCases = [
303+
'{% if variable unknown > 5 %}hello{% endif %}',
304+
"{% if user.role badop 'admin' %}hello{% endif %}",
305+
'{% if price fake 100 %}hello{% endif %}',
306+
'{% if "str" blue == something %}hello{% endif %}',
307+
'{% if red blue > something %}hello{% endif %}',
308+
];
309+
310+
for (const testCase of testCases) {
311+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase);
312+
expect(offenses).to.have.length(0);
313+
}
314+
});
315+
316+
it('should not report an offense for unknown operators in complex expressions', async () => {
317+
const testCases = [
318+
"{% if user.active and name fake 'test' %}hello{% endif %}",
319+
"{% unless 'test' some > thing %}hello{% endunless %}",
320+
];
321+
322+
for (const testCase of testCases) {
323+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase);
324+
expect(offenses).to.have.length(0);
325+
}
326+
});
327+
328+
it('should not report an offense for pipe filter expressions', async () => {
329+
const testCases = ['{% if wat | something == something %}hello{% endif %}'];
330+
331+
for (const testCase of testCases) {
332+
const offenses = await runLiquidCheck(LiquidHTMLSyntaxError, testCase);
333+
expect(offenses).to.have.length(0);
334+
}
335+
});
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+
});
373+
});

0 commit comments

Comments
 (0)