Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ignore else expressions and superfluous blocks #20

Merged
merged 2 commits into from
Feb 11, 2024
Merged

fix: ignore else expressions and superfluous blocks #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e148c41
fix: ignore `else` expressions and superfluous blocks
jg-rp Feb 10, 2024
e06d39e
Fix golden test file.
jg-rp Feb 11, 2024
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading