Update grammar and models to fix failing tests

All tests now pass! The old filter_expr rules failed with a reduce-reduce conflict in the default LALR(1) mode due to a bug in jison: zaach/jison#205 $ jison src/xpath.jison src/xpath.jisonlex ... States with conflicts: State 3 expr -> base_expr . #lookaheads= EOF OR... filter_expr -> base_expr . #lookaheads= ... Aside: the old grammar (with filter_expr uncommented) did work in LR(1) mode, but it takes a LOOOOOONG time to generate which is annoying. $ jison -p lr src/xpath.jison src/xpath.jisonlex
dimagi · Jul 9, 2015 · dae36a9 · dae36a9
1 parent 25ea674
commit dae36a9
Show file tree

Hide file tree

Showing 4 changed files with 157 additions and 66 deletions.
diff --git a/models.js b/models.js
@@ -271,6 +271,9 @@ debuglog = function () {
             // the two methods below it can call itx
             var parts = self.steps.map(func), ret = [], curPart, prevPart, sep;
             var root = (self.initial_context === xpm.XPathInitialContextEnum.ROOT) ? "/" : "";
+            if (self.filter) {
+                parts.splice(0, 0, func(self.filter));
+            }
             if (parts.length === 0) {
                 return root;
             }
@@ -328,6 +331,38 @@ debuglog = function () {
     };
 
 
+    xpm.XPathFilterExpr = function (definition) {
+        /**
+         * Representation of an xpath filter expression.
+         */
+        this.expr = definition.expr;
+        this.predicates = definition.predicates || [];
+        this.toString = function() {
+            var stringArray = [];
+            stringArray.push("{filt-expr:", this.expr.toString(), ",{");
+            stringArray.push(this.predicates.join(","));
+            stringArray.push("}}");
+            return stringArray.join("");
+        };
+        this.toXPath = function() {
+            var predicates = "";
+            if (this.predicates.length > 0) {
+                predicates = "[" + this.predicates.map(objToXPath).join("][") + "]";
+            }
+            var expr = objToXPath(this.expr);
+            // FIXME should all non-function expressions be parenthesized?
+            if (!(this.expr instanceof xpm.XPathFuncExpr)) {
+                expr = "(" + expr + ")";
+            }
+            return expr + predicates;
+        };
+        this.getChildren = function () {
+           return this.predicates;
+        };
+        return this;
+    };
+
+
     // expressions
 
     xpm.XPathExpressionTypeEnum = {

diff --git a/src/xpath.jison b/src/xpath.jison
@@ -35,6 +35,7 @@ xpath_expr:  expr EOF   { return $1; }
 expr:   base_expr                   {  $$ = $1; } /* not necessary as this is the default */
     |   op_expr                     {  $$ = $1; }
     |   path_expr                   {  $$ = $1; }
+    |   filter_expr                 {  $$ = $1; }
     ;
 
 base_expr:  LPAREN expr RPAREN            { $$ = $2; }
@@ -71,22 +72,37 @@ arg_list:   arg_list COMMA expr     { var args = $1;
         ;
 
 path_expr:  loc_path
-        ; 
-
-/* This is commented out because there might be a bug in jison that thinks this is ambiguous
- * when in fact it's not. The first group belongs as part of the path_expr. The second should
- * be added as a new filter_expr.
- */
-
-/*
-        |   filter_expr SLASH rel_loc_path          { $$ = "fe.unwrapPathExpr(rlp)"; }
-        |   filter_expr DBL_SLASH rel_loc_path      { $$ = "fe.unwrapPathExpr(Vprepend(rlp, xpathmodels.XPathStep.ABBR_DESCENDANTS()))"; }
+        |   filter_expr SLASH rel_loc_path          { $$ = new xpathmodels.XPathPathExpr({
+                                                                    initial_context: xpathmodels.XPathInitialContextEnum.EXPR,
+                                                                    filter: $1, steps: $3}); }
+        |   filter_expr DBL_SLASH rel_loc_path      { var steps = $3;
+                                                      steps.splice(0, 0, new xpathmodels.XPathStep({
+                                                                                axis: xpathmodels.XPathAxisEnum.DESCENDANT_OR_SELF, 
+                                                                                test: xpathmodels.XPathTestEnum.TYPE_NODE}));
+                                                      $$ = new xpathmodels.XPathPathExpr({
+                                                                    initial_context: xpathmodels.XPathInitialContextEnum.EXPR,
+                                                                    filter: $1, steps: steps}); }
+        |   base_expr SLASH rel_loc_path            { // could eliminate filterExpr wrapper, but this makes tests pass as-is
+                                                      var filterExpr = new xpathmodels.XPathFilterExpr({expr: $1});
+                                                      $$ = new xpathmodels.XPathPathExpr({
+                                                                    initial_context: xpathmodels.XPathInitialContextEnum.EXPR,
+                                                                    filter: filterExpr, steps: $3}); }
+        |   base_expr DBL_SLASH rel_loc_path        { var steps = $3;
+                                                      // could eliminate filterExpr wrapper, but this makes tests pass as-is
+                                                      var filterExpr = new xpathmodels.XPathFilterExpr({expr: $1});
+                                                      steps.splice(0, 0, new xpathmodels.XPathStep({
+                                                                                axis: xpathmodels.XPathAxisEnum.DESCENDANT_OR_SELF, 
+                                                                                test: xpathmodels.XPathTestEnum.TYPE_NODE}));
+                                                      $$ = new xpathmodels.XPathPathExpr({
+                                                                    initial_context: xpathmodels.XPathInitialContextEnum.EXPR,
+                                                                    filter: filterExpr, steps: steps}); }
         ;
 
-filter_expr:  filter_expr predicate     { $$ = "Vappend(fe.v, p); RESULT = fe;" }
-|   base_expr                   { $$ = "new vectorWrapper(be);"; } ***** THIS IS THE LINE THAT BREAKS *****  
+filter_expr:  base_expr predicate     { $$ = new xpathmodels.XPathFilterExpr({expr: $1, predicates: [$2]}); }
+        |   filter_expr predicate     { var filterExpr = $1;
+                                        filterExpr.predicates.push($2);
+                                        $$ = filterExpr; }
         ;
-*/ 
 
 predicate:   LBRACK expr RBRACK            { $$ = $2; }
         ;

diff --git a/test/generatortests.js b/test/generatortests.js
@@ -142,7 +142,7 @@ test("generator function calls that are actually node tests", function () {
 test("generator filter expressions", function () {
     runGeneratorTests({
         "bunch-o-nodes()[3]": "bunch-o-nodes()[3]",
-        "bunch-o-nodes()[3]['predicates'!='galore']": "bunch-o-nodes()[3]['predicates'!='galore']",
+        "bunch-o-nodes()[3]['predicates'!='galore']": "bunch-o-nodes()[3]['predicates' != 'galore']",
         "(bunch-o-nodes)[3]": "(bunch-o-nodes)[3]",
         "bunch-o-nodes[3]": "bunch-o-nodes[3]",
     });