Skip to content

Commit

Permalink
fix(rosetta): constants are incorrectly turned into getters
Browse files Browse the repository at this point in the history
Enum and public static const accesses are turned into getters,
which they shouldn't be.

Related #2984.
  • Loading branch information
rix0rrr committed Oct 8, 2021
1 parent 2120d34 commit dada654
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 8 deletions.
11 changes: 8 additions & 3 deletions packages/jsii-rosetta/lib/jsii/jsii-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<A extends number>(flags: A, test: A) {
export function hasAllFlags<A extends number>(flags: A, test: A) {
// tslint:disable-next-line:no-bitwise
return (flags & test) !== 0;
return test !== 0 && (flags & test) === test;
}

export function hasAnyFlag<A extends number>(flags: A, test: A) {
// tslint:disable-next-line:no-bitwise
return test !== 0 && (flags & test) !== 0;
}

export interface StructProperty {
Expand Down
24 changes: 19 additions & 5 deletions packages/jsii-rosetta/lib/languages/java.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -496,11 +502,19 @@ export class JavaVisitor extends DefaultVisitor<JavaContext> {
// 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];
}
Expand Down
36 changes: 36 additions & 0 deletions packages/jsii-rosetta/lib/typescript/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as ts from 'typescript';

import { hasAllFlags, hasAnyFlag } from '../jsii/jsii-utils';
import { AstRenderer } from '../renderer';

/**
Expand Down Expand Up @@ -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).
*/
Expand Down Expand Up @@ -153,3 +158,34 @@ export function arrayElementType(type: ts.Type): ts.Type | undefined {
function isDefined<A>(x: A): x is NonNullable<A> {
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<string, number | string>) {
if (flags === undefined) {
return '';
}

return Object.values(flagObject)
.filter(isNumber)
.filter((f) => hasAllFlags(flags, f))
.map((f) => flagObject[f])
.join(',');
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Console.WriteLine(EnumType.ENUM_VALUE_A);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
System.out.println(EnumType.ENUM_VALUE_A);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
print(EnumType.ENUM_VALUE_A)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/// !hide
enum EnumType {
ENUM_VALUE_A = 'a',
ENUM_VALUE_B = 'b',
}
/// !show
console.log(EnumType.ENUM_VALUE_A);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Console.WriteLine(EnumType.ENUM_VALUE_A);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
System.out.println(EnumType.ENUM_VALUE_A);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
print(EnumType.ENUM_VALUE_A)
Original file line number Diff line number Diff line change
@@ -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);

0 comments on commit dada654

Please sign in to comment.