Repository: obsidian-tasks-group/obsidian-tasks
Pull request #1054 (2022-08-20 21:05:14)

refactor: Extract text searching out of TextField
<!--- Provide a general summary of your changes in the Title above -->

# Description

Extract 3 classes out of `TextField`:

- `IStringMatcher`
- `SubstringMatcher`
- `RegexMatcher`

These hide away the details of how the `includes` and `regex matches` searches are constructed and executed, which will allow for re-use in the `TagsField` class later.

All steps were done as small, safe refactorings, mostly done automatically via WebStorm's 'Refactor this' feature.

### Later work...

**Note that there is scope for more simplification of the code in a future PR**

There are now two identical copies of this code:

```ts
                result.filter = (task: Task) => {
                    return TextField.maybeNegate(
                        matcher.matches(this.value(task)),
                        filterMethod,
                    );
                };
```

In other words, the new interface means that `TextSearch` now doesn't care whether it is doing a substring search or a regex search - the resulting filtering code is identical.

However, when I went through the next refactoring steps in this branch, I felt that the changes were too large, and they obscured the simplicity of this first step, so I reverted them for now.

## Motivation and Context

Preparation for adding regex-based searching of tags, without needing to duplicate all the regex-handling code from `TextField`.

<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->



## How has this been tested?

- Running existing tests
- Adding new tests
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, tests ran to see how -->
<!--- your change affects other areas of the code, etc. -->

## Screenshots (if appropriate)

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

Internal changes:

- [x] **Refactor** (prefix: `refactor` - non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)
- [x] **Tests** (prefix: `test` - additions and improvements to unit tests and the smoke tests)

## Checklist

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [x] My code follows the code style of this project and passes `yarn run lint`.
- [ ] My change requires a change to the documentation.
- [ ] I have [updated the documentation](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md#updating-documentation) accordingly.
- [x] My change has adequate [Unit Test coverage](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md#maintaining-the-tests).

## Terms

<!--
By submitting this pull request, you must agree to follow our
[contributing guide](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md) and
[Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md).
Put an x in the boxes to confirm you agree.
-->

- [x] My contribution follow this project's [contributing guide](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md)
- [x] I agree to follow this project's [Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md)



Patch:
diff --git a/src/Query/Filter/TextField.ts b/src/Query/Filter/TextField.ts
index bca075b7..1036887b 100644
--- a/src/Query/Filter/TextField.ts
+++ b/src/Query/Filter/TextField.ts
@@ -1,4 +1,6 @@
 import type { Task } from '../../Task';
+import { SubstringMatcher } from '../Matchers/SubstringMatcher';
+import { RegexMatcher } from '../Matchers/RegexMatcher';
 import { Field } from './Field';
 import { FilterOrErrorMessage } from './Filter';
 
@@ -14,30 +16,24 @@ export abstract class TextField extends Field {
         if (match !== null) {
             const filterMethod = match[1];
             if (['includes', 'does not include'].includes(filterMethod)) {
-                result.filter = (task: Task) =>
-                    TextField.maybeNegate(
-                        TextField.stringIncludesCaseInsensitive(
-                            this.value(task),
-                            match[2],
-                        ),
+                const matcher = new SubstringMatcher(match[2]);
+                result.filter = (task: Task) => {
+                    return TextField.maybeNegate(
+                        matcher.matches(this.value(task)),
                         filterMethod,
                     );
+                };
             } else if (
                 ['regex matches', 'regex does not match'].includes(filterMethod)
             ) {
-                // Courtesy of https://stackoverflow.com/questions/17843691/javascript-regex-to-match-a-regex
-                const regexPattern =
-                    /\/((?![*+?])(?:[^\r\n[/\\]|\\.|\[(?:[^\r\n\]\\]|\\.)*])+)\/((?:g(?:im?|mi?)?|i(?:gm?|mg?)?|m(?:gi?|ig?)?)?)/;
-                const query = match[2].match(regexPattern);
-
-                if (query !== null) {
-                    result.filter = (task: Task) =>
-                        TextField.maybeNegate(
-                            this.value(task).match(
-                                new RegExp(query[1], query[2]),
-                            ) !== null,
+                const matcher = RegexMatcher.validateAndConstruct(match[2]);
+                if (matcher !== null) {
+                    result.filter = (task: Task) => {
+                        return TextField.maybeNegate(
+                            matcher.matches(this.value(task)),
                             filterMethod,
                         );
+                    };
                 } else {
                     result.error = `cannot parse regex (${this.fieldName()}); check your leading and trailing slashes for your query`;
                 }
@@ -54,9 +50,7 @@ export abstract class TextField extends Field {
         haystack: string,
         needle: string,
     ): boolean {
-        return haystack
-            .toLocaleLowerCase()
-            .includes(needle.toLocaleLowerCase());
+        return SubstringMatcher.stringIncludesCaseInsensitive(haystack, needle);
     }
 
     protected filterRegexp(): RegExp {
diff --git a/src/Query/Matchers/IStringMatcher.ts b/src/Query/Matchers/IStringMatcher.ts
new file mode 100644
index 00000000..7b861454
--- /dev/null
+++ b/src/Query/Matchers/IStringMatcher.ts
@@ -0,0 +1,13 @@
+/**
+ * An interface for determining whether a string value matches a particular condition.
+ *
+ * This is used to hide away the details of various text searches, such as the
+ * simple inclusion of a sub-string, or the more complex regular expression searches.
+ */
+export interface IStringMatcher {
+    /**
+     * Return whether the given string matches this condition.
+     * @param stringToSearch
+     */
+    matches(stringToSearch: string): boolean;
+}
diff --git a/src/Query/Matchers/RegexMatcher.ts b/src/Query/Matchers/RegexMatcher.ts
new file mode 100644
index 00000000..612f2387
--- /dev/null
+++ b/src/Query/Matchers/RegexMatcher.ts
@@ -0,0 +1,45 @@
+import type { IStringMatcher } from './IStringMatcher';
+
+/**
+ * Regular-expression-based implementation of IStringMatcher.
+ */
+export class RegexMatcher implements IStringMatcher {
+    private readonly regex: RegExp;
+
+    /**
+     * Construct a RegexMatcher object.
+     *
+     * @param regex {RegExp} - A valid regular expression
+     */
+    public constructor(regex: RegExp) {
+        this.regex = regex;
+    }
+
+    /**
+     * Construct a RegexMatcher object if the supplied string is a valid regular expression.
+     * and null if not.
+     *
+     * @param {string} regexInput - A string that can be converted to a regular expression.
+     *                              It must begin with a /, and end either with / and optionally any
+     *                              valid flags.
+     */
+    public static validateAndConstruct(
+        regexInput: string,
+    ): RegexMatcher | null {
+        // Courtesy of https://stackoverflow.com/questions/17843691/javascript-regex-to-match-a-regex
+        const regexPattern =
+            /\/((?![*+?])(?:[^\r\n[/\\]|\\.|\[(?:[^\r\n\]\\]|\\.)*])+)\/((?:g(?:im?|mi?)?|i(?:gm?|mg?)?|m(?:gi?|ig?)?)?)/;
+        const query = regexInput.match(regexPattern);
+
+        if (query !== null) {
+            const regExp = new RegExp(query[1], query[2]);
+            return new RegexMatcher(regExp);
+        } else {
+            return null;
+        }
+    }
+
+    public matches(stringToSearch: string): boolean {
+        return stringToSearch.match(this.regex) !== null;
+    }
+}
diff --git a/src/Query/Matchers/SubstringMatcher.ts b/src/Query/Matchers/SubstringMatcher.ts
new file mode 100644
index 00000000..a24ce654
--- /dev/null
+++ b/src/Query/Matchers/SubstringMatcher.ts
@@ -0,0 +1,36 @@
+import type { IStringMatcher } from './IStringMatcher';
+
+/**
+ * Substring-based implementation of IStringMatcher.
+ *
+ * This does a case-insensitive search for the given string.
+ */
+export class SubstringMatcher implements IStringMatcher {
+    private readonly stringToFind: string;
+
+    /**
+     * Construct a SubstringMatcher object
+     *
+     * @param {string} stringToFind - The string to search for.
+     *                                Searches will be case-insensitive.
+     */
+    public constructor(stringToFind: string) {
+        this.stringToFind = stringToFind;
+    }
+
+    public matches(stringToSearch: string): boolean {
+        return SubstringMatcher.stringIncludesCaseInsensitive(
+            stringToSearch,
+            this.stringToFind,
+        );
+    }
+
+    public static stringIncludesCaseInsensitive(
+        haystack: string,
+        needle: string,
+    ): boolean {
+        return haystack
+            .toLocaleLowerCase()
+            .includes(needle.toLocaleLowerCase());
+    }
+}


Test patch:
diff --git a/tests/Query/Matchers/RegexMatcher.test.ts b/tests/Query/Matchers/RegexMatcher.test.ts
new file mode 100644
index 00000000..4c1a5778
--- /dev/null
+++ b/tests/Query/Matchers/RegexMatcher.test.ts
@@ -0,0 +1,24 @@
+import { RegexMatcher } from '../../../src/Query/Matchers/RegexMatcher';
+
+describe('RegexMatcher', () => {
+    it('should match simple regular expression', () => {
+        const regex: RegExp = /^hello world$/;
+        const matcher = new RegexMatcher(regex);
+        expect(matcher.matches('hello world')).toStrictEqual(true);
+        expect(matcher.matches('xx hello world yy')).toStrictEqual(false);
+        expect(matcher.matches('search me')).toStrictEqual(false);
+    });
+
+    it('should construct regex from a valid string', () => {
+        const matcher = RegexMatcher.validateAndConstruct('/hello world/i');
+        expect(matcher).not.toBeNull();
+        expect(matcher!.matches('hello world')).toStrictEqual(true);
+        expect(matcher!.matches('HELLO world')).toStrictEqual(true); // Confirm that the flag 'i' is retained
+        expect(matcher!.matches('Bye Bye')).toStrictEqual(false);
+    });
+
+    it('should not construct regex from an INvalid string', () => {
+        const matcher = RegexMatcher.validateAndConstruct('I have no slashes');
+        expect(matcher).toBeNull();
+    });
+});
diff --git a/tests/Query/Matchers/SubstringMatcher.test.ts b/tests/Query/Matchers/SubstringMatcher.test.ts
new file mode 100644
index 00000000..7e488df6
--- /dev/null
+++ b/tests/Query/Matchers/SubstringMatcher.test.ts
@@ -0,0 +1,10 @@
+import { SubstringMatcher } from '../../../src/Query/Matchers/SubstringMatcher';
+
+describe('SubstringMatcher', () => {
+    it('should match simple text', () => {
+        const matcher = new SubstringMatcher('find me');
+        expect(matcher.matches('find me')).toStrictEqual(true);
+        expect(matcher.matches('prefix find me suffix')).toStrictEqual(true);
+        expect(matcher.matches('FIND ME')).toStrictEqual(true);
+    });
+});