Repository: DestinyItemManager/DIM
Pull request #5540 (2020-08-02 05:59:14)

Fix query parsing when starting with multiple parenthesized exprs
Thanks for the test case!


Patch:
diff --git a/src/app/search/query-parser.ts b/src/app/search/query-parser.ts
index 8a3f363f6..2780457e4 100644
--- a/src/app/search/query-parser.ts
+++ b/src/app/search/query-parser.ts
@@ -140,7 +140,7 @@ export function parseQuery(query: string): QueryAST {
    * not a binary operator. "not" is also in here because it's really just a modifier on an atom.
    */
   function parseAtom(tokens: PeekableGenerator<Token>): QueryAST {
-    const token: Token | undefined = tokens.peek();
+    const token: Token | undefined = tokens.pop();
 
     if (!token) {
       throw new Error('expected an atom');
@@ -148,7 +148,6 @@ export function parseQuery(query: string): QueryAST {
 
     switch (token[0]) {
       case 'filter': {
-        tokens.pop();
         const keyword = token[1];
         if (keyword === 'not') {
           // `not:` a synonym for `-is:`. We could fix this up in filter execution but I chose to normalize it here.
@@ -169,7 +168,6 @@ export function parseQuery(query: string): QueryAST {
         }
       }
       case 'not': {
-        tokens.pop();
         return {
           op: 'not',
           // The operand should always be an atom
@@ -177,8 +175,7 @@ export function parseQuery(query: string): QueryAST {
         };
       }
       case '(': {
-        tokens.pop();
-        const result = parse(new PeekableGenerator(untilCloseParen(tokens)));
+        const result = parse(tokens);
         if (tokens.peek()?.[0] === ')') {
           tokens.pop();
         }
@@ -201,6 +198,9 @@ export function parseQuery(query: string): QueryAST {
 
       let token: Token | undefined;
       while ((token = tokens.peek())) {
+        if (token[0] === ')') {
+          break;
+        }
         const operator = operators[token[0] as keyof typeof operators];
         if (!operator) {
           throw new Error('Expected an operator, got ' + token);
@@ -246,19 +246,6 @@ function isSameOp<T extends 'and' | 'or'>(binOp: T, op: QueryAST): op is AndOp |
   return binOp === op.op;
 }
 
-/**
- * Yield tokens from the passed in generator until we reach a closing parenthesis token.
- */
-function* untilCloseParen(tokens: PeekableGenerator<Token>): Generator<Token> {
-  let token: Token | undefined;
-  while ((token = tokens.pop())) {
-    if (token[0] === ')') {
-      return;
-    }
-    yield token;
-  }
-}
-
 /* **** Lexer **** */
 
 // Lexer token types


Test patch:
diff --git a/src/app/search/__snapshots__/query-parser.test.ts.snap b/src/app/search/__snapshots__/query-parser.test.ts.snap
index 746ea3ac1..e3d33648c 100644
--- a/src/app/search/__snapshots__/query-parser.test.ts.snap
+++ b/src/app/search/__snapshots__/query-parser.test.ts.snap
@@ -84,6 +84,72 @@ Array [
 ]
 `;
 
+exports[`parse |(("test" or "test") and "test")|: ast 1`] = `
+Object {
+  "op": "and",
+  "operands": Array [
+    Object {
+      "op": "or",
+      "operands": Array [
+        Object {
+          "args": "test",
+          "op": "filter",
+          "type": "keyword",
+        },
+        Object {
+          "args": "test",
+          "op": "filter",
+          "type": "keyword",
+        },
+      ],
+    },
+    Object {
+      "args": "test",
+      "op": "filter",
+      "type": "keyword",
+    },
+  ],
+}
+`;
+
+exports[`parse |(("test" or "test") and "test")|: lexer 1`] = `
+Array [
+  Array [
+    "(",
+  ],
+  Array [
+    "(",
+  ],
+  Array [
+    "filter",
+    "keyword",
+    "test",
+  ],
+  Array [
+    "or",
+  ],
+  Array [
+    "filter",
+    "keyword",
+    "test",
+  ],
+  Array [
+    ")",
+  ],
+  Array [
+    "and",
+  ],
+  Array [
+    "filter",
+    "keyword",
+    "test",
+  ],
+  Array [
+    ")",
+  ],
+]
+`;
+
 exports[`parse |(is:hunter power:>=540) or (is:warlock power:>=560)|: ast 1`] = `
 Object {
   "op": "or",
diff --git a/src/app/search/query-parser.test.ts b/src/app/search/query-parser.test.ts
index 08ca122c0..4aee43ff1 100644
--- a/src/app/search/query-parser.test.ts
+++ b/src/app/search/query-parser.test.ts
@@ -45,6 +45,7 @@ const cases = [
   // These test copy/pasting from somewhere that automatically converts quotes to "smart quotes"
   ['“grenade launcher reserves”'],
   ['‘grenade launcher reserves’'],
+  ['(("test" or "test") and "test")'],
 ];
 
 // Each of these asserts that the first query is the same as the second query once parsed