Repository: mozilla/nunjucks
Pull request #336 (2014-12-10 23:52:18)

Bind 'in' operator more tightly than boolean operators.
When I added support for the 'in' operator a couple months ago, I think I got the precedence slightly wrong.

JS has no 'in' operator, so there's nothing to base nunjucks' behavior off there, but Python (https://docs.python.org/2/reference/expressions.html#operator-precedence) and Jinja both bind 'in' more tightly than boolean operators, so that this `{% if msg.status in ['pending', 'confirmed'] and msg.body %}` does not evaluate as `{% if msg.status in (['pending', 'confirmed'] and msg.body) %}` (which is what nunjucks master currently does).

I think the Python/Jinja behavior is more useful and logical than nunjucks' current behavior: `in` is a comparison operator, like `==` et al, and should bind with similar precedence. You wouldn't expect `{% if msg.status == 'pending' and msg.body %}` to evaluate as `{% if msg.status == ('pending' and msg.body) %}`.

This pull request fixes the precedence (and while I was at it, also adds a test to confirm that `and` binds more tightly than `or`, which is correct and matches both Python and JS).


Patch:
diff --git a/src/parser.js b/src/parser.js
index 870f305..5aec650 100644
--- a/src/parser.js
+++ b/src/parser.js
@@ -632,15 +632,15 @@ var Parser = Object.extend({
     },
 
     parseInlineIf: function() {
-        var node = this.parseIn();
+        var node = this.parseOr();
         if(this.skipSymbol('if')) {
-            var cond_node = this.parseIn();
+            var cond_node = this.parseOr();
             var body_node = node;
             node = new nodes.InlineIf(node.lineno, node.colno);
             node.body = body_node;
             node.cond = cond_node;
             if(this.skipSymbol('else')) {
-                node.else_ = this.parseIn();
+                node.else_ = this.parseOr();
             } else {
                 node.else_ = null;
             }
@@ -649,36 +649,6 @@ var Parser = Object.extend({
         return node;
     },
 
-    parseIn: function() {
-      var node = this.parseOr();
-      while(1) {
-        // check if the next token is 'not'
-        var tok = this.nextToken();
-        if (!tok) { break; }
-        var invert = tok.type == lexer.TOKEN_SYMBOL && tok.value == 'not';
-        // if it wasn't 'not', put it back
-        if (!invert) { this.pushToken(tok); }
-        if (this.skipSymbol('in')) {
-          var node2 = this.parseOr();
-          node = new nodes.In(node.lineno,
-                              node.colno,
-                              node,
-                              node2);
-          if (invert) {
-            node = new nodes.Not(node.lineno,
-                                 node.colno,
-                                 node);
-          }
-        }
-        else {
-          // if we'd found a 'not' but this wasn't an 'in', put back the 'not'
-          if (invert) { this.pushToken(tok); }
-          break;
-        }
-      }
-      return node;
-    },
-
     parseOr: function() {
         var node = this.parseAnd();
         while(this.skipSymbol('or')) {
@@ -710,7 +680,37 @@ var Parser = Object.extend({
                                  tok.colno,
                                  this.parseNot());
         }
-        return this.parseCompare();
+        return this.parseIn();
+    },
+
+    parseIn: function() {
+      var node = this.parseCompare();
+      while(1) {
+        // check if the next token is 'not'
+        var tok = this.nextToken();
+        if (!tok) { break; }
+        var invert = tok.type == lexer.TOKEN_SYMBOL && tok.value == 'not';
+        // if it wasn't 'not', put it back
+        if (!invert) { this.pushToken(tok); }
+        if (this.skipSymbol('in')) {
+          var node2 = this.parseCompare();
+          node = new nodes.In(node.lineno,
+                              node.colno,
+                              node,
+                              node2);
+          if (invert) {
+            node = new nodes.Not(node.lineno,
+                                 node.colno,
+                                 node);
+          }
+        }
+        else {
+          // if we'd found a 'not' but this wasn't an 'in', put back the 'not'
+          if (invert) { this.pushToken(tok); }
+          break;
+        }
+      }
+      return node;
     },
 
     parseCompare: function() {


Test patch:
diff --git a/tests/parser.js b/tests/parser.js
index 2b213b7..8e6ae41 100644
--- a/tests/parser.js
+++ b/tests/parser.js
@@ -236,6 +236,36 @@
                       [nodes.Symbol, 'y']]]]]);
         });
 
+        it('should parse operators with correct precedence', function() {
+            isAST(parser.parse('{{ x in y and z }}'),
+                  [nodes.Root,
+                   [nodes.Output,
+                    [nodes.And,
+                     [nodes.In,
+                      [nodes.Symbol, 'x'],
+                      [nodes.Symbol, 'y']],
+                     [nodes.Symbol, 'z']]]]);
+
+            isAST(parser.parse('{{ x not in y or z }}'),
+                  [nodes.Root,
+                   [nodes.Output,
+                    [nodes.Or,
+                     [nodes.Not,
+                      [nodes.In,
+                      [nodes.Symbol, 'x'],
+                       [nodes.Symbol, 'y']]],
+                     [nodes.Symbol, 'z']]]]);
+
+            isAST(parser.parse('{{ x or y and z }}'),
+                  [nodes.Root,
+                   [nodes.Output,
+                    [nodes.Or,
+                     [nodes.Symbol, 'x'],
+                     [nodes.And,
+                      [nodes.Symbol, 'y'],
+                      [nodes.Symbol, 'z']]]]]);
+        });
+
         it('should parse blocks', function() {
             var n = parser.parse('want some {% if hungry %}pizza{% else %}' +
                                  'water{% endif %}?');