Detect and correct "incorrect" use of new FnJoin (#516)

A common developer error is to pass an array as the second argument of
`new FnJoin`, but that argument is variadic. This causes invalid
CloudFormation templates to be generated. This change corrects the
API to match the `Fn::Join` syntax where the second argument is an
array.

Fixes #512

examples/cdk-examples-typescript/advanced-usage/index.ts

@@ -157,7 +157,7 @@ class CloudFormationExample extends cdk.Stack {
         // outputs are constructs the synthesize into the template's "Outputs" section
         new cdk.Output(this, 'Output', {
             description: 'This is an output of the template',
-            value: new cdk.FnJoin(',', new cdk.AwsAccountId(), '/', param.ref)
+            value: new cdk.FnConcat(new cdk.AwsAccountId(), '/', param.ref)
         });
 
         // stack.templateOptions can be used to specify template-level options

packages/@aws-cdk/aws-events/test/test.rule.ts

@@ -232,7 +232,7 @@ export = {
         // tokens can also used for JSON templates, but that means escaping is
         // the responsibility of the user.
         rule.addTarget(t4, {
-            jsonTemplate: new cdk.FnJoin(' ', '"', 'hello', '\"world\"', '"'),
+            jsonTemplate: new cdk.FnJoin(' ', ['"', 'hello', '\"world\"', '"']),
         });
 
         expect(stack).toMatch({

packages/@aws-cdk/aws-rds/lib/cluster-ref.ts

@@ -211,6 +211,6 @@ export class Endpoint {
     constructor(address: DBClusterEndpointAddress, port: Port) {
         this.hostname = address;
         this.port = port;
-        this.socketAddress = new cdk.FnJoin(":", address, port);
+        this.socketAddress = new cdk.FnJoin(":", [address, port]);
     }
 }

packages/@aws-cdk/cdk/lib/cloudformation/fn.ts

@@ -12,12 +12,12 @@ export class Fn extends Token {
 }
 
 /**
- * The intrinsic function Fn::FindInMap returns the value corresponding to keys in a two-level
+ * The intrinsic function ``Fn::FindInMap`` returns the value corresponding to keys in a two-level
  * map that is declared in the Mappings section.
  */
 export class FnFindInMap extends Fn {
     /**
-     * Creates an Fn::FindInMap function.
+     * Creates an ``Fn::FindInMap`` function.
      * @param mapName The logical name of a mapping declared in the Mappings section that contains the keys and values.
      * @param topLevelKey The top-level key name. Its value is a list of key-value pairs.
      * @param secondLevelKey The second-level key name, which is set to one of the keys from the list assigned to TopLevelKey.
@@ -28,11 +28,11 @@ export class FnFindInMap extends Fn {
 }
 
 /**
- * The Fn::GetAtt intrinsic function returns the value of an attribute from a resource in the template.
+ * The ``Fn::GetAtt`` intrinsic function returns the value of an attribute from a resource in the template.
  */
 export class FnGetAtt extends Fn {
     /**
-     * Creates a Fn::GetAtt function.
+     * Creates a ``Fn::GetAtt`` function.
      * @param logicalNameOfResource The logical name (also called logical ID) of the resource that contains the attribute that you want.
      * @param attributeName The name of the resource-specific attribute whose value you want. See the resource's reference page for details about the attributes available for that resource type.
      */
@@ -42,15 +42,15 @@ export class FnGetAtt extends Fn {
 }
 
 /**
- * The intrinsic function Fn::GetAZs returns an array that lists Availability Zones for a
+ * The intrinsic function ``Fn::GetAZs`` returns an array that lists Availability Zones for a
  * specified region. Because customers have access to different Availability Zones, the intrinsic
- * function Fn::GetAZs enables template authors to write templates that adapt to the calling
+ * function ``Fn::GetAZs`` enables template authors to write templates that adapt to the calling
  * user's access. That way you don't have to hard-code a full list of Availability Zones for a
  * specified region.
  */
 export class FnGetAZs extends Fn {
     /**
-     * Creates an Fn::GetAZs function.
+     * Creates an ``Fn::GetAZs`` function.
      * @param region The name of the region for which you want to get the Availability Zones.
      *               You can use the AWS::Region pseudo parameter to specify the region in
      *               which the stack is created. Specifying an empty string is equivalent to
@@ -62,13 +62,13 @@ export class FnGetAZs extends Fn {
 }
 
 /**
- * The intrinsic function Fn::ImportValue returns the value of an output exported by another stack.
+ * The intrinsic function ``Fn::ImportValue`` returns the value of an output exported by another stack.
  * You typically use this function to create cross-stack references. In the following example
  * template snippets, Stack A exports VPC security group values and Stack B imports them.
  */
 export class FnImportValue extends Fn {
     /**
-     * Creates an Fn::ImportValue function.
+     * Creates an ``Fn::ImportValue`` function.
      * @param sharedValueToImport The stack output value that you want to import.
      */
     constructor(sharedValueToImport: string) {
@@ -77,40 +77,44 @@ export class FnImportValue extends Fn {
 }
 
 /**
- * The intrinsic function Fn::Join appends a set of values into a single value, separated by
+ * The intrinsic function ``Fn::Join`` appends a set of values into a single value, separated by
  * the specified delimiter. If a delimiter is the empty string, the set of values are concatenated
  * with no delimiter.
  */
 export class FnJoin extends Fn {
     /**
-     * Creates an Fn::Join function.
-     * @param delimiter The value you want to occur between fragments. The delimiter will occur between fragments only. It will not terminate the final value.
+     * Creates an ``Fn::Join`` function.
+     * @param delimiter The value you want to occur between fragments. The delimiter will occur between fragments only.
+     *                  It will not terminate the final value.
      * @param listOfValues The list of values you want combined.
      */
-    constructor(delimiter: string, ...listOfValues: any[]) {
+    constructor(delimiter: string, listOfValues: any[]) {
+        if (listOfValues.length === 0) {
+            throw new Error(`FnJoin requires at least one value to be provided`);
+        }
         super('Fn::Join', [ delimiter, listOfValues ]);
     }
 }
 
 /**
- * Alias for Fn::Join('', [ values ]).
+ * Alias for ``FnJoin('', listOfValues)``.
  */
 export class FnConcat extends FnJoin {
     /**
-     * Creates an Fn::Join function with an empty delimiter.
+     * Creates an ``Fn::Join`` function with an empty delimiter.
      * @param listOfValues The list of values to concatenate.
      */
     constructor(...listOfValues: any[]) {
-        super('', ...listOfValues);
+        super('', listOfValues);
     }
 }
 
 /**
- * The intrinsic function Fn::Select returns a single object from a list of objects by index.
+ * The intrinsic function ``Fn::Select`` returns a single object from a list of objects by index.
  */
 export class FnSelect extends Fn {
     /**
-     * Creates an Fn::Select function.
+     * Creates an ``Fn::Select`` function.
      * @param index The index of the object to retrieve. This must be a value from zero to N-1, where N represents the number of elements in the array.
      * @param array The list of objects to select from. This list must not be null, nor can it have null entries.
      */
@@ -121,13 +125,13 @@ export class FnSelect extends Fn {
 
 /**
  * To split a string into a list of string values so that you can select an element from the
- * resulting string list, use the Fn::Split intrinsic function. Specify the location of splits
- * with a delimiter, such as , (a comma). After you split a string, use the Fn::Select function
+ * resulting string list, use the ``Fn::Split`` intrinsic function. Specify the location of splits
+ * with a delimiter, such as , (a comma). After you split a string, use the ``Fn::Select`` function
  * to pick a specific element.
  */
 export class FnSplit extends Fn {
     /**
-     * Create an Fn::Split function.
+     * Create an ``Fn::Split`` function.
      * @param delimiter A string value that determines where the source string is divided.
      * @param source The string value that you want to split.
      */
@@ -137,13 +141,13 @@ export class FnSplit extends Fn {
 }
 
 /**
- * The intrinsic function Fn::Sub substitutes variables in an input string with values that
+ * The intrinsic function ``Fn::Sub`` substitutes variables in an input string with values that
  * you specify. In your templates, you can use this function to construct commands or outputs
  * that include values that aren't available until you create or update a stack.
  */
 export class FnSub extends Fn {
     /**
-     * Creates an Fn::Sub function.
+     * Creates an ``Fn::Sub`` function.
      * @param body A string with variables that AWS CloudFormation substitutes with their
      *             associated values at runtime. Write variables as ${MyVarName}. Variables
      *             can be template parameter names, resource logical IDs, resource attributes,
@@ -158,14 +162,14 @@ export class FnSub extends Fn {
 }
 
 /**
- * The intrinsic function Fn::Base64 returns the Base64 representation of the input string.
+ * The intrinsic function ``Fn::Base64`` returns the Base64 representation of the input string.
  * This function is typically used to pass encoded data to Amazon EC2 instances by way of
  * the UserData property.
  */
 export class FnBase64 extends Fn {
 
     /**
-     * Creates an Fn::Base64 function.
+     * Creates an ``Fn::Base64`` function.
      * @param data The string value you want to convert to Base64.
      */
     constructor(data: any) {
@@ -174,11 +178,11 @@ export class FnBase64 extends Fn {
 }
 
 /**
- * The intrinsic function Fn::Cidr returns the specified Cidr address block.
+ * The intrinsic function ``Fn::Cidr`` returns the specified Cidr address block.
  */
 export class FnCidr extends Fn {
     /**
-     * Creates an Fn::Cidr function.
+     * Creates an ``Fn::Cidr`` function.
      * @param ipBlock  The user-specified default Cidr address block.
      * @param count    The number of subnets' Cidr block wanted. Count can be 1 to 256.
      * @param sizeMask The digit covered in the subnet.
@@ -192,14 +196,14 @@ export class FnCidr extends Fn {
 }
 
 /**
- * You can use intrinsic functions, such as Fn::If, Fn::Equals, and Fn::Not, to conditionally
+ * You can use intrinsic functions, such as ``Fn::If``, ``Fn::Equals``, and ``Fn::Not``, to conditionally
  * create stack resources. These conditions are evaluated based on input parameters that you
  * declare when you create or update a stack. After you define all your conditions, you can
  * associate them with resources or resource properties in the Resources and Outputs sections
  * of a template.
  *
- * You define all conditions in the Conditions section of a template except for Fn::If conditions.
- * You can use the Fn::If condition in the metadata attribute, update policy attribute, and property
+ * You define all conditions in the Conditions section of a template except for ``Fn::If`` conditions.
+ * You can use the ``Fn::If`` condition in the metadata attribute, update policy attribute, and property
  * values in the Resources section and Outputs sections of a template.
  *
  * You might use conditions when you want to reuse a template that can create resources in different
@@ -216,7 +220,7 @@ export class FnCondition extends Fn {
 
 /**
  * Returns true if all the specified conditions evaluate to true, or returns false if any one
- *  of the conditions evaluates to false. Fn::And acts as an AND operator. The minimum number of
+ *  of the conditions evaluates to false. ``Fn::And`` acts as an AND operator. The minimum number of
  * conditions that you can include is 2, and the maximum is 10.
  */
 export class FnAnd extends FnCondition {
@@ -231,7 +235,7 @@ export class FnAnd extends FnCondition {
  */
 export class FnEquals extends FnCondition {
     /**
-     * Creates an Fn::Equals condition function.
+     * Creates an ``Fn::Equals`` condition function.
      * @param lhs A value of any type that you want to compare.
      * @param rhs A value of any type that you want to compare.
      */
@@ -242,14 +246,14 @@ export class FnEquals extends FnCondition {
 
 /**
  * Returns one value if the specified condition evaluates to true and another value if the
- * specified condition evaluates to false. Currently, AWS CloudFormation supports the Fn::If
+ * specified condition evaluates to false. Currently, AWS CloudFormation supports the ``Fn::If``
  * intrinsic function in the metadata attribute, update policy attribute, and property values
  * in the Resources section and Outputs sections of a template. You can use the AWS::NoValue
  * pseudo parameter as a return value to remove the corresponding property.
  */
 export class FnIf extends FnCondition {
     /**
-     * Creates an Fn::If condition function.
+     * Creates an ``Fn::If`` condition function.
      * @param condition A reference to a condition in the Conditions section. Use the condition's name to reference it.
      * @param valueIfTrue A value to be returned if the specified condition evaluates to true.
      * @param valueIfFalse A value to be returned if the specified condition evaluates to false.
@@ -261,12 +265,12 @@ export class FnIf extends FnCondition {
 
 /**
  * Returns true for a condition that evaluates to false or returns false for a condition that evaluates to true.
- * Fn::Not acts as a NOT operator.
+ * ``Fn::Not`` acts as a NOT operator.
  */
 export class FnNot extends FnCondition {
     /**
-     * Creates an Fn::Not condition function.
-     * @param condition A condition such as Fn::Equals that evaluates to true or false.
+     * Creates an ``Fn::Not`` condition function.
+     * @param condition A condition such as ``Fn::Equals`` that evaluates to true or false.
      */
     constructor(condition: FnCondition) {
         super('Fn::Not', [ condition ]);
@@ -275,12 +279,12 @@ export class FnNot extends FnCondition {
 
 /**
  * Returns true if any one of the specified conditions evaluate to true, or returns false if
- * all of the conditions evaluates to false. Fn::Or acts as an OR operator. The minimum number
+ * all of the conditions evaluates to false. ``Fn::Or`` acts as an OR operator. The minimum number
  * of conditions that you can include is 2, and the maximum is 10.
  */
 export class FnOr extends FnCondition {
     /**
-     * Creates an Fn::Or condition function.
+     * Creates an ``Fn::Or`` condition function.
      * @param condition A condition that evaluates to true or false.
      */
     constructor(...condition: FnCondition[]) {
@@ -293,7 +297,7 @@ export class FnOr extends FnCondition {
  */
 export class FnContains extends FnCondition {
     /**
-     * Creates an Fn::Contains function.
+     * Creates an ``Fn::Contains`` function.
      * @param listOfStrings A list of strings, such as "A", "B", "C".
      * @param value A string, such as "A", that you want to compare against a list of strings.
      */
@@ -307,7 +311,7 @@ export class FnContains extends FnCondition {
  */
 export class FnEachMemberEquals extends FnCondition {
     /**
-     * Creates an Fn::EachMemberEquals function.
+     * Creates an ``Fn::EachMemberEquals`` function.
      * @param listOfStrings A list of strings, such as "A", "B", "C".
      * @param value A string, such as "A", that you want to compare against a list of strings.
      */
@@ -322,7 +326,7 @@ export class FnEachMemberEquals extends FnCondition {
  */
 export class FnEachMemberIn extends FnCondition {
     /**
-     * Creates an Fn::EachMemberIn function.
+     * Creates an ``Fn::EachMemberIn`` function.
      * @param stringsToCheck A list of strings, such as "A", "B", "C". AWS CloudFormation checks whether each member in the strings_to_check parameter is in the strings_to_match parameter.
      * @param stringsToMatch A list of strings, such as "A", "B", "C". Each member in the strings_to_match parameter is compared against the members of the strings_to_check parameter.
      */
@@ -336,7 +340,7 @@ export class FnEachMemberIn extends FnCondition {
  */
 export class FnRefAll extends FnCondition {
     /**
-     * Creates an Fn::RefAll function.
+     * Creates an ``Fn::RefAll`` function.
      * @param parameterType An AWS-specific parameter type, such as AWS::EC2::SecurityGroup::Id or
      *                      AWS::EC2::VPC::Id. For more information, see Parameters in the AWS
      *                      CloudFormation User Guide.
@@ -351,7 +355,7 @@ export class FnRefAll extends FnCondition {
  */
 export class FnValueOf extends FnCondition {
     /**
-     * Creates an Fn::ValueOf function.
+     * Creates an ``Fn::ValueOf`` function.
      * @param parameterOrLogicalId The name of a parameter for which you want to retrieve attribute values. The parameter must be declared in the Parameters section of the template.
      * @param attribute The name of an attribute from which you want to retrieve a value.
      */
@@ -365,7 +369,7 @@ export class FnValueOf extends FnCondition {
  */
 export class FnValueOfAll extends FnCondition {
     /**
-     * Creates an Fn::ValueOfAll function.
+     * Creates an ``Fn::ValueOfAll`` function.
      * @param parameterType An AWS-specific parameter type, such as AWS::EC2::SecurityGroup::Id or AWS::EC2::VPC::Id. For more information, see Parameters in the AWS CloudFormation User Guide.
      * @param attribute The name of an attribute from which you want to retrieve a value. For more information about attributes, see Supported Attributes.
      */

packages/@aws-cdk/cdk/lib/cloudformation/output.ts

@@ -207,7 +207,7 @@ export class StringListOutput extends Construct {
             condition: props.condition,
             disableExport: props.disableExport,
             export: props.export,
-            value: new FnJoin(this.separator, ...props.values)
+            value: new FnJoin(this.separator, props.values)
         });
     }
 
@@ -224,4 +224,4 @@ export class StringListOutput extends Construct {
 
         return ret;
     }
-}
+}

packages/@aws-cdk/cdk/test/cloudformation/test.fn.ts

@@ -0,0 +1,11 @@
+import nodeunit = require('nodeunit');
+import fn = require('../../lib/cloudformation/fn');
+
+export = nodeunit.testCase({
+    'Fn::Join': {
+        'rejects empty list of arguments to join'(test: nodeunit.Test) {
+            test.throws(() => new fn.FnJoin('.', []));
+            test.done();
+        }
+    }
+});

packages/aws-cdk-docs/src/cloudformation.rst

@@ -172,7 +172,7 @@ Intrinsic Functions
 .. code-block:: js
 
     import cdk = require('@aws-cdk/cdk');
-    new cdk.FnJoin(",", ...)
+    new cdk.FnJoin(",", [...])
 
 .. _pseudo_parameters: