Enforce balanced parens

Added check and forced a parsing error if parens are not matched

Added notes to do a large refactor in the future for better parsing of parens

resolves #80

Author: paustint

src/parser/lexer.ts

@@ -89,6 +89,12 @@ export const UsingScopeEnumeration = createToken({
   pattern: Lexer.NA,
 });
 
+// This is a token that will be invoked to force a parsing error if there is a paren mismatch
+export const RParenMismatch = createToken({
+  name: 'RParenMismatch',
+  pattern: Lexer.NA,
+});
+
 const identifierRegex = /[a-zA-Z][a-zA-Z0-9_.]*/y;
 
 /**

src/parser/parser.ts

@@ -166,6 +166,17 @@ export class SoqlParser extends CstParser {
         this.SUBRULE(this.whereClauseExpression, { ARGS: [parenCount] });
       },
     });
+
+    // TODO: this is a hack and the paren expressions should be re-worked, but it is a significant amount of work and difficult to handle in the visitor
+    // If There are more open parens than closing parens, force parsing error - RParenMismatch is not a valid token and does not have a pattern
+    // Because we are not actually doing any calculations with results, our strategy for calculating parens within an expression is semi-ok
+    // but should be considered for a refactor at some point
+    this.ACTION(() => {
+      const parenMatch = parenCount.left - parenCount.right;
+      if (parenMatch !== 0) {
+        this.CONSUME(lexer.RParenMismatch);
+      }
+    });
   });
 
   private whereClauseSubqueryIdentifier = this.RULE('whereClauseSubqueryIdentifier', () => {

test/test-cases-for-is-valid.ts

@@ -286,5 +286,60 @@ export const testCases: TestCaseForFormat[] = [
     soql: `SELECT Id, Name FROM Opportunity WHERE Amount > USD5000`,
     isValid: true,
   },
+  {
+    testCase: 123,
+    soql: `SELECT Id, Name FROM Opportunity WHERE (((((Amount > USD5000`,
+    isValid: false,
+  },
+  {
+    testCase: 124,
+    soql: `SELECT Id, Name FROM Opportunity WHERE (((((Amount > USD5000))))`,
+    isValid: false,
+  },
+  {
+    testCase: 125,
+    soql: `SELECT Id, Name FROM Opportunity WHERE (((((Amount > USD5000)))))`,
+    isValid: true,
+  },
+  {
+    testCase: 126,
+    soql: `SELECT Id FROM Account WHERE (((Name = '1' OR Name = '2') AND Name = '3')) AND (((Description = '4') OR (Id = '5' AND Id = '6'))) AND Id = '7'`,
+    isValid: true,
+  },
+  {
+    testCase: 127,
+    soql: `SELECT Id FROM Account WHERE ((Name = '1' OR Name = '2') AND Name = '3')) AND (((Description = '4') OR (Id = '5' AND Id = '6'))) AND Id = '7'`,
+    isValid: false,
+  },
+  {
+    testCase: 128,
+    soql: `SELECT Id FROM Account WHERE (((Name = '1' OR Name = '2' AND Name = '3')) AND (((Description = '4') OR (Id = '5' AND Id = '6'))) AND Id = '7'`,
+    isValid: false,
+  },
+  {
+    testCase: 129,
+    soql: `SELECT Id FROM Account WHERE (((Name = '1' OR Name = '2') AND Name = '3') AND (((Description = '4') OR (Id = '5' AND Id = '6'))) AND Id = '7'`,
+    isValid: false,
+  },
+  {
+    testCase: 130,
+    soql: `SELECT Id FROM Account WHERE (((Name = '1' OR Name = '2') AND Name = '3') AND (((Description = '4') OR (Id = '5' AND Id = '6')) AND Id = '7'`,
+    isValid: false,
+  },
+  {
+    testCase: 131,
+    soql: `SELECT Id FROM Account WHERE (((Name = '1' OR Name = '2') AND Name = '3')) AND ((Description = '4') OR (Id = '5' AND Id = '6'))) AND Id = '7'`,
+    isValid: false,
+  },
+  {
+    testCase: 132,
+    soql: `SELECT Id FROM Account WHERE (((Name = '1' OR Name = '2') AND Name = '3')) AND (((Description = '4' OR (Id = '5' AND Id = '6'))) AND Id = '7'`,
+    isValid: false,
+  },
+  {
+    testCase: 133,
+    soql: `SELECT Id FROM Account WHERE (((Name = '1' OR Name = '2') AND Name = '3')) AND (((Description = '4') OR Id = '5' AND Id = '6'))) AND Id = '7'`,
+    isValid: false,
+  },
 ];
 export default testCases;

test/test.spec.ts

@@ -40,10 +40,10 @@ describe('parse queries', () => {
 
 // describe.only('Test valid queries', () => {
 //   testCasesForIsValid
-//     .filter(testCase => testCase.testCase === 122)
+//     .filter(testCase => testCase.testCase === 123)
 //     .forEach(testCase => {
 //       it(`should identify validity of query - test case ${testCase.testCase} - ${testCase.soql}`, () => {
-//         const soqlQuery = parseQuery(testCase.soql, testCase.options);
+//         // const soqlQuery = parseQuery(testCase.soql, testCase.options);
 //         const isValid = isQueryValid(testCase.soql);
 //         expect(isValid).equal(testCase.isValid);
 //       });