fix(rosetta): Python translation for implements is wrong (#3280)

For Python, interface implementation needs to be written as
`@jsii.implements(Iface)`.

Also add a check that people don't put functions in object literals.

Fixes aws/aws-cdk#17928



---

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/jsii/jsii-utils.ts

@@ -1,3 +1,4 @@
+import * as spec from '@jsii/spec';
 import { symbolIdentifier } from 'jsii';
 import * as ts from 'typescript';
 
@@ -29,6 +30,60 @@ export function analyzeStructType(typeChecker: ts.TypeChecker, type: ts.Type): O
   return { kind: 'local-struct', type };
 }
 
+/**
+ * Whether the given type is a protocol AND comes from jsii
+ *
+ * - Protocol: a TypeScript interface that is *not* a "struct" type.
+ *   A.k.a. "behavioral interface".
+ * - From jsii: whether the interface type is defined in and exported
+ *   via a jsii assembly. There can be literal interfaces defined
+ *   in an example, and they will not be mangled in the same way
+ *   as a jsii interface would be.
+ *
+ *
+ * Examples:
+ *
+ * ```ts
+ * // isJsiiProtocolType() -> false: not a protocol
+ * interface Banana {
+ *   readonly arc: number;
+ * }
+ *
+ * // isJsiiProtocolType() -> might be true: depends on whether it was defined
+ * // in a jsii assembly.
+ * interface IHello {
+ *   sayIt(): void;
+ * }
+ *
+ * // isJsiiProtocolType() -> false: declared to not be a protocol, even though
+ * // it has the naming scheme of one
+ * /**
+ *  * @struct
+ *  * /
+ * interface IPAddress {
+ *   readonly octets: number[];
+ * }
+ * ```
+ */
+export function isJsiiProtocolType(typeChecker: ts.TypeChecker, type: ts.Type): boolean | undefined {
+  if (!type.isClassOrInterface() || !hasAllFlags(type.objectFlags, ts.ObjectFlags.Interface)) {
+    return false;
+  }
+
+  const sym = lookupJsiiSymbol(typeChecker, type.symbol);
+  if (!sym) {
+    return false;
+  }
+
+  if (!sym.sourceAssembly) {
+    // No source assembly, so this is a 'fake-from-jsii' type
+    return !isNamedLikeStruct(type.symbol.name);
+  }
+
+  const jsiiType = resolveJsiiSymbolType(sym);
+  return spec.isInterfaceType(jsiiType) && !jsiiType.datatype;
+}
+
 export function hasAllFlags<A extends number>(flags: A, test: A) {
   // tslint:disable-next-line:no-bitwise
   return test !== 0 && (flags & test) === test;
@@ -100,6 +155,26 @@ export function lookupJsiiSymbolFromNode(typeChecker: ts.TypeChecker, node: ts.N
   return fmap(typeChecker.getSymbolAtLocation(node), (s) => lookupJsiiSymbol(typeChecker, s));
 }
 
+export function resolveJsiiSymbolType(jsiiSymbol: JsiiSymbol): spec.Type {
+  if (jsiiSymbol.symbolType !== 'type') {
+    throw new Error(
+      `Expected symbol to refer to a 'type', got '${jsiiSymbol.fqn}' which is a '${jsiiSymbol.symbolType}'`,
+    );
+  }
+
+  if (!jsiiSymbol.sourceAssembly) {
+    throw new Error('`resolveJsiiSymbolType: requires an actual source assembly');
+  }
+
+  const type = jsiiSymbol.sourceAssembly?.assembly.types?.[jsiiSymbol.fqn];
+  if (!type) {
+    throw new Error(
+      `resolveJsiiSymbolType: ${jsiiSymbol.fqn} not found in assembly ${jsiiSymbol.sourceAssembly.assembly.name}`,
+    );
+  }
+  return type;
+}
+
 /**
  * Returns the jsii FQN for a TypeScript (class or type) symbol
  *

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

@@ -1,7 +1,7 @@
 import * as ts from 'typescript';
 
 import { analyzeObjectLiteral, ObjectLiteralStruct } from '../jsii/jsii-types';
-import { isNamedLikeStruct } from '../jsii/jsii-utils';
+import { isNamedLikeStruct, isJsiiProtocolType } from '../jsii/jsii-utils';
 import { OTree, NO_SYNTAX } from '../o-tree';
 import { AstRenderer, AstHandler, nimpl, CommentSyntax } from '../renderer';
 import { voidExpressionString } from '../typescript/ast-utils';
@@ -153,6 +153,22 @@ export abstract class DefaultVisitor<C> implements AstHandler<C> {
       context.report(unsup, `Use of ${ts.SyntaxKind[unsup.kind]} in an object literal is not supported.`);
     }
 
+    const anyMembersFunctions = node.properties.some((p) =>
+      ts.isPropertyAssignment(p)
+        ? isExpressionOfFunctionType(context.typeChecker, p.initializer)
+        : ts.isShorthandPropertyAssignment(p)
+        ? isExpressionOfFunctionType(context.typeChecker, p.name)
+        : false,
+    );
+
+    const inferredType = context.inferredTypeOfExpression(node);
+    if ((inferredType && isJsiiProtocolType(context.typeChecker, inferredType)) || anyMembersFunctions) {
+      context.report(
+        node,
+        `You cannot use an object literal to make an instance of an interface. Define a class instead.`,
+      );
+    }
+
     const lit = analyzeObjectLiteral(context.typeChecker, node);
 
     switch (lit.kind) {
@@ -342,3 +358,24 @@ const UNARY_OPS: { [op in ts.PrefixUnaryOperator]: string } = {
   [ts.SyntaxKind.TildeToken]: '~',
   [ts.SyntaxKind.ExclamationToken]: '!',
 };
+
+/**
+ * Whether the given expression evaluates to a value that is of type "function"
+ *
+ * Examples of function types:
+ *
+ * ```ts
+ * // GIVEN
+ * function someFunction() { }
+ *
+ * // THEN
+ * const x = someFunction; // <- function type
+ * const y = () => 42; // <- function type
+ * const z = x; // <- function type
+ * Array.isArray; // <- function type
+ * ```
+ */
+function isExpressionOfFunctionType(typeChecker: ts.TypeChecker, expr: ts.Expression) {
+  const type = typeChecker.getTypeAtLocation(expr).getNonNullableType();
+  return type.getCallSignatures().length > 0;
+}

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

@@ -9,6 +9,7 @@ import {
   JsiiSymbol,
   simpleName,
   namespaceName,
+  isJsiiProtocolType,
 } from '../jsii/jsii-utils';
 import { jsiiTargetParameter } from '../jsii/packages';
 import { TargetLanguage } from '../languages/target-language';
@@ -23,7 +24,7 @@ import {
 } from '../typescript/ast-utils';
 import { ImportStatement } from '../typescript/imports';
 import { parameterAcceptsUndefined } from '../typescript/types';
-import { startsWithUppercase, flat, sortBy, groupBy, fmap } from '../util';
+import { startsWithUppercase, sortBy, groupBy, fmap } from '../util';
 import { DefaultVisitor } from './default';
 
 interface StructVar {
@@ -102,7 +103,7 @@ export class PythonVisitor extends DefaultVisitor<PythonLanguageContext> {
    * Bump this when you change something in the implementation to invalidate
    * existing cached translations.
    */
-  public static readonly VERSION = '1';
+  public static readonly VERSION = '2';
 
   public readonly language = TargetLanguage.PYTHON;
   public readonly defaultContext = {};
@@ -532,10 +533,18 @@ export class PythonVisitor extends DefaultVisitor<PythonLanguageContext> {
   }
 
   public classDeclaration(node: ts.ClassDeclaration, context: PythonVisitorContext): OTree {
-    const heritage = flat(Array.from(node.heritageClauses ?? []).map((h) => Array.from(h.types))).map((t) =>
-      context.convert(t.expression),
+    const allHeritageClauses = Array.from(node.heritageClauses ?? []).flatMap((h) => Array.from(h.types));
+
+    // List of booleans matching `allHeritage` array
+    const isJsii = allHeritageClauses.map(
+      (e) =>
+        fmap(context.typeOfExpression(e.expression), (type) => isJsiiProtocolType(context.typeChecker, type)) ?? false,
     );
-    const hasHeritage = heritage.length > 0;
+
+    const jsiiImplements = allHeritageClauses.filter((_, i) => isJsii[i]);
+
+    const inlineHeritage = allHeritageClauses.filter((_, i) => !isJsii[i]);
+    const hasHeritage = inlineHeritage.length > 0;
 
     const members = context.updateContext({ inClass: true }).convertAll(node.members);
     if (members.length === 0) {
@@ -544,10 +553,11 @@ export class PythonVisitor extends DefaultVisitor<PythonLanguageContext> {
 
     const ret = new OTree(
       [
+        ...jsiiImplements.flatMap((i) => ['@jsii.implements(', context.convert(i.expression), ')\n']),
         'class ',
         node.name ? context.textOf(node.name) : '???',
         hasHeritage ? '(' : '',
-        ...heritage,
+        ...inlineHeritage.map((t) => context.convert(t.expression)),
         hasHeritage ? ')' : '',
         ': ',
       ],

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

@@ -144,3 +144,41 @@ test('didSuccessfullyCompile is undefined when compilation is not attempted', ()
 
   expect(subject.didSuccessfullyCompile).toBeUndefined();
 });
+
+test('refuse to translate object literal with function member', () => {
+  const visibleSource = 'const x: any = { mem: () => 42 };';
+
+  const snippet: TypeScriptSnippet = {
+    visibleSource,
+    location: { api: { api: 'type', fqn: 'my.class' } },
+  };
+
+  // WHEN
+  const subject = new SnippetTranslator(snippet);
+  subject.renderUsing(new PythonVisitor());
+
+  expect(subject.diagnostics).toContainEqual(
+    expect.objectContaining({
+      messageText: expect.stringMatching(/You cannot use an object literal/),
+    }),
+  );
+});
+
+test('refuse to translate object literal with function member in shorthand property', () => {
+  const visibleSource = 'const mem = () => 42; const x: any = { mem };';
+
+  const snippet: TypeScriptSnippet = {
+    visibleSource,
+    location: { api: { api: 'type', fqn: 'my.class' } },
+  };
+
+  // WHEN
+  const subject = new SnippetTranslator(snippet);
+  subject.renderUsing(new PythonVisitor());
+
+  expect(subject.diagnostics).toContainEqual(
+    expect.objectContaining({
+      messageText: expect.stringMatching(/You cannot use an object literal/),
+    }),
+  );
+});

packages/jsii-rosetta/test/translations/classes/class_implementing_jsii_interface.cs

@@ -0,0 +1,7 @@
+class MyClass : IResolvable
+{
+    public object Resolve()
+    {
+        return 42;
+    }
+}

packages/jsii-rosetta/test/translations/classes/class_implementing_jsii_interface.java

@@ -0,0 +1,5 @@
+public class MyClass implements IResolvable {
+    public Object resolve() {
+        return 42;
+    }
+}

packages/jsii-rosetta/test/translations/classes/class_implementing_jsii_interface.py

@@ -0,0 +1,4 @@
+@jsii.implements(IResolvable)
+class MyClass:
+    def resolve(self):
+        return 42

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

@@ -0,0 +1,12 @@
+/// !hide
+/// fake-from-jsii
+interface IResolvable {
+  resolve(): any;
+}
+/// !show
+
+class MyClass implements IResolvable {
+  public resolve(): any {
+    return 42;
+  }
+}