fix(lib): ForExpression missing iteratorContext and OperatorExpression drops falsy right operands

packages/cdktn/lib/terraform-dynamic-expression.ts

@@ -49,7 +49,7 @@ export class TerraformDynamicExpression implements IResolvable {
         // context.resolve is required for the iteratorContext to be correctly passed
         // to Lazy values within this.content
         return context.resolve(
-          forExpression(this.iterator._getForEachExpression(), this.content),
+          forExpression(this.iterator._getForExpressionInput(), this.content),
         );
       },
     });

packages/cdktn/lib/terraform-iterator.ts

@@ -33,6 +33,13 @@ export interface ITerraformIterator {
    * @internal used by TerraformResource to set the for_each expression
    */
   _getForEachExpression(): any;
+
+  /**
+   * @internal input used when this iterator appears inside a for-expression
+   * body (keys / values / pluckProperty / forExpressionForList /
+   * forExpressionForMap / dynamic).
+   */
+  _getForExpressionInput(): any;
 }
 
 type ListType =
@@ -66,6 +73,17 @@ export abstract class TerraformIterator implements ITerraformIterator {
    */
   abstract _getForEachExpression(): any;
 
+  /**
+   * @internal input used when this iterator appears inside a for-expression
+   * body (keys / values / pluckProperty / forExpressionForList /
+   * forExpressionForMap / dynamic). Defaults to the for_each expression;
+   * subclasses that need a different shape in for-expression bodies
+   * (e.g. DynamicListTerraformIterator) can override.
+   */
+  public _getForExpressionInput(): any {
+    return this._getForEachExpression();
+  }
+
   /**
    * Creates a new iterator from a list
    */
@@ -303,7 +321,7 @@ export abstract class TerraformIterator implements ITerraformIterator {
    */
   public keys(): IResolvable {
     return Token.asAny(
-      forExpression(this._getForEachExpression(), FOR_EXPRESSION_KEY),
+      forExpression(this._getForExpressionInput(), FOR_EXPRESSION_KEY),
     );
   }
 
@@ -315,7 +333,7 @@ export abstract class TerraformIterator implements ITerraformIterator {
    */
   public values(): IResolvable {
     return Token.asAny(
-      forExpression(this._getForEachExpression(), FOR_EXPRESSION_VALUE),
+      forExpression(this._getForExpressionInput(), FOR_EXPRESSION_VALUE),
     );
   }
 
@@ -328,7 +346,7 @@ export abstract class TerraformIterator implements ITerraformIterator {
   public pluckProperty(property: string): IResolvable {
     return Token.asAny(
       forExpression(
-        this._getForEachExpression(),
+        this._getForExpressionInput(),
         propertyAccess(FOR_EXPRESSION_VALUE, [property]),
       ),
     );
@@ -349,7 +367,9 @@ export abstract class TerraformIterator implements ITerraformIterator {
    * @param expression The expression to use in the for mapping
    */
   public forExpressionForList(expression: string | IResolvable) {
-    return Token.asAny(forExpression(this._getForEachExpression(), expression));
+    return Token.asAny(
+      forExpression(this._getForExpressionInput(), expression),
+    );
   }
 
   /**
@@ -373,7 +393,7 @@ export abstract class TerraformIterator implements ITerraformIterator {
   ) {
     return Token.asAny(
       forExpression(
-        this._getForEachExpression(),
+        this._getForExpressionInput(),
         valueExpression,
         keyExpression,
       ),
@@ -491,31 +511,25 @@ export class DynamicListTerraformIterator extends MapTerraformIterator {
   }
 
   /**
-   * @internal used by TerraformResource to set the for_each expression
+   * @internal used by TerraformResource to set the for_each expression.
+   * Always returns the map-wrapped form so each.key is the mapKeyAttributeName
+   * and entries are uniquely keyed.
    */
   public _getForEachExpression(): any {
-    // uses a Lazy value to be able to render a conversion into a map in the context of a TerraformResource
-    return Lazy.anyValue(
-      {
-        produce: (context) => {
-          switch (context.iteratorContext) {
-            case "FOR_EXPRESSION":
-              return this.list;
-            case "DYNAMIC_BLOCK": // fallthrough
-            default: // same as dynamic block, as this is the case when a iterator is passed to the root level of e.g. a resource
-              // Turn list into a map
-              // { for k,v in <input> : <keyExpression> => <valueExpression>}
-              return forExpression(
-                this.list, // input
-                FOR_EXPRESSION_VALUE, // valueExpression
-                Fn.lookupNested(FOR_EXPRESSION_VALUE, [
-                  this.mapKeyAttributeName,
-                ]), // keyExpression
-              );
-          }
-        },
-      },
-      { displayHint: "<iterator value>" },
+    // Turn list into a map: { for k,v in <input> : <keyExpression> => <valueExpression> }
+    return forExpression(
+      this.list,
+      FOR_EXPRESSION_VALUE,
+      Fn.lookupNested(FOR_EXPRESSION_VALUE, [this.mapKeyAttributeName]),
     );
   }
+
+  /**
+   * @internal inside a for-expression body we walk the raw list so
+   * `[ for key, val in <list> : ... ]` iterates list elements once,
+   * rather than the doubly-wrapped form the for_each conversion produces.
+   */
+  public _getForExpressionInput(): any {
+    return this.list;
+  }
 }

packages/cdktn/lib/tfExpression.ts

@@ -322,7 +322,10 @@ export class OperatorExpression extends TFExpression {
     context.suppressBraces = true;
 
     const left = this.resolveArg(context, this.left);
-    const right = this.right ? this.resolveArg(context, this.right) : undefined;
+    const right =
+      this.right !== undefined
+        ? this.resolveArg(context, this.right)
+        : undefined;
 
     let expr = "";
     switch (this.operator) {
@@ -331,7 +334,7 @@ export class OperatorExpression extends TFExpression {
         break;
       }
       case "-": {
-        if (right) {
+        if (right !== undefined) {
           // subtraction
           expr = `(${left} - ${right})`;
         } else {
@@ -402,6 +405,7 @@ class ForExpression extends TFExpression {
   public resolve(context: IResolveContext): string {
     const suppressBraces = context.suppressBraces;
     context.suppressBraces = true;
+    context.iteratorContext = "FOR_EXPRESSION";
     const key = this.resolveArg(context, FOR_EXPRESSION_KEY);
     const value = this.resolveArg(context, FOR_EXPRESSION_VALUE);
     const input = this.resolveArg(context, this.input);

packages/cdktn/test/iterator.test.ts

@@ -529,3 +529,116 @@ test("for expressions from iterators", () => {
     "${[ for key, val in toset(var.list): key]}",
   );
 });
+
+test("forExpressionForList with fromComplexList uses raw list, not map conversion", () => {
+  const app = Testing.app();
+  const stack = new TerraformStack(app, "test");
+  const variable = new TerraformVariable(stack, "list", {});
+  const it = TerraformIterator.fromComplexList(
+    variable.listValue as any,
+    "name",
+  );
+
+  new TestResource(stack, "test", {
+    name: Token.asString(
+      it.forExpressionForList(`val.id if val.name == "foo"`),
+    ),
+  });
+
+  const synth = JSON.parse(Testing.synth(stack));
+  // Should iterate the raw list, not the map conversion
+  expect(synth).toHaveProperty(
+    "resource.test_resource.test.name",
+    '${[ for key, val in var.list: val.id if val.name == "foo"]}',
+  );
+});
+
+test("keys() on fromComplexList yields raw list indices", () => {
+  const app = Testing.app();
+  const stack = new TerraformStack(app, "test");
+  const variable = new TerraformVariable(stack, "list", {});
+  const it = TerraformIterator.fromComplexList(
+    variable.listValue as any,
+    "name",
+  );
+
+  new TestResource(stack, "test", {
+    name: Token.asString(it.keys()),
+  });
+
+  const synth = JSON.parse(Testing.synth(stack));
+  expect(synth).toHaveProperty(
+    "resource.test_resource.test.name",
+    "${[ for key, val in var.list: key]}",
+  );
+});
+
+test("values() on fromComplexList iterates raw list", () => {
+  const app = Testing.app();
+  const stack = new TerraformStack(app, "test");
+  const variable = new TerraformVariable(stack, "list", {});
+  const it = TerraformIterator.fromComplexList(
+    variable.listValue as any,
+    "name",
+  );
+
+  new TestResource(stack, "test", {
+    name: Token.asString(it.values()),
+  });
+
+  const synth = JSON.parse(Testing.synth(stack));
+  expect(synth).toHaveProperty(
+    "resource.test_resource.test.name",
+    "${[ for key, val in var.list: val]}",
+  );
+});
+
+test("pluckProperty on fromComplexList iterates raw list", () => {
+  const app = Testing.app();
+  const stack = new TerraformStack(app, "test");
+  const variable = new TerraformVariable(stack, "list", {});
+  const it = TerraformIterator.fromComplexList(
+    variable.listValue as any,
+    "name",
+  );
+
+  new TestResource(stack, "test", {
+    name: Token.asString(it.pluckProperty("id")),
+  });
+
+  const synth = JSON.parse(Testing.synth(stack));
+  expect(synth).toHaveProperty(
+    "resource.test_resource.test.name",
+    "${[ for key, val in var.list: val.id]}",
+  );
+});
+
+test("fromComplexList renders map-wrapped for_each and raw-list for-expressions on the same iterator", () => {
+  const app = Testing.app();
+  const stack = new TerraformStack(app, "test");
+  const variable = new TerraformVariable(stack, "list", {});
+  const it = TerraformIterator.fromComplexList(
+    variable.listValue as any,
+    "name",
+  );
+
+  new TestResource(stack, "test", {
+    forEach: it,
+    name: it.getString("id"),
+    tags: { ids: Token.asString(it.pluckProperty("id")) },
+  });
+
+  const synth = JSON.parse(Testing.synth(stack));
+  expect(synth).toHaveProperty(
+    "resource.test_resource.test.for_each",
+    "${{ for key, val in var.list: val.name => val }}",
+  );
+  expect(synth).toHaveProperty(
+    "resource.test_resource.test.name",
+    "${each.value.id}",
+  );
+  expect(synth).toHaveProperty(
+    "resource.test_resource.test.tags.ids",
+    "${[ for key, val in var.list: val.id]}",
+  );
+});

packages/cdktn/test/terraform-operator.test.ts

@@ -78,3 +78,18 @@ test("Op.or renders correctly", () => {
     `"\${(true || true)}"`,
   );
 });
+
+// Regression tests for falsy right-hand operands
+test("Op.gt renders correctly with 0 as right operand", () => {
+  expect(resolveExpression(Op.gt(1, 0))).toMatchInlineSnapshot(`"\${(1 > 0)}"`);
+});
+test("Op.eq renders correctly with false as right operand", () => {
+  expect(resolveExpression(Op.eq(true, false))).toMatchInlineSnapshot(
+    `"\${(true == false)}"`,
+  );
+});
+test("Op.sub renders correctly with 0 as right operand", () => {
+  expect(resolveExpression(Op.sub(5, 0))).toMatchInlineSnapshot(
+    `"\${(5 - 0)}"`,
+  );
+});

packages/cdktn/test/tfExpression.test.ts

@@ -200,7 +200,7 @@ test("string index expression argument renders correctly", () => {
 
 test("null expression argument renders correctly", () => {
   expect(resolveExpression(Op.or(true, null))).toMatchInlineSnapshot(
-    `"\${(true || undefined)}"`,
+    `"\${(true || null)}"`,
   );
 });