From dada654326b323bceabc23c2d0905438abb49fa5 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 8 Oct 2021 20:28:12 +0200 Subject: [PATCH 1/2] fix(rosetta): constants are incorrectly turned into getters Enum and public static const accesses are turned into getters, which they shouldn't be. Related #2984. --- packages/jsii-rosetta/lib/jsii/jsii-utils.ts | 11 ++++-- packages/jsii-rosetta/lib/languages/java.ts | 24 ++++++++++--- packages/jsii-rosetta/lib/typescript/types.ts | 36 +++++++++++++++++++ .../translations/expressions/enum_access.cs | 1 + .../translations/expressions/enum_access.java | 1 + .../translations/expressions/enum_access.py | 1 + .../translations/expressions/enum_access.ts | 7 ++++ .../expressions/enum_like_access.cs | 1 + .../expressions/enum_like_access.java | 1 + .../expressions/enum_like_access.py | 1 + .../expressions/enum_like_access.ts | 7 ++++ 11 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 packages/jsii-rosetta/test/translations/expressions/enum_access.cs create mode 100644 packages/jsii-rosetta/test/translations/expressions/enum_access.java create mode 100644 packages/jsii-rosetta/test/translations/expressions/enum_access.py create mode 100644 packages/jsii-rosetta/test/translations/expressions/enum_access.ts create mode 100644 packages/jsii-rosetta/test/translations/expressions/enum_like_access.cs create mode 100644 packages/jsii-rosetta/test/translations/expressions/enum_like_access.java create mode 100644 packages/jsii-rosetta/test/translations/expressions/enum_like_access.py create mode 100644 packages/jsii-rosetta/test/translations/expressions/enum_like_access.ts diff --git a/packages/jsii-rosetta/lib/jsii/jsii-utils.ts b/packages/jsii-rosetta/lib/jsii/jsii-utils.ts index 82d77ae5a4..d45d8a942d 100644 --- a/packages/jsii-rosetta/lib/jsii/jsii-utils.ts +++ b/packages/jsii-rosetta/lib/jsii/jsii-utils.ts @@ -11,14 +11,19 @@ export function isStructInterface(name: string) { export function isStructType(type: ts.Type) { return ( type.isClassOrInterface() && - hasFlag(type.objectFlags, ts.ObjectFlags.Interface) && + hasAllFlags(type.objectFlags, ts.ObjectFlags.Interface) && isStructInterface(type.symbol.name) ); } -export function hasFlag(flags: A, test: A) { +export function hasAllFlags(flags: A, test: A) { // tslint:disable-next-line:no-bitwise - return (flags & test) !== 0; + return test !== 0 && (flags & test) === test; +} + +export function hasAnyFlag(flags: A, test: A) { + // tslint:disable-next-line:no-bitwise + return test !== 0 && (flags & test) !== 0; } export interface StructProperty { diff --git a/packages/jsii-rosetta/lib/languages/java.ts b/packages/jsii-rosetta/lib/languages/java.ts index c45d6e0076..905c696584 100644 --- a/packages/jsii-rosetta/lib/languages/java.ts +++ b/packages/jsii-rosetta/lib/languages/java.ts @@ -7,7 +7,13 @@ 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, firstTypeInUnion, mapElementType } from '../typescript/types'; +import { + builtInTypeName, + mapElementType, + isEnumAccess, + isStaticReadonlyAccess, + firstTypeInUnion, +} from '../typescript/types'; import { DefaultVisitor } from './default'; interface JavaContext { @@ -496,11 +502,19 @@ export class JavaVisitor extends DefaultVisitor { // for 'this', assume this is a field, and access it directly parts = ['this', '.', rightHandSide]; } else { + let convertToGetter = renderer.currentContext.convertPropertyToGetter !== false; + + // See if we're not accessing an enum member or public static readonly property (const). + if (isEnumAccess(renderer.typeChecker, node)) { + convertToGetter = false; + } + if (isStaticReadonlyAccess(renderer.typeChecker, node)) { + convertToGetter = false; + } + // add a 'get' prefix to the property name, and change the access to a method call, if required - const renderedRightHandSide = - renderer.currentContext.convertPropertyToGetter === false - ? rightHandSide - : `get${capitalize(node.name.text)}()`; + const renderedRightHandSide = convertToGetter ? `get${capitalize(node.name.text)}()` : rightHandSide; + // strip any trailing ! from the left-hand side, as they're not meaningful in Java parts = [stripTrailingBang(leftHandSide), '.', renderedRightHandSide]; } diff --git a/packages/jsii-rosetta/lib/typescript/types.ts b/packages/jsii-rosetta/lib/typescript/types.ts index 6a645a270e..e995e67067 100644 --- a/packages/jsii-rosetta/lib/typescript/types.ts +++ b/packages/jsii-rosetta/lib/typescript/types.ts @@ -1,5 +1,6 @@ import * as ts from 'typescript'; +import { hasAllFlags, hasAnyFlag } from '../jsii/jsii-utils'; import { AstRenderer } from '../renderer'; /** @@ -77,6 +78,10 @@ export function typeContainsUndefined(type: ts.Type): boolean { return false; } +export function renderTypeFlags(type: ts.Type): string { + return renderFlags(type.flags, ts.TypeFlags); +} + /** * If this is a map type, return the type mapped *to* (key must always be `string` anyway). */ @@ -153,3 +158,34 @@ export function arrayElementType(type: ts.Type): ts.Type | undefined { function isDefined(x: A): x is NonNullable { return x !== undefined; } + +export function isNumber(x: any): x is number { + return typeof x === 'number'; +} + +export function isEnumAccess(typeChecker: ts.TypeChecker, access: ts.PropertyAccessExpression) { + const symbol = typeChecker.getSymbolAtLocation(access.expression); + return symbol ? hasAnyFlag(symbol.flags, ts.SymbolFlags.Enum) : false; +} + +export function isStaticReadonlyAccess(typeChecker: ts.TypeChecker, access: ts.PropertyAccessExpression) { + const symbol = typeChecker.getSymbolAtLocation(access); + const decl = symbol?.getDeclarations(); + if (decl && decl[0] && ts.isPropertyDeclaration(decl[0])) { + const flags = ts.getCombinedModifierFlags(decl[0]); + return hasAllFlags(flags, ts.ModifierFlags.Readonly | ts.ModifierFlags.Static); + } + return false; +} + +export function renderFlags(flags: number | undefined, flagObject: Record) { + if (flags === undefined) { + return ''; + } + + return Object.values(flagObject) + .filter(isNumber) + .filter((f) => hasAllFlags(flags, f)) + .map((f) => flagObject[f]) + .join(','); +} diff --git a/packages/jsii-rosetta/test/translations/expressions/enum_access.cs b/packages/jsii-rosetta/test/translations/expressions/enum_access.cs new file mode 100644 index 0000000000..319d38fc46 --- /dev/null +++ b/packages/jsii-rosetta/test/translations/expressions/enum_access.cs @@ -0,0 +1 @@ +Console.WriteLine(EnumType.ENUM_VALUE_A); \ No newline at end of file diff --git a/packages/jsii-rosetta/test/translations/expressions/enum_access.java b/packages/jsii-rosetta/test/translations/expressions/enum_access.java new file mode 100644 index 0000000000..77e3b782b2 --- /dev/null +++ b/packages/jsii-rosetta/test/translations/expressions/enum_access.java @@ -0,0 +1 @@ +System.out.println(EnumType.ENUM_VALUE_A); \ No newline at end of file diff --git a/packages/jsii-rosetta/test/translations/expressions/enum_access.py b/packages/jsii-rosetta/test/translations/expressions/enum_access.py new file mode 100644 index 0000000000..370d3f04d2 --- /dev/null +++ b/packages/jsii-rosetta/test/translations/expressions/enum_access.py @@ -0,0 +1 @@ +print(EnumType.ENUM_VALUE_A) \ No newline at end of file diff --git a/packages/jsii-rosetta/test/translations/expressions/enum_access.ts b/packages/jsii-rosetta/test/translations/expressions/enum_access.ts new file mode 100644 index 0000000000..aab29f4288 --- /dev/null +++ b/packages/jsii-rosetta/test/translations/expressions/enum_access.ts @@ -0,0 +1,7 @@ +/// !hide +enum EnumType { + ENUM_VALUE_A = 'a', + ENUM_VALUE_B = 'b', +} +/// !show +console.log(EnumType.ENUM_VALUE_A); \ No newline at end of file diff --git a/packages/jsii-rosetta/test/translations/expressions/enum_like_access.cs b/packages/jsii-rosetta/test/translations/expressions/enum_like_access.cs new file mode 100644 index 0000000000..319d38fc46 --- /dev/null +++ b/packages/jsii-rosetta/test/translations/expressions/enum_like_access.cs @@ -0,0 +1 @@ +Console.WriteLine(EnumType.ENUM_VALUE_A); \ No newline at end of file diff --git a/packages/jsii-rosetta/test/translations/expressions/enum_like_access.java b/packages/jsii-rosetta/test/translations/expressions/enum_like_access.java new file mode 100644 index 0000000000..77e3b782b2 --- /dev/null +++ b/packages/jsii-rosetta/test/translations/expressions/enum_like_access.java @@ -0,0 +1 @@ +System.out.println(EnumType.ENUM_VALUE_A); \ No newline at end of file diff --git a/packages/jsii-rosetta/test/translations/expressions/enum_like_access.py b/packages/jsii-rosetta/test/translations/expressions/enum_like_access.py new file mode 100644 index 0000000000..370d3f04d2 --- /dev/null +++ b/packages/jsii-rosetta/test/translations/expressions/enum_like_access.py @@ -0,0 +1 @@ +print(EnumType.ENUM_VALUE_A) \ No newline at end of file diff --git a/packages/jsii-rosetta/test/translations/expressions/enum_like_access.ts b/packages/jsii-rosetta/test/translations/expressions/enum_like_access.ts new file mode 100644 index 0000000000..f742e5b95f --- /dev/null +++ b/packages/jsii-rosetta/test/translations/expressions/enum_like_access.ts @@ -0,0 +1,7 @@ +/// !hide +class EnumType { + public static readonly ENUM_VALUE_A = 'a'; + public static readonly ENUM_VALUE_B = 'b'; +} +/// !show +console.log(EnumType.ENUM_VALUE_A); \ No newline at end of file From eff21f0cac9928978ca963d5f46bccd051f11251 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 11 Oct 2021 09:26:28 +0200 Subject: [PATCH 2/2] Fix build issues --- packages/jsii-rosetta/lib/languages/default.ts | 4 ++-- packages/jsii-rosetta/lib/typescript/types.ts | 10 ---------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/packages/jsii-rosetta/lib/languages/default.ts b/packages/jsii-rosetta/lib/languages/default.ts index d3de4ec2fe..9b4666a0cb 100644 --- a/packages/jsii-rosetta/lib/languages/default.ts +++ b/packages/jsii-rosetta/lib/languages/default.ts @@ -1,6 +1,6 @@ import * as ts from 'typescript'; -import { hasFlag, isStructInterface, isStructType } from '../jsii/jsii-utils'; +import { isStructInterface, isStructType, hasAllFlags } from '../jsii/jsii-utils'; import { OTree, NO_SYNTAX } from '../o-tree'; import { AstRenderer, AstHandler, nimpl, CommentSyntax } from '../renderer'; import { voidExpressionString } from '../typescript/ast-utils'; @@ -133,7 +133,7 @@ export abstract class DefaultVisitor implements AstHandler { const type = context.inferredTypeOfExpression(node); let isUnknownType = !type; - if (type && hasFlag(type.flags, ts.TypeFlags.Any)) { + if (type && hasAllFlags(type.flags, ts.TypeFlags.Any)) { // The type checker by itself won't tell us the difference between an `any` that // was literally declared as a type in the code, vs an `any` it assumes because it // can't find a function's type declaration. diff --git a/packages/jsii-rosetta/lib/typescript/types.ts b/packages/jsii-rosetta/lib/typescript/types.ts index e995e67067..c2939c282b 100644 --- a/packages/jsii-rosetta/lib/typescript/types.ts +++ b/packages/jsii-rosetta/lib/typescript/types.ts @@ -42,16 +42,6 @@ export function renderType(type: ts.Type): string { 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;