Repository: obsidian-tasks-group/obsidian-tasks (ts)

Issue:
Work out how to prevent `path regex matches /a/b/c/d/` from confusingly only searching `path regex matches /a/`.
From code review in https://github.com/obsidian-tasks-group/obsidian-tasks/pull/1015

> Open an Issue against this?
> 
> Fixable with another regex. 😉😂🤣🤪

_Originally posted by @elight in https://github.com/obsidian-tasks-group/obsidian-tasks/pull/1015#discussion_r945393546_

### Expected Behavior

```text
path regex matches /a/b/c/d/
```

Should give an error message, something along the lines of:

> An unescaped delimiter must be escaped; in most languages with a backslash (\)

So that the user can work out that the correct search is:

```text
path regex matches /\a/b\/c\/d/
```

### Current behaviour

The search proceeds, doing this search, and potentially matching many unwanted hits:

```text
path regex matches /a/
```

### Steps to reproduce

Construct a search similar to the above, and run in Tasks 1.12.0, when it is relased.

Or open the file `Regular Expression Searches.md` in the Tasks-Demo vault, and read the section 'Extra slashes truncate the query'

Or run this test, and remove all the `failing` text:

https://github.com/obsidian-tasks-group/obsidian-tasks/blob/60c5cd45d6d236f3fe8b4b1628b03953a4c19867/tests/Query/Filter/PathField.test.ts#L158-L187

### Which Operating Systems are you using?

- [ ] Android
- [ ] iPhone/iPad
- [ ] Linux
- [x] macOS
- [ ] Windows

### Obsidian Version

0.15.9

### Tasks Plugin Version

1.12.0 (pre-release)

### Checks

- [ ] I have tried it with all other plugins disabled and the error still occurs

### Suggested fix


#### Current behaviour

I believe that the problem is that the regular expression is the problem:

https://github.com/obsidian-tasks-group/obsidian-tasks/blob/114c8cd7614bbcba1f746b742658c9bcf99fd5bf/src/Query/Filter/TextField.ts#L32-L33

It gets as much of the regex as is valid, and ignores any later text.

This hides any errors from the user.

#### Requested behaviour

- I would like the full regular expression that is supplied by the user to be passed to the regex library
- and then for Tasks to check if there were any error messages detected
- and if there was an error, report it the the user and then refuse to search.

In other words, I don't feel that it is Tasks' job to validate the regex supplied to the user.

I feel that the regex library should do this, as it is bound to do a better and more informative job than we can possibly do.



Pull request:
fix: prevent truncation of regex /a/b/c/d/ searching only /a/
<!--- Provide a general summary of your changes in the Title above -->

# Description

I finally worked out how to prevent `path regex matches /a/b/c/d/` from confusingly only searching `path regex matches /a/`


It turned out that when creating `RegExp` objects from strings, TypeScript automatically escapes un-escaped forward slashes (/).

But the previous validation code in `validateAndConstruct()` was wrongly truncating queries at the first unescaped forward slash.

By removing that validation, it is now possible to use unescaped paths in regular expressions (providing that they don't have any other unescaped special characters, of course).

And if the regular expression does have an error, the user will now get a meaningful error message via the exception thrown by the `RegExp` constructor.

Previous searches where '/' characters are escaped will still work, but this escaping is no longer necessary.

Note: Some documentation will need updating for this change, but for now I'm focussing on finishing a series of RegExp fixes.

## Motivation and Context

This fixes #1037.


<!--- 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?

<!--- 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. -->

- Running and updating existing tests
- Adding new tests
- Testing manually in demo vault

## Screenshots (if appropriate)

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

Changes visible to users:

- [x] **Bug fix** (prefix: `fix` - non-breaking change which fixes an issue)

Internal changes:

- [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`.
- [x] My change requires a change to the documentation.
- [ ] I have [updated the documentation](https://publish.obsidian.md/tasks-contributing/Documentation/About+Documentation) accordingly.
- [x] My change has adequate [Unit Test coverage](https://publish.obsidian.md/tasks-contributing/Testing/About+Testing).

## Terms

<!--
By submitting this pull request, you must agree to follow our
[contributing guide](https://publish.obsidian.md/tasks-contributing) 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://publish.obsidian.md/tasks-contributing)
- [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/Matchers/RegexMatcher.ts b/src/Query/Matchers/RegexMatcher.ts
index 855e39e4..dd6693ac 100644
--- a/src/Query/Matchers/RegexMatcher.ts
+++ b/src/Query/Matchers/RegexMatcher.ts
@@ -27,8 +27,7 @@ export class RegexMatcher extends IStringMatcher {
      * @throws {SyntaxError} Throws an exception if there was an error in {@link regexInput}.
      */
     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\]\\]|\\.)*])+)\/([igmu]*)/;
+        const regexPattern = /^\/(.+)\/([igmu]*)/;
         const query = regexInput.match(regexPattern);
 
         if (query !== null) {


Test patch:
diff --git a/tests/Query/Filter/PathField.test.ts b/tests/Query/Filter/PathField.test.ts
index dce08b18..56e95c24 100644
--- a/tests/Query/Filter/PathField.test.ts
+++ b/tests/Query/Filter/PathField.test.ts
@@ -76,33 +76,18 @@ describe('path', () => {
     });
 });
 
-describe('invalid unescaped slash should give helpful error text and not search', () => {
+describe('should use whole path with un-escaped slashes in query', () => {
     const filterWithUnescapedSlashes = new PathField().createFilterOrErrorMessage(
         String.raw`path regex matches /a/b/c/d/`,
     );
 
-    // This test demonstrates the issue logged in
-    // https://github.com/obsidian-tasks-group/obsidian-tasks/issues/1037
-    //      'Work out how to prevent `path regex matches /a/b/c/d/` from
-    //       confusingly only searching `path regex matches /a/`.
-    // All these tests are marked as 'failing' because the code currently accepts
-    // an invalid search.
-    it.failing('should not be valid', () => {
-        expect(filterWithUnescapedSlashes).not.toBeValid();
+    it('should escape backslashes in query automatically', () => {
+        expect(filterWithUnescapedSlashes).toBeValid();
+        expect(filterWithUnescapedSlashes).toHaveExplanation("using regex:     'a\\/b\\/c\\/d' with no flags");
     });
 
-    it.failing('should have a meaningful error message', () => {
-        // The error message does not have to be exactly this.
-        // The main thing is that it should convey what the user needs to do
-        // to fix the expression.
-        expect(filterWithUnescapedSlashes.error).toEqual(
-            'An unescaped delimiter must be escaped; in most languages with a backslash (\\)',
-        );
-    });
-
-    it.failing('should not match a subset of requested path', () => {
-        // Once the issue is fixed, and filterWithUnescapedSlashes is not valid,
-        // this test should be deleted.
+    it('should match the requested path', () => {
+        expect(filterWithUnescapedSlashes).toMatchTaskWithPath('/a/b/c/d/e.md');
         expect(filterWithUnescapedSlashes).not.toMatchTaskWithPath('/a/b.md');
     });
 });
diff --git a/tests/Query/Filter/TextField.test.explains_regular_expression_searches_bulk_test.approved.txt b/tests/Query/Filter/TextField.test.explains_regular_expression_searches_bulk_test.approved.txt
index f25c2958..35a0cc14 100644
--- a/tests/Query/Filter/TextField.test.explains_regular_expression_searches_bulk_test.approved.txt
+++ b/tests/Query/Filter/TextField.test.explains_regular_expression_searches_bulk_test.approved.txt
@@ -8,7 +8,7 @@ description regex matches /  / =>
   using regex:            '  ' with no flags
 
 description regex matches /#context/pc_photos|#context/pc_clare|#context/pc_macbook/i =>
-  using regex:            '#context' with no flags
+  using regex:            '#context\/pc_photos|#context\/pc_clare|#context\/pc_macbook' with flag 'i'
 
 description regex matches /#context\/pc_photos|#context\/pc_clare|#context\/pc_macbook/i =>
   using regex:            '#context\/pc_photos|#context\/pc_clare|#context\/pc_macbook' with flag 'i'
@@ -41,7 +41,7 @@ filename regex does not match /^Tasks User Support Kanban\.md$/ =>
   using regex:                '^Tasks User Support Kanban\.md$' with no flags
 
 folder regex matches /root/sub-folder/sub-sub-folder/ =>
-  using regex:       'root' with no flags
+  using regex:       'root\/sub-folder\/sub-sub-folder' with no flags
 
 folder regex matches /root\/sub-folder\/sub-sub-folder/ =>
   using regex:       'root\/sub-folder\/sub-sub-folder' with no flags
@@ -77,7 +77,7 @@ path regex matches /clare/i =>
   using regex:     'clare' with flag 'i'
 
 path regex matches /root/sub-folder/sub-sub-folder/index\.md/ =>
-  using regex:     'root' with no flags
+  using regex:     'root\/sub-folder\/sub-sub-folder\/index\.md' with no flags
 
 path regex matches /root\/sub-folder\/sub-sub-folder\/index\.md/ =>
   using regex:     'root\/sub-folder\/sub-sub-folder\/index\.md' with no flags
diff --git a/tests/Query/Matchers/RegexMatcher.test.ts b/tests/Query/Matchers/RegexMatcher.test.ts
index 70346d91..288dc0b4 100644
--- a/tests/Query/Matchers/RegexMatcher.test.ts
+++ b/tests/Query/Matchers/RegexMatcher.test.ts
@@ -23,6 +23,18 @@ describe('RegexMatcher', () => {
     });
 });
 
+describe('RegexMatcher source', () => {
+    it('should allow multiple slashes in source', () => {
+        const matcher = RegexMatcher.validateAndConstruct('/a/b/c/d/');
+        expect(matcher!.regex.source).toEqual(String.raw`a\/b\/c\/d`);
+    });
+
+    it('should allow multiple slashes and delimiters in source', () => {
+        const matcher = RegexMatcher.validateAndConstruct('//a/b/c/d//');
+        expect(matcher!.regex.source).toEqual(String.raw`\/a\/b\/c\/d\/`);
+    });
+});
+
 describe('RegexMatcher flags', () => {
     it('should allow "i" flag - case-insensitive', () => {
         const matcher = RegexMatcher.validateAndConstruct('/expression/i');