feat!!: Disallow mixing delimiter types in Boolean queries

src/Query/Filter/BooleanDelimiters.ts

@@ -5,7 +5,9 @@ export function anyOfTheseChars(allowedChars: string): string {
 /**
  * A class to try to identify the type of delimiter used between Boolean operators.
  *
- * Currently under development
+ * Note that this only checks the first and last non-operator characters on the line,
+ * so where there is more than one binary operator, it is still possible for the user
+ * to mix delimiters, and for the error to not be detected until later in the parsing process.
  */
 export class BooleanDelimiters {
     public readonly openFilterChars;
@@ -28,4 +30,31 @@ export class BooleanDelimiters {
     public static allSupportedDelimiters(): BooleanDelimiters {
         return new BooleanDelimiters('("', ')"', '()"');
     }
+
+    public static fromInstructionLine(instruction: string) {
+        const trimmedInstruction = instruction.trim();
+
+        // We use a set of capitals and spaces as a short-cut to match AND, OR, NOT, AND NOT etc.
+        // The only valid initial operator is NOT, so this may be worth tightening up, if
+        // further tests show that it would be worthwhile.
+        const findAnyInitialUnaryOperator = /^[A-Z ]*\s*(.*)/;
+        const matches = findAnyInitialUnaryOperator.exec(trimmedInstruction);
+        if (matches) {
+            const instructionWithoutAnyLeadingOperators = matches[1];
+            const firstChar = instructionWithoutAnyLeadingOperators[0];
+            const lastChar = instructionWithoutAnyLeadingOperators.slice(-1);
+
+            if (firstChar === '(' && lastChar === ')') {
+                return new BooleanDelimiters('(', ')', '()');
+            }
+
+            if (firstChar === '"' && lastChar === '"') {
+                return new BooleanDelimiters('"', '"', '"');
+            }
+        }
+
+        throw new Error(
+            'All filters in a Boolean instruction must be inside one of these pairs of delimiter characters: (...) or "...". Combinations of those delimiters are no longer supported.',
+        );
+    }
 }

src/Query/Filter/BooleanField.ts

@@ -75,7 +75,15 @@ export class BooleanField extends Field {
             return FilterOrErrorMessage.fromError(line, 'empty line');
         }
 
-        const parseResult = BooleanPreprocessor.preprocessExpression(line);
+        let delimiters;
+        try {
+            delimiters = BooleanDelimiters.fromInstructionLine(line);
+        } catch (error) {
+            const message = error instanceof Error ? error.message : 'unknown error type';
+            return FilterOrErrorMessage.fromError(line, this.helpMessageFromSimpleError(line, message));
+        }
+
+        const parseResult = BooleanPreprocessor.preprocessExpression(line, delimiters);
         const simplifiedLine = parseResult.simplifiedLine;
         const filters = parseResult.filters;
         try {

src/Query/Filter/BooleanPreprocessor.ts

@@ -6,14 +6,12 @@ export type BooleanPreprocessorResult = {
 };
 
 export class BooleanPreprocessor {
-    public static preprocessExpression(line: string): BooleanPreprocessorResult {
-        const parts = BooleanPreprocessor.splitLine(line);
-        return BooleanPreprocessor.getFiltersAndSimplifiedLine(parts);
+    public static preprocessExpression(line: string, delimiters: BooleanDelimiters): BooleanPreprocessorResult {
+        const parts = BooleanPreprocessor.splitLine(line, delimiters);
+        return BooleanPreprocessor.getFiltersAndSimplifiedLine(parts, delimiters);
     }
 
-    public static splitLine(line: string) {
-        const delimiters = BooleanDelimiters.allSupportedDelimiters();
-
+    public static splitLine(line: string, delimiters: BooleanDelimiters) {
         // Here, we split the input line in to separate operators-plus-adjacent-delimiters
         // and the remaining filter text.
 
@@ -69,9 +67,7 @@ export class BooleanPreprocessor {
             .filter((substring) => substring !== '');
     }
 
-    private static getFiltersAndSimplifiedLine(parts: string[]) {
-        const delimiters = BooleanDelimiters.allSupportedDelimiters();
-
+    private static getFiltersAndSimplifiedLine(parts: string[], delimiters: BooleanDelimiters) {
         // Holds the reconstructed expression with placeholders
         let simplifiedLine = '';
         let currentIndex = 1; // Placeholder index starts at 1

tests/Query/Filter/BooleanDelimiters.test.ts

@@ -1,5 +1,31 @@
 import { BooleanDelimiters } from '../../../src/Query/Filter/BooleanDelimiters';
 
+function shouldDelimitWithParentheses(line: string) {
+    const delimiters = BooleanDelimiters.fromInstructionLine(line);
+
+    expect(delimiters.openFilterChars).toEqual('(');
+    expect(delimiters.closeFilterChars).toEqual(')');
+    expect(delimiters.openAndCloseFilterChars).toEqual('()');
+}
+
+function shouldDelimitWithDoubleQuotes(line: string) {
+    const delimiters = BooleanDelimiters.fromInstructionLine(line);
+
+    expect(delimiters.openFilterChars).toEqual('"');
+    expect(delimiters.closeFilterChars).toEqual('"');
+    expect(delimiters.openAndCloseFilterChars).toEqual('"');
+}
+
+function shouldThrow(line: string) {
+    const t = () => {
+        BooleanDelimiters.fromInstructionLine(line);
+    };
+    expect(t).toThrow(Error);
+    expect(t).toThrowError(
+        'All filters in a Boolean instruction must be inside one of these pairs of delimiter characters: (...) or "...". Combinations of those delimiters are no longer supported.',
+    );
+}
+
 describe('BooleanDelimiters', () => {
     it('construction - all delimiters', () => {
         const delimiters = BooleanDelimiters.allSupportedDelimiters();
@@ -12,4 +38,51 @@ describe('BooleanDelimiters', () => {
 
         expect(delimiters.openAndCloseFilterChars).toEqual('()"');
     });
+
+    describe('construct from line with binary operators', () => {
+        it('should recognise () delimiters', () => {
+            shouldDelimitWithParentheses('(not done) OR (done)');
+        });
+
+        it('does not recognise inconsistent delimiters in middle of line', () => {
+            shouldDelimitWithParentheses('(not done) OR "done" OR (done)');
+        });
+
+        it('should recognise "" delimiters', () => {
+            shouldDelimitWithDoubleQuotes('"not done" OR "done"');
+        });
+
+        it('should reject line with mixed delimiters', () => {
+            shouldThrow('(not done) OR "done"');
+        });
+
+        it('should reject line with unknown delimiters', () => {
+            shouldThrow('{not done} OR {done}');
+        });
+    });
+
+    describe('construct from line starting with NOT', () => {
+        it('should recognise () delimiters', () => {
+            shouldDelimitWithParentheses('NOT (not done)');
+        });
+
+        it('should recognise "" delimiters', () => {
+            shouldDelimitWithDoubleQuotes('NOT "not done"');
+        });
+
+        it('should reject line with mixed delimiters', () => {
+            shouldThrow('NOT (not done"');
+        });
+
+        it('should reject line with unknown delimiters', () => {
+            shouldThrow('NOT {not done}');
+        });
+    });
+
+    describe('error cases', () => {
+        it('should throw if line is too short', () => {
+            shouldThrow('');
+            shouldThrow('x');
+        });
+    });
 });