Repository: mozilla/nunjucks
Pull request #480 (2015-07-21 23:37:49)

Add 'none' as a literal evaluating to null.
I ran across this in working on the fix for #478.

Jinja2 has three special constants: `true`, `false`, and `none` (mapping to Python's `True`, `False`, and `None`, respectively).

Nunjucks has `true` and `false` (mapping to JS `true` and `false`), but no explicit support for `none`.

This has gone unaddressed thus far, because in practice a Jinja template using `none` ports just fine to nunjucks anyway, as the symbol `none` evaluates to `undefined`, which behaves similarly-enough to Python's `None` (mostly just in that it's falsy) to make no real difference.

But there are some subtle problems with this.

First, it's pointlessly inefficient. There's no reason for use of `none` to require a `contextOrFrameLookup` at runtime every place it is used.

Second, it allows for the name `none` to be re-bound to some other value, resulting in confusing code and needless Jinja incompatibility.

Third, the logical mapping for `none` to JS is really `null`, not `undefined`. Most of the time this makes no difference, but it does make a difference with the `default` filter. In Jinja, `{{ none|default('d', false) }}` evaluates to `None` (because `None` is not an undefined value), whereas in nunjucks it prints `d` (because it is undefined).

This pull request turns `none` into a lexed constant just like `true` and `false`, with the value `null`.

This could be backwards incompatible, if someone is relying on the current behavior of `none` with the `default` filter, or is re-binding the name `none` to some other value. I think this backwards-incompatibility is acceptable as a bugfix, and the time to do it is now (before 2.0 is released).


Patch:
diff --git a/src/compiler.js b/src/compiler.js
index 374c950..a19dd34 100644
--- a/src/compiler.js
+++ b/src/compiler.js
@@ -298,6 +298,9 @@ var Compiler = Object.extend({
             val = val.replace(/\t/g, '\\t');
             this.emit('"' + val  + '"');
         }
+        else if (node.value === null) {
+            this.emit('null');
+        }
         else {
             this.emit(node.value.toString());
         }
diff --git a/src/lexer.js b/src/lexer.js
index 3ab4575..301c21e 100644
--- a/src/lexer.js
+++ b/src/lexer.js
@@ -35,6 +35,7 @@ var TOKEN_PIPE = 'pipe';
 var TOKEN_INT = 'int';
 var TOKEN_FLOAT = 'float';
 var TOKEN_BOOLEAN = 'boolean';
+var TOKEN_NONE = 'none';
 var TOKEN_SYMBOL = 'symbol';
 var TOKEN_SPECIAL = 'special';
 var TOKEN_REGEX = 'regex';
@@ -195,6 +196,9 @@ Tokenizer.prototype.nextToken = function() {
             else if(tok.match(/^(true|false)$/)) {
                 return token(TOKEN_BOOLEAN, tok, lineno, colno);
             }
+            else if(tok === 'none') {
+                return token(TOKEN_NONE, tok, lineno, colno);
+            }
             else if(tok) {
                 return token(TOKEN_SYMBOL, tok, lineno, colno);
             }
@@ -493,6 +497,7 @@ module.exports = {
     TOKEN_INT: TOKEN_INT,
     TOKEN_FLOAT: TOKEN_FLOAT,
     TOKEN_BOOLEAN: TOKEN_BOOLEAN,
+    TOKEN_NONE: TOKEN_NONE,
     TOKEN_SYMBOL: TOKEN_SYMBOL,
     TOKEN_SPECIAL: TOKEN_SPECIAL,
     TOKEN_REGEX: TOKEN_REGEX
diff --git a/src/parser.js b/src/parser.js
index 5835f49..06e911d 100644
--- a/src/parser.js
+++ b/src/parser.js
@@ -891,7 +891,7 @@ var Parser = Object.extend({
 
     parsePrimary: function (noPostfix) {
         var tok = this.nextToken();
-        var val = null;
+        var val;
         var node = null;
 
         if(!tok) {
@@ -919,11 +919,14 @@ var Parser = Object.extend({
                           tok.colno);
             }
         }
+        else if(tok.type === lexer.TOKEN_NONE) {
+            val = null;
+        }
         else if (tok.type === lexer.TOKEN_REGEX) {
             val = new RegExp(tok.value.body, tok.value.flags);
         }
 
-        if(val !== null) {
+        if(val !== undefined) {
             node = new nodes.Literal(tok.lineno, tok.colno, val);
         }
         else if(tok.type === lexer.TOKEN_SYMBOL) {


Test patch:
diff --git a/tests/compiler.js b/tests/compiler.js
index 9bf559d..293ff9a 100644
--- a/tests/compiler.js
+++ b/tests/compiler.js
@@ -67,6 +67,21 @@
             finish(done);
         });
 
+        it('should display none as empty string', function(done) {
+            equal('{{ none }}', '');
+            finish(done);
+        });
+
+        it('should compile none as falsy', function(done) {
+            equal('{% if not none %}yes{% endif %}', 'yes');
+            finish(done);
+        });
+
+        it('should compile none as null, not undefined', function(done) {
+            equal('{{ none|default("d", false) }}', '');
+            finish(done);
+        });
+
         it('should compile function calls', function(done) {
             equal('{{ foo("msg") }}',
                   { foo: function(str) { return str + 'hi'; }},
diff --git a/tests/lexer.js b/tests/lexer.js
index f84b1f9..9316b50 100644
--- a/tests/lexer.js
+++ b/tests/lexer.js
@@ -148,13 +148,14 @@
         });
 
         it('should parse basic types', function() {
-            tokens = lexer.lex('{{ 3 4.5 true false foo "hello" \'boo\' r/regex/ }}');
+            tokens = lexer.lex('{{ 3 4.5 true false none foo "hello" \'boo\' r/regex/ }}');
             hasTokens(tokens,
                       lexer.TOKEN_VARIABLE_START,
                       lexer.TOKEN_INT,
                       lexer.TOKEN_FLOAT,
                       lexer.TOKEN_BOOLEAN,
                       lexer.TOKEN_BOOLEAN,
+                      lexer.TOKEN_NONE,
                       lexer.TOKEN_SYMBOL,
                       lexer.TOKEN_STRING,
                       lexer.TOKEN_STRING,
diff --git a/tests/parser.js b/tests/parser.js
index d5e58cf..e287b99 100644
--- a/tests/parser.js
+++ b/tests/parser.js
@@ -156,6 +156,11 @@
                    [nodes.Output,
                     [nodes.Literal, false]]]);
 
+            isAST(parser.parse('{{ none }}'),
+                  [nodes.Root,
+                   [nodes.Output,
+                    [nodes.Literal, null]]]);
+
             isAST(parser.parse('{{ foo }}'),
                   [nodes.Root,
                    [nodes.Output,
diff --git a/tests/runtime.js b/tests/runtime.js
index 2389f98..8a7e6d4 100644
--- a/tests/runtime.js
+++ b/tests/runtime.js
@@ -58,7 +58,7 @@
         it('should allow for undefined macro arguments in the last position', function(done) {
             render('{% macro foo(bar, baz) %}' +
                    '{{ bar }} {{ baz }}{% endmacro %}' +
-                   '{{ foo("hello", none) }}',
+                   '{{ foo("hello", nosuchvar) }}',
                    {},
                    { noThrow: true },
                    function(err, res) {