fix(rosetta): variadic arguments may use kwargs syntax in Python (#1832)

When processing function calls, the signature of the function was not
considered when it was available, and an object literal used as the
final argument of a variadic call would be rendered as keyword arguments
when this is actually not a correct way to render variadic values.

This change attempts to read the resolved signature of the function
beging called, in order to correctly render variadic values.

Fixes #1821



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

packages/jsii-rosetta/lib/languages/python.ts

@@ -71,6 +71,11 @@ interface PythonLanguageContext {
    * (Used to render super() call.);
    */
   readonly currentMethodName?: string;
+
+  /**
+   * If we're rendering a variadic argument value
+   */
+  readonly variadicArgument?: boolean;
 }
 
 type PythonVisitorContext = AstRenderer<PythonLanguageContext>;
@@ -296,11 +301,19 @@ export class PythonVisitor extends DefaultVisitor<PythonLanguageContext> {
       expressionText = `super().${context.currentContext.currentMethodName}`;
     }
 
+    const signature = context.typeChecker.getResolvedSignature(node);
+
     return new OTree(
       [
         expressionText,
         '(',
-        this.convertFunctionCallArguments(node.arguments, context),
+        this.convertFunctionCallArguments(
+          node.arguments,
+          context,
+          signature?.parameters?.map(
+            (p) => p.valueDeclaration as ts.ParameterDeclaration,
+          ),
+        ),
         ')',
       ],
       [],
@@ -396,11 +409,32 @@ export class PythonVisitor extends DefaultVisitor<PythonLanguageContext> {
     node: ts.ObjectLiteralExpression,
     context: PythonVisitorContext,
   ): OTree {
-    if (context.currentContext.tailPositionArgument) {
+    // Neutralize local modifiers if any for transforming further down.
+    const downContext = context.updateContext({
+      tailPositionArgument: false,
+      variadicArgument: false,
+    });
+
+    if (
+      context.currentContext.tailPositionArgument &&
+      !context.currentContext.variadicArgument
+    ) {
       // Guess that it's a struct we can probably inline the kwargs for
-      return this.renderObjectLiteralExpression('', '', true, node, context);
+      return this.renderObjectLiteralExpression(
+        '',
+        '',
+        true,
+        node,
+        downContext,
+      );
     }
-    return this.renderObjectLiteralExpression('{', '}', false, node, context);
+    return this.renderObjectLiteralExpression(
+      '{',
+      '}',
+      false,
+      node,
+      downContext,
+    );
   }
 
   public knownStructObjectLiteralExpression(
@@ -754,17 +788,22 @@ export class PythonVisitor extends DefaultVisitor<PythonLanguageContext> {
   private convertFunctionCallArguments(
     args: ts.NodeArray<ts.Expression> | undefined,
     context: PythonVisitorContext,
+    parameterDeclarations?: readonly ts.ParameterDeclaration[],
   ) {
     if (!args) {
       return NO_SYNTAX;
     }
 
-    const converted: Array<string | OTree> = context.convertLastDifferently(
-      args,
-      {
-        tailPositionArgument: true,
-      },
-    );
+    const converted = context.convertWithModifier(args, (ctx, _arg, index) => {
+      const decl =
+        parameterDeclarations?.[
+          Math.min(index, parameterDeclarations.length - 1)
+        ];
+      const variadicArgument = decl?.dotDotDotToken != null;
+      const tailPositionArgument = index >= args.length - 1;
+
+      return ctx.updateContext({ variadicArgument, tailPositionArgument });
+    });
 
     return new OTree([], converted, { separator: ', ', indent: 4 });
   }

packages/jsii-rosetta/lib/renderer.ts

@@ -79,6 +79,26 @@ export class AstRenderer<C> {
     return filterVisible(nodes).map(this.convert.bind(this));
   }
 
+  public convertWithModifier(
+    nodes: readonly ts.Node[],
+    makeContext: (
+      context: this,
+      node: ts.Node,
+      index: number,
+    ) => AstRenderer<C>,
+  ): OTree[] {
+    const vis = assignVisibility(nodes);
+    const result = new Array<OTree>();
+    for (const [idx, { node, visible, maskingVoid }] of vis.entries()) {
+      const renderedNode = visible ? node : maskingVoid;
+      if (renderedNode) {
+        const context = makeContext(this, renderedNode, idx);
+        result.push(context.convert(renderedNode));
+      }
+    }
+    return result;
+  }
+
   /**
    * Convert a set of nodes, but update the context for the last one.
    *

packages/jsii-rosetta/test/translations/statements/vararg_any_call.cs

@@ -0,0 +1,7 @@
+public void Test(Array _args)
+{
+}
+
+Test(new Struct { Key = "Value", Also = 1337 });
+
+Test(new Struct { Key = "Value" }, new Struct { Also = 1337 });

packages/jsii-rosetta/test/translations/statements/vararg_any_call.java

@@ -0,0 +1,6 @@
+public void test(Array _args) {
+}
+
+test(Map.of("Key", "Value", "also", 1337));
+
+test(Map.of("Key", "Value"), Map.of("also", 1337));

packages/jsii-rosetta/test/translations/statements/vararg_any_call.py

@@ -0,0 +1,6 @@
+def test(_args):
+    pass
+
+test({"Key": "Value", "also": 1337})
+
+test({"Key": "Value"}, {"also": 1337})

packages/jsii-rosetta/test/translations/statements/vararg_any_call.ts

@@ -0,0 +1,7 @@
+function test(..._args: any[]): void {
+  // ...
+}
+
+test({ Key: 'Value', also: 1337 });
+
+test({ Key: 'Value' }, { also: 1337 });