Repository: paustint/soql-parser-js (ts)

Issue:
Some date formats are not properly parsed
- [ ] Feature
- [x] Bug

<!--If this is a bug, please include the steps to reproduce the bug-->
## Description
Dates in this format are valid with Salesforce, but not parsed properly: `2020-04-15T02:40:03.000+0000`

Example: `SELECT Id FROM LoginHistory WHERE LoginTime > 2019-04-15T02:40:03.000+0000 AND LoginTime < 2020-04-15T02:40:03.000+0000`
Order By does not allow aggregate queries
- [ ] Feature
- [x] Bug

<!--If this is a bug, please include the steps to reproduce the bug-->
## Description
Salesforce supports aggregate functions in the order by, but parsing fails.
Example query: `SELECT ProductCode FROM Product2 GROUP BY ProductCode HAVING COUNT(Id) > 1 ORDER BY COUNT(Id) DESC`
Functions in order by are not composed correctly
- [ ] Feature
- [x] Bug

<!--If this is a bug, please include the steps to reproduce the bug-->
## Description
If ASC or DESC is included in an order by that uses a function, then the query is not composed properly.
```diff
-SELECT ProductCode FROM Product2 GROUP BY ProductCode HAVING COUNT(Id) > 1 ORDER BY COUNT(Id)DESC
+SELECT ProductCode FROM Product2 GROUP BY ProductCode HAVING COUNT(Id) > 1 ORDER BY COUNT(Id) DESC
```

Pull request description:
Misc bugfixes
Added support for dates formatted with nanoseconds, such as . (#102)
Added support for aggregate function in the  clause (#103)
Queries would not be properly composed if an order by had a function and also specified ASC or DESC (#104)

resolves #102
resolves #103
resolves #104:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index eac1923..2a1c563 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,13 @@
 # Changelog
 
+## 2.5.2
+
+April 23, 2020
+
+1. Added support for dates formatted with nanoseconds, such as `2020-04-15T02:40:03.000+0000`. (#102)
+2. Added support for aggregate function in the `ORDER BY` clause (#103)
+3. Queries would not be properly composed if an order by had a function and also specified ASC or DESC (#104)
+
 ## 2.5.1
 
 April 23, 2020
diff --git a/src/composer/composer.ts b/src/composer/composer.ts
index ae0b4f0..1fffc34 100644
--- a/src/composer/composer.ts
+++ b/src/composer/composer.ts
@@ -354,7 +354,7 @@ export class Compose {
       return this.formatter.formatOrderByArray(orderBy.map(ob => this.parseOrderBy(ob)));
     } else {
       let output = `${utils.get(orderBy.field, ' ')}`;
-      output += orderBy.fn ? this.parseFn(orderBy.fn) : '';
+      output += orderBy.fn ? `${this.parseFn(orderBy.fn)} ` : '';
       output += `${utils.get(orderBy.order, ' ')}${utils.get(orderBy.nulls, '', 'NULLS ')}`;
       return output.trim();
     }
diff --git a/src/models.ts b/src/models.ts
index 08d5a7d..eb435b4 100644
--- a/src/models.ts
+++ b/src/models.ts
@@ -141,8 +141,9 @@ export interface OrderByFunctionExpressionContext extends WithIdentifier {
   nulls?: IToken[];
 }
 
-export interface OrderByLocationExpressionContext {
-  locationFunction: CstNode[];
+export interface orderByAggregateOrLocationExpressionContext {
+  locationFunction?: CstNode[];
+  aggregateFunction?: CstNode[];
   order?: IToken[];
   nulls?: IToken[];
 }
diff --git a/src/parser/lexer.ts b/src/parser/lexer.ts
index 7b6484b..a912a9c 100644
--- a/src/parser/lexer.ts
+++ b/src/parser/lexer.ts
@@ -714,7 +714,7 @@ export const Minus = createToken({ name: 'MINUS', pattern: '-', categories: [Sym
 
 export const DateTime = createToken({
   name: 'DATETIME',
-  pattern: /[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}(\Z|\.[0-9]+Z|\+[0-9]{2}:[0-9]{2}|\-[0-9]{2}:[0-9]{2})/i,
+  pattern: /[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}(\.[0-9]{3})?(\Z|\.[0-9]+Z|\+[0-9]{2}:[0-9]{2}|\-[0-9]{2}:[0-9]{2}|\+[0-9]{4}|\-[0-9]{4})/i,
   categories: [DateIdentifier],
 });
 
diff --git a/src/parser/parser.ts b/src/parser/parser.ts
index 5d15618..8b71533 100644
--- a/src/parser/parser.ts
+++ b/src/parser/parser.ts
@@ -328,7 +328,7 @@ export class SoqlParser extends CstParser {
       DEF: () => {
         this.OR([
           { ALT: () => this.SUBRULE(this.orderByFunctionExpression, { LABEL: 'orderByExpressionOrFn' }) },
-          { ALT: () => this.SUBRULE(this.orderByLocationExpression, { LABEL: 'orderByExpressionOrFn' }) },
+          { ALT: () => this.SUBRULE(this.orderByAggregateOrLocationExpression, { LABEL: 'orderByExpressionOrFn' }) },
           { ALT: () => this.SUBRULE(this.orderByExpression, { LABEL: 'orderByExpressionOrFn' }) },
         ]);
       },
@@ -351,14 +351,14 @@ export class SoqlParser extends CstParser {
     this.SUBRULE(this.functionExpression);
   });
 
-  private orderByLocationExpression = this.RULE('orderByLocationExpression', () => {
-    this.SUBRULE(this.locationFunction);
+  private orderByAggregateOrLocationExpression = this.RULE('orderByAggregateOrLocationExpression', () => {
+    this.OR([{ ALT: () => this.SUBRULE(this.locationFunction) }, { ALT: () => this.SUBRULE(this.aggregateFunction) }]);
     this.OPTION(() => {
-      this.OR([{ ALT: () => this.CONSUME(lexer.Asc, { LABEL: 'order' }) }, { ALT: () => this.CONSUME(lexer.Desc, { LABEL: 'order' }) }]);
+      this.OR1([{ ALT: () => this.CONSUME(lexer.Asc, { LABEL: 'order' }) }, { ALT: () => this.CONSUME(lexer.Desc, { LABEL: 'order' }) }]);
     });
     this.OPTION1(() => {
       this.CONSUME(lexer.Nulls);
-      this.OR1([{ ALT: () => this.CONSUME(lexer.First, { LABEL: 'nulls' }) }, { ALT: () => this.CONSUME(lexer.Last, { LABEL: 'nulls' }) }]);
+      this.OR2([{ ALT: () => this.CONSUME(lexer.First, { LABEL: 'nulls' }) }, { ALT: () => this.CONSUME(lexer.Last, { LABEL: 'nulls' }) }]);
     });
   });
 
diff --git a/src/parser/visitor.ts b/src/parser/visitor.ts
index ae40a58..8bbc3ec 100644
--- a/src/parser/visitor.ts
+++ b/src/parser/visitor.ts
@@ -58,7 +58,7 @@ import {
   WithDateCategoryContext,
   LocationFunctionContext,
   GeoLocationFunctionContext,
-  OrderByLocationExpressionContext,
+  orderByAggregateOrLocationExpressionContext,
   GroupByFieldListContext,
   SelectClauseIdentifierContext,
 } from '../models';
@@ -473,10 +473,13 @@ class SOQLVisitor extends BaseSoqlVisitor {
     return orderByClause;
   }
 
-  orderByLocationExpression(ctx: OrderByLocationExpressionContext): OrderByClause {
-    const orderByClause: OrderByClause = {
-      fn: this.visit(ctx.locationFunction, { includeType: false }),
-    };
+  orderByAggregateOrLocationExpression(ctx: orderByAggregateOrLocationExpressionContext): OrderByClause {
+    const orderByClause: OrderByClause = {};
+    if (ctx.locationFunction) {
+      orderByClause.fn = this.visit(ctx.locationFunction, { includeType: false });
+    } else {
+      orderByClause.fn = this.visit(ctx.aggregateFunction, { includeType: false });
+    }
     if (ctx.order && ctx.order[0]) {
       orderByClause.order = ctx.order[0].tokenType.name as OrderByCriterion;
     }


Test patch:
diff --git a/test/test-cases.ts b/test/test-cases.ts
index cd4bafc..98e52f7 100644
--- a/test/test-cases.ts
+++ b/test/test-cases.ts
@@ -1610,7 +1610,7 @@ export const testCases: TestCase[] = [
   },
   {
     testCase: 89,
-    soql: `SELECT Name, StreetAddress__c FROM Warehouse__c WHERE DISTANCE(Location__c, GEOLOCATION(37.775, -122.418), 'mi') < 20 ORDER BY DISTANCE(Location__c, GEOLOCATION(37.775, -122.418), 'mi') LIMIT 10`,
+    soql: `SELECT Name, StreetAddress__c FROM Warehouse__c WHERE DISTANCE(Location__c, GEOLOCATION(37.775, -122.418), 'mi') < 20 ORDER BY DISTANCE(Location__c, GEOLOCATION(37.775, -122.418), 'mi') DESC LIMIT 10`,
     output: {
       fields: [
         { type: 'Field', field: 'Name' },
@@ -1638,6 +1638,7 @@ export const testCases: TestCase[] = [
         },
       },
       orderBy: {
+        order: 'DESC',
         fn: {
           rawValue: `DISTANCE(Location__c, GEOLOCATION(37.775, -122.418), 'mi')`,
           functionName: 'DISTANCE',
@@ -1874,6 +1875,42 @@ export const testCases: TestCase[] = [
       },
     },
   },
+  {
+    testCase: 101,
+    soql: 'SELECT Id FROM LoginHistory WHERE LoginTime > 2019-04-15T02:40:03.000+0000 AND LoginTime < 2020-04-15T02:40:03.000+0000',
+    output: {
+      fields: [{ type: 'Field', field: 'Id' }],
+      sObject: 'LoginHistory',
+      where: {
+        left: { field: 'LoginTime', operator: '>', value: '2019-04-15T02:40:03.000+0000', literalType: 'DATETIME' },
+        operator: 'AND',
+        right: { left: { field: 'LoginTime', operator: '<', value: '2020-04-15T02:40:03.000+0000', literalType: 'DATETIME' } },
+      },
+    },
+  },
+  {
+    testCase: 102,
+    soql: 'SELECT ProductCode FROM Product2 GROUP BY ProductCode HAVING COUNT(Id) > 1 ORDER BY COUNT(Id) DESC',
+    output: {
+      fields: [{ type: 'Field', field: 'ProductCode' }],
+      sObject: 'Product2',
+      groupBy: {
+        field: 'ProductCode',
+        having: {
+          left: {
+            operator: '>',
+            value: '1',
+            literalType: 'INTEGER',
+            fn: { rawValue: 'COUNT(Id)', functionName: 'COUNT', parameters: ['Id'] },
+          },
+        },
+      },
+      orderBy: {
+        fn: { rawValue: 'COUNT(Id)', functionName: 'COUNT', parameters: ['Id'] },
+        order: 'DESC',
+      },
+    },
+  },
 ];
 
 export default testCases;
diff --git a/test/test.spec.ts b/test/test.spec.ts
index e7ebfa2..003b639 100644
--- a/test/test.spec.ts
+++ b/test/test.spec.ts
@@ -11,7 +11,7 @@ const replacements = [{ matching: / last /i, replace: ' LAST ' }];
 // Uncomment these to easily test one specific query - useful for troubleshooting/bugfixing
 
 // describe.only('parse queries', () => {
-//   const testCase = testCases.find(tc => tc.testCase === 100);
+//   const testCase = testCases.find(tc => tc.testCase === 102);
 //   it(`should correctly parse test case ${testCase.testCase} - ${testCase.soql}`, () => {
 //     const soqlQuery = parseQuery(testCase.soql, testCase.options);
 //     const soqlQueryWithoutUndefinedProps = JSON.parse(JSON.stringify(soqlQuery));