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

Issue:
Reduce cyclic dependencies in source code
### Description

Currently the following cyclic dependencies exist in `src/`

```text
Processed 85 files (1s) (11 warnings)

1) Config/Settings.ts > Config/StatusSettings.ts > StatusRegistry.ts > Task.ts
2) Config/Settings.ts > Config/StatusSettings.ts > StatusRegistry.ts > Task.ts > DateFallback.ts
3) Task.ts > DateFallback.ts
4) Task.ts > Query/Filter/DateField.ts > Query/Filter/Field.ts > Query/Filter/Filter.ts
5) Task.ts > Query/Filter/DateField.ts > Query/Filter/Field.ts > Query/Grouper.ts
6) Task.ts > Query/Filter/DateField.ts > Query/Filter/Field.ts > Query/Sorter.ts
7) Task.ts > Query/Filter/DateField.ts
8) StatusRegistry.ts > Task.ts
9) Config/Settings.ts > Config/StatusSettings.ts > StatusRegistry.ts > Task.ts > TaskLineRenderer.ts
10) Config/Settings.ts > Config/StatusSettings.ts > StatusRegistry.ts > Task.ts > TaskLineRenderer.ts > File.ts
11) Task.ts > TaskLineRenderer.ts > File.ts
12) Task.ts > TaskLineRenderer.ts
13) Task.ts > Urgency.ts
14) Cache.ts > TasksEvents.ts
15) Config/CustomStatusModal.ts > main.ts > Config/SettingsTab.ts
16) main.ts > Config/SettingsTab.ts
17) Query/TaskGroups.ts > Query/GroupHeadings.ts > Query/IntermediateTaskGroups.ts > Query/Group.ts
18) Query/FilterParser.ts > Query/Filter/BooleanField.ts
```

These were found by running:

```bash
npm add madge
madge --circular --extensions ts ./src
```

These do have real consequences on my productivity. Twice recently I have gone to move code around and got stuck because the cyclic dependencies caused build failures.



### Required Steps

DateField is an obvious one to address.
I don't think I will be able to fix all of them at this stage.


### How to Test

_No response_

Pull request:
refactor: Reduce cyclic dependencies
<!--- Provide a general summary of your changes in the Title above -->

# Description

<!--- Describe your changes in detail -->

Cyclic dependencies before:

```text
Processed 85 files (1s) (11 warnings)

1) Config/Settings.ts > Config/StatusSettings.ts > StatusRegistry.ts > Task.ts
2) Config/Settings.ts > Config/StatusSettings.ts > StatusRegistry.ts > Task.ts > DateFallback.ts
3) Task.ts > DateFallback.ts
4) Task.ts > Query/Filter/DateField.ts > Query/Filter/Field.ts > Query/Filter/Filter.ts
5) Task.ts > Query/Filter/DateField.ts > Query/Filter/Field.ts > Query/Grouper.ts
6) Task.ts > Query/Filter/DateField.ts > Query/Filter/Field.ts > Query/Sorter.ts
7) Task.ts > Query/Filter/DateField.ts
8) StatusRegistry.ts > Task.ts
9) Config/Settings.ts > Config/StatusSettings.ts > StatusRegistry.ts > Task.ts > TaskLineRenderer.ts
10) Config/Settings.ts > Config/StatusSettings.ts > StatusRegistry.ts > Task.ts > TaskLineRenderer.ts > File.ts
11) Task.ts > TaskLineRenderer.ts > File.ts
12) Task.ts > TaskLineRenderer.ts
13) Task.ts > Urgency.ts
14) Cache.ts > TasksEvents.ts
15) Config/CustomStatusModal.ts > main.ts > Config/SettingsTab.ts
16) main.ts > Config/SettingsTab.ts
17) Query/TaskGroups.ts > Query/GroupHeadings.ts > Query/IntermediateTaskGroups.ts > Query/Group.ts
18) Query/FilterParser.ts > Query/Filter/BooleanField.ts
```

After:

```text
Processed 86 files (1s) (11 warnings)

1) DateFallback.ts > Task.ts
2) Task.ts > TaskLineRenderer.ts > File.ts
3) Task.ts > TaskLineRenderer.ts
4) Task.ts > Urgency.ts
5) Cache.ts > TasksEvents.ts
6) Config/SettingsTab.ts > main.ts
7) Query/TaskGroups.ts > Query/GroupHeadings.ts > Query/IntermediateTaskGroups.ts > Query/Group.ts
8) Query/FilterParser.ts > Query/Filter/BooleanField.ts
```

## Motivation and Context

Fixes #1591 - as in, this has fixed the low-hanging fruit. I'm happy this is a good-enough improvement.

## 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 existing tests
- Running the demo vault & testing

## 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)

## 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/Config/CustomStatusModal.ts b/src/Config/CustomStatusModal.ts
index 806d33e5..696ee5be 100644
--- a/src/Config/CustomStatusModal.ts
+++ b/src/Config/CustomStatusModal.ts
@@ -1,6 +1,6 @@
 import { Modal, Notice, Setting, TextComponent } from 'obsidian';
+import type { Plugin } from 'obsidian';
 import { StatusConfiguration, StatusType } from '../StatusConfiguration';
-import type TasksPlugin from '../main';
 import { StatusValidator } from '../StatusValidator';
 import { Status } from '../Status';
 
@@ -16,7 +16,7 @@ export class CustomStatusModal extends Modal {
     saved: boolean = false;
     error: boolean = false;
     private isCoreStatus: boolean;
-    constructor(public plugin: TasksPlugin, statusType: StatusConfiguration, isCoreStatus: boolean) {
+    constructor(public plugin: Plugin, statusType: StatusConfiguration, isCoreStatus: boolean) {
         super(plugin.app);
         this.statusSymbol = statusType.symbol;
         this.statusName = statusType.name;
diff --git a/src/Config/SettingsTab.ts b/src/Config/SettingsTab.ts
index 7f0c841c..ed03a398 100644
--- a/src/Config/SettingsTab.ts
+++ b/src/Config/SettingsTab.ts
@@ -454,7 +454,10 @@ export class SettingsTab extends PluginSettingTab {
                 .setCta()
                 .onClick(async () => {
                     const tasks = this.plugin.getTasks();
-                    const unknownStatuses = StatusRegistry.getInstance().findUnknownStatuses(tasks!);
+                    const allStatuses = tasks!.map((task) => {
+                        return task.status;
+                    });
+                    const unknownStatuses = StatusRegistry.getInstance().findUnknownStatuses(allStatuses);
                     if (unknownStatuses.length === 0) {
                         return;
                     }
diff --git a/src/Query/Filter/DateField.ts b/src/Query/Filter/DateField.ts
index 68881d9f..1f68cbda 100644
--- a/src/Query/Filter/DateField.ts
+++ b/src/Query/Filter/DateField.ts
@@ -3,6 +3,7 @@ import type { Task } from '../../Task';
 import { DateParser } from '../DateParser';
 import { Explanation } from '../Explain/Explanation';
 import type { Comparator } from '../Sorter';
+import { compareByDate } from '../../lib/DateTools';
 import { Field } from './Field';
 import { Filter, FilterOrErrorMessage } from './Filter';
 import { FilterInstructions } from './FilterInstructions';
@@ -127,31 +128,7 @@ export abstract class DateField extends Field {
 
     public comparator(): Comparator {
         return (a: Task, b: Task) => {
-            return DateField.compareByDate(this.date(a), this.date(b));
+            return compareByDate(this.date(a), this.date(b));
         };
     }
-
-    public static compareByDate(a: moment.Moment | null, b: moment.Moment | null): -1 | 0 | 1 {
-        if (a !== null && b === null) {
-            return -1;
-        } else if (a === null && b !== null) {
-            return 1;
-        } else if (a !== null && b !== null) {
-            if (a.isValid() && !b.isValid()) {
-                return -1;
-            } else if (!a.isValid() && b.isValid()) {
-                return 1;
-            }
-
-            if (a.isAfter(b)) {
-                return 1;
-            } else if (a.isBefore(b)) {
-                return -1;
-            } else {
-                return 0;
-            }
-        } else {
-            return 0;
-        }
-    }
 }
diff --git a/src/Query/Filter/HappensDateField.ts b/src/Query/Filter/HappensDateField.ts
index bf9cf187..88562a51 100644
--- a/src/Query/Filter/HappensDateField.ts
+++ b/src/Query/Filter/HappensDateField.ts
@@ -3,6 +3,7 @@ import type { Task } from '../../Task';
 import { DateParser } from '../DateParser';
 import { Explanation } from '../Explain/Explanation';
 import type { Comparator } from '../Sorter';
+import { compareByDate } from '../../lib/DateTools';
 import { Field } from './Field';
 import { Filter, FilterOrErrorMessage } from './Filter';
 import { FilterInstructions } from './FilterInstructions';
@@ -92,7 +93,7 @@ export class HappensDateField extends Field {
      */
     public earliestDate(task: Task): Moment | null {
         const happensDates = new HappensDateField().dates(task);
-        const sortedHappensDates = happensDates.sort(DateField.compareByDate);
+        const sortedHappensDates = happensDates.sort(compareByDate);
         return sortedHappensDates[0];
     }
 
@@ -120,7 +121,7 @@ export class HappensDateField extends Field {
      */
     public comparator(): Comparator {
         return (a: Task, b: Task) => {
-            return DateField.compareByDate(this.earliestDate(a), this.earliestDate(b));
+            return compareByDate(this.earliestDate(a), this.earliestDate(b));
         };
     }
 }
diff --git a/src/Recurrence.ts b/src/Recurrence.ts
index 8d8d91d7..af001e65 100644
--- a/src/Recurrence.ts
+++ b/src/Recurrence.ts
@@ -1,6 +1,6 @@
 import type { Moment } from 'moment';
 import { RRule } from 'rrule';
-import { DateField } from './Query/Filter/DateField';
+import { compareByDate } from './lib/DateTools';
 
 export class Recurrence {
     private readonly rrule: RRule;
@@ -195,13 +195,13 @@ export class Recurrence {
         }
 
         // Compare Date fields
-        if (DateField.compareByDate(this.startDate, other.startDate) !== 0) {
+        if (compareByDate(this.startDate, other.startDate) !== 0) {
             return false;
         }
-        if (DateField.compareByDate(this.scheduledDate, other.scheduledDate) !== 0) {
+        if (compareByDate(this.scheduledDate, other.scheduledDate) !== 0) {
             return false;
         }
-        if (DateField.compareByDate(this.dueDate, other.dueDate) !== 0) {
+        if (compareByDate(this.dueDate, other.dueDate) !== 0) {
             return false;
         }
 
diff --git a/src/StatusRegistry.ts b/src/StatusRegistry.ts
index 2070d3e9..3492cf12 100644
--- a/src/StatusRegistry.ts
+++ b/src/StatusRegistry.ts
@@ -1,6 +1,5 @@
 import { Status } from './Status';
 import { StatusConfiguration, StatusType } from './StatusConfiguration';
-import type { Task } from './Task';
 
 /**
  * Tracks all the registered statuses a task can have.
@@ -187,19 +186,15 @@ export class StatusRegistry {
     }
 
     /**
-     * Find any statuses in the given tasks that are not known to this registry.
+     * Find any statuses in the given list that are not known to this registry.
      * This can be used to add all unknown status types to the settings,
      * to save users from having to do that manually.
      *
      * Statuses are returned in the order that they are first found in the
-     * supplied tasks.
-     * @param tasks
+     * supplied list.
+     * @param allStatuses
      */
-    public findUnknownStatuses(tasks: Task[]): Status[] {
-        const allStatuses = tasks.map((task) => {
-            return task.status;
-        });
-
+    public findUnknownStatuses(allStatuses: Status[]): Status[] {
         const unknownStatuses = allStatuses.filter((s) => {
             return !this.hasSymbol(s.symbol);
         });
diff --git a/src/Task.ts b/src/Task.ts
index cc542405..0aa65d3b 100644
--- a/src/Task.ts
+++ b/src/Task.ts
@@ -6,11 +6,11 @@ import { getSettings } from './Config/Settings';
 import { StatusRegistry } from './StatusRegistry';
 import type { Status } from './Status';
 import { Urgency } from './Urgency';
-import { DateField } from './Query/Filter/DateField';
 import { renderTaskLine } from './TaskLineRenderer';
 import type { TaskLineRenderDetails } from './TaskLineRenderer';
 import { DateFallback } from './DateFallback';
 import * as RegExpTools from './lib/RegExpTools';
+import { compareByDate } from './lib/DateTools';
 
 /**
  * When sorting, make sure low always comes after none. This way any tasks with low will be below any exiting
@@ -686,7 +686,7 @@ export class Task {
         for (const el of args) {
             const date1 = this[el] as Moment | null;
             const date2 = other[el] as Moment | null;
-            if (DateField.compareByDate(date1, date2) !== 0) {
+            if (compareByDate(date1, date2) !== 0) {
                 return false;
             }
         }
diff --git a/src/lib/DateTools.ts b/src/lib/DateTools.ts
new file mode 100644
index 00000000..f6581527
--- /dev/null
+++ b/src/lib/DateTools.ts
@@ -0,0 +1,23 @@
+export function compareByDate(a: moment.Moment | null, b: moment.Moment | null): -1 | 0 | 1 {
+    if (a !== null && b === null) {
+        return -1;
+    } else if (a === null && b !== null) {
+        return 1;
+    } else if (a !== null && b !== null) {
+        if (a.isValid() && !b.isValid()) {
+            return -1;
+        } else if (!a.isValid() && b.isValid()) {
+            return 1;
+        }
+
+        if (a.isAfter(b)) {
+            return 1;
+        } else if (a.isBefore(b)) {
+            return -1;
+        } else {
+            return 0;
+        }
+    } else {
+        return 0;
+    }
+}


Test patch:
diff --git a/tests/CustomMatchers/CustomMatchersForSorting.ts b/tests/CustomMatchers/CustomMatchersForSorting.ts
index f5a18b8b..32a2d6b3 100644
--- a/tests/CustomMatchers/CustomMatchersForSorting.ts
+++ b/tests/CustomMatchers/CustomMatchersForSorting.ts
@@ -1,8 +1,8 @@
 import type moment from 'moment';
 import { DateParser } from '../../src/Query/DateParser';
-import { DateField } from '../../src/Query/Filter/DateField';
 import type { Sorter } from '../../src/Query/Sorter';
 import type { Task } from '../../src/Task';
+import { compareByDate } from '../../src/lib/DateTools';
 
 declare global {
     namespace jest {
@@ -26,7 +26,7 @@ expect.extend({
         let b: moment.Moment | null = null;
         if (dateB !== null) b = DateParser.parseDate(dateB);
 
-        const actual = DateField.compareByDate(a, b);
+        const actual = compareByDate(a, b);
 
         const pass = actual === expected;
         const message = () => `${dateA} < ${dateB}: expected=${expected} actual=${actual}`;
diff --git a/tests/StatusRegistry.test.ts b/tests/StatusRegistry.test.ts
index 2168e8b2..9361efca 100644
--- a/tests/StatusRegistry.test.ts
+++ b/tests/StatusRegistry.test.ts
@@ -7,7 +7,6 @@ import { Status } from '../src/Status';
 import { StatusConfiguration, StatusType } from '../src/StatusConfiguration';
 import { Task } from '../src/Task';
 import * as TestHelpers from './TestHelpers';
-import { TaskBuilder } from './TestingTools/TaskBuilder';
 
 jest.mock('obsidian');
 window.moment = moment;
@@ -278,20 +277,20 @@ describe('StatusRegistry', () => {
             expect(registry.bySymbol('!').type).toEqual(StatusType.EMPTY);
             expect(registry.bySymbol('X').type).toEqual(StatusType.EMPTY);
             expect(registry.bySymbol('d').type).toEqual(StatusType.EMPTY);
-            const tasks = [
-                new TaskBuilder().statusValues('!', 'Unknown', 'X', false, StatusType.TODO).build(),
-                new TaskBuilder().statusValues('X', 'Unknown', '!', false, StatusType.DONE).build(),
-                new TaskBuilder().statusValues('d', 'Unknown', '!', false, StatusType.IN_PROGRESS).build(),
+            const allStatuses = [
+                new Status(new StatusConfiguration('!', 'Unknown', 'X', false, StatusType.TODO)),
+                new Status(new StatusConfiguration('X', 'Unknown', '!', false, StatusType.DONE)),
+                new Status(new StatusConfiguration('d', 'Unknown', '!', false, StatusType.IN_PROGRESS)),
                 // Include some tasks with duplicate statuses, to make sure duplicates are discarded
-                new TaskBuilder().statusValues('!', 'Unknown', 'X', false, StatusType.TODO).build(),
-                new TaskBuilder().statusValues('X', 'Unknown', '!', false, StatusType.DONE).build(),
-                new TaskBuilder().statusValues('d', 'Unknown', '!', false, StatusType.IN_PROGRESS).build(),
+                new Status(new StatusConfiguration('!', 'Unknown', 'X', false, StatusType.TODO)),
+                new Status(new StatusConfiguration('X', 'Unknown', '!', false, StatusType.DONE)),
+                new Status(new StatusConfiguration('d', 'Unknown', '!', false, StatusType.IN_PROGRESS)),
                 // Check that it does not add copies of any core statuses
-                new TaskBuilder().statusValues('-', 'Unknown', '!', false, StatusType.IN_PROGRESS).build(),
+                new Status(new StatusConfiguration('-', 'Unknown', '!', false, StatusType.IN_PROGRESS)),
             ];
 
             // Act
-            const unknownStatuses = registry.findUnknownStatuses(tasks);
+            const unknownStatuses = registry.findUnknownStatuses(allStatuses);
 
             // Assert
             expect(unknownStatuses.length).toEqual(3);
diff --git a/tests/Query/Filter/DateField.test.ts b/tests/lib/DateTools.test.ts
similarity index 94%
rename from tests/Query/Filter/DateField.test.ts
rename to tests/lib/DateTools.test.ts
index af4524a1..f9f6d7fc 100644
--- a/tests/Query/Filter/DateField.test.ts
+++ b/tests/lib/DateTools.test.ts
@@ -9,7 +9,7 @@ import {
     expectDateComparesAfter,
     expectDateComparesBefore,
     expectDateComparesEqual,
-} from '../../CustomMatchers/CustomMatchersForSorting';
+} from '../CustomMatchers/CustomMatchersForSorting';
 
 // These are lower-level tests that the Task-based ones above, for ease of test coverage.
 describe('compareBy', () => {