Skip to content

Commit

Permalink
fix(rosetta): fix usage of Builders in Java
Browse files Browse the repository at this point in the history
* Builders were never used for structs, even though that's required.
* The integrated class+struct builder wasn't always used for class
  translations, even if it was available.

Rectify both of these issues. Relates to #2984.
  • Loading branch information
rix0rrr committed Oct 11, 2021
1 parent 2120d34 commit b86632d
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 26 deletions.
42 changes: 42 additions & 0 deletions packages/jsii-rosetta/lib/jsii/jsii-types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import * as ts from 'typescript';
import { inferredTypeOfExpression, mapElementType } from '../typescript/types';
import { hasFlag } from './jsii-utils';




export type ObjectLiteralAnalysis = (
{ readonly kind: 'struct', readonly type: ts.Type }
| { readonly kind: 'map', readonly elementType?: ts.Type });


export function analyzeObjectLiteral(typeChecker: ts.TypeChecker, node: ts.ObjectLiteralExpression): ObjectLiteralAnalysis {
const type = inferredTypeOfExpression(typeChecker, node);
if (!type) { return { kind: 'map' }};

if (hasFlag(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.
//
// Search for the function's declaration and only if we can't find it,
// the type is actually unknown (otherwise it's a literal 'any').
const call = findEnclosingCallExpression(node);
const signature = call ? typeChecker.getResolvedSignature(call) : undefined;
if (!signature?.declaration) {

return { kind: 'map', elementType: mapElementType(type, };
}
}
}

function findEnclosingCallExpression(node?: ts.Node): ts.CallLikeExpression | undefined {
while (node) {
if (ts.isCallLikeExpression(node)) {
return node;
}
node = node.parent;
}

return undefined;
}
11 changes: 0 additions & 11 deletions packages/jsii-rosetta/lib/languages/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,14 +331,3 @@ const UNARY_OPS: { [op in ts.PrefixUnaryOperator]: string } = {
[ts.SyntaxKind.TildeToken]: '~',
[ts.SyntaxKind.ExclamationToken]: '~',
};

function findEnclosingCallExpression(node?: ts.Node): ts.CallLikeExpression | undefined {
while (node) {
if (ts.isCallLikeExpression(node)) {
return node;
}
node = node.parent;
}

return undefined;
}
18 changes: 13 additions & 5 deletions packages/jsii-rosetta/lib/languages/java.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,9 @@ export class JavaVisitor extends DefaultVisitor<JavaContext> {
const lastArgIsObjectLiteral = lastArg && ts.isObjectLiteralExpression(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)
// if the last argument is an object literal, and either a known struct (because
// those are the jsii rules) or an unknown type (because the example didn't
// compile but most of the target examples will intend this to be a struct).
const renderBuilderInsteadOfNew = lastArgIsObjectLiteral && (!lastArgType || !isStructType(lastArgType));

return new OTree(
Expand Down Expand Up @@ -463,9 +464,16 @@ export class JavaVisitor extends DefaultVisitor<JavaContext> {
structType: ts.Type,
renderer: JavaRenderer,
): OTree {
return new OTree(['new ', structType.symbol.name, '()'], [...renderer.convertAll(node.properties)], {
indent: 8,
});
return new OTree(
['', structType.symbol.name, '.builder()'],
[
...renderer.convertAll(node.properties),
new OTree([renderer.mirrorNewlineBefore(node.properties[0])], ['.build()']),
],
{
indent: 8,
},
);
}

public propertyAssignment(node: ts.PropertyAssignment, renderer: JavaRenderer): OTree {
Expand Down
6 changes: 4 additions & 2 deletions packages/jsii-rosetta/lib/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
scanText,
} from './typescript/ast-utils';
import { analyzeImportDeclaration, analyzeImportEquals, ImportStatement } from './typescript/imports';
import { inferredTypeOfExpression } from './typescript/types';

/**
* Render a TypeScript AST to some other representation (encoded in OTrees)
Expand Down Expand Up @@ -137,10 +138,11 @@ export class AstRenderer<C> {
* optional), `undefined` will be removed from the union.
*
* (Will return undefined for object literals not unified with a declared type)
*
* @deprecated Use `inferredTypeOfExpression` instead
*/
public inferredTypeOfExpression(node: ts.Expression) {
const type = this.typeChecker.getContextualType(node);
return type ? this.typeChecker.getNonNullableType(type) : undefined;
return inferredTypeOfExpression(this.typeChecker, node);
}

/**
Expand Down
13 changes: 13 additions & 0 deletions packages/jsii-rosetta/lib/typescript/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,3 +153,16 @@ export function arrayElementType(type: ts.Type): ts.Type | undefined {
function isDefined<A>(x: A): x is NonNullable<A> {
return x !== undefined;
}

/**
* 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)
*/
export function inferredTypeOfExpression(typeChecker: ts.TypeChecker, node: ts.Expression) {
const type = typeChecker.getContextualType(node);
return type ? typeChecker.getNonNullableType(type) : undefined;
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public OuterStruct deeper(DeeperStruct deeper) {
public void foo(Number x, OuterStruct outer) {
}

foo(25, new OuterStruct().foo(3).deeper(new DeeperStruct()
foo(25, OuterStruct.builder().foo(3).deeper(DeeperStruct.builder()
.a(1)
.b(2)));
.b(2)
.build()).build());
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
new Vpc(this, "Something", new VpcProps()
.argument(5));
new Vpc(this, "Something", VpcProps.builder()
.argument(5)
.build());
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
new Integration(this, "Something", new IntegrationOptions()
.argument(5));
new Integration(this, "Something", IntegrationOptions.builder()
.argument(5)
.build());
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
Vpc vpc = new Vpc(this, "Something", new VpcProps()
.argument(5));
Object vpc = Vpc.Builder.create(this, "Something")
.argument(5)
.build();

0 comments on commit b86632d

Please sign in to comment.