fix: draft() emits ciceromark Formula node without required value field (#146)

src/TemplateMarkInterpreter.ts

@@ -393,7 +393,12 @@ async function generateAgreement(modelManager: ModelManager, clauseLibrary: obje
             else if (FORMULA_DEFINITION_RE.test(nodeClass)) {
                 if (context.code) {
                     const result = userCodeResults[this.path.join('/')];
-                    if (result === null) {
+                    if (result === undefined) {
+                        // JSON.stringify(undefined) is undefined, which would leave the
+                        // required `value` field unset and fail downstream validation.
+                        throw new Error(`Formula '${context.name}' did not return a value. Formulas must be an expression or use 'return' to produce a value.`);
+                    }
+                    else if (result === null) {
                         context.value = '<null>';
                     }
                     else if (typeof result === 'string') {

src/utils.ts

@@ -20,6 +20,7 @@ import { templatemarkutil } from '@accordproject/markdown-template';
 
 import { existsSync, mkdirSync, rmSync } from 'fs';
 import traverse from 'traverse';
+import * as ts from 'typescript';
 
 export function ensureDirSync(path:string) {
     if(!existsSync(path)) {
@@ -40,13 +41,50 @@ export function writeFunctionToString(templateClass:ClassDeclaration, functionNa
     templateClass.getProperties().forEach((p: Property) => {
         result += `   const ${p.getName()} = data.${p.getName()};\n`;
     });
-    result += '   ' + code.trim() + '\n';
+    result += '   ' + wrapExpressionWithReturn(code) + '\n';
     result += '}\n';
     result += '\n';
 
     return result;
 }
 
+/**
+ * Wraps user-supplied formula/condition code with `return` when the user did
+ * not write an explicit `return`, so that an inline expression like
+ * `{{% amount / 2.0 %}}` produces a value rather than an undefined result that
+ * fails downstream validation.
+ *
+ * Uses the TypeScript AST to inspect only top-level statements, so a nested
+ * function/arrow that contains its own `return` does not confuse the outer
+ * detection (e.g. `[1,2,3].map(x => { return x*2 })[0]`).
+ */
+export function wrapExpressionWithReturn(code: string): string {
+    const trimmed = code.trim();
+    if (trimmed.length === 0) {
+        return trimmed;
+    }
+    const sourceFile = ts.createSourceFile('formula.ts', trimmed, ts.ScriptTarget.Latest, false, ts.ScriptKind.TS);
+    const statements = sourceFile.statements;
+    if (statements.length === 0) {
+        return trimmed;
+    }
+    for (const stmt of statements) {
+        if (ts.isReturnStatement(stmt)) {
+            return trimmed;
+        }
+    }
+    const last = statements[statements.length - 1];
+    if (!ts.isExpressionStatement(last)) {
+        return trimmed;
+    }
+    const prefix = statements
+        .slice(0, -1)
+        .map(s => s.getText(sourceFile))
+        .join('\n');
+    const expr = last.expression.getText(sourceFile);
+    return prefix.length > 0 ? `${prefix}\nreturn ${expr};` : `return ${expr};`;
+}
+
 export function nameUserCode(templateMarkDom: any) {
     return traverse(templateMarkDom).map(function (x) {
         if (x && ((x.$class === `${TemplateMarkModel.NAMESPACE}.ConditionalDefinition` && x.condition) ||

test/__snapshots__/TemplateMarkInterpreter.test.ts.snap

@@ -17,43 +17,23 @@ exports[`templatemark interpreter should fail to generate formula-invalid 1`] =
       },
       {
         "category": 1,
-        "character": 8,
+        "character": 0,
         "code": 2304,
         "id": "err-2304-473-2",
         "length": 2,
-        "line": 82,
+        "line": 83,
         "renderedMessage": "Cannot find name 'IS'.",
         "start": 1790,
       },
       {
         "category": 1,
-        "character": 11,
+        "character": 7,
         "code": 2304,
-        "id": "err-2304-476-7",
+        "id": "err-2304-483-7",
         "length": 7,
-        "line": 82,
+        "line": 84,
         "renderedMessage": "Cannot find name 'GARBAGE'.",
-        "start": 1793,
-      },
-      {
-        "category": 1,
-        "character": 3,
-        "code": 1435,
-        "id": "err-1435-468-4",
-        "length": 4,
-        "line": 82,
-        "renderedMessage": "Unknown keyword or identifier. Did you mean 'this'?",
-        "start": 1785,
-      },
-      {
-        "category": 1,
-        "character": 8,
-        "code": 1434,
-        "id": "err-1434-473-2",
-        "length": 2,
-        "line": 82,
-        "renderedMessage": "Unexpected keyword or identifier.",
-        "start": 1790,
+        "start": 1800,
       },
     ],
     "nodeId": "formula_adef0feb9fc1b6e7513409c93d86aa4d9999d35608e1aaeecc07dca99bc591c8",
@@ -1300,6 +1280,64 @@ exports[`templatemark interpreter should generate helloformula 1`] = `
 }
 `;
 
+exports[`templatemark interpreter should generate helloformula-implicit-return 1`] = `
+{
+  "$class": "org.accordproject.commonmark@0.5.0.Document",
+  "nodes": [
+    {
+      "$class": "org.accordproject.commonmark@0.5.0.Paragraph",
+      "nodes": [
+        {
+          "$class": "org.accordproject.commonmark@0.5.0.Paragraph",
+          "nodes": [
+            {
+              "$class": "org.accordproject.commonmark@0.5.0.Text",
+              "text": "Hello ",
+            },
+            {
+              "$class": "org.accordproject.ciceromark@0.6.0.Variable",
+              "elementType": "String",
+              "name": "message",
+              "value": "World",
+            },
+            {
+              "$class": "org.accordproject.commonmark@0.5.0.Text",
+              "text": "! Half of ",
+            },
+            {
+              "$class": "org.accordproject.ciceromark@0.6.0.Variable",
+              "elementType": "Double",
+              "name": "importerLOCAmount",
+              "value": "1000.0",
+            },
+            {
+              "$class": "org.accordproject.commonmark@0.5.0.Text",
+              "text": " is ",
+            },
+            {
+              "$class": "org.accordproject.commonmark@0.5.0.Strong",
+              "nodes": [
+                {
+                  "$class": "org.accordproject.ciceromark@0.6.0.Formula",
+                  "dependencies": [],
+                  "name": "formula_6a1c6cfe8791ac1da78736898d0ee7ead395e709aa1c4f3e890f2fff0b51497f",
+                  "value": "500",
+                },
+              ],
+            },
+            {
+              "$class": "org.accordproject.commonmark@0.5.0.Text",
+              "text": ".",
+            },
+          ],
+        },
+      ],
+    },
+  ],
+  "xmlns": "http://commonmark.org/xml/1.0",
+}
+`;
+
 exports[`templatemark interpreter should generate helloworld 1`] = `
 {
   "$class": "org.accordproject.commonmark@0.5.0.Document",

test/templates/good/helloformula-implicit-return/data.json

@@ -0,0 +1,5 @@
+{
+    "$class" : "helloformula@1.0.0.TemplateData",
+    "message": "World",
+    "importerLOCAmount": 1000.0
+}

test/templates/good/helloformula-implicit-return/model.cto

@@ -0,0 +1,7 @@
+namespace helloformula@1.0.0
+
+@template
+concept TemplateData {
+    o String message
+    o Double importerLOCAmount
+}

test/templates/good/helloformula-implicit-return/template.md

@@ -0,0 +1 @@
+Hello {{message}}! Half of {{importerLOCAmount}} is **{{% importerLOCAmount / 2.0 %}}**.

test/utils.test.ts

@@ -0,0 +1,32 @@
+import { wrapExpressionWithReturn } from '../src/utils';
+
+describe('wrapExpressionWithReturn', () => {
+    test('wraps a bare expression with return', () => {
+        expect(wrapExpressionWithReturn('importerLOCAmount / 2.0')).toBe('return importerLOCAmount / 2.0;');
+    });
+
+    test('leaves explicit top-level return unchanged', () => {
+        const code = 'const x = 1;\nreturn x + 2;';
+        expect(wrapExpressionWithReturn(code)).toBe(code);
+    });
+
+    test('wraps when only a nested function/arrow contains return (regression for #146 follow-up)', () => {
+        const code = '[1,2,3].map(x => { return x*2 })[0]';
+        expect(wrapExpressionWithReturn(code)).toBe('return [1,2,3].map(x => { return x*2 })[0];');
+    });
+
+    test('wraps only the trailing expression of a multi-statement body', () => {
+        const code = 'const x = 1;\nconst y = 2;\nx + y';
+        expect(wrapExpressionWithReturn(code)).toBe('const x = 1;\nconst y = 2;\nreturn x + y;');
+    });
+
+    test('leaves a statements-only body (no trailing expression) unchanged', () => {
+        const code = 'const x = 1;\nconst y = 2;';
+        expect(wrapExpressionWithReturn(code)).toBe(code);
+    });
+
+    test('returns an empty string as-is', () => {
+        expect(wrapExpressionWithReturn('')).toBe('');
+        expect(wrapExpressionWithReturn('   \n\t  ')).toBe('');
+    });
+});