Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(rosetta): literal map type is rendered as __object in C sharp #3047

Merged
merged 10 commits into from
Oct 11, 2021
9 changes: 7 additions & 2 deletions packages/jsii-rosetta/lib/jsii/jsii-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,13 @@ export function determineJsiiType(typeChecker: ts.TypeChecker, type: ts.Type): J
}

const mapValuesType = mapElementType(type, typeChecker);
if (mapValuesType) {
return { kind: 'map', elementType: determineJsiiType(typeChecker, mapValuesType) };
if (mapValuesType.result === 'map') {
return {
kind: 'map',
elementType: mapValuesType.elementType
? determineJsiiType(typeChecker, mapValuesType.elementType)
: { kind: 'builtIn', builtIn: 'any' },
};
}

// User-defined or aliased type
Expand Down
12 changes: 3 additions & 9 deletions packages/jsii-rosetta/lib/languages/csharp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ export class CSharpVisitor extends DefaultVisitor<CSharpLanguageContext> {
});
}
// Type information missing and from context we prefer a map
return this.keyValueObjectLiteralExpression(node, undefined, renderer);
return this.keyValueObjectLiteralExpression(node, renderer);
}

public knownStructObjectLiteralExpression(
Expand All @@ -424,15 +424,9 @@ export class CSharpVisitor extends DefaultVisitor<CSharpLanguageContext> {
});
}

public keyValueObjectLiteralExpression(
node: ts.ObjectLiteralExpression,
valueType: ts.Type | undefined,
renderer: CSharpRenderer,
): OTree {
public keyValueObjectLiteralExpression(node: ts.ObjectLiteralExpression, renderer: CSharpRenderer): OTree {
// Try to infer an element type from the elements
if (valueType === undefined) {
valueType = inferMapElementType(node.properties, renderer);
}
const valueType = inferMapElementType(node.properties, renderer.typeChecker);

return new OTree(
['new Dictionary<string, ', this.renderType(node, valueType, false, 'object', renderer), '> { '],
Expand Down
9 changes: 2 additions & 7 deletions packages/jsii-rosetta/lib/languages/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ 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 } from '../typescript/types';

import { TargetLanguage } from '.';

Expand Down Expand Up @@ -162,7 +161,7 @@ export abstract class DefaultVisitor<C> implements AstHandler<C> {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
return this.knownStructObjectLiteralExpression(node, type!, context);
}
return this.keyValueObjectLiteralExpression(node, type && mapElementType(type, context.typeChecker), context);
return this.keyValueObjectLiteralExpression(node, context);
}

public unknownTypeObjectLiteralExpression(node: ts.ObjectLiteralExpression, context: AstRenderer<C>): OTree {
Expand All @@ -177,11 +176,7 @@ export abstract class DefaultVisitor<C> implements AstHandler<C> {
return this.notImplemented(node, context);
}

public keyValueObjectLiteralExpression(
node: ts.ObjectLiteralExpression,
_valueType: ts.Type | undefined,
context: AstRenderer<C>,
): OTree {
public keyValueObjectLiteralExpression(node: ts.ObjectLiteralExpression, context: AstRenderer<C>): OTree {
return this.notImplemented(node, context);
}

Expand Down
8 changes: 2 additions & 6 deletions packages/jsii-rosetta/lib/languages/java.ts
Original file line number Diff line number Diff line change
Expand Up @@ -452,14 +452,10 @@ export class JavaVisitor extends DefaultVisitor<JavaContext> {
public unknownTypeObjectLiteralExpression(node: ts.ObjectLiteralExpression, renderer: JavaRenderer): OTree {
return renderer.currentContext.inNewExprWithObjectLiteralAsLastArg
? this.renderObjectLiteralAsBuilder(node, renderer)
: this.keyValueObjectLiteralExpression(node, undefined, renderer);
: this.keyValueObjectLiteralExpression(node, renderer);
}

public keyValueObjectLiteralExpression(
node: ts.ObjectLiteralExpression,
_valueType: ts.Type | undefined,
renderer: JavaRenderer,
): OTree {
public keyValueObjectLiteralExpression(node: ts.ObjectLiteralExpression, renderer: JavaRenderer): OTree {
return new OTree(['Map.of('], renderer.updateContext({ inKeyValueList: true }).convertAll(node.properties), {
suffix: ')',
separator: ', ',
Expand Down
6 changes: 1 addition & 5 deletions packages/jsii-rosetta/lib/languages/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,11 +346,7 @@ export class PythonVisitor extends DefaultVisitor<PythonLanguageContext> {
return this.renderObjectLiteralExpression(`${structType.symbol.name}(`, ')', true, node, context);
}

public keyValueObjectLiteralExpression(
node: ts.ObjectLiteralExpression,
_valueType: ts.Type | undefined,
context: PythonVisitorContext,
): OTree {
public keyValueObjectLiteralExpression(node: ts.ObjectLiteralExpression, context: PythonVisitorContext): OTree {
return this.renderObjectLiteralExpression('{', '}', false, node, context);
}

Expand Down
45 changes: 23 additions & 22 deletions packages/jsii-rosetta/lib/typescript/types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as ts from 'typescript';

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

/**
* Return the first non-undefined type from a union
Expand Down Expand Up @@ -72,53 +71,55 @@ export function renderTypeFlags(type: ts.Type): string {
return renderFlags(type.flags, ts.TypeFlags);
}

export type MapAnalysis = { result: 'nonMap' } | { result: 'map'; elementType: ts.Type | undefined };

/**
* If this is a map type, return the type mapped *to* (key must always be `string` anyway).
*/
export function mapElementType(type: ts.Type, typeChecker: ts.TypeChecker): ts.Type | undefined {
if (type.flags & ts.TypeFlags.Object && type.symbol) {
export function mapElementType(type: ts.Type, typeChecker: ts.TypeChecker): MapAnalysis {
if (hasAnyFlag(type.flags, ts.TypeFlags.Object) && type.symbol) {
if (type.symbol.name === '__type') {
// Declared map type: {[k: string]: A}
return type.getStringIndexType();
return { result: 'map', elementType: type.getStringIndexType() };
}

if (type.symbol.name === '__object') {
// Derived map type from object literal: typeof({ k: "value" })
// For every property, get the node that created it (PropertyAssignment), and get the type of the initializer of that node
const initializerTypes = type.getProperties().map((p) => {
if (ts.isPropertyAssignment(p.valueDeclaration)) {
return typeOfExpression(typeChecker, p.valueDeclaration.initializer);
}
return undefined;
const expression = p.valueDeclaration;
return typeOfObjectLiteralProperty(typeChecker, expression);
});
return typeIfSame([...initializerTypes, type.getStringIndexType()]);
return {
result: 'map',
elementType: typeIfSame([...initializerTypes, type.getStringIndexType()].filter(isDefined)),
};
}
}

return undefined;
return { result: 'nonMap' };
}

/**
* Try to infer the map element type from the properties if they're all the same
*/
export function inferMapElementType(
elements: ts.NodeArray<ts.ObjectLiteralElementLike>,
renderer: AstRenderer<any>,
elements: readonly ts.ObjectLiteralElementLike[],
typeChecker: ts.TypeChecker,
): ts.Type | undefined {
const nodes = elements.map(elementValueNode).filter(isDefined);
const types = nodes.map((x) => renderer.typeOfExpression(x));
const types = elements.map((e) => typeOfObjectLiteralProperty(typeChecker, e)).filter(isDefined);

return types.every((t) => isSameType(types[0], t)) ? types[0] : undefined;
}

function elementValueNode(el: ts.ObjectLiteralElementLike): ts.Expression | undefined {
if (ts.isPropertyAssignment(el)) {
return el.initializer;
}
if (ts.isShorthandPropertyAssignment(el)) {
return el.name;
}
return undefined;
function typeOfObjectLiteralProperty(typeChecker: ts.TypeChecker, el: ts.Node): ts.Type | undefined {
if (ts.isPropertyAssignment(el)) {
return typeOfExpression(typeChecker, el.initializer);
}
if (ts.isShorthandPropertyAssignment(el)) {
return typeOfExpression(typeChecker, el.name);
}
return undefined;
}

function isSameType(a: ts.Type, b: ts.Type) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
IDictionary<string, object> expected = new Dictionary<string, object> {
{ "Foo", "Bar" },
{ "Baz", 5 },
{ "Qux", new [] { "Waldo", "Fred" } }
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Map<String, Object> expected = Map.of(
"Foo", "Bar",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the desired indentation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

"Baz", 5,
"Qux", List.of("Waldo", "Fred"));
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
expected = {
"Foo": "Bar",
"Baz": 5,
"Qux": ["Waldo", "Fred"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const expected = {
Foo: 'Bar',
Baz: 5,
Qux: [ 'Waldo', 'Fred' ],
};