Skip to content

Commit

Permalink
fix(rosetta): incorrect transliteration of map keys in python (#3449)
Browse files Browse the repository at this point in the history
The Python transliterator failed to properly account for maps when
transliterating keys, resulting in name-mangling being applied on these
when it should not have been.

This PR adds a new `inMap` context entry to track whether a property
assignment must be interpreted as a map key-value entry, or an actual
keyword/value mapping; and in that case circumvents the mangling.

Added a new test to verify that such map literals render appropriately
using a key similar to that which caused #3448.

Fixes #3448



---

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
  • Loading branch information
RomainMuller authored Mar 29, 2022
1 parent 5b4b852 commit 54cebaa
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 14 deletions.
9 changes: 9 additions & 0 deletions .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -1339,6 +1339,15 @@
"contributions": [
"bug"
]
},
{
"login": "wendysophie",
"name": "wendysophie",
"avatar_url": "https://avatars.githubusercontent.com/u/54415551?v=4",
"profile": "https://github.com/wendysophie",
"contributions": [
"bug"
]
}
],
"repoType": "github",
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
<tr>
<td align="center"><a href="https://github.com/sullis"><img src="https://avatars3.githubusercontent.com/u/30938?v=4?s=100" width="100px;" alt=""/><br /><sub><b>sullis</b></sub></a><br /><a href="https://github.com/aws/jsii/commits?author=sullis" title="Code">💻</a></td>
<td align="center"><a href="https://github.com/vaneek"><img src="https://avatars1.githubusercontent.com/u/8113305?v=4?s=100" width="100px;" alt=""/><br /><sub><b>vaneek</b></sub></a><br /><a href="https://github.com/aws/jsii/issues?q=author%3Avaneek+label%3Abug" title="Bug reports">🐛</a></td>
<td align="center"><a href="https://github.com/wendysophie"><img src="https://avatars.githubusercontent.com/u/54415551?v=4?s=100" width="100px;" alt=""/><br /><sub><b>wendysophie</b></sub></a><br /><a href="https://github.com/aws/jsii/issues?q=author%3Awendysophie+label%3Abug" title="Bug reports">🐛</a></td>
</tr>
</table>

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 25 additions & 12 deletions packages/jsii-rosetta/lib/languages/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ interface PythonLanguageContext {
*/
readonly inClass?: boolean;

/**
* Whether the current property assignment is in the context of a map value.
* In this case, the keys should be strings (quoted where needed), and should
* not get mangled or case-converted.
*/
readonly inMap?: boolean;

/**
* If we're in a method, what is it's name
*
Expand Down Expand Up @@ -415,7 +422,7 @@ export class PythonVisitor extends DefaultVisitor<PythonLanguageContext> {
}

public keyValueObjectLiteralExpression(node: ts.ObjectLiteralExpression, context: PythonVisitorContext): OTree {
return this.renderObjectLiteralExpression('{', '}', false, node, context);
return this.renderObjectLiteralExpression('{', '}', false, node, context.updateContext({ inMap: true }));
}

public translateUnaryOperator(operator: ts.PrefixUnaryOperator) {
Expand Down Expand Up @@ -451,19 +458,26 @@ export class PythonVisitor extends DefaultVisitor<PythonLanguageContext> {
const mid = context.currentContext.renderObjectLiteralAsKeywords ? '=' : ': ';

// node.name is either an identifier or a string literal. The string literal
// needs to be converted differently.
let name = context.convert(node.name);
matchAst(node.name, nodeOfType('stringLiteral', ts.SyntaxKind.StringLiteral), (captured) => {
name = new OTree([mangleIdentifier(captured.stringLiteral.text)]);
});
// needs to be converted differently depending on whether it needs to be a
// string or a keyword argument.
let name = ts.isStringLiteral(node.name)
? new OTree([
context.currentContext.inMap // If in map, don't mangle the keys
? node.name.text
: mangleIdentifier(node.name.text),
])
: context.convert(node.name);

// If this isn't a computed property, we must quote the key (unless it's rendered as a keyword)
if (!context.currentContext.renderObjectLiteralAsKeywords && !ts.isComputedPropertyName(node.name)) {
if (
context.currentContext.inMap ||
(!context.currentContext.renderObjectLiteralAsKeywords && !ts.isComputedPropertyName(node.name))
) {
name = new OTree(['"', name, '"']);
}

return new OTree(
[name, mid, context.updateContext({ tailPositionArgument: false }).convert(node.initializer)],
[name, mid, context.updateContext({ inMap: false, tailPositionArgument: false }).convert(node.initializer)],
[],
{ canBreakLine: true },
);
Expand Down Expand Up @@ -608,19 +622,18 @@ export class PythonVisitor extends DefaultVisitor<PythonLanguageContext> {
node: ts.StringLiteral | ts.NoSubstitutionTemplateLiteral,
_context: PythonVisitorContext,
): OTree {
const rawText = node.text;
if (rawText.includes('\n')) {
if (node.getText(node.getSourceFile()).includes('\n')) {
return new OTree([
'"""',
rawText
node.text
// Escape all occurrences of back-slash once more
.replace(/\\/g, '\\\\')
// Escape only the first one in triple-quotes
.replace(/"""/g, '\\"""'),
'"""',
]);
}
return new OTree([JSON.stringify(rawText)]);
return new OTree([JSON.stringify(node.text)]);
}

public templateExpression(node: ts.TemplateExpression, context: PythonVisitorContext): OTree {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
IDictionary<string, object> map = new Dictionary<string, object> {
{ "Access-Control-Allow-Origin", "\"*\"" }
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
map := map[string]interface{}{
"Access-Control-Allow-Origin": jsii.String("\"*\""),
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Map<String, Object> map = Map.of(
"Access-Control-Allow-Origin", "\"*\"");
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
map = {
"Access-Control-Allow-Origin": "\"*\""
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const map: { [key: string]: unknown; } = {
'Access-Control-Allow-Origin': '"*"',
};

0 comments on commit 54cebaa

Please sign in to comment.