Repository: obsidian-tasks-group/obsidian-tasks
Pull request #1406 (2022-12-17 20:57:32)

refactor: Field classes do all 'sort by' options except 'tag'
# Description

**This PR**

- Each step of this PR involved:
    - Picking a symbol from `Query.SortingProperty`
    - Making sure test coverage of the corresponding `Field` implementation was good enough
    - Moving the corresponding `Sort.compare...` implementation (and any associated helper functions) to new sort methods in the `Field` implementation
    - Remove the field name from `Query.SortingProperty` and `Query.sortByRegexp`
    - Removing all traces of the field name from `Sort`
    - Updating any tests that used the old `Sort.makeLegacySorting(...)` to `new PathField().createNormalSorter()` or similar.
- All remaining `sort by` instructions except `sort by tag` are now implemented in classes derived from `Field.`
- The only remaining legacy sort function is `sort by tag`
- This involved adding `UrgencyField` - which can sort, but not yet search

**Next steps**

I'll put `sort by tag`  in a separate PR as it is rather inconsistent with the others, especially due to its manipulation of the global variable `Sort.tagPropertyInstance`.

## Motivation and Context

This is step 8 in splitting out all the sort functions to the relevant fields, to make it easier to add new sort instructions, and to be able to re-use the sort code to sort groups in reverse order.

## How has this been tested?

- By running existing tests
- And adding new tests

## Screenshots (if appropriate)

## Types of changes


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

- [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

- [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/DateField.ts b/src/Query/Filter/DateField.ts
index a64256c6..b2f3d186 100644
--- a/src/Query/Filter/DateField.ts
+++ b/src/Query/Filter/DateField.ts
@@ -2,6 +2,7 @@ import type { Moment } from 'moment';
 import type { Task } from '../../Task';
 import { DateParser } from '../DateParser';
 import { Explanation } from '../Explain/Explanation';
+import type { Comparator } from '../Sorting';
 import { Field } from './Field';
 import { Filter, FilterOrErrorMessage } from './Filter';
 import { FilterInstructions } from './FilterInstructions';
@@ -120,6 +121,16 @@ export abstract class DateField extends Field {
      */
     protected abstract filterResultIfFieldMissing(): boolean;
 
+    public supportsSorting(): boolean {
+        return true;
+    }
+
+    public comparator(): Comparator {
+        return (a: Task, b: Task) => {
+            return DateField.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;
diff --git a/src/Query/Filter/DescriptionField.ts b/src/Query/Filter/DescriptionField.ts
index 167c9a32..61ffe153 100644
--- a/src/Query/Filter/DescriptionField.ts
+++ b/src/Query/Filter/DescriptionField.ts
@@ -1,5 +1,6 @@
 import { getSettings } from '../../Config/Settings';
 import type { Task } from '../../Task';
+import type { Comparator } from '../Sorting';
 import { TextField } from './TextField';
 
 /**
@@ -27,4 +28,61 @@ export class DescriptionField extends TextField {
         const globalFilter = getSettings().globalFilter;
         return task.description.replace(globalFilter, '').trim();
     }
+
+    public supportsSorting(): boolean {
+        return true;
+    }
+
+    /**
+     * Return a function to compare the description by how it is rendered in markdown.
+     *
+     * Does not use the MarkdownRenderer, but tries to match regexes instead
+     * in order to be simpler, faster, and not async.
+     *
+     * Only searches at the start of the description. Markdown later in the tak is unchanged.
+     */
+    public comparator(): Comparator {
+        return (a: Task, b: Task) => {
+            return DescriptionField.cleanDescription(a.description).localeCompare(
+                DescriptionField.cleanDescription(b.description),
+            );
+        };
+    }
+
+    /**
+     * Removes `*`, `=`, and `[` from the beginning of the description.
+     *
+     * Will remove them only if they are closing.
+     * Properly reads links [[like this|one]] (note pipe).
+     */
+    private static cleanDescription(description: string): string {
+        const globalFilter = getSettings().globalFilter;
+        description = description.replace(globalFilter, '').trim();
+
+        const startsWithLinkRegex = /^\[\[?([^\]]*)\]/;
+        const linkRegexMatch = description.match(startsWithLinkRegex);
+        if (linkRegexMatch !== null) {
+            const innerLinkText = linkRegexMatch[1];
+            // For a link, we have to check whether it has another visible name set.
+            // For example `[[this is the link|but this is actually shown]]`.
+            description =
+                innerLinkText.substring(innerLinkText.indexOf('|') + 1) + description.replace(startsWithLinkRegex, '');
+        }
+
+        const startsWithItalicOrBoldRegex = /^\*\*?([^*]*)\*/;
+        const italicBoldRegexMatch = description.match(startsWithItalicOrBoldRegex);
+        if (italicBoldRegexMatch !== null) {
+            const innerItalicBoldText = italicBoldRegexMatch[1];
+            description = innerItalicBoldText + description.replace(startsWithLinkRegex, '');
+        }
+
+        const startsWithHighlightRegex = /^==?([^=]*)==/;
+        const highlightRegexMatch = description.match(startsWithHighlightRegex);
+        if (highlightRegexMatch !== null) {
+            const innerHighlightsText = highlightRegexMatch[1];
+            description = innerHighlightsText + description.replace(startsWithHighlightRegex, '');
+        }
+
+        return description;
+    }
 }
diff --git a/src/Query/Filter/DueDateField.ts b/src/Query/Filter/DueDateField.ts
index fc11d2c6..0bd4e8ed 100644
--- a/src/Query/Filter/DueDateField.ts
+++ b/src/Query/Filter/DueDateField.ts
@@ -1,6 +1,5 @@
 import type { Moment } from 'moment';
 import type { Task } from '../../Task';
-import type { Comparator } from '../Sorting';
 import { DateField } from './DateField';
 
 /**
@@ -21,17 +20,4 @@ export class DueDateField extends DateField {
     protected filterResultIfFieldMissing() {
         return false;
     }
-
-    public supportsSorting(): boolean {
-        return true;
-    }
-
-    /**
-     * Return a function to compare two Task objects, for use in sorting by due.
-     */
-    public comparator(): Comparator {
-        return (a: Task, b: Task) => {
-            return DateField.compareByDate(this.date(a), this.date(b));
-        };
-    }
 }
diff --git a/src/Query/Filter/PathField.ts b/src/Query/Filter/PathField.ts
index 4e5caf37..c018ab7d 100644
--- a/src/Query/Filter/PathField.ts
+++ b/src/Query/Filter/PathField.ts
@@ -1,4 +1,5 @@
 import type { Task } from '../../Task';
+import type { Comparator } from '../Sorting';
 import { TextField } from './TextField';
 
 /** Support the 'path' search instruction.
@@ -20,4 +21,20 @@ export class PathField extends TextField {
     public value(task: Task): string {
         return task.path;
     }
+
+    public supportsSorting(): boolean {
+        return true;
+    }
+
+    public comparator(): Comparator {
+        return (a: Task, b: Task) => {
+            if (a.path < b.path) {
+                return -1;
+            } else if (a.path > b.path) {
+                return 1;
+            } else {
+                return 0;
+            }
+        };
+    }
 }
diff --git a/src/Query/Filter/PriorityField.ts b/src/Query/Filter/PriorityField.ts
index 19c5511f..07110d8d 100644
--- a/src/Query/Filter/PriorityField.ts
+++ b/src/Query/Filter/PriorityField.ts
@@ -1,5 +1,6 @@
 import { Priority, Task } from '../../Task';
 import { Explanation } from '../Explain/Explanation';
+import type { Comparator } from '../Sorting';
 import { Field } from './Field';
 import { Filter, FilterOrErrorMessage } from './Filter';
 
@@ -68,4 +69,14 @@ export class PriorityField extends Field {
     protected filterRegExp(): RegExp {
         return PriorityField.priorityRegexp;
     }
+
+    public supportsSorting(): boolean {
+        return true;
+    }
+
+    public comparator(): Comparator {
+        return (a: Task, b: Task) => {
+            return a.priority.localeCompare(b.priority);
+        };
+    }
 }
diff --git a/src/Query/Filter/UrgencyField.ts b/src/Query/Filter/UrgencyField.ts
new file mode 100644
index 00000000..93bab692
--- /dev/null
+++ b/src/Query/Filter/UrgencyField.ts
@@ -0,0 +1,38 @@
+import type { Comparator } from '../Sorting';
+import type { Task } from '../../Task';
+import { Field } from './Field';
+import { FilterOrErrorMessage } from './Filter';
+
+/**
+ * Support 'urgency' sorting.
+ *
+ * Note: Searching by urgency is not yet implemented.
+ */
+export class UrgencyField extends Field {
+    canCreateFilterForLine(_line: string): boolean {
+        return false;
+    }
+
+    createFilterOrErrorMessage(line: string): FilterOrErrorMessage {
+        return FilterOrErrorMessage.fromError(line, 'Filtering by urgency is not yet supported');
+    }
+
+    fieldName(): string {
+        return 'urgency';
+    }
+
+    protected filterRegExp(): RegExp | null {
+        throw Error(`filterRegExp() unimplemented for ${this.fieldName()}`);
+    }
+
+    supportsSorting(): boolean {
+        return true;
+    }
+
+    public comparator(): Comparator {
+        return (a: Task, b: Task) => {
+            // Higher urgency should be sorted earlier.
+            return b.urgency - a.urgency;
+        };
+    }
+}
diff --git a/src/Query/FilterParser.ts b/src/Query/FilterParser.ts
index cee5b614..1f25dbbd 100644
--- a/src/Query/FilterParser.ts
+++ b/src/Query/FilterParser.ts
@@ -13,6 +13,7 @@ import { StatusField } from './Filter/StatusField';
 import { TagsField } from './Filter/TagsField';
 import { BooleanField } from './Filter/BooleanField';
 import { FilenameField } from './Filter/FilenameField';
+import { UrgencyField } from './Filter/UrgencyField';
 
 import type { FilterOrErrorMessage } from './Filter/Filter';
 import type { Sorting } from './Sorting';
@@ -33,6 +34,7 @@ const fieldCreators = [
     () => new ExcludeSubItemsField(),
     () => new BooleanField(),
     () => new FilenameField(),
+    () => new UrgencyField(),
 ];
 
 export function parseFilter(filterString: string): FilterOrErrorMessage | null {
diff --git a/src/Query/Query.ts b/src/Query/Query.ts
index b8e258d1..bc8c78d7 100644
--- a/src/Query/Query.ts
+++ b/src/Query/Query.ts
@@ -12,7 +12,7 @@ import type { Filter } from './Filter/Filter';
 // TODO Work through the values in SortingProperty, moving their comparison
 //      functions to the corresponding Field implementations.
 //      Then remove SortingProperty.
-export type SortingProperty = 'urgency' | 'priority' | 'start' | 'scheduled' | 'done' | 'path' | 'description' | 'tag';
+export type SortingProperty = 'tag';
 
 export type GroupingProperty =
     | 'backlink'
@@ -45,8 +45,7 @@ export class Query implements IQuery {
 
     // If a tag is specified the user can also add a number to specify
     // which one to sort by if there is more than one.
-    private readonly sortByRegexp =
-        /^sort by (urgency|priority|start|scheduled|done|path|description|tag)( reverse)?[\s]*(\d+)?/;
+    private readonly sortByRegexp = /^sort by (tag)( reverse)?[\s]*(\d+)?/;
 
     private readonly groupByRegexp =
         /^group by (backlink|done|due|filename|folder|happens|heading|path|priority|recurrence|recurring|root|scheduled|start|status|tags)/;
diff --git a/src/Query/Sort.ts b/src/Query/Sort.ts
index fd4d863b..a3f6814e 100644
--- a/src/Query/Sort.ts
+++ b/src/Query/Sort.ts
@@ -1,12 +1,13 @@
 import type { Task } from '../Task';
-import { getSettings } from '../Config/Settings';
 import { Sorting } from './Sorting';
 import type { Comparator } from './Sorting';
 import type { Query, SortingProperty } from './Query';
 // TODO Remove the cyclic dependency between StatusField and Sort.
 import { StatusField } from './Filter/StatusField';
 import { DueDateField } from './Filter/DueDateField';
-import { DateField } from './Filter/DateField';
+import { PriorityField } from './Filter/PriorityField';
+import { PathField } from './Filter/PathField';
+import { UrgencyField } from './Filter/UrgencyField';
 
 export class Sort {
     static tagPropertyInstance: number = 1;
@@ -14,11 +15,11 @@ export class Sort {
     public static by(query: Pick<Query, 'sorting'>, tasks: Task[]): Task[] {
         // TODO Move code for creating default comparators to separate file
         const defaultComparators: Comparator[] = [
-            Sort.compareByUrgency,
+            new UrgencyField().comparator(),
             new StatusField().comparator(),
             new DueDateField().comparator(),
-            Sort.compareByPriority,
-            Sort.compareByPath,
+            new PriorityField().comparator(),
+            new PathField().comparator(),
         ];
 
         const userComparators: Comparator[] = [];
@@ -34,13 +35,6 @@ export class Sort {
     }
 
     public static comparators: Record<SortingProperty, Comparator> = {
-        urgency: Sort.compareByUrgency,
-        description: Sort.compareByDescription,
-        priority: Sort.compareByPriority,
-        start: Sort.compareByStartDate,
-        scheduled: Sort.compareByScheduledDate,
-        done: Sort.compareByDoneDate,
-        path: Sort.compareByPath,
         tag: Sort.compareByTag,
     };
 
@@ -87,27 +81,6 @@ export class Sort {
         };
     }
 
-    private static compareByUrgency(a: Task, b: Task): number {
-        // Higher urgency should be sorted earlier.
-        return b.urgency - a.urgency;
-    }
-
-    private static compareByPriority(a: Task, b: Task): number {
-        return a.priority.localeCompare(b.priority);
-    }
-
-    private static compareByStartDate(a: Task, b: Task): -1 | 0 | 1 {
-        return DateField.compareByDate(a.startDate, b.startDate);
-    }
-
-    private static compareByScheduledDate(a: Task, b: Task): -1 | 0 | 1 {
-        return DateField.compareByDate(a.scheduledDate, b.scheduledDate);
-    }
-
-    private static compareByDoneDate(a: Task, b: Task): -1 | 0 | 1 {
-        return DateField.compareByDate(a.doneDate, b.doneDate);
-    }
-
     private static compareByTag(a: Task, b: Task): -1 | 0 | 1 {
         // If no tags then assume they are equal.
         if (a.tags.length === 0 && b.tags.length === 0) {
@@ -139,60 +112,4 @@ export class Sort {
             return 0;
         }
     }
-    private static compareByPath(a: Task, b: Task): -1 | 0 | 1 {
-        if (a.path < b.path) {
-            return -1;
-        } else if (a.path > b.path) {
-            return 1;
-        } else {
-            return 0;
-        }
-    }
-
-    /**
-     * Compare the description by how it is rendered in markdown.
-     *
-     * Does not use the MarkdownRenderer, but tries to match regexes instead
-     * in order to be simpler, faster, and not async.
-     */
-    private static compareByDescription(a: Task, b: Task) {
-        return Sort.cleanDescription(a.description).localeCompare(Sort.cleanDescription(b.description));
-    }
-
-    /**
-     * Removes `*`, `=`, and `[` from the beginning of the description.
-     *
-     * Will remove them only if they are closing.
-     * Properly reads links [[like this|one]] (note pipe).
-     */
-    private static cleanDescription(description: string): string {
-        const globalFilter = getSettings().globalFilter;
-        description = description.replace(globalFilter, '').trim();
-
-        const startsWithLinkRegex = /^\[\[?([^\]]*)\]/;
-        const linkRegexMatch = description.match(startsWithLinkRegex);
-        if (linkRegexMatch !== null) {
-            const innerLinkText = linkRegexMatch[1];
-            // For a link, we have to check whether it has another visible name set.
-            // For example `[[this is the link|but this is actually shown]]`.
-            description =
-                innerLinkText.substring(innerLinkText.indexOf('|') + 1) + description.replace(startsWithLinkRegex, '');
-        }
-
-        const startsWithItalicOrBoldRegex = /^\*\*?([^*]*)\*/;
-        const italicBoldRegexMatch = description.match(startsWithItalicOrBoldRegex);
-        if (italicBoldRegexMatch !== null) {
-            const innerItalicBoldText = italicBoldRegexMatch[1];
-            description = innerItalicBoldText + description.replace(startsWithLinkRegex, '');
-        }
-
-        const startsWithHighlightRegex = /^==?([^=]*)==/;
-        const highlightRegexMatch = description.match(startsWithHighlightRegex);
-        if (highlightRegexMatch !== null) {
-            const innerHighlightsText = highlightRegexMatch[1];
-            description = innerHighlightsText + description.replace(startsWithHighlightRegex, '');
-        }
-
-        return description;
-    }
 }


Test patch:
diff --git a/tests/Query/Filter/DescriptionField.test.ts b/tests/Query/Filter/DescriptionField.test.ts
index ca78a533..b41ca7cd 100644
--- a/tests/Query/Filter/DescriptionField.test.ts
+++ b/tests/Query/Filter/DescriptionField.test.ts
@@ -10,7 +10,11 @@ import type { FilterOrErrorMessage } from '../../../src/Query/Filter/Filter';
 import { BooleanField } from '../../../src/Query/Filter/BooleanField';
 import { toMatchTaskFromLine } from '../../CustomMatchers/CustomMatchersForFilters';
 import { Sort } from '../../../src/Query/Sort';
-import { expectTaskComparesBefore, expectTaskComparesEqual } from '../../CustomMatchers/CustomMatchersForSorting';
+import {
+    expectTaskComparesAfter,
+    expectTaskComparesBefore,
+    expectTaskComparesEqual,
+} from '../../CustomMatchers/CustomMatchersForSorting';
 import { TaskBuilder } from '../../TestingTools/TaskBuilder';
 
 window.moment = moment;
@@ -249,8 +253,35 @@ describe('search description for Alternation (OR)', () => {
 });
 
 describe('sorting by description', () => {
+    it('supports Field sorting methods correctly', () => {
+        const field = new DescriptionField();
+        expect(field.supportsSorting()).toEqual(true);
+    });
+
+    // Helper function to create a task with a given path
+    function with_description(description: string) {
+        return new TaskBuilder().description(description).build();
+    }
+
+    it('sort by path', () => {
+        // Arrange
+        const sorter = new DescriptionField().createNormalSorter();
+
+        // Assert
+        expectTaskComparesEqual(sorter, with_description('Aaa'), with_description('Aaa'));
+        expectTaskComparesBefore(sorter, with_description('AAA'), with_description('ZZZ'));
+        expectTaskComparesAfter(sorter, with_description('AAA'), with_description('aaa')); // case-sensitive - capitals come last - TODO WHY? In description searches, capitals appear to come first.
+    });
+
+    it('sort by path reverse', () => {
+        // Single example just to prove reverse works.
+        // (There's no need to repeat all the examples above)
+        const sorter = new DescriptionField().createReverseSorter();
+        expectTaskComparesAfter(sorter, with_description('AAA'), with_description('ZZZ'));
+    });
+
     describe('show how markdown in descriptions gets cleaned', () => {
-        const sorter = Sort.makeLegacySorting(false, 1, 'description');
+        const sorter = new DescriptionField().createNormalSorter();
 
         it('characters that are not stripped out', () => {
             // expectTaskComparesBefore() shows that the initial * is not removed removed
@@ -332,7 +363,7 @@ describe('sorting by description', () => {
         expect(
             Sort.by(
                 {
-                    sorting: [Sort.makeLegacySorting(false, 1, 'description')],
+                    sorting: [new DescriptionField().createNormalSorter()],
                 },
                 [two, one, five, four, three],
             ),
diff --git a/tests/Query/Filter/DoneDateField.test.ts b/tests/Query/Filter/DoneDateField.test.ts
index 824629e2..122833c5 100644
--- a/tests/Query/Filter/DoneDateField.test.ts
+++ b/tests/Query/Filter/DoneDateField.test.ts
@@ -8,7 +8,6 @@ import { TaskBuilder } from '../../TestingTools/TaskBuilder';
 import { testFilter } from '../../TestingTools/FilterTestHelpers';
 import { toHaveExplanation } from '../../CustomMatchers/CustomMatchersForFilters';
 import { expectTaskComparesAfter, expectTaskComparesBefore } from '../../CustomMatchers/CustomMatchersForSorting';
-import { Sort } from '../../../src/Query/Sort';
 
 window.moment = moment;
 
@@ -75,7 +74,7 @@ describe('explain done date queries', () => {
 describe('sorting by done', () => {
     it('supports Field sorting methods correctly', () => {
         const field = new DoneDateField();
-        expect(field.supportsSorting()).toEqual(false);
+        expect(field.supportsSorting()).toEqual(true);
     });
 
     // These are minimal tests just to confirm basic behaviour is set up for this field.
@@ -85,10 +84,10 @@ describe('sorting by done', () => {
     const date2 = new TaskBuilder().doneDate('2022-12-23').build();
 
     it('sort by done', () => {
-        expectTaskComparesBefore(Sort.makeLegacySorting(false, 1, 'done'), date1, date2);
+        expectTaskComparesBefore(new DoneDateField().createNormalSorter(), date1, date2);
     });
 
     it('sort by done reverse', () => {
-        expectTaskComparesAfter(Sort.makeLegacySorting(true, 1, 'done'), date1, date2);
+        expectTaskComparesAfter(new DoneDateField().createReverseSorter(), date1, date2);
     });
 });
diff --git a/tests/Query/Filter/PathField.test.ts b/tests/Query/Filter/PathField.test.ts
index 3a3ed75a..ecc6957b 100644
--- a/tests/Query/Filter/PathField.test.ts
+++ b/tests/Query/Filter/PathField.test.ts
@@ -8,7 +8,6 @@ import {
     expectTaskComparesBefore,
     expectTaskComparesEqual,
 } from '../../CustomMatchers/CustomMatchersForSorting';
-import { Sort } from '../../../src/Query/Sort';
 import { fromLine } from '../../TestHelpers';
 
 function testTaskFilterForTaskWithPath(filter: FilterOrErrorMessage, path: string, expected: boolean) {
@@ -120,7 +119,7 @@ describe('invalid unescaped slash should give helpful error text and not search'
 describe('sorting by path', () => {
     it('supports Field sorting methods correctly', () => {
         const field = new PathField();
-        expect(field.supportsSorting()).toEqual(false);
+        expect(field.supportsSorting()).toEqual(true);
     });
 
     // Helper function to create a task with a given path
@@ -130,7 +129,7 @@ describe('sorting by path', () => {
 
     it('sort by path', () => {
         // Arrange
-        const sorter = Sort.makeLegacySorting(false, 1, 'path');
+        const sorter = new PathField().createNormalSorter();
 
         // Assert
         expectTaskComparesEqual(sorter, with_path('a/b.md'), with_path('a/b.md'));
@@ -145,7 +144,7 @@ describe('sorting by path', () => {
     it('sort by path reverse', () => {
         // Single example just to prove reverse works.
         // (There's no need to repeat all the examples above)
-        const sorter = Sort.makeLegacySorting(true, 1, 'path');
+        const sorter = new PathField().createReverseSorter();
         expectTaskComparesAfter(sorter, with_path('a/b.md'), with_path('c/d.md'));
     });
 });
diff --git a/tests/Query/Filter/PriorityField.test.ts b/tests/Query/Filter/PriorityField.test.ts
index a7184d51..74a6372a 100644
--- a/tests/Query/Filter/PriorityField.test.ts
+++ b/tests/Query/Filter/PriorityField.test.ts
@@ -10,7 +10,6 @@ import {
     expectTaskComparesBefore,
     expectTaskComparesEqual,
 } from '../../CustomMatchers/CustomMatchersForSorting';
-import { Sort } from '../../../src/Query/Sort';
 
 expect.extend({
     toBeValid,
@@ -132,10 +131,9 @@ describe('explain priority', () => {
 });
 
 describe('sorting by priority', () => {
-    it('does not yet support Field sorting methods', () => {
+    it('supports Field sorting methods correctly', () => {
         const field = new PriorityField();
-        // Not yet supported - TODO - rename this test when implementing Priority sorting
-        expect(field.supportsSorting()).toEqual(false);
+        expect(field.supportsSorting()).toEqual(true);
     });
 
     // Helper function to create a task with a given priority
@@ -145,7 +143,7 @@ describe('sorting by priority', () => {
 
     it('sort by priority', () => {
         // Arrange
-        const sorter = Sort.makeLegacySorting(false, 1, 'priority');
+        const sorter = new PriorityField().createNormalSorter();
 
         // Assert
         // This tests each adjacent pair of priority values, in descending order,
@@ -160,7 +158,7 @@ describe('sorting by priority', () => {
     it('sort by priority reverse', () => {
         // Single example just to prove reverse works.
         // (There's no need to repeat all the examples above)
-        const sorter = Sort.makeLegacySorting(true, 1, 'priority');
+        const sorter = new PriorityField().createReverseSorter();
         expectTaskComparesAfter(sorter, with_priority(Priority.High), with_priority(Priority.Medium));
     });
 });
diff --git a/tests/Query/Filter/ScheduledDateField.test.ts b/tests/Query/Filter/ScheduledDateField.test.ts
index 473f921a..21cffa2d 100644
--- a/tests/Query/Filter/ScheduledDateField.test.ts
+++ b/tests/Query/Filter/ScheduledDateField.test.ts
@@ -6,7 +6,6 @@ import { toHaveExplanation } from '../../CustomMatchers/CustomMatchersForFilters
 import { ScheduledDateField } from '../../../src/Query/Filter/ScheduledDateField';
 import { TaskBuilder } from '../../TestingTools/TaskBuilder';
 import { expectTaskComparesAfter, expectTaskComparesBefore } from '../../CustomMatchers/CustomMatchersForSorting';
-import { Sort } from '../../../src/Query/Sort';
 
 window.moment = moment;
 
@@ -29,7 +28,7 @@ describe('explain scheduled date queries', () => {
 describe('sorting by scheduled', () => {
     it('supports Field sorting methods correctly', () => {
         const field = new ScheduledDateField();
-        expect(field.supportsSorting()).toEqual(false);
+        expect(field.supportsSorting()).toEqual(true);
     });
 
     // These are minimal tests just to confirm basic behaviour is set up for this field.
@@ -39,10 +38,10 @@ describe('sorting by scheduled', () => {
     const date2 = new TaskBuilder().scheduledDate('2022-12-23').build();
 
     it('sort by scheduled', () => {
-        expectTaskComparesBefore(Sort.makeLegacySorting(false, 1, 'scheduled'), date1, date2);
+        expectTaskComparesBefore(new ScheduledDateField().createNormalSorter(), date1, date2);
     });
 
     it('sort by scheduled reverse', () => {
-        expectTaskComparesAfter(Sort.makeLegacySorting(true, 1, 'scheduled'), date1, date2);
+        expectTaskComparesAfter(new ScheduledDateField().createReverseSorter(), date1, date2);
     });
 });
diff --git a/tests/Query/Filter/StartDateField.test.ts b/tests/Query/Filter/StartDateField.test.ts
index 887d7aad..17811df7 100644
--- a/tests/Query/Filter/StartDateField.test.ts
+++ b/tests/Query/Filter/StartDateField.test.ts
@@ -6,7 +6,6 @@ import { toHaveExplanation } from '../../CustomMatchers/CustomMatchersForFilters
 import { StartDateField } from '../../../src/Query/Filter/StartDateField';
 import { TaskBuilder } from '../../TestingTools/TaskBuilder';
 import { expectTaskComparesAfter, expectTaskComparesBefore } from '../../CustomMatchers/CustomMatchersForSorting';
-import { Sort } from '../../../src/Query/Sort';
 
 window.moment = moment;
 
@@ -33,7 +32,7 @@ describe('explain start date queries', () => {
 describe('sorting by start', () => {
     it('supports Field sorting methods correctly', () => {
         const field = new StartDateField();
-        expect(field.supportsSorting()).toEqual(false);
+        expect(field.supportsSorting()).toEqual(true);
     });
 
     // These are minimal tests just to confirm basic behaviour is set up for this field.
@@ -43,10 +42,10 @@ describe('sorting by start', () => {
     const date2 = new TaskBuilder().startDate('2022-12-23').build();
 
     it('sort by start', () => {
-        expectTaskComparesBefore(Sort.makeLegacySorting(false, 1, 'start'), date1, date2);
+        expectTaskComparesBefore(new StartDateField().createNormalSorter(), date1, date2);
     });
 
     it('sort by start reverse', () => {
-        expectTaskComparesAfter(Sort.makeLegacySorting(true, 1, 'start'), date1, date2);
+        expectTaskComparesAfter(new StartDateField().createReverseSorter(), date1, date2);
     });
 });
diff --git a/tests/Query/Filter/UrgencyField.test.ts b/tests/Query/Filter/UrgencyField.test.ts
index c1133eec..c97d5cb5 100644
--- a/tests/Query/Filter/UrgencyField.test.ts
+++ b/tests/Query/Filter/UrgencyField.test.ts
@@ -4,6 +4,7 @@
 import moment from 'moment';
 import { Priority } from '../../../src/Task';
 import { TaskBuilder } from '../../TestingTools/TaskBuilder';
+import { UrgencyField } from '../../../src/Query/Filter/UrgencyField';
 
 import { toBeValid, toHaveExplanation } from '../../CustomMatchers/CustomMatchersForFilters';
 
@@ -12,7 +13,6 @@ import {
     expectTaskComparesBefore,
     expectTaskComparesEqual,
 } from '../../CustomMatchers/CustomMatchersForSorting';
-import { Sort } from '../../../src/Query/Sort';
 
 window.moment = moment;
 
@@ -21,13 +21,21 @@ expect.extend({
     toHaveExplanation,
 });
 
+describe('urgency', () => {
+    it('should not yet be implemented', () => {
+        // Arrange
+        const filter = new UrgencyField().createFilterOrErrorMessage('any old nonsense');
+
+        // Act, Assert
+        expect(filter).not.toBeValid();
+    });
+});
+
 describe('sorting by urgency', () => {
-    // TODO Activate this when creating UrgencyField
-    // it('does not yet support Field sorting methods', () => {
-    //     const field = new UrgencyField();
-    //     // Not yet supported - TODO - rename this test when implementing urgency sorting
-    //     expect(field.supportsSorting()).toEqual(false);
-    // });
+    it('supports Field sorting methods correctly', () => {
+        const field = new UrgencyField();
+        expect(field.supportsSorting()).toEqual(true);
+    });
 
     // Helper function to create a task with a given priority
     function with_priority(priority: Priority) {
@@ -40,7 +48,7 @@ describe('sorting by urgency', () => {
 
     it('sort by urgency', () => {
         // Arrange
-        const sorter = Sort.makeLegacySorting(false, 1, 'urgency');
+        const sorter = new UrgencyField().createNormalSorter();
 
         // Assert
         // Just some minimal tests to confirm that the urgency value is respected.
@@ -66,7 +74,7 @@ describe('sorting by urgency', () => {
 
     it('sort by urgency reverse', () => {
         // Single example just to prove reverse works.
-        const sorter = Sort.makeLegacySorting(true, 1, 'urgency');
+        const sorter = new UrgencyField().createReverseSorter();
         expectTaskComparesAfter(
             sorter,
             with_priority(Priority.High), // Higher priority comes last
diff --git a/tests/Sort.test.ts b/tests/Sort.test.ts
index 83aa205e..8544a514 100644
--- a/tests/Sort.test.ts
+++ b/tests/Sort.test.ts
@@ -10,7 +10,10 @@ import { Sort } from '../src/Query/Sort';
 import { Sorting } from '../src/Query/Sorting';
 import type { Task } from '../src/Task';
 import { StatusField } from '../src/Query/Filter/StatusField';
+import { DoneDateField } from '../src/Query/Filter/DoneDateField';
 import { DueDateField } from '../src/Query/Filter/DueDateField';
+import { PathField } from '../src/Query/Filter/PathField';
+import { DescriptionField } from '../src/Query/Filter/DescriptionField';
 import { fromLine } from './TestHelpers';
 import { TaskBuilder } from './TestingTools/TaskBuilder';
 import {
@@ -85,7 +88,7 @@ describe('Sort', () => {
                 {
                     sorting: [
                         new DueDateField().createNormalSorter(),
-                        Sort.makeLegacySorting(false, 1, 'path'),
+                        new PathField().createNormalSorter(),
                         new StatusField().createNormalSorter(),
                     ],
                 },
@@ -116,10 +119,7 @@ describe('Sort', () => {
         expect(
             Sort.by(
                 {
-                    sorting: [
-                        Sort.makeLegacySorting(false, 1, 'description'),
-                        Sort.makeLegacySorting(false, 1, 'done'),
-                    ],
+                    sorting: [new DescriptionField().createNormalSorter(), new DoneDateField().createNormalSorter()],
                 },
                 [three, one, two, four],
             ),
@@ -148,7 +148,7 @@ describe('Sort', () => {
         expect(
             Sort.by(
                 {
-                    sorting: [Sort.makeLegacySorting(true, 1, 'description'), Sort.makeLegacySorting(false, 1, 'done')],
+                    sorting: [new DescriptionField().createReverseSorter(), new DoneDateField().createNormalSorter()],
                 },
                 [two, four, three, one],
             ),
@@ -171,7 +171,7 @@ describe('Sort', () => {
                     sorting: [
                         new StatusField().createReverseSorter(),
                         new DueDateField().createReverseSorter(),
-                        Sort.makeLegacySorting(false, 1, 'path'),
+                        new PathField().createNormalSorter(),
                     ],
                 },
                 [six, five, one, four, three, two],