Lazy state errors

.changeset/four-lights-see.md

@@ -0,0 +1,6 @@
+---
+'@openfn/compiler': patch
+'@openfn/cli': patch
+---
+
+Improve messaging and position reporting in lazy state compilation errors

packages/cli/src/util/abort.ts

@@ -7,18 +7,35 @@ class AbortError extends Error {
   handled = true;
 }
 
+interface CLIFriendlyError extends Error {
+  fix?: string;
+  details?: string;
+}
+
 // This is always the CLI logger, can I trap it?
 export default (
   logger: Logger,
   reason: string,
-  error?: Error,
+  error?: CLIFriendlyError,
   help?: string
 ) => {
   const e = new AbortError(reason);
   logger.break();
   logger.error(reason);
   if (error) {
     logger.error(error.message);
+    logger.break();
+
+    if (error.details) {
+      logger.error('ERROR DETAILS:');
+      logger.error(error.details);
+      logger.break();
+    }
+    if (error.fix) {
+      logger.error('FIX HINT:');
+      logger.error(error.fix);
+      logger.break();
+    }
   }
   if (help) {
     logger.always(help);

packages/compiler/src/transforms/lazy-state.ts

@@ -11,6 +11,32 @@ import type { NodePath } from 'ast-types/lib/node-path';
 import type { Transformer } from '../transform';
 import IgnoreRules from '../transform-ignore';
 
+type LazyStateErrorOptions = {
+  details?: string;
+  fix?: string;
+  pos?: any;
+};
+
+export class LazyStateError extends Error {
+  fix?: string;
+  details?: string;
+  pos?: any;
+
+  constructor(
+    message: string,
+    { details, fix, pos }: LazyStateErrorOptions = {}
+  ) {
+    const posStr = (pos && `(${pos.start.line}:${pos.start.column})`) ?? '';
+    super(`Lazy State Error: ${message} ${posStr}`);
+
+    this.fix = fix;
+    this.details = details;
+    const { start, end } = pos;
+    this.pos = { start, end };
+    delete this.stack;
+  }
+}
+
 // Walk up the AST and work out where the parent arrow function should go
 const ensureParentArrow = (path: NodePath<n.MemberExpression>) => {
   let root = path;
@@ -23,9 +49,12 @@ const ensureParentArrow = (path: NodePath<n.MemberExpression>) => {
     root = root.parent;
 
     // if this is any kind of statement, we should throw
-    // TODO we may relax this, see https://github.com/OpenFn/kit/issues/660
     if (n.Statement.check(root.node) || n.Declaration.check(root.node)) {
-      throw new Error(`invalid state operator: must be inside an expression`);
+      throw new LazyStateError('Must be inside an operation', {
+        pos: path.node.loc,
+        details:
+          'The Lazy State operation must be used inside a top-level operation, like fn(). It cannot be used inside a regular JavaScript statement because no valid state reference is available.',
+      });
     }
   }
 
@@ -47,9 +76,9 @@ const ensureParentArrow = (path: NodePath<n.MemberExpression>) => {
     }
   } else {
     // Actually I don't think we'll ever get here
-    throw new Error(
-      `invalid state operator: must be be passed as an argument to an operator`
-    );
+    throw new LazyStateError('must be passed as an argument to an operator', {
+      pos: path.node.loc,
+    });
   }
 };
 
@@ -66,11 +95,11 @@ const isOpenFunction = (path: NodePath) => {
         // We already have a valid open function here
         return true;
       }
-      throw new Error(
-        `invalid state operator: parameter "${name}" should be called "state"`
-      );
+      throw new LazyStateError(`parameter "${name}" should be called "state"`, {
+        pos: path.node.loc,
+      });
     }
-    throw new Error('invalid state operator: parent has wrong arity');
+    throw new LazyStateError('parent has wrong arity', { pos: path.node.loc });
   }
 
   // if we get here, then path is:

packages/compiler/test/transforms/lazy-state.test.ts

@@ -5,7 +5,7 @@ import { namedTypes, NodePath, builders as b } from 'ast-types';
 import parse from '../../src/parse';
 
 import transform from '../../src/transform';
-import visitors from '../../src/transforms/lazy-state';
+import visitors, { LazyStateError } from '../../src/transforms/lazy-state';
 
 test('convert a simple dollar reference', (t) => {
   const ast = parse('get($.data)');
@@ -86,7 +86,8 @@ test('throw if a $ is already inside a non-compatible arrow (state name)', (t) =
   const ast = parse(src);
 
   t.throws(() => transform(ast, [visitors]), {
-    message: `invalid state operator: parameter "s" should be called "state"`,
+    instanceOf: LazyStateError,
+    message: /parameter "s" should be called "state" \(1:4\)/i,
   });
 });
 
@@ -96,7 +97,8 @@ test('throw if a $ is already inside a non-compatible arrow (arity)', (t) => {
   const ast = parse(src);
 
   t.throws(() => transform(ast, [visitors]), {
-    message: 'invalid state operator: parent has wrong arity',
+    instanceOf: LazyStateError,
+    message: /parent has wrong arity \(1:4\)/i,
   });
 });
 
@@ -106,7 +108,8 @@ test('throw if $ is not inside an operation', (t) => {
   const ast = parse(src);
 
   t.throws(() => transform(ast, [visitors]), {
-    message: 'invalid state operator: must be inside an expression',
+    instanceOf: LazyStateError,
+    message: /must be inside an operation \(1:10\)/i,
   });
 });
 
@@ -116,7 +119,8 @@ test('throw if $ is on the left hand side of an assignment', (t) => {
   const ast = parse(src);
 
   t.throws(() => transform(ast, [visitors]), {
-    message: 'invalid state operator: must be inside an expression',
+    instanceOf: LazyStateError,
+    message: /must be inside an operation \(1:0\)/i,
   });
 });
 
@@ -126,7 +130,8 @@ test('throw if $ is on the left hand side of a nested assignment', (t) => {
   const ast = parse(src);
 
   t.throws(() => transform(ast, [visitors]), {
-    message: 'invalid state operator: must be inside an expression',
+    instanceOf: LazyStateError,
+    message: /must be inside an operation \(1:11\)/i,
   });
 });
 
@@ -136,7 +141,8 @@ test('throw if $ is on the left hand side of a multi assignment', (t) => {
   const ast = parse(src);
 
   t.throws(() => transform(ast, [visitors]), {
-    message: 'invalid state operator: must be inside an expression',
+    instanceOf: LazyStateError,
+    message: /must be inside an operation \(1:10\)/i,
   });
 });