fix(rosetta): structs starting with I are incorrectly interpreted a…

…s non-structs (#3040)

Not all interfaces that start with the name `I` are structs: only if
the second letter is also uppercase.

Also clean up type handling a bit, we were doing some things that the
TypeScript compiler has built-ins for.

---

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/commands/extract.ts

@@ -123,7 +123,7 @@ export function singleThreadedTranslateAll(
       failures.push({
         category: ts.DiagnosticCategory.Error,
         code: 999,
-        messageText: `rosetta: error translating snippet: ${e}\n${block.completeSource}`,
+        messageText: `rosetta: error translating snippet: ${e}\n${e.stack}\n${block.completeSource}`,
         file: undefined,
         start: undefined,
         length: undefined,

packages/jsii-rosetta/lib/jsii/jsii-utils.ts

@@ -4,7 +4,8 @@ import { AstRenderer } from '../renderer';
 import { typeContainsUndefined } from '../typescript/types';
 
 export function isStructInterface(name: string) {
-  return !name.startsWith('I');
+  // Start with an I and another uppercase character
+  return !/^I[A-Z]/.test(name);
 }
 
 export function isStructType(type: ts.Type) {

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

@@ -14,12 +14,12 @@ import {
 } from '../typescript/ast-utils';
 import { ImportStatement } from '../typescript/imports';
 import {
-  typeWithoutUndefinedUnion,
   builtInTypeName,
   typeContainsUndefined,
   parameterAcceptsUndefined,
   mapElementType,
   inferMapElementType,
+  firstTypeInUnion,
 } from '../typescript/types';
 import { flat, partition, setExtend } from '../util';
 import { DefaultVisitor } from './default';
@@ -623,9 +623,9 @@ export class CSharpVisitor extends DefaultVisitor<CSharpLanguageContext> {
       return fallback;
     }
 
-    const nonUnionType = typeWithoutUndefinedUnion(type);
+    const nonUnionType = firstTypeInUnion(renderer.typeChecker, type);
     if (!nonUnionType) {
-      renderer.report(typeNode, 'Type unions in examples are not supported');
+      renderer.report(typeNode, 'Type unions in examples are not supported for csharp');
       return '...';
     }
 

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

@@ -5,7 +5,7 @@ import { OTree, NO_SYNTAX } from '../o-tree';
 import { AstRenderer, AstHandler, nimpl, CommentSyntax } from '../renderer';
 import { voidExpressionString } from '../typescript/ast-utils';
 import { ImportStatement } from '../typescript/imports';
-import { mapElementType, typeWithoutUndefinedUnion } from '../typescript/types';
+import { mapElementType } from '../typescript/types';
 
 import { TargetLanguage } from '.';
 
@@ -130,7 +130,7 @@ export abstract class DefaultVisitor<C> implements AstHandler<C> {
    *     - It's not a struct (render as key-value map)
    */
   public objectLiteralExpression(node: ts.ObjectLiteralExpression, context: AstRenderer<C>): OTree {
-    const type = typeWithoutUndefinedUnion(context.inferredTypeOfExpression(node));
+    const type = context.inferredTypeOfExpression(node);
 
     let isUnknownType = !type;
     if (type && hasFlag(type.flags, ts.TypeFlags.Any)) {

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

@@ -7,7 +7,7 @@ import { OTree, NO_SYNTAX } from '../o-tree';
 import { AstRenderer } from '../renderer';
 import { isReadOnly, matchAst, nodeOfType, quoteStringLiteral, visibility } from '../typescript/ast-utils';
 import { ImportStatement } from '../typescript/imports';
-import { builtInTypeName, mapElementType, typeWithoutUndefinedUnion } from '../typescript/types';
+import { builtInTypeName, firstTypeInUnion, mapElementType } from '../typescript/types';
 import { DefaultVisitor } from './default';
 
 interface JavaContext {
@@ -407,7 +407,7 @@ export class JavaVisitor extends DefaultVisitor<JavaContext> {
     const argsLength = node.arguments ? node.arguments.length : 0;
     const lastArg = argsLength > 0 ? node.arguments![argsLength - 1] : undefined;
     const lastArgIsObjectLiteral = lastArg && ts.isObjectLiteralExpression(lastArg);
-    const lastArgType = lastArg && typeWithoutUndefinedUnion(renderer.inferredTypeOfExpression(lastArg));
+    const lastArgType = lastArg && renderer.inferredTypeOfExpression(lastArg);
     // we only render the ClassName.Builder.create(...) expression
     // if the last argument is an object literal, and NOT a known struct
     // (in that case, it has its own creation method)
@@ -654,9 +654,9 @@ export class JavaVisitor extends DefaultVisitor<JavaContext> {
       return fallback;
     }
 
-    const nonUnionType = typeWithoutUndefinedUnion(type);
+    const nonUnionType = firstTypeInUnion(renderer.typeChecker, type);
     if (!nonUnionType) {
-      renderer.report(owningNode, 'Type unions in examples are not supported');
+      renderer.report(owningNode, 'Type unions in examples are not supported for java');
       return fallback;
     }
 

packages/jsii-rosetta/lib/renderer.ts

@@ -133,10 +133,14 @@ export class AstRenderer<C> {
   /**
    * Infer type of expression by the argument it is assigned to
    *
+   * If the type of the expression can include undefined (if the value is
+   * optional), `undefined` will be removed from the union.
+   *
    * (Will return undefined for object literals not unified with a declared type)
    */
   public inferredTypeOfExpression(node: ts.Expression) {
-    return this.typeChecker.getContextualType(node);
+    const type = this.typeChecker.getContextualType(node);
+    return type ? this.typeChecker.getNonNullableType(type) : undefined;
   }
 
   /**
@@ -152,6 +156,10 @@ export class AstRenderer<C> {
     return this.typeChecker.getTypeFromTypeNode(node);
   }
 
+  public typeToString(type: ts.Type) {
+    return this.typeChecker.typeToString(type);
+  }
+
   public report(node: ts.Node, messageText: string, category: ts.DiagnosticCategory = ts.DiagnosticCategory.Error) {
     this.diagnostics.push({
       category,

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

@@ -3,17 +3,16 @@ import * as ts from 'typescript';
 import { AstRenderer } from '../renderer';
 
 /**
- * Return the OTHER type from undefined from a union, returns undefined if there is more than one
+ * Return the first non-undefined type from a union
  */
-export function typeWithoutUndefinedUnion(type: ts.Type | undefined): ts.Type | undefined {
-  if (!type || !type.isUnion()) {
+export function firstTypeInUnion(typeChecker: ts.TypeChecker, type: ts.Type): ts.Type {
+  type = typeChecker.getNonNullableType(type);
+
+  if (!type.isUnion()) {
     return type;
   }
-  const remaining = type.types.filter((t) => t.flags !== ts.TypeFlags.Undefined);
-  if (remaining.length > 1) {
-    return undefined;
-  }
-  return remaining[0];
+
+  return type.types[0];
 }
 
 type BuiltInType = 'any' | 'boolean' | 'number' | 'string';
@@ -31,6 +30,27 @@ export function builtInTypeName(type: ts.Type): BuiltInType | undefined {
   return map[type.flags];
 }
 
+export function renderType(type: ts.Type): string {
+  if (type.isClassOrInterface()) {
+    return type.symbol.name;
+  }
+  if (type.isLiteral()) {
+    // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
+    return `${type.value}`;
+  }
+  return renderTypeFlags(type);
+}
+
+export function renderTypeFlags(type: ts.Type) {
+  const ret = [];
+  for (const flag of Object.values(ts.TypeFlags)) {
+    if (typeof flag === 'number' && type.flags & flag) {
+      ret.push(ts.TypeFlags[flag]);
+    }
+  }
+  return ret.join(',');
+}
+
 export function parameterAcceptsUndefined(param: ts.ParameterDeclaration, type?: ts.Type): boolean {
   if (param.initializer !== undefined) {
     return true;
@@ -111,18 +131,12 @@ function isSameType(a: ts.Type, b: ts.Type) {
 }
 
 function typeIfSame(types: Array<ts.Type | undefined>): ts.Type | undefined {
-  let ret: ts.Type | undefined;
-  for (const type of types) {
-    if (ret === undefined) {
-      ret = type;
-    } else {
-      // Not all the same
-      if (type !== undefined && ret.flags !== type.flags) {
-        return undefined;
-      }
-    }
+  const ttypes = types.filter(isDefined);
+  if (types.length === 0) {
+    return undefined;
   }
-  return ret;
+
+  return ttypes.every((t) => isSameType(ttypes[0], t)) ? ttypes[0] : undefined;
 }
 
 /**

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

@@ -15,9 +15,9 @@ import { AstHandler } from '../lib/renderer';
 //
 // To run only the tests for a certain language you're working on, do this:
 //
-//    yarn test test/translations.test.js -t 'Translating .* to Python'
-//    yarn test test/translations.test.js -t 'Translating .* to Java'
-//    yarn test test/translations.test.js -t 'Translating .* to C#'
+//    yarn test test/translations.test -t 'Translating .* to Python'
+//    yarn test test/translations.test -t 'Translating .* to Java'
+//    yarn test test/translations.test -t 'Translating .* to C#'
 //
 // To narrow it down even more you can of course replace the '.*' regex with
 // whatever file indication you desire.

packages/jsii-rosetta/test/translations/structs/struct_starting_with_i.cs

@@ -0,0 +1,3 @@
+new Integration(this, "Something", new IntegrationOptions {
+    Argument = 5
+});

packages/jsii-rosetta/test/translations/structs/struct_starting_with_i.java

@@ -0,0 +1,2 @@
+new Integration(this, "Something", new IntegrationOptions()
+        .argument(5));

packages/jsii-rosetta/test/translations/structs/struct_starting_with_i.py

@@ -0,0 +1,3 @@
+Integration(self, "Something",
+    argument=5
+)

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

@@ -0,0 +1,9 @@
+/// !hide
+class Integration {
+  constructor(_something: any, id: string, props?: IntegrationOptions) { }
+}
+interface IntegrationOptions { readonly argument: number }
+/// !show
+new Integration(this, 'Something', {
+  argument: 5
+});