Repository: cdklabs/decdk (ts)

Issue:
Provide better error messages when invalid values are passed
It appears I can't use a `Bucket` where an `IBucket` is expected. Example:
Edit: Whoops: looks like I just forgot to include Ref:. We should improve the error messaging to make this more clear.

```
{
  "$schema": "../cdk.schema.json",
  "Resources": {
    "MyBucket": {
      "Type": "aws-cdk-lib.aws_s3.Bucket"
    },
    "MyDist": {
      "Type": "aws-cdk-lib.aws_cloudfront.Distribution",
      "Properties": {
        "defaultBehavior": {
          "origin": {
            "aws-cdk-lib.aws_cloudfront_origins.S3Origin": {
              "bucket": "MyBucket"
            }
          }
        }
      }
    }
  }
}
```

throws the error:

```
Error: Cannot parse class type aws-cdk-lib.aws_s3.IBucket with value MyBucket
```

Pull request:
fix: provide useful type resolution errors
Fixes #62

---

Wrong type:
```
Old: Error: Expected object, got: {"type":"string","value":"MyBucket"}
New: TypeError: Expected aws-cdk-lib.aws_s3.IBucket, got: "MyBucket"
```

Static Method not found:
```
Old: Error: Assembly "" not found
New: TypeError: Expected static method not found, got: "bucket"
```


Patch:
diff --git a/src/parser/template/expression.ts b/src/parser/template/expression.ts
index 2738cf0..ea2f648 100644
--- a/src/parser/template/expression.ts
+++ b/src/parser/template/expression.ts
@@ -249,6 +249,25 @@ export function assertExpression(x: unknown): TemplateExpression {
   return x as TemplateExpression;
 }
 
+export function unparseExpression(x: TemplateExpression): any {
+  switch (x.type) {
+    case 'string':
+    case 'number':
+    case 'boolean':
+      return x.value;
+    case 'null':
+      return null;
+    case 'array':
+      return x.array.map(unparseExpression);
+    case 'object':
+      return Object.fromEntries(
+        Object.entries(x.fields).map(([k, v]) => [k, unparseExpression(v)])
+      );
+    default:
+      return x;
+  }
+}
+
 export function parseExpression(x: unknown): TemplateExpression {
   if (x == null) {
     return { type: 'null' };
diff --git a/src/type-resolution/callables.ts b/src/type-resolution/callables.ts
index dd39e44..9a924ea 100644
--- a/src/type-resolution/callables.ts
+++ b/src/type-resolution/callables.ts
@@ -55,7 +55,7 @@ export function resolveStaticMethodCallExpression(
   typeSystem: reflect.TypeSystem,
   resultType?: reflect.Type
 ): StaticMethodCallExpression {
-  const { method, args } = inferMethodCall(typeSystem, call);
+  const { method, args } = inferStaticMethodCall(typeSystem, call);
 
   if (resultType) {
     assertImplements(method.returns.type, resultType);
@@ -94,7 +94,7 @@ export function resolveInstanceMethodCallExpression(
   };
 }
 
-function inferMethodCall(
+function inferStaticMethodCall(
   typeSystem: reflect.TypeSystem,
   call: FactoryMethodCall
 ): MethodCall {
@@ -118,7 +118,7 @@ function inferInstanceMethodCall(
   call: Required<FactoryMethodCall>
 ): InstanceMethodCall {
   const factory = inferType(call.target);
-  const method = inferMethod(factory, call.methodName);
+  const method = inferInstanceMethod(factory, call.methodName);
   const args = resolveArguments(call.arguments, method);
 
   return {
@@ -145,12 +145,14 @@ function inferInstanceMethodCall(
       }
 
       if (res.call?.target) {
-        return inferMethod(inferType(res.call.target), res.call.methodName)
-          .returns.type;
+        return inferInstanceMethod(
+          inferType(res.call.target),
+          res.call.methodName
+        ).returns.type;
       }
 
       if (res.call != null) {
-        const methodCall = inferMethodCall(typeSystem, res.call);
+        const methodCall = inferStaticMethodCall(typeSystem, res.call);
         return methodCall.method.returns.type;
       }
 
@@ -164,7 +166,7 @@ function inferInstanceMethodCall(
   }
 }
 
-function inferMethod(
+function inferInstanceMethod(
   typeRef: reflect.TypeReference,
   methodName: string
 ): reflect.Method {
@@ -173,7 +175,7 @@ function inferMethod(
   const methodNames = methods.map((m) => m.name);
 
   if (!methodNames.includes(methodName)) {
-    throw new Error(
+    throw new TypeError(
       `'${candidateType.fqn}' has no method called '${methodName}'`
     );
   }
@@ -200,7 +202,13 @@ function resolveTypeFromPath(
 
 function enclosingClassFqn(methodFqn: string): string {
   const parts = methodFqn.split('.');
-  return parts.slice(0, parts.length - 1).join('.');
+  const enclosingFqn = parts.slice(0, parts.length - 1).join('.');
+  if (!enclosingFqn) {
+    throw new TypeError(
+      `Expected static method, got: ${JSON.stringify(methodFqn)}`
+    );
+  }
+  return enclosingFqn;
 }
 
 export interface InitializerExpression {
diff --git a/src/type-resolution/enums.ts b/src/type-resolution/enums.ts
index 0ef78ac..27ad6c7 100644
--- a/src/type-resolution/enums.ts
+++ b/src/type-resolution/enums.ts
@@ -6,7 +6,7 @@ import {
   resolveInstanceExpression,
   StaticMethodCallExpression,
 } from './callables';
-import { assertExpressionType } from './expression';
+import { assertExpressionForType } from './expression';
 
 export interface StaticPropertyExpression {
   readonly type: 'staticProperty';
@@ -31,7 +31,10 @@ export function resolveEnumLikeExpression(
     };
   }
 
-  return resolveInstanceExpression(assertExpressionType(x, 'object'), type);
+  return resolveInstanceExpression(
+    assertExpressionForType(x, 'object', type.reference),
+    type
+  );
 }
 
 export function assertClass(typeRef: reflect.TypeReference): reflect.ClassType {
diff --git a/src/type-resolution/expression.ts b/src/type-resolution/expression.ts
index 75b3cff..0f7f73a 100644
--- a/src/type-resolution/expression.ts
+++ b/src/type-resolution/expression.ts
@@ -1,4 +1,4 @@
-import { ParserError } from '../parser/private/types';
+import * as reflect from 'jsii-reflect';
 import {
   ArrayExpression,
   BooleanLiteral,
@@ -8,6 +8,7 @@ import {
   ObjectExpression,
   StringLiteral,
   TemplateExpression,
+  unparseExpression,
 } from '../parser/template';
 import {
   InitializerExpression,
@@ -46,12 +47,15 @@ export interface TypedArrayExpression
 export interface TypedObjectExpression
   extends ObjectExpression<TypedTemplateExpression> {}
 
-export function assertExpressionType<T extends TemplateExpression['type']>(
+export function assertExpressionForType<T extends TemplateExpression['type']>(
   x: TemplateExpression,
-  type: T
+  type: T,
+  typeRef: reflect.TypeReference
 ): TemplateExpression & { type: T } {
   if (x.type !== type) {
-    throw new ParserError(`Expected ${type}, got: ${JSON.stringify(x)}`);
+    throw new TypeError(
+      `Expected ${typeRef.fqn}, got: ${JSON.stringify(unparseExpression(x))}`
+    );
   }
 
   return x as TemplateExpression & { type: T };
diff --git a/src/type-resolution/resolve.ts b/src/type-resolution/resolve.ts
index cbab8a3..022772a 100644
--- a/src/type-resolution/resolve.ts
+++ b/src/type-resolution/resolve.ts
@@ -21,7 +21,7 @@ import {
 } from './enums';
 import {
   assertExpressionShaped,
-  assertExpressionType,
+  assertExpressionForType,
   TypedTemplateExpression,
 } from './expression';
 import { resolveDateLiteral } from './literals';
@@ -52,15 +52,15 @@ export function resolveExpressionType(
     case ResolvableExpressionType.STRING:
       return assertLiteralOrIntrinsic(x, 'string');
     case ResolvableExpressionType.DATE:
-      return resolveDateLiteral(assertExpressionType(x, 'string'));
+      return resolveDateLiteral(assertExpressionForType(x, 'string', typeRef));
     case ResolvableExpressionType.ARRAY_OF_TYPE:
       return resolveArrayOfTypeExpression(
-        assertExpressionType(x, 'array'),
+        assertExpressionForType(x, 'array', typeRef),
         assertArrayOfType(typeRef)
       );
     case ResolvableExpressionType.MAP_OF_TYPE:
       return resolveMapOfTypeExpression(
-        assertExpressionType(x, 'object'),
+        assertExpressionForType(x, 'object', typeRef),
         assertMapOfType(typeRef)
       );
     case ResolvableExpressionType.UNION_OF_TYPES:
@@ -68,13 +68,13 @@ export function resolveExpressionType(
     case ResolvableExpressionType.STRUCT:
       return refOrResolve(x, (y) =>
         resolveStructExpression(
-          assertExpressionType(y, 'object'),
+          assertExpressionForType(y, 'object', typeRef),
           assertInterface(typeRef)
         )
       );
     case ResolvableExpressionType.ENUM:
       return resolveEnumExpression(
-        assertExpressionType(x, 'string'),
+        assertExpressionForType(x, 'string', typeRef),
         assertEnum(typeRef)
       );
     case ResolvableExpressionType.ENUM_LIKE_CLASS:
@@ -85,14 +85,14 @@ export function resolveExpressionType(
     case ResolvableExpressionType.BEHAVIORAL_INTERFACE:
       return refOrResolve(x, (y) =>
         resolveInstanceExpression(
-          assertExpressionType(y, 'object'),
+          assertExpressionForType(y, 'object', typeRef),
           assertInterface(typeRef)
         )
       );
     case ResolvableExpressionType.OTHER_CLASS:
       return refOrResolve(x, (y) =>
         resolveInstanceExpression(
-          assertExpressionType(y, 'object'),
+          assertExpressionForType(y, 'object', typeRef),
           assertClass(typeRef)
         )
       );


Test patch:
diff --git a/test/evaluate/errors.test.ts b/test/evaluate/errors.test.ts
index 8e1e47f..de724bf 100644
--- a/test/evaluate/errors.test.ts
+++ b/test/evaluate/errors.test.ts
@@ -3,27 +3,95 @@ import { Template } from '../../src/parser/template';
 import { Testing } from '../util';
 
 suite('Evaluation errors', () => {
-  test('invalid enum option raises an error', async () => {
-    // GIVEN
-    const template = {
-      Resources: {
-        Hello: {
-          Type: 'aws-cdk-lib.aws_sqs.Queue',
-          Properties: {
-            encryption: 'boom',
+  suite('Enums', () => {
+    test('invalid enum option raises an error', async () => {
+      // GIVEN
+      const template = {
+        Resources: {
+          Hello: {
+            Type: 'aws-cdk-lib.aws_sqs.Queue',
+            Properties: {
+              encryption: 'boom',
+            },
           },
         },
-      },
-    };
+      };
 
-    // THEN
-    expect(template).not.toBeValidTemplate();
-    await expect(
-      Testing.synth(await Template.fromObject(template), {
+      // THEN
+      expect(template).not.toBeValidTemplate();
+      await expect(
+        Testing.synth(await Template.fromObject(template), {
+          validateTemplate: false,
+        })
+      ).rejects.toThrow(
+        'Expected choice for enum type aws-cdk-lib.aws_sqs.QueueEncryption to be one of UNENCRYPTED|KMS_MANAGED|KMS|SQS_MANAGED, got: boom'
+      );
+    });
+  });
+
+  suite('Invalid FQNs', () => {
+    test('invalid method name option raises an error', async () => {
+      // GIVEN
+      const template = {
+        Resources: {
+          MyBucket: {
+            Type: 'aws-cdk-lib.aws_s3.Bucket',
+          },
+          MyDist: {
+            Type: 'aws-cdk-lib.aws_cloudfront.Distribution',
+            Properties: {
+              defaultBehavior: {
+                origin: {
+                  'aws-cdk-lib.aws_cloudfront_origins.S3Origin': {
+                    bucket: 'MyBucket',
+                  },
+                },
+              },
+            },
+          },
+        },
+      };
+
+      // THEN
+      const synth = Testing.synth(await Template.fromObject(template), {
+        validateTemplate: false,
+      });
+      await expect(synth).rejects.toThrow(TypeError);
+      await expect(synth).rejects.toThrow(
+        'Expected static method, got: "bucket"'
+      );
+    });
+  });
+
+  suite('Invalid Types', () => {
+    test('string in place of reference raised an error', async () => {
+      // GIVEN
+      const template = {
+        Resources: {
+          MyBucket: {
+            Type: 'aws-cdk-lib.aws_s3.Bucket',
+          },
+          MyDist: {
+            Type: 'aws-cdk-lib.aws_cloudfront.Distribution',
+            Properties: {
+              defaultBehavior: {
+                origin: {
+                  'aws-cdk-lib.aws_cloudfront_origins.S3Origin': 'MyBucket',
+                },
+              },
+            },
+          },
+        },
+      };
+
+      // THEN
+      const synth = Testing.synth(await Template.fromObject(template), {
         validateTemplate: false,
-      })
-    ).rejects.toThrow(
-      'Expected choice for enum type aws-cdk-lib.aws_sqs.QueueEncryption to be one of UNENCRYPTED|KMS_MANAGED|KMS|SQS_MANAGED, got: boom'
-    );
+      });
+      await expect(synth).rejects.toThrow(TypeError);
+      await expect(synth).rejects.toThrow(
+        'Expected aws-cdk-lib.aws_s3.IBucket, got: "MyBucket'
+      );
+    });
   });
 });