Merge branch 'main' into rmuller/rosetta-opt-in-validation

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

@@ -612,7 +612,7 @@ export class CSharpVisitor extends DefaultVisitor<CSharpLanguageContext> {
           renderer
             .updateContext({
               propertyOrMethod: false,
-              identifierAsString: true,
+              identifierAsString: !ts.isComputedPropertyName(key),
             })
             .convert(key),
           ', ',

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

@@ -323,6 +323,13 @@ export abstract class DefaultVisitor<C> implements AstHandler<C> {
     return this.notImplemented(node, context);
   }
 
+  public computedPropertyName(
+    node: ts.Expression,
+    context: AstRenderer<C>,
+  ): OTree {
+    return context.convert(node);
+  }
+
   public methodDeclaration(
     node: ts.MethodDeclaration,
     context: AstRenderer<C>,

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

@@ -72,6 +72,15 @@ interface JavaContext {
    * @default false
    */
   readonly stringLiteralAsIdentifier?: boolean;
+
+  /**
+   * Used to denote that a type is being rendered in a position where a generic
+   * type parameter is expected, so only reference types are valid (not
+   * primitives).
+   *
+   * @default false
+   */
+  readonly requiresReferenceType?: boolean;
 }
 
 /**
@@ -342,16 +351,17 @@ export class JavaVisitor extends DefaultVisitor<JavaContext> {
       (node.initializer && renderer.typeOfExpression(node.initializer));
 
     const renderedType = type
-      ? this.renderType(node, type, renderer, 'var')
-      : 'var';
+      ? this.renderType(node, type, renderer, 'Object')
+      : 'Object';
 
     return new OTree(
       [
         renderedType,
         ' ',
         renderer.convert(node.name),
-        ' = ',
-        renderer.convert(node.initializer),
+        ...(node.initializer
+          ? [' = ', renderer.convert(node.initializer)]
+          : []),
         ';',
       ],
       [],
@@ -443,23 +453,41 @@ export class JavaVisitor extends DefaultVisitor<JavaContext> {
     node: ts.TemplateExpression,
     renderer: JavaRenderer,
   ): OTree {
-    const result = new Array<string>();
-    let first = true;
+    let template = '';
+    const parameters = new Array<OTree>();
 
     if (node.head.rawText) {
-      result.push(`"${quoteStringLiteral(node.head.rawText)}"`);
-      first = false;
+      template += node.head.rawText;
     }
 
     for (const span of node.templateSpans) {
-      result.push(`${first ? '' : ' + '}${renderer.textOf(span.expression)}`);
-      first = false;
+      template += '%s';
+      parameters.push(
+        renderer
+          .updateContext({
+            convertPropertyToGetter: true,
+            identifierAsString: false,
+          })
+          .convert(span.expression),
+      );
       if (span.literal.rawText) {
-        result.push(` + "${quoteStringLiteral(span.literal.rawText)}"`);
+        template += span.literal.rawText;
       }
     }
 
-    return new OTree(result);
+    if (parameters.length === 0) {
+      return new OTree([`"${quoteStringLiteral(template)}"`]);
+    }
+
+    return new OTree([
+      'String.format(',
+      `"${quoteStringLiteral(template)}"`,
+      ...parameters.reduce(
+        (list, element) => list.concat(', ', element),
+        new Array<string | OTree>(),
+      ),
+      ')',
+    ]);
   }
 
   public asExpression(node: ts.AsExpression, renderer: JavaRenderer): OTree {
@@ -694,7 +722,11 @@ export class JavaVisitor extends DefaultVisitor<JavaContext> {
     return new OTree(
       [],
       [
-        renderer.updateContext({ identifierAsString: true }).convert(name),
+        renderer
+          .updateContext({
+            identifierAsString: !ts.isComputedPropertyName(name),
+          })
+          .convert(name),
         ', ',
         renderer.updateContext({ inKeyValueList: false }).convert(initializer),
       ],
@@ -851,7 +883,7 @@ export class JavaVisitor extends DefaultVisitor<JavaContext> {
       return `Map<String, ${this.renderType(
         owningNode,
         mapValuesType,
-        renderer,
+        renderer.updateContext({ requiresReferenceType: true }),
         'Object',
       )}>`;
     }
@@ -870,10 +902,14 @@ export class JavaVisitor extends DefaultVisitor<JavaContext> {
     }
 
     switch (typeScriptBuiltInType) {
+      case 'boolean':
+        return renderer.currentContext.requiresReferenceType
+          ? 'Boolean'
+          : 'boolean';
+      case 'number':
+        return 'Number';
       case 'string':
         return 'String';
-      case 'number':
-        return 'int';
       case 'any':
         return 'Object';
       default:

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

@@ -498,13 +498,9 @@ export class PythonVisitor extends DefaultVisitor<PythonLanguageContext> {
     node: ts.PropertyAssignment,
     context: PythonVisitorContext,
   ): OTree {
-    let before = '"';
-    let mid = '": ';
-
-    if (context.currentContext.renderObjectLiteralAsKeywords) {
-      before = '';
-      mid = '=';
-    }
+    const mid = context.currentContext.renderObjectLiteralAsKeywords
+      ? '='
+      : ': ';
 
     // node.name is either an identifier or a string literal. The string literal
     // needs to be converted differently.
@@ -517,9 +513,16 @@ export class PythonVisitor extends DefaultVisitor<PythonLanguageContext> {
       },
     );
 
+    // 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)
+    ) {
+      name = new OTree(['"', name, '"']);
+    }
+
     return new OTree(
       [
-        before,
         name,
         mid,
         context

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

@@ -192,6 +192,13 @@ export class VisualizeAstVisitor implements AstHandler<void> {
     return this.defaultNode('propertyDeclaration', node, context);
   }
 
+  public computedPropertyName(
+    node: ts.Expression,
+    context: AstRenderer<void>,
+  ): OTree {
+    return this.defaultNode('computedPropertyName', node, context);
+  }
+
   public methodDeclaration(
     node: ts.MethodDeclaration,
     context: AstRenderer<void>,

packages/jsii-rosetta/lib/renderer.ts

@@ -340,6 +340,9 @@ export class AstRenderer<C> {
     if (ts.isPropertyDeclaration(tree)) {
       return visitor.propertyDeclaration(tree, this);
     }
+    if (ts.isComputedPropertyName(tree)) {
+      return visitor.computedPropertyName(tree.expression, this);
+    }
     if (ts.isMethodDeclaration(tree)) {
       return visitor.methodDeclaration(tree, this);
     }
@@ -541,6 +544,7 @@ export interface AstHandler<C> {
     node: ts.PropertyDeclaration,
     context: AstRenderer<C>,
   ): OTree;
+  computedPropertyName(node: ts.Expression, context: AstRenderer<C>): OTree;
   methodDeclaration(node: ts.MethodDeclaration, context: AstRenderer<C>): OTree;
   interfaceDeclaration(
     node: ts.InterfaceDeclaration,

packages/jsii-rosetta/lib/typescript/types.ts

@@ -20,9 +20,11 @@ export function typeWithoutUndefinedUnion(
   return remaining[0];
 }
 
-export function builtInTypeName(type: ts.Type): string | undefined {
-  const map: { [k: number]: string } = {
+type BuiltInType = 'any' | 'boolean' | 'number' | 'string';
+export function builtInTypeName(type: ts.Type): BuiltInType | undefined {
+  const map: { readonly [k: number]: BuiltInType } = {
     [ts.TypeFlags.Any]: 'any',
+    [ts.TypeFlags.Unknown]: 'any',
     [ts.TypeFlags.Boolean]: 'boolean',
     [ts.TypeFlags.Number]: 'number',
     [ts.TypeFlags.String]: 'string',
@@ -84,7 +86,7 @@ export function mapElementType(
         }
         return undefined;
       });
-      return typeIfSame(initializerTypes);
+      return typeIfSame([...initializerTypes, type.getStringIndexType()]);
     }
   }
 

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

@@ -48,71 +48,60 @@ const SUPPORTED_LANGUAGES = new Array<SupportedLanguage>(
   },
 );
 
-/**
- * Automatically setup tests from source files found in the directory
- */
-function makeTests() {
-  const translationsRoot = path.join(__dirname, 'translations');
-  const typeScriptTests = allFiles(translationsRoot)
-    .filter((f) => f.endsWith('.ts') && !f.endsWith('.d.ts'))
-    .filter((f) => !f.endsWith('.test.ts')); // Exclude self and other jest tests in this dir
-
-  typeScriptTests.forEach((typeScriptTest) => {
-    describe(`Translating ${path.relative(
-      translationsRoot,
-      typeScriptTest,
-    )}`, () => {
-      const typeScriptSource = fs.readFileSync(typeScriptTest, {
-        encoding: 'utf-8',
-      });
+const translationsRoot = path.join(__dirname, 'translations');
+const typeScriptTests = allFiles(translationsRoot)
+  .filter((f) => f.endsWith('.ts') && !f.endsWith('.d.ts'))
+  .filter((f) => !f.endsWith('.test.ts')); // Exclude self and other jest tests in this dir
+
+for (const typeScriptTest of typeScriptTests) {
+  describe(`Translating ${path.relative(
+    translationsRoot,
+    typeScriptTest,
+  )}`, () => {
+    const typeScriptSource = fs.readFileSync(typeScriptTest, {
+      encoding: 'utf-8',
+    });
 
-      let translator: SnippetTranslator;
-      let anyFailed = false;
-      beforeAll(() => {
-        translator = new SnippetTranslator({
-          visibleSource: typeScriptSource,
-          where: typeScriptTest,
-        });
-      });
-      afterAll(() => {
-        // Print the AST for tests that failed (to help debugging)
-        if (anyFailed && translator) {
-          const vis = translator.renderUsing(new VisualizeAstVisitor(true));
-          console.log(`${vis}\n`);
-        }
-        translator = undefined as any; // Need this to properly release memory
+    let translator: SnippetTranslator;
+    let anyFailed = false;
+    beforeAll(() => {
+      translator = new SnippetTranslator({
+        visibleSource: typeScriptSource,
+        where: typeScriptTest,
       });
+    });
 
-      SUPPORTED_LANGUAGES.forEach((supportedLanguage) => {
-        const languageFile = replaceExtension(
-          typeScriptTest,
-          supportedLanguage.extension,
-        );
-
-        // Use 'test.skip' if the file doesn't exist so that we can clearly see it's missing.
-        const testConstructor = fs.existsSync(languageFile) ? test : test.skip;
-
-        testConstructor(`to ${supportedLanguage.name}`, () => {
-          const expected = fs.readFileSync(languageFile, { encoding: 'utf-8' });
-          try {
-            const translation = translator.renderUsing(
-              supportedLanguage.visitor,
-            );
-            expect(stripEmptyLines(translation)).toEqual(
-              stripEmptyLines(stripCommonWhitespace(expected)),
-            );
-          } catch (e) {
-            anyFailed = true;
-            throw e;
-          }
-        });
-      });
+    afterAll(() => {
+      // Print the AST for tests that failed (to help debugging)
+      if (anyFailed && translator) {
+        const vis = translator.renderUsing(new VisualizeAstVisitor(true));
+        console.log(`${vis}\n`);
+      }
+      translator = undefined as any; // Need this to properly release memory
     });
+
+    for (const { name, extension, visitor } of SUPPORTED_LANGUAGES) {
+      const languageFile = replaceExtension(typeScriptTest, extension);
+
+      // Use 'test.skip' if the file doesn't exist so that we can clearly see it's missing.
+      const testConstructor = fs.existsSync(languageFile) ? test : test.skip;
+
+      testConstructor(`to ${name}`, () => {
+        const expected = fs.readFileSync(languageFile, { encoding: 'utf-8' });
+        try {
+          const translation = translator.renderUsing(visitor);
+          expect(stripEmptyLines(translation)).toEqual(
+            stripEmptyLines(stripCommonWhitespace(expected)),
+          );
+        } catch (e) {
+          anyFailed = true;
+          throw e;
+        }
+      });
+    }
   });
 }
 
-makeTests();
-
 function allFiles(root: string) {
   const ret: string[] = [];
   recurse(root);

packages/jsii-rosetta/test/translations/calls/shorthand_property.java

@@ -1,2 +1,2 @@
 String foo = "hello";
-callFunction(Map.of("foo", foo));
+callFunction(Map.of("foo", foo));