Fix incorrect parsing for NOT negation

Ensure that all `expressionNegation` entries are visited instead of just the first one in the case where there aer multiple negations in a row

Removed `NOT IN` keyword in lexer to avoid false positives for identifiers (fields) that start with `In` preceded by `NOT`

Enable typescript strict mode to help enforce handling data properly - this required many minor updates.

Improved visitor types to better express actual shape of context

Author: paustint

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## 4.10.0
+
+Jan 13, 2024
+
+- Fixed where clause's that have a field name beginning with `In` preceded by the `NOT` operator. These were parsed as `NOT IN` instead of `NOT` followed by a field name, example: `NOT Invoice__c`
+  - https://github.com/jetstreamapp/jetstream/issues/702
+- Fixed queries that have two consecutive `NOT` operators (#237)
+- Enabled Typescript strict mode and made a number of minor fixes related to this.
+- When using `getField` which return `FieldFunctionExpression` will now always return an empty array even if no parameters are provided.
+
 ## 4.9.2
 
 July 24, 2023

package-lock.json

@@ -157,7 +157,7 @@ export interface Subquery extends QueryBase {
 export type WhereClause = WhereClauseWithoutOperator | WhereClauseWithRightCondition;
 
 export interface WhereClauseWithoutOperator {
-  left: ConditionWithValueQuery;
+  left: ConditionWithValueQuery | null;
 }
 
 export interface WhereClauseWithRightCondition extends WhereClauseWithoutOperator {

src/api/api-models.ts

@@ -100,15 +100,15 @@ export function getField(input: string | ComposeFieldInput): SoqlModels.FieldTyp
       field: input,
     };
   } else if (isComposeFieldFunction(input)) {
-    let parameters: string[] | SoqlModels.FieldFunctionExpression[];
+    let parameters: string[] | SoqlModels.FieldFunctionExpression[] = [];
     if (input.parameters) {
       parameters = (Array.isArray(input.parameters) ? input.parameters : [input.parameters]) as
         | string[]
         | SoqlModels.FieldFunctionExpression[];
     }
     return {
       type: 'FieldFunctionExpression',
-      functionName: input.functionName || input.fn,
+      functionName: (input.functionName || input.fn)!,
       parameters,
       alias: input.alias,
     };
@@ -122,7 +122,7 @@ export function getField(input: string | ComposeFieldInput): SoqlModels.FieldTyp
   } else if (isComposeFieldSubquery(input)) {
     return {
       type: 'FieldSubquery',
-      subquery: input.subquery,
+      subquery: input.subquery!,
     };
   } else if (isComposeFieldTypeof(input)) {
     return {
@@ -259,7 +259,7 @@ export function getFlattenedFields(
           break;
       }
     })
-    .filter(field => isString(field));
+    .filter(field => isString(field)) as string[];
 
   return parsedFields;
 }

src/api/public-utils.ts

@@ -76,8 +76,8 @@ export class Compose {
   constructor(private soql: Query, config: Partial<SoqlComposeConfig> = {}) {
     config = { autoCompose: true, ...config };
     const { logging, format } = config;
-    this.logging = logging;
-    this.format = format;
+    this.logging = !!logging;
+    this.format = !!format;
     this.query = '';
 
     this.formatter = new Formatter(this.format, {
@@ -126,7 +126,7 @@ export class Compose {
       output += ` ${fn.alias}`;
     }
 
-    return output;
+    return output!;
   }
 
   /**
@@ -262,7 +262,7 @@ export class Compose {
   public parseFields(fields: FieldType[]): { text: string; typeOfClause?: string[] }[] {
     return fields.map(field => {
       let text = '';
-      let typeOfClause: string[];
+      let typeOfClause: string[] | undefined;
 
       const objPrefix = (field as any).objectPrefix ? `${(field as any).objectPrefix}.` : '';
       switch (field.type) {

src/composer/composer.ts

@@ -16,12 +16,14 @@ export interface FieldData {
 export interface FormatOptions {
   /**
    * @default 1
+   *
    * Number of tabs to indent by
    * These defaults to one
    */
   numIndent?: number;
   /**
    * @default 60
+   *
    * Number of characters before wrapping to a new line.
    * Set to 0 or 1 to force every field on a new line.
    * TYPEOF fields do not honor this setting, they will always be on one line unless `newLineAfterKeywords` is true,
@@ -30,6 +32,7 @@ export interface FormatOptions {
   fieldMaxLineLength?: number;
   /**
    * @default true
+   *
    * Set to true to have a subquery parentheses start and end on a new line.
    * This will be set to true if `newLineAfterKeywords` is true, in which case this property can be omitted
    */
@@ -38,6 +41,7 @@ export interface FormatOptions {
   whereClauseOperatorsIndented?: boolean;
   /**
    * @default false
+   *
    * Adds a new line and indent after all keywords (such as SELECT, FROM, WHERE, ORDER BY, etc..)
    * Setting this to true will add new lines in other places as well, such as complex WHERE clauses
    */
@@ -52,18 +56,18 @@ export interface FormatOptions {
  */
 export class Formatter {
   enabled: boolean;
-  private options: FormatOptions;
+  private options: Required<FormatOptions>;
   private currIndent = 1;
 
   constructor(enabled: boolean, options: FormatOptions) {
     this.enabled = enabled;
     this.options = {
-      numIndent: 1,
-      fieldMaxLineLength: 60,
-      fieldSubqueryParensOnOwnLine: true,
-      newLineAfterKeywords: false,
-      logging: false,
-      ...options,
+      numIndent: options.numIndent ?? 1,
+      fieldMaxLineLength: options.fieldMaxLineLength ?? 60,
+      fieldSubqueryParensOnOwnLine: options.fieldSubqueryParensOnOwnLine ?? true,
+      whereClauseOperatorsIndented: true,
+      newLineAfterKeywords: options.newLineAfterKeywords ?? false,
+      logging: options.logging ?? false,
     };
     if (this.options.newLineAfterKeywords) {
       this.options.fieldSubqueryParensOnOwnLine = true;
@@ -77,7 +81,7 @@ export class Formatter {
   }
 
   private getIndent(additionalIndent = 0) {
-    return this.repeatChar((this.currIndent + additionalIndent) * this.options.numIndent, '\t');
+    return this.repeatChar((this.currIndent + additionalIndent) * (this.options.numIndent || 1), '\t');
   }
 
   private repeatChar(numTimes: number, char: string) {
@@ -236,7 +240,7 @@ export class Formatter {
       groupBy.forEach((token, i) => {
         const nextToken = groupBy[i + 1];
         currLen += token.length;
-        if (nextToken && (currLen + nextToken.length > this.options.fieldMaxLineLength || this.options.newLineAfterKeywords)) {
+        if (nextToken && (currLen + nextToken.length > (this.options.fieldMaxLineLength || 0) || this.options.newLineAfterKeywords)) {
           output += `${token},\n\t`;
           currLen = 0;
         } else {
@@ -256,7 +260,7 @@ export class Formatter {
    * @param [leadingParenInline] Make the leading paren inline (last for "(" and first for ")") used for negation condition
    * @returns
    */
-  formatParens(count: number, character: '(' | ')', leadingParenInline = false) {
+  formatParens(count: number | undefined, character: '(' | ')', leadingParenInline = false) {
     let output = '';
     if (isNumber(count) && count > 0) {
       if (this.enabled) {

src/formatter/formatter.ts

@@ -18,7 +18,7 @@ export interface ExpressionTree<T> {
  */
 
 interface WithIdentifier {
-  Identifier?: IToken[];
+  Identifier: IToken[];
 }
 
 export interface SelectStatementContext {
@@ -61,7 +61,7 @@ export interface SelectClauseFunctionIdentifierContext extends WithIdentifier {
 }
 
 export interface SelectClauseSubqueryIdentifierContext extends WithIdentifier {
-  selectStatement?: CstNode[];
+  selectStatement: CstNode[];
 }
 
 export interface SelectClauseTypeOfContext extends WithIdentifier {
@@ -102,10 +102,24 @@ export interface ConditionExpressionContext {
   expression: CstNode[];
 }
 
-export interface WithClauseContext {
-  withSecurityEnforced?: CstNode[];
-  withAccessLevel?: IToken[];
-  withDataCategory?: CstNode[];
+export type WithClauseContext = WithSecurityEnforcedClauseContext | WithAccessLevelClauseContext | WithDataCategoryClauseContext;
+
+export interface WithSecurityEnforcedClauseContext {
+  withSecurityEnforced: CstNode[];
+  withAccessLevel?: never;
+  withDataCategory?: never;
+}
+
+export interface WithAccessLevelClauseContext {
+  withSecurityEnforced?: never;
+  withAccessLevel: IToken[];
+  withDataCategory?: never;
+}
+
+export interface WithDataCategoryClauseContext {
+  withSecurityEnforced?: never;
+  withAccessLevel?: never;
+  withDataCategory: CstNode[];
 }
 
 export interface WithDateCategoryContext {
@@ -163,6 +177,17 @@ export interface OperatorContext {
   operator: IToken[];
 }
 
+export type OperatorOrNotInContext = OperatorWithoutNotInContext | OperatorNotInContext;
+
+export interface OperatorWithoutNotInContext extends OperatorContext {
+  notIn?: never;
+}
+
+export interface OperatorNotInContext {
+  operator?: never;
+  notIn: CstNode[];
+}
+
 export interface BooleanContext {
   boolean: IToken[];
 }
@@ -249,10 +274,22 @@ export interface ApexBindVariableFunctionArrayAccessorContext {
   value: IToken[];
 }
 
-export interface ExpressionOperatorContext {
+export type ExpressionOperatorContext = ExpressionOperatorRhsContext & ExpressionWithRelationalOrSetOperatorContext;
+
+export interface ExpressionOperatorRhsContext {
   rhs: CstNode[];
-  relationalOperator?: CstNode[];
-  setOperator?: CstNode[];
+}
+
+type ExpressionWithRelationalOrSetOperatorContext = ExpressionWithRelationalOperatorContext | ExpressionWithSetOperatorOperatorContext;
+
+export interface ExpressionWithRelationalOperatorContext {
+  relationalOperator: CstNode[];
+  setOperator?: never;
+}
+
+export interface ExpressionWithSetOperatorOperatorContext {
+  relationalOperator?: never;
+  setOperator: CstNode[];
 }
 
 export interface FromClauseContext extends WithIdentifier {

src/models.ts

@@ -230,9 +230,6 @@ export const GroupBy = createToken({ name: 'GROUP_BY', pattern: /GROUP BY/i, lon
 export const Having = createToken({ name: 'HAVING', pattern: /HAVING/i, longer_alt: Identifier, categories: [Keyword, ReservedKeyword] });
 export const In = createToken({ name: 'IN', pattern: /IN/i, longer_alt: Identifier, categories: [Keyword, ReservedKeyword] });
 
-// FIXME: split into two tokens, NOT is a reserved keyword, IN is not
-export const NotIn = createToken({ name: 'NOT_IN', pattern: /NOT IN/i, longer_alt: Identifier });
-
 export const Includes = createToken({
   name: 'INCLUDES',
   pattern: /INCLUDES/i,
@@ -1025,7 +1022,6 @@ export const allTokens = [
   Standard,
 
   In,
-  NotIn,
   For,
   Or,
   Last,

src/parser/lexer.ts

@@ -45,14 +45,14 @@ export class SoqlParser extends CstParser {
   public allowApexBindVariables = false;
   public ignoreParseErrors = false;
 
-  constructor({ ignoreParseErrors }: { ignoreParseErrors: boolean } = { ignoreParseErrors: false }) {
+  constructor({ ignoreParseErrors }: { ignoreParseErrors?: boolean | null } = { ignoreParseErrors: false }) {
     super(lexer.allTokens, {
       // true in production (webpack replaces this string)
       skipValidations: false,
-      recoveryEnabled: ignoreParseErrors,
+      recoveryEnabled: !!ignoreParseErrors,
       // nodeLocationTracking: 'full', // not sure if needed, could look at
     });
-    this.ignoreParseErrors = ignoreParseErrors;
+    this.ignoreParseErrors = !!ignoreParseErrors;
     this.performSelfAnalysis();
   }
 
@@ -157,7 +157,7 @@ export class SoqlParser extends CstParser {
         this.$_selectClauseFunctionIdentifier ||
           (this.$_selectClauseFunctionIdentifier = [
             { ALT: () => this.SUBRULE(this.dateFunction, { LABEL: 'fn' }) },
-            { ALT: () => this.SUBRULE(this.aggregateFunction, { LABEL: 'fn', ARGS: [true] }) },
+            { ALT: () => this.SUBRULE(this.aggregateFunction, { LABEL: 'fn' }) },
             { ALT: () => this.SUBRULE(this.locationFunction, { LABEL: 'fn' }) },
             { ALT: () => this.SUBRULE(this.fieldsFunction, { LABEL: 'fn' }) },
             { ALT: () => this.SUBRULE(this.otherFunction, { LABEL: 'fn' }) },
@@ -261,7 +261,7 @@ export class SoqlParser extends CstParser {
     const parenCount = this.getParenCount();
     this.AT_LEAST_ONE({
       DEF: () => {
-        this.SUBRULE(this.conditionExpression, { ARGS: [parenCount, false, true, true] });
+        this.SUBRULE(this.conditionExpression, { ARGS: [parenCount, true, true] });
       },
     });
 
@@ -280,7 +280,7 @@ export class SoqlParser extends CstParser {
 
   private conditionExpression = this.RULE(
     'conditionExpression',
-    (parenCount?: ParenCount, allowSubquery?: boolean, allowAggregateFn?: boolean, allowLocationFn?: boolean) => {
+    (parenCount?: ParenCount, allowAggregateFn?: boolean, allowLocationFn?: boolean) => {
       // argument is undefined during self-analysis, need to initialize to avoid exception
       parenCount = this.getParenCount(parenCount);
       this.OPTION(() => {
@@ -292,15 +292,15 @@ export class SoqlParser extends CstParser {
 
       // MAX_LOOKAHEAD -> this is increased because an arbitrary number of parens could be used causing a parsing error
       // this does not allow infinite parentheses, but is more than enough for any real use-cases
-      // Under no circumstances would large numbers of nested expressions not be expressable with fewer conditions
+      // Under no circumstances would large numbers of nested expressions not be expressible with fewer conditions
       this.MANY({
         MAX_LOOKAHEAD: 10,
         DEF: () => this.SUBRULE(this.expressionPartWithNegation, { ARGS: [parenCount], LABEL: 'expressionNegation' }),
       });
 
       this.OR1({
         MAX_LOOKAHEAD: 10,
-        DEF: [{ ALT: () => this.SUBRULE(this.expression, { ARGS: [parenCount, allowSubquery, allowAggregateFn, allowLocationFn] }) }],
+        DEF: [{ ALT: () => this.SUBRULE(this.expression, { ARGS: [parenCount, false, allowAggregateFn, allowLocationFn] }) }],
       });
     },
   );
@@ -372,7 +372,7 @@ export class SoqlParser extends CstParser {
     const parenCount = this.getParenCount();
     this.AT_LEAST_ONE({
       DEF: () => {
-        this.SUBRULE(this.conditionExpression, { ARGS: [parenCount, true, undefined, undefined] });
+        this.SUBRULE(this.conditionExpression, { ARGS: [parenCount, true, false] });
       },
     });
 
@@ -575,8 +575,8 @@ export class SoqlParser extends CstParser {
       });
 
       this.OR1([
-        { GATE: () => allowAggregateFn, ALT: () => this.SUBRULE(this.aggregateFunction, { LABEL: 'lhs' }) },
-        { GATE: () => allowLocationFn, ALT: () => this.SUBRULE(this.locationFunction, { LABEL: 'lhs' }) },
+        { GATE: () => !!allowAggregateFn, ALT: () => this.SUBRULE(this.aggregateFunction, { LABEL: 'lhs' }) },
+        { GATE: () => !!allowLocationFn, ALT: () => this.SUBRULE(this.locationFunction, { LABEL: 'lhs' }) },
         { ALT: () => this.SUBRULE(this.dateFunction, { LABEL: 'lhs' }) },
         { ALT: () => this.SUBRULE(this.otherFunction, { LABEL: 'lhs' }) },
         { ALT: () => this.CONSUME(lexer.Identifier, { LABEL: 'lhs' }) },
@@ -757,12 +757,17 @@ export class SoqlParser extends CstParser {
   private setOperator = this.RULE('setOperator', () => {
     this.OR([
       { ALT: () => this.CONSUME(lexer.In, { LABEL: 'operator' }) },
-      { ALT: () => this.CONSUME(lexer.NotIn, { LABEL: 'operator' }) },
+      { ALT: () => this.SUBRULE(this.notInOperator, { LABEL: 'notIn' }) },
       { ALT: () => this.CONSUME(lexer.Includes, { LABEL: 'operator' }) },
       { ALT: () => this.CONSUME(lexer.Excludes, { LABEL: 'operator' }) },
     ]);
   });
 
+  private notInOperator = this.RULE('notInOperator', () => {
+    this.CONSUME(lexer.Not, { LABEL: 'operator' });
+    this.CONSUME(lexer.In, { LABEL: 'operator' });
+  });
+
   private booleanValue = this.RULE('booleanValue', () => {
     this.OR([
       { ALT: () => this.CONSUME(lexer.True, { LABEL: 'boolean' }) },