Skip to content

Commit

Permalink
fix(rosetta): arrays aren't handled properly (#3098)
Browse files Browse the repository at this point in the history
This PR introduces many fixes to array handling:

- Array index (`array[5]`) was not supported.
- Array types weren't being rendered properly (they were being rendered
  as the named type `Array` instead of, say, `string[]`).
- Functions with variable length arguments were not being marked as
  varargs, simply saying the parameter type was an `Array`.



---

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
rix0rrr committed Oct 27, 2021
1 parent f160d3c commit de4648b
Show file tree
Hide file tree
Showing 14 changed files with 56 additions and 5 deletions.
16 changes: 16 additions & 0 deletions packages/jsii-rosetta/lib/jsii/jsii-types.ts
Expand Up @@ -31,6 +31,22 @@ export function determineJsiiType(typeChecker: ts.TypeChecker, type: ts.Type): J
};
}

if (type.symbol?.name === 'Array') {
const typeRef = type as ts.TypeReference;

if (typeRef.typeArguments?.length === 1) {
return {
kind: 'list',
elementType: determineJsiiType(typeChecker, typeRef.typeArguments[0]),
};
}

return {
kind: 'list',
elementType: { kind: 'builtIn', builtIn: 'any' },
};
}

// User-defined or aliased type
if (type.aliasSymbol) {
return { kind: 'namedType', name: type.aliasSymbol.name };
Expand Down
1 change: 1 addition & 0 deletions packages/jsii-rosetta/lib/languages/csharp.ts
Expand Up @@ -317,6 +317,7 @@ export class CSharpVisitor extends DefaultVisitor<CSharpLanguageContext> {

public parameterDeclaration(node: ts.ParameterDeclaration, renderer: CSharpRenderer): OTree {
return new OTree([
...(node.dotDotDotToken ? ['params '] : []), // Varargs. Render with 'params' keyword
this.renderTypeNode(node.type, node.questionToken !== undefined, renderer),
' ',
renderer.convert(node.name),
Expand Down
7 changes: 7 additions & 0 deletions packages/jsii-rosetta/lib/languages/default.ts
Expand Up @@ -287,6 +287,13 @@ export abstract class DefaultVisitor<C> implements AstHandler<C> {
return this.notImplemented(node, context);
}

public elementAccessExpression(node: ts.ElementAccessExpression, context: AstRenderer<C>): OTree {
const expression = context.convert(node.expression);
const index = context.convert(node.argumentExpression);

return new OTree([expression, '[', index, ']']);
}

public nonNullExpression(node: ts.NonNullExpression, context: AstRenderer<C>): OTree {
// We default we drop the non-null assertion
return context.convert(node.expression);
Expand Down
9 changes: 8 additions & 1 deletion packages/jsii-rosetta/lib/languages/java.ts
Expand Up @@ -262,7 +262,13 @@ export class JavaVisitor extends DefaultVisitor<JavaContext> {
}

public parameterDeclaration(node: ts.ParameterDeclaration, renderer: JavaRenderer): OTree {
return new OTree([this.renderTypeNode(node.type, renderer), ' ', renderer.convert(node.name)]);
let renderedType = this.renderTypeNode(node.type, renderer);
if (node.dotDotDotToken && renderedType.endsWith('[]')) {
// Varargs. In Java, render this as `Element...` (instead of `Element[]` which is what we'll have gotten).
renderedType = `${renderedType.substr(0, renderedType.length - 2)}...`;
}

return new OTree([renderedType, ' ', renderer.convert(node.name)]);
}

public block(node: ts.Block, renderer: JavaRenderer): OTree {
Expand Down Expand Up @@ -691,6 +697,7 @@ export class JavaVisitor extends DefaultVisitor<JavaContext> {
if (!type) {
return fallback;
}

return doRender(determineJsiiType(renderer.typeChecker, type), false);

// eslint-disable-next-line consistent-return
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/languages/python.ts
Expand Up @@ -301,7 +301,7 @@ export class PythonVisitor extends DefaultVisitor<PythonLanguageContext> {

const suffix = parameterAcceptsUndefined(node, type) ? '=None' : '';

return new OTree([context.convert(node.name), suffix]);
return new OTree([node.dotDotDotToken ? '*' : '', context.convert(node.name), suffix]);

function renderStructProperty(prop: StructProperty): string {
const sfx = structPropertyAcceptsUndefined(prop) ? '=None' : '';
Expand Down
4 changes: 4 additions & 0 deletions packages/jsii-rosetta/lib/languages/visualize.ts
Expand Up @@ -170,6 +170,10 @@ export class VisualizeAstVisitor implements AstHandler<void> {
return this.defaultNode('templateExpression', node, context);
}

public elementAccessExpression(node: ts.ElementAccessExpression, context: AstRenderer<void>): OTree {
return this.defaultNode('elementAccessExpression', node, context);
}

public nonNullExpression(node: ts.NonNullExpression, context: AstRenderer<void>): OTree {
return this.defaultNode('nonNullExpression', node, context);
}
Expand Down
4 changes: 4 additions & 0 deletions packages/jsii-rosetta/lib/renderer.ts
Expand Up @@ -362,6 +362,9 @@ export class AstRenderer<C> {
}
return visitor.spreadElement(tree, this);
}
if (ts.isElementAccessExpression(tree)) {
return visitor.elementAccessExpression(tree, this);
}
if (ts.isTemplateExpression(tree)) {
return visitor.templateExpression(tree, this);
}
Expand Down Expand Up @@ -492,6 +495,7 @@ export interface AstHandler<C> {
nonNullExpression(node: ts.NonNullExpression, context: AstRenderer<C>): OTree;
parenthesizedExpression(node: ts.ParenthesizedExpression, context: AstRenderer<C>): OTree;
maskingVoidExpression(node: ts.VoidExpression, context: AstRenderer<C>): OTree;
elementAccessExpression(node: ts.ElementAccessExpression, context: AstRenderer<C>): OTree;

// Not a node, called when we recognize a spread element/assignment that is only
// '...' and nothing else.
Expand Down
@@ -0,0 +1,3 @@
string[] array;

Console.WriteLine(array[3]);
@@ -0,0 +1,3 @@
String[] array;

System.out.println(array[3]);
@@ -0,0 +1,3 @@
# array is of type list of string

print(array[3])
@@ -0,0 +1,3 @@
declare const array: string[];

console.log(array[3]);
@@ -1,4 +1,4 @@
public void Test(Array _args)
public void Test(params object[] _args)
{
}

Expand Down
@@ -1,4 +1,4 @@
public void test(Array _args) {
public void test(Object... _args) {
}

test(Map.of("Key", "Value", "also", 1337));
Expand Down
@@ -1,4 +1,4 @@
def test(_args):
def test(*_args):
pass

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

0 comments on commit de4648b

Please sign in to comment.