Repository: obsidian-tasks-group/obsidian-tasks
Pull request #2951 (2024-07-11 16:38:45)

fix: Prevent task.frontmatter.tags being null if no tags in frontmatter
# 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)
    - A small fix to an unreleased feature
    - See https://github.com/obsidian-tasks-group/obsidian-tasks/issues/2480
- [x] **Sample vault** (prefix: `vault` - improvements to the [Tasks-Demo sample vault](https://github.com/obsidian-tasks-group/obsidian-tasks/tree/main/resources/sample_vaults/Tasks-Demo))

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)

## Description

In `src/`:

- fix: Prevent `task.frontmatter.tags` being null if no tags in frontmatter
- Remove `TasksFile.frontmatterTags`
- Improve JSDocs of `TasksFile.ts`

In `tests/`:

- Add safety checks to the mock functions `getAllTags()` and `parseFrontMatterTags()`
- Add more tests, just exploring the behaviour

In sample vault:

- Update sample code that demoed use of now-deleted `task.file.frontmatterTags`

## Motivation and Context

More progress on https://github.com/obsidian-tasks-group/obsidian-tasks/issues/2480

## How has this been tested?

- Adding new tests
- Exploratory testing in demo vault


## 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 follows 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/resources/sample_vaults/Tasks-Demo/How To/Find tasks in notes with particular tag.md b/resources/sample_vaults/Tasks-Demo/How To/Find tasks in notes with particular tag.md
index a58759c1..8334b8dd 100644
--- a/resources/sample_vaults/Tasks-Demo/How To/Find tasks in notes with particular tag.md
+++ b/resources/sample_vaults/Tasks-Demo/How To/Find tasks in notes with particular tag.md
@@ -36,7 +36,7 @@ Credit: jonlemon in [this Obsidian Forum thread](https://forum.obsidian.md/t/how
 #### Presence of tag - in frontmatter
 
 ```tasks
-filter by function task.file.frontmatterTags.includes('#examples')
+filter by function task.file.frontmatter.tags?.includes('#examples') ?? false
 ```
 
 ### Tasks in files that have a Tag anywhere - in frontmatter or body
diff --git a/src/Scripting/TasksFile.ts b/src/Scripting/TasksFile.ts
index f2111eaa..549ff6d5 100644
--- a/src/Scripting/TasksFile.ts
+++ b/src/Scripting/TasksFile.ts
@@ -15,7 +15,7 @@ export class TasksFile {
         const rawFrontmatter = cachedMetadata.frontmatter;
         if (rawFrontmatter !== undefined) {
             this._frontmatter = JSON.parse(JSON.stringify(rawFrontmatter));
-            this._frontmatter.tags = parseFrontMatterTags(rawFrontmatter);
+            this._frontmatter.tags = parseFrontMatterTags(rawFrontmatter) ?? [];
         }
     }
 
@@ -29,49 +29,44 @@ export class TasksFile {
     /**
      * Return all the tags in the file, both from frontmatter and the body of the file.
      *
-     * It adds the `#` prefix to tags in the frontmatter.
-     * For now, it includes any global filter that is a tag, if there are any tasks in the file
-     * that have the global filter. This decision will be reviewed later.
+     * - It adds the `#` prefix to tags in the frontmatter.
+     * - It removes any duplicate tag values.
+     * - For now, it includes any global filter that is a tag, if there are any tasks in the file
+     *   that have the global filter. This decision will be reviewed later.
      *
      * @todo Review presence of global filter tag in the results.
      */
     get tags(): string[] {
+        // TODO Replace this with storing the sanitised tags, to avoid repeated re-calculation.
         const tags = getAllTags(this.cachedMetadata) ?? [];
         return [...new Set(tags)];
     }
 
-    /**
-     * Return the sanitised tags from the frontmatter.
-     *
-     * It recognises both frontmatter.tags and frontmatter.tag (and various capitalisation combinations too)
-     * It adds the `#` prefix, and removes any null tags.
-     * We would like to make it accessible via task.frontmatter.tags eventually.
-     *
-     * For now, it includes any global filter that is a tag, if there are any tasks in the file
-     * that have the global filter. This decision will be reviewed later.
-     *
-     * @todo Review presence of global filter tag in the results.
-     *       Or should that not be relevant for tags in the frontmatter?
-     */
-    get frontmatterTags(): string[] {
-        return parseFrontMatterTags(this.cachedMetadata.frontmatter) ?? [];
-    }
-
     /**
      * Return Obsidian's [CachedMetadata](https://docs.obsidian.md/Reference/TypeScript+API/CachedMetadata)
      * for this file, if available.
      *
+     * Any raw frontmatter may be accessed via `cachedMetadata.frontmatter`.
+     * See [FrontMatterCache](https://docs.obsidian.md/Reference/TypeScript+API/FrontMatterCache).
+     * But prefer using {@link frontmatter} where possible.
+     *
      * @note This is currently only populated for Task objects when read in the Obsidian plugin.
      *       It's not populated for queries in the plugin, nor in most unit tests.
      *       If not available, it returns an empty object, {}.
+     *
+     * @see frontmatter, which provides a cleaned-up version of the raw frontmatter.
      */
     public get cachedMetadata(): CachedMetadata {
         return this._cachedMetadata;
     }
 
     /**
-     * Return Obsidian's [FrontMatterCache](https://docs.obsidian.md/Reference/TypeScript+API/FrontMatterCache)
-     * for this file, if available.
+     * Returns a cleaned-up version of the frontmatter.
+     *
+     * If accessing tags, please note:
+     * - If there are any tags in the frontmatter, `frontmatter.tags` will have the values with '#' prefix added.
+     * - It recognises both `frontmatter.tags` and `frontmatter.tag` (and various capitalisation combinations too).
+     * - It removes any null tags.
      *
      * @note This is currently only populated for Task objects when read in the Obsidian plugin.
      *       It's not populated for queries in the plugin, nor in most unit tests.


Test patch:
diff --git a/tests/Scripting/TasksFile.test.ts b/tests/Scripting/TasksFile.test.ts
index 4f2ede64..2f6acde4 100644
--- a/tests/Scripting/TasksFile.test.ts
+++ b/tests/Scripting/TasksFile.test.ts
@@ -7,6 +7,11 @@ import { empty_yaml } from '../Obsidian/__test_data__/empty_yaml';
 import { yaml_tags_has_multiple_values } from '../Obsidian/__test_data__/yaml_tags_has_multiple_values';
 import { yaml_custom_number_property } from '../Obsidian/__test_data__/yaml_custom_number_property';
 import { yaml_tags_with_one_value_on_new_line } from '../Obsidian/__test_data__/yaml_tags_with_one_value_on_new_line';
+import { yaml_tags_field_added_by_obsidian_but_not_populated } from '../Obsidian/__test_data__/yaml_tags_field_added_by_obsidian_but_not_populated';
+import { yaml_tags_had_value_then_was_emptied_by_obsidian } from '../Obsidian/__test_data__/yaml_tags_had_value_then_was_emptied_by_obsidian';
+import { yaml_tags_is_empty_list } from '../Obsidian/__test_data__/yaml_tags_is_empty_list';
+import { yaml_tags_is_empty } from '../Obsidian/__test_data__/yaml_tags_is_empty';
+import { example_kanban } from '../Obsidian/__test_data__/example_kanban';
 
 describe('TasksFile', () => {
     it('should provide access to path', () => {
@@ -61,6 +66,11 @@ function getTasksFileFromMockData(data: any) {
     return new TasksFile(data.filePath, cachedMetadata);
 }
 
+function listPathAndData(inputs: any[]) {
+    // We use map() to extract the path, to use it as a test name in it.each()
+    return inputs.map((data) => [data.filePath, data]);
+}
+
 describe('TasksFile - reading frontmatter', () => {
     it('should read file if not given CachedMetadata', () => {
         const tasksFile = new TasksFile('some path.md', {});
@@ -73,12 +83,14 @@ describe('TasksFile - reading frontmatter', () => {
         const tasksFile = getTasksFileFromMockData(no_yaml);
         expect(tasksFile.cachedMetadata.frontmatter).toBeUndefined();
         expect(tasksFile.frontmatter).toEqual({});
+        expect(tasksFile.frontmatter.tags).toEqual(undefined); // TODO This will be inconvenient for users - should be []?
     });
 
     it('should read file with empty yaml metadata', () => {
         const tasksFile = getTasksFileFromMockData(empty_yaml);
         expect(tasksFile.cachedMetadata.frontmatter).toBeUndefined();
         expect(tasksFile.frontmatter).toEqual({});
+        expect(tasksFile.frontmatter.tags).toEqual(undefined); // TODO This will be inconvenient for users - should be []?
     });
 
     it('should provide an independent copy of frontmatter', () => {
@@ -99,6 +111,19 @@ describe('TasksFile - reading frontmatter', () => {
         expect(tasksFile.cachedMetadata.frontmatter?.tags).toEqual(['multiple1', 'multiple2']);
     });
 
+    it('should detect whether file is a kanban plugin board', () => {
+        const tasksFile = getTasksFileFromMockData(example_kanban);
+
+        expect(tasksFile.frontmatter['kanban-plugin']).toEqual('basic');
+        // An example search might be:
+        //      filter by function task.file.frontmatter['kanban-plugin'] !== undefined
+
+        // TODO Consider sanitising property names:
+        //      See dataview docs for ideas:
+        //          https://blacksmithgu.github.io/obsidian-dataview/annotation/add-metadata/#field-names
+        expect(tasksFile.frontmatter.kanban_plugin).toBeUndefined();
+    });
+
     // See property types: https://help.obsidian.md/Editing+and+formatting/Properties#Property+types
     // Obsidian supports the following property types:
     //  Text
@@ -124,4 +149,17 @@ describe('TasksFile - reading tags', () => {
         const tasksFile = getTasksFileFromMockData(callouts_nested_issue_2890_unlabelled);
         expect(tasksFile.tags).toEqual(['#task']);
     });
+
+    it.each(
+        listPathAndData([
+            yaml_custom_number_property, // no tags value in frontmatter
+            yaml_tags_field_added_by_obsidian_but_not_populated,
+            yaml_tags_had_value_then_was_emptied_by_obsidian,
+            yaml_tags_is_empty_list,
+            yaml_tags_is_empty,
+        ]),
+    )('should provide empty list if no tags in frontmatter: "%s"', (_path: string, data: any) => {
+        const tasksFile = getTasksFileFromMockData(data);
+        expect(tasksFile.frontmatter.tags).toEqual([]);
+    });
 });
diff --git a/tests/__mocks__/obsidian.ts b/tests/__mocks__/obsidian.ts
index 20d56642..c59ffa9b 100644
--- a/tests/__mocks__/obsidian.ts
+++ b/tests/__mocks__/obsidian.ts
@@ -130,11 +130,17 @@ export function setCurrentCacheFile(mockData: any) {
     mockedFileData = mockData;
 }
 
-export function getAllTags(_cachedMetadata: CachedMetadata): string[] {
+export function getAllTags(cachedMetadata: CachedMetadata): string[] {
+    if (cachedMetadata !== mockedFileData.cachedMetadata) {
+        throw new Error('Inconsistent test data used in mock getAllTags()');
+    }
     return mockedFileData.getAllTags;
 }
 
-export function parseFrontMatterTags(_frontmatter: any | null): string[] | null {
+export function parseFrontMatterTags(frontmatter: any | null): string[] | null {
+    if (frontmatter !== mockedFileData.cachedMetadata.frontmatter) {
+        throw new Error('Inconsistent test data used in mock parseFrontMatterTags()');
+    }
     return mockedFileData.parseFrontMatterTags;
 }