Merge pull request #3380 from obsidian-tasks-group/prevent-unused-placeholders-in-placeholders

fix: Prevent use of missing properties in placeholders (#3249)

Author: claremacrae

src/Scripting/ExpandPlaceholders.ts

@@ -72,6 +72,13 @@ function evaluateAnyFunctionCalls(template: string, view: any) {
         const functionOrError = parseExpression(paramsArgs, reconstructed);
         if (functionOrError.isValid()) {
             const result = evaluateExpression(functionOrError.queryComponent!, paramsArgs);
+            if (result === null) {
+                throw Error(
+                    `Invalid placeholder result 'null'.
+    Check for missing file property in this expression:
+        {{${reconstructed}}}`,
+                );
+            }
             if (result !== undefined) {
                 return result;
             }

tests/Query/Query.test.ts

@@ -892,19 +892,21 @@ group by folder
                 `);
             });
 
-            it('visualise the need to guard against undefined query properties in placeholders', () => {
+            it('visualise the guard against undefined query properties in placeholders', () => {
                 const source = "{{query.file.property('no_such_property')}}";
                 const query = new Query(source, file);
 
                 expect(query.error).not.toBeUndefined();
-                // The 'null' value is because we are embedding regardless of whether the property
-                // exists in the query file.
                 expect(query.error).toMatchInlineSnapshot(`
-                    "do not understand query
-                    Problem statement:
-                        {{query.file.property('no_such_property')}} =>
-                        null
-                    "
+                    "There was an error expanding one or more placeholders.
+
+                    The error message was:
+                        Invalid placeholder result 'null'.
+                        Check for missing file property in this expression:
+                            {{query.file.property('no_such_property')}}
+
+                    The problem is in:
+                        {{query.file.property('no_such_property')}}"
                 `);
             });
 

tests/Scripting/ExpandPlaceholders.test.ts

@@ -90,13 +90,22 @@ The problem is in:
         );
     });
 
-    it.failing('should not treat absent property values as string ', () => {
-        // TODO We really must prevent use of null - and probably undefined - as strings from property placeholders
+    it('should not treat absent property values as string, but report the error ', () => {
         const rawString = '{{ query.file.property("non-existent")}}';
 
         const queryContext = makeQueryContext(tasksFile);
-        const expanded = expandPlaceholders(rawString, queryContext);
-        expect(expanded).not.toContain('null');
+
+        expect(() => expandPlaceholders(rawString, queryContext)).toThrow(
+            `There was an error expanding one or more placeholders.
+
+The error message was:
+    Invalid placeholder result 'null'.
+    Check for missing file property in this expression:
+        {{ query.file.property("non-existent")}}
+
+The problem is in:
+    {{ query.file.property("non-existent")}}`,
+        );
     });
 });