Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jul 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 49 additions & 10 deletions packages/jsii-rosetta/lib/languages/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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,
),
),
')',
],
[],
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 });
}
Expand Down
20 changes: 20 additions & 0 deletions packages/jsii-rosetta/lib/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -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 });
Original file line number Diff line number Diff line change
@@ -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));
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
def test(_args):
pass

test({"Key": "Value", "also": 1337})

test({"Key": "Value"}, {"also": 1337})
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
function test(..._args: any[]): void {
// ...
}

test({ Key: 'Value', also: 1337 });

test({ Key: 'Value' }, { also: 1337 });