Merge pull request #3390 from ilandikov/refactor-null-from-list-item-line

refactor:  `ListItem.fromListItemLine()` now correctly returns `null`

Author: claremacrae

src/Task/ListItem.ts

@@ -63,23 +63,22 @@ export class ListItem {
         taskLocation: TaskLocation,
     ): ListItem | null {
         const nonTaskMatch = RegExp(TaskRegularExpressions.nonTaskRegex).exec(originalMarkdown);
-        let indentation = '';
-        let listMarker = '';
-        let statusCharacter = null;
-        let description = '';
-        if (nonTaskMatch) {
-            indentation = nonTaskMatch[1];
-            listMarker = nonTaskMatch[2];
-            statusCharacter = nonTaskMatch[4] ?? null;
-            description = nonTaskMatch[5].trim();
+        if (!nonTaskMatch) {
+            // In practice we never reach here, because the regexp matches any text even '', but the compiler doesn't know that.
+            return null;
+        }
+
+        const listMarker = nonTaskMatch[2];
+        if (listMarker === undefined) {
+            return null;
         }
 
         return new ListItem({
             originalMarkdown,
-            indentation,
+            indentation: nonTaskMatch[1],
             listMarker,
-            statusCharacter,
-            description,
+            statusCharacter: nonTaskMatch[4] ?? null,
+            description: nonTaskMatch[5].trim(),
             taskLocation,
             parent,
         });

tests/Task/ListItem.test.ts

@@ -16,26 +16,26 @@ const taskLocation = TaskLocation.fromUnknownPosition(new TasksFile('anything.md
 
 describe('list item tests', () => {
     it('should create list item with empty children and absent parent', () => {
-        const listItem = ListItem.fromListItemLine('', null, taskLocation)!;
+        const listItem = ListItem.fromListItemLine('- list item', null, taskLocation)!;
         expect(listItem).toBeDefined();
         expect(listItem.children).toEqual([]);
         expect(listItem.parent).toEqual(null);
         expect(listItem.taskLocation).toBe(taskLocation);
     });
 
     it('should create a list item with 2 children', () => {
-        const listItem = ListItem.fromListItemLine('', null, taskLocation)!;
-        const childItem1 = ListItem.fromListItemLine('', listItem, taskLocation)!;
-        const childItem2 = ListItem.fromListItemLine('', listItem, taskLocation)!;
+        const listItem = ListItem.fromListItemLine('- list item', null, taskLocation)!;
+        const childItem1 = ListItem.fromListItemLine('- list item', listItem, taskLocation)!;
+        const childItem2 = ListItem.fromListItemLine('- list item', listItem, taskLocation)!;
         expect(listItem).toBeDefined();
         expect(childItem1.parent).toEqual(listItem);
         expect(childItem2.parent).toEqual(listItem);
         expect(listItem.children).toEqual([childItem1, childItem2]);
     });
 
     it('should create a list item with a parent', () => {
-        const parentItem = ListItem.fromListItemLine('', null, taskLocation)!;
-        const listItem = ListItem.fromListItemLine('', parentItem, taskLocation)!;
+        const parentItem = ListItem.fromListItemLine('- list item', null, taskLocation)!;
+        const listItem = ListItem.fromListItemLine('- list item', parentItem, taskLocation)!;
         expect(listItem).toBeDefined();
         expect(listItem.parent).toEqual(parentItem);
         expect(parentItem.children).toEqual([listItem]);
@@ -149,14 +149,10 @@ describe('list item parsing', () => {
         expect(ListItem.fromListItemLine('2. xxx', null, taskLocation)!.listMarker).toEqual('2.');
     });
 
-    it('should accept a non list item', () => {
-        // we tried making the constructor throw if given a non list item
-        // but it broke lots of normal Task uses in the tests (TaskBuilder)
+    it('should detect a non-list item', () => {
         const item = ListItem.fromListItemLine('# Heading', null, taskLocation)!;
 
-        expect(item.description).toEqual('# Heading');
-        expect(item.originalMarkdown).toEqual('# Heading');
-        expect(item.statusCharacter).toEqual(null);
+        expect(item).toBeNull();
     });
 });