Skip to content

Commit

Permalink
fix: ignore else expressions and superfluous blocks (#20)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jg-rp
jg-rp authored Feb 11, 2024
1 parent e3fdeae commit 0db7c40
Show file tree
Hide file tree
Showing 5 changed files with 216 additions and 17 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
**Fixes**

- Fixed comparison of strings in logical expressions. Previously we only supported comparing strings for equality with `==` and `!=`, now we support `<`, `>`, `<=` and `>=` too.
- Fixed handling of superfluous expressions in `{% else %}` tags. We now silently ignore anything between `else` and `%}`, matching the behavior of Shopify/Liquid.
- Fixed handling of extra `{% else %}` and `{% elsif %}` blocks after the first `{% else %}` block. We now silently ignore extraneous blocks, matching the behavior of Shopify/Liquid.


## Version 1.8.1
Expand Down
37 changes: 29 additions & 8 deletions src/builtin/tags/if.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class IfTag implements Tag {
TOKEN_EOF,
]);
protected static END_ELSEIF_BLOCK = new Set([TAG_ENDIF, TAG_ELSIF, TAG_ELSE]);
protected static END_ELSE_BLOCK = new Set([TAG_ENDIF]);
protected static END_ELSE_BLOCK = new Set([TAG_ENDIF, TAG_ELSIF, TAG_ELSE]);

readonly block = true;
readonly name: string = TAG_IF;
Expand Down Expand Up @@ -71,24 +71,45 @@ export class IfTag implements Tag {
});
}

let alternative: BlockNode | undefined = undefined;

if (
stream.current.kind === TOKEN_TAG &&
stream.current.value === TAG_ELSE
) {
return new this.nodeClass(
token,
condition,
consequence,
conditionalAlternatives,
parser.parseBlock(stream, IfTag.END_ELSE_BLOCK, stream.next()),
);
const tok = stream.next();
// @ts-expect-error: stream.current has changed, so `kind` will have changed too.
if (stream.current.kind === TOKEN_EXPRESSION) {
// Superfluous expressions inside an `else` tag are ignored.
stream.next();
}

alternative = parser.parseBlock(stream, IfTag.END_ELSE_BLOCK, tok);
}

// Extraneous `else` and `elsif` blocks are ignored.
if (
!(stream.current.kind === TOKEN_TAG && stream.current.value === TAG_ENDIF)
) {
while (stream.current.kind !== TOKEN_EOF) {
if (
stream.current.kind === TOKEN_TAG &&
stream.current.value === TAG_ENDIF
) {
break;
}
stream.next();
}
}

stream.expectTag(TAG_ENDIF);

return new this.nodeClass(
token,
condition,
consequence,
conditionalAlternatives,
alternative,
);
}
}
Expand Down
44 changes: 36 additions & 8 deletions src/builtin/tags/unless.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,18 @@ export class UnlessTag implements Tag {
TAG_ELSE,
TOKEN_EOF,
]);

protected static END_ELSEIF_BLOCK = new Set([
TAG_ENDUNLESS,
TAG_ELSIF,
TAG_ELSE,
]);
protected static END_ELSE_BLOCK = new Set([TAG_ENDUNLESS]);

protected static END_ELSE_BLOCK = new Set([
TAG_ENDUNLESS,
TAG_ELSIF,
TAG_ELSE,
]);

readonly block = true;
readonly name: string = TAG_UNLESS;
Expand Down Expand Up @@ -79,24 +85,46 @@ export class UnlessTag implements Tag {
});
}

let alternative: BlockNode | undefined = undefined;

if (
stream.current.kind === TOKEN_TAG &&
stream.current.value === TAG_ELSE
) {
return new this.nodeClass(
token,
condition,
consequence,
conditionalAlternatives,
parser.parseBlock(stream, UnlessTag.END_ELSE_BLOCK, stream.next()),
);
const tok = stream.next();
// @ts-expect-error: stream.current has changed, so `kind` will have changed too.
if (stream.current.kind === TOKEN_EXPRESSION) {
// Superfluous expressions inside an `else` tag are ignored.
stream.next();
}

alternative = parser.parseBlock(stream, UnlessTag.END_ELSE_BLOCK, tok);
}

// Extraneous `else` and `elsif` blocks are ignored.
if (
!(
stream.current.kind === TOKEN_TAG &&
stream.current.value === TAG_ENDUNLESS
)
) {
while (stream.current.kind !== TOKEN_EOF) {
if (
stream.current.kind === TOKEN_TAG &&
stream.current.value === TAG_ENDUNLESS
) {
break;
}
stream.next();
}
}

return new this.nodeClass(
token,
condition,
consequence,
conditionalAlternatives,
alternative,
);
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/expression.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,10 @@ function eq(left: unknown, right: unknown): boolean {
)
[left, right] = [right, left];

if (left instanceof Undefined && right instanceof Undefined) {
if (
(isUndefined(left) || left instanceof Nil || left === null) &&
(isUndefined(right) || right instanceof Nil || right === null)
) {
return true;
}

Expand Down
145 changes: 145 additions & 0 deletions tests/golden/golden_liquid.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3865,6 +3865,86 @@
"error": false,
"strict": false
},
{
"name": "else tag expressions are ignored",
"template": "{% if false %}1{% else nonsense %}2{% endif %}",
"want": "2",
"context": {},
"partials": {},
"error": false,
"strict": true
},
{
"name": "empty array equals special empty",
"template": "{% if x == empty %}TRUE{% else %}FALSE{% endif %}",
"want": "TRUE",
"context": {
"x": []
},
"partials": {},
"error": false,
"strict": false
},
{
"name": "empty array is truthy",
"template": "{% if x %}TRUE{% else %}FALSE{% endif %}",
"want": "TRUE",
"context": {
"x": []
},
"partials": {},
"error": false,
"strict": false
},
{
"name": "empty object equals special empty",
"template": "{% if x == empty %}TRUE{% else %}FALSE{% endif %}",
"want": "TRUE",
"context": {
"x": {}
},
"partials": {},
"error": false,
"strict": false
},
{
"name": "empty object is truthy",
"template": "{% if x %}TRUE{% else %}FALSE{% endif %}",
"want": "TRUE",
"context": {
"x": {}
},
"partials": {},
"error": false,
"strict": false
},
{
"name": "empty string is truthy",
"template": "{% if '' %}TRUE{% else %}FALSE{% endif %}",
"want": "TRUE",
"context": {},
"partials": {},
"error": false,
"strict": false
},
{
"name": "extra else blocks are ignored",
"template": "{% if false %}1{% else %}2{% else %}3{% endif %}",
"want": "2",
"context": {},
"partials": {},
"error": false,
"strict": true
},
{
"name": "extra elsif blocks are ignored",
"template": "{% if false %}1{% else %}2{% elsif true %}3{% endif %}",
"want": "2",
"context": {},
"partials": {},
"error": false,
"strict": true
},
{
"name": "int does not equal string",
"template": "{% if 1 == '1' %}true{% else %}false{% endif %}",
Expand Down Expand Up @@ -3982,6 +4062,17 @@
"error": false,
"strict": false
},
{
"name": "string contains int",
"template": "{% if 'hel9lo' contains 9 %}TRUE{% else %}FALSE{% endif %}",
"want": "TRUE",
"context": {
"x": {}
},
"partials": {},
"error": false,
"strict": false
},
{
"name": "string does not equal int",
"template": "{% if '1' == 1 %}true{% else %}false{% endif %}",
Expand Down Expand Up @@ -4072,6 +4163,33 @@
"error": false,
"strict": false
},
{
"name": "undefined is equal to nil",
"template": "{% if nosuchthing == nil %}TRUE{% else %}FALSE{% endif %}",
"want": "TRUE",
"context": {},
"partials": {},
"error": false,
"strict": false
},
{
"name": "nil is equal to undefined",
"template": "{% if nil == nosuchthing %}TRUE{% else %}FALSE{% endif %}",
"want": "TRUE",
"context": {},
"partials": {},
"error": false,
"strict": false
},
{
"name": "undefined is equal to null",
"template": "{% if nosuchthing == null %}TRUE{% else %}FALSE{% endif %}",
"want": "TRUE",
"context": {},
"partials": {},
"error": false,
"strict": false
},
{
"name": "undefined variables are falsy",
"template": "{% if nosuchthing %}bar{% else %}foo{% endif %}",
Expand Down Expand Up @@ -9335,6 +9453,33 @@
"error": false,
"strict": false
},
{
"name": "else tag expressions are ignored",
"template": "{% unless true %}1{% else nonsense %}2{% endunless %}",
"want": "2",
"context": {},
"partials": {},
"error": false,
"strict": true
},
{
"name": "extra else blocks are ignored",
"template": "{% unless true %}1{% else %}2{% else %}3{% endunless %}",
"want": "2",
"context": {},
"partials": {},
"error": false,
"strict": true
},
{
"name": "extra elsif blocks are ignored",
"template": "{% unless true %}1{% else %}2{% elsif true %}3{% endunless %}",
"want": "2",
"context": {},
"partials": {},
"error": false,
"strict": true
},
{
"name": "literal false condition",
"template": "{% unless false %}foo{% endunless %}",
Expand Down

0 comments on commit 0db7c40

Please sign in to comment.