Skip to content

Commit

Permalink
🐛 Jinja - treat none as null instead of a variable name (#846)
Browse files Browse the repository at this point in the history
I encountered the issue where when a Jinja template sets a variable to
`none`, it treats it as a variable name and sets its value to
`undefined`, and then checking that the variable value is not `none`
fails.

Given this template:
```
{%- if not null_val is defined -%}
    {%- set null_val = none -%}
{%- endif -%}
{%- if null_val is not none -%}
    {{- 'fail' -}}
{%- else -%}
    {{- 'pass' -}}
{%- endif -%}
```

The current code will set `null_val` to `undefined`, then `null_val is
not none` will be interpreted as `true` (since `undefined !== null`),
and thus it'll render "fail".

This PR fixes that by interpreting a variable named `none` as a
`NullValue`, since `none` should be a reserved keyword.

---------

Co-authored-by: Joshua Lochner <admin@xenova.com>
  • Loading branch information
giladgd and xenova authored Aug 27, 2024
1 parent c367ae4 commit 80cd456
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 1 deletion.
7 changes: 7 additions & 0 deletions packages/jinja/src/ast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,13 @@ export class BooleanLiteral extends Literal<boolean> {
override type = "BooleanLiteral";
}

/**
* Represents null (none) in the template.
*/
export class NullLiteral extends Literal<null> {
override type = "NullLiteral";
}

/**
* Represents an array literal in the template.
*/
Expand Down
4 changes: 4 additions & 0 deletions packages/jinja/src/lexer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export const TOKEN_TYPES = Object.freeze({

NumericLiteral: "NumericLiteral", // e.g., 123
BooleanLiteral: "BooleanLiteral", // true or false
NullLiteral: "NullLiteral", // none
StringLiteral: "StringLiteral", // 'string'
Identifier: "Identifier", // Variables, functions, etc.
Equals: "Equals", // =
Expand Down Expand Up @@ -73,13 +74,15 @@ const KEYWORDS = Object.freeze({
// Literals
true: TOKEN_TYPES.BooleanLiteral,
false: TOKEN_TYPES.BooleanLiteral,
none: TOKEN_TYPES.NullLiteral,

// NOTE: According to the Jinja docs: The special constants true, false, and none are indeed lowercase.
// Because that caused confusion in the past, (True used to expand to an undefined variable that was considered false),
// all three can now also be written in title case (True, False, and None). However, for consistency, (all Jinja identifiers are lowercase)
// you should use the lowercase versions.
True: TOKEN_TYPES.BooleanLiteral,
False: TOKEN_TYPES.BooleanLiteral,
None: TOKEN_TYPES.NullLiteral,
});

/**
Expand Down Expand Up @@ -271,6 +274,7 @@ export function tokenize(source: string, options: PreprocessOptions = {}): Token
case TOKEN_TYPES.Identifier:
case TOKEN_TYPES.NumericLiteral:
case TOKEN_TYPES.BooleanLiteral:
case TOKEN_TYPES.NullLiteral:
case TOKEN_TYPES.StringLiteral:
case TOKEN_TYPES.CloseParen:
case TOKEN_TYPES.CloseSquareBracket:
Expand Down
6 changes: 6 additions & 0 deletions packages/jinja/src/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
NumericLiteral,
StringLiteral,
BooleanLiteral,
NullLiteral,
ArrayLiteral,
ObjectLiteral,
BinaryExpression,
Expand Down Expand Up @@ -486,6 +487,8 @@ export function parse(tokens: Token[]): Program {
if (filter instanceof BooleanLiteral) {
// Special case: treat boolean literals as identifiers
filter = new Identifier(filter.value.toString());
} else if (filter instanceof NullLiteral) {
filter = new Identifier("none");
}
if (!(filter instanceof Identifier)) {
throw new SyntaxError(`Expected identifier for the test`);
Expand Down Expand Up @@ -527,6 +530,9 @@ export function parse(tokens: Token[]): Program {
case TOKEN_TYPES.BooleanLiteral:
++current;
return new BooleanLiteral(token.value.toLowerCase() === "true");
case TOKEN_TYPES.NullLiteral:
++current;
return new NullLiteral(null);
case TOKEN_TYPES.Identifier:
++current;
return new Identifier(token.value);
Expand Down
4 changes: 4 additions & 0 deletions packages/jinja/src/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type {
NumericLiteral,
StringLiteral,
BooleanLiteral,
NullLiteral,
ArrayLiteral,
Statement,
Program,
Expand Down Expand Up @@ -257,6 +258,7 @@ export class Environment {
],
["false", (operand) => operand.type === "BooleanValue" && !(operand as BooleanValue).value],
["true", (operand) => operand.type === "BooleanValue" && (operand as BooleanValue).value],
["none", (operand) => operand.type === "NullValue"],
["string", (operand) => operand.type === "StringValue"],
["number", (operand) => operand.type === "NumericValue"],
["integer", (operand) => operand.type === "NumericValue" && Number.isInteger((operand as NumericValue).value)],
Expand Down Expand Up @@ -1039,6 +1041,8 @@ export class Interpreter {
return new StringValue((statement as StringLiteral).value);
case "BooleanLiteral":
return new BooleanValue((statement as BooleanLiteral).value);
case "NullLiteral":
return new NullValue((statement as NullLiteral).value);
case "ArrayLiteral":
return new ArrayValue((statement as ArrayLiteral).value.map((x) => this.evaluate(x, environment)));
case "TupleLiteral":
Expand Down
50 changes: 49 additions & 1 deletion packages/jinja/test/templates.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ const TEST_STRINGS = {
UNDEFINED_VARIABLES: `{{ undefined_variable }}`,
UNDEFINED_ACCESS: `{{ object.undefined_attribute }}`,

// Null
NULL_VARIABLE: `{% if not null_val is defined %}{% set null_val = none %}{% endif %}{% if null_val is not none %}{{ 'fail' }}{% else %}{{ 'pass' }}{% endif %}`,

// Ternary operator
TERNARY_OPERATOR: `|{{ 'a' if true else 'b' }}|{{ 'a' if false else 'b' }}|{{ 'a' if 1 + 1 == 2 else 'b' }}|{{ 'a' if 1 + 1 == 3 or 1 * 2 == 3 else 'b' }}|`,

Expand Down Expand Up @@ -2210,7 +2213,7 @@ const TEST_PARSED = {
{ value: "unknown", type: "StringLiteral" },
{ value: ")", type: "CloseParen" },
{ value: "is", type: "Is" },
{ value: "none", type: "Identifier" },
{ value: "none", type: "NullLiteral" },
{ value: "}}", type: "CloseExpression" },
{ value: "|", type: "Text" },
{ value: "{{", type: "OpenExpression" },
Expand Down Expand Up @@ -2355,6 +2358,45 @@ const TEST_PARSED = {
{ value: "}}", type: "CloseExpression" },
],

// Null
NULL_VARIABLE: [
{ value: "{%", type: "OpenStatement" },
{ value: "if", type: "If" },
{ value: "not", type: "UnaryOperator" },
{ value: "null_val", type: "Identifier" },
{ value: "is", type: "Is" },
{ value: "defined", type: "Identifier" },
{ value: "%}", type: "CloseStatement" },
{ value: "{%", type: "OpenStatement" },
{ value: "set", type: "Set" },
{ value: "null_val", type: "Identifier" },
{ value: "=", type: "Equals" },
{ value: "none", type: "NullLiteral" },
{ value: "%}", type: "CloseStatement" },
{ value: "{%", type: "OpenStatement" },
{ value: "endif", type: "EndIf" },
{ value: "%}", type: "CloseStatement" },
{ value: "{%", type: "OpenStatement" },
{ value: "if", type: "If" },
{ value: "null_val", type: "Identifier" },
{ value: "is", type: "Is" },
{ value: "not", type: "UnaryOperator" },
{ value: "none", type: "NullLiteral" },
{ value: "%}", type: "CloseStatement" },
{ value: "{{", type: "OpenExpression" },
{ value: "fail", type: "StringLiteral" },
{ value: "}}", type: "CloseExpression" },
{ value: "{%", type: "OpenStatement" },
{ value: "else", type: "Else" },
{ value: "%}", type: "CloseStatement" },
{ value: "{{", type: "OpenExpression" },
{ value: "pass", type: "StringLiteral" },
{ value: "}}", type: "CloseExpression" },
{ value: "{%", type: "OpenStatement" },
{ value: "endif", type: "EndIf" },
{ value: "%}", type: "CloseStatement" },
],

// Ternary operator
TERNARY_OPERATOR: [
{ value: "|", type: "Text" },
Expand Down Expand Up @@ -2894,6 +2936,9 @@ const TEST_CONTEXT = {
UNDEFINED_VARIABLES: {},
UNDEFINED_ACCESS: { object: {} },

// Null
NULL_VARIABLE: { a: null },

// Ternary operator
TERNARY_OPERATOR: {},

Expand Down Expand Up @@ -3037,6 +3082,9 @@ const EXPECTED_OUTPUTS = {
UNDEFINED_VARIABLES: ``,
UNDEFINED_ACCESS: ``,

// Null
NULL_VARIABLE: `pass`,

// Ternary operator
TERNARY_OPERATOR: `|a|b|a|b|`,

Expand Down

0 comments on commit 80cd456

Please sign in to comment.