Skip to content

Commit

Permalink
fix(jsii): Correctly handle singleton enums
Browse files Browse the repository at this point in the history
When an enum has only one option, TypeScript handles it in a special way
and tries very hard to hide the enum declaration in favor of the sole
member. This caused incorrect type names and kinds to be emitted in the
JSII assembly, resulting in incorrect behavior.

This uses a non-public part of the TSC API (possibly an omission from
the hand-written type model), so it includes an additional guard check
to fail explicitly in case the API's behavior happens to change in a
breaking way.

Fixes #231
  • Loading branch information
RomainMuller committed Jun 13, 2019
1 parent e804cab commit 907df8f
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 7 deletions.
34 changes: 34 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1692,3 +1692,37 @@ export class WithPrivatePropertyInConstructor {
return this.privateField === 'Success!';
}
}

/**
* Verifies that singleton enums are handled correctly
*
* https://github.com/awslabs/jsii/issues/231
*/
export class SingletonString {
private constructor() { }

public isSingletonString(value: string): boolean {
return value === SingletonStringEnum.SingletonString;
}
}
/** A singleton string */
export enum SingletonStringEnum {
/** 1337 */
SingletonString = '3L1T3!'
}
/**
* Verifies that singleton enums are handled correctly
*
* https://github.com/awslabs/jsii/issues/231
*/
export class SingletonInt {
private constructor() { }
public isSingletonInt(value: number): boolean {
return value === SingletonIntEnum.SingletonInt;
}
}
/** A singleton integer. */
export enum SingletonIntEnum {
/** Elite! */
SingletonInt = 1337
}
128 changes: 127 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -7121,6 +7121,132 @@
],
"name": "SingleInstanceTwoTypes"
},
"jsii-calc.SingletonInt": {
"assembly": "jsii-calc",
"docs": {
"remarks": "https://github.com/awslabs/jsii/issues/231",
"stability": "experimental",
"summary": "Verifies that singleton enums are handled correctly."
},
"fqn": "jsii-calc.SingletonInt",
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1718
},
"methods": [
{
"docs": {
"stability": "experimental"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1720
},
"name": "isSingletonInt",
"parameters": [
{
"name": "value",
"type": {
"primitive": "number"
}
}
],
"returns": {
"type": {
"primitive": "boolean"
}
}
}
],
"name": "SingletonInt"
},
"jsii-calc.SingletonIntEnum": {
"assembly": "jsii-calc",
"docs": {
"stability": "experimental",
"summary": "A singleton integer."
},
"fqn": "jsii-calc.SingletonIntEnum",
"kind": "enum",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1725
},
"members": [
{
"docs": {
"stability": "experimental",
"summary": "Elite!"
},
"name": "SingletonInt"
}
],
"name": "SingletonIntEnum"
},
"jsii-calc.SingletonString": {
"assembly": "jsii-calc",
"docs": {
"remarks": "https://github.com/awslabs/jsii/issues/231",
"stability": "experimental",
"summary": "Verifies that singleton enums are handled correctly."
},
"fqn": "jsii-calc.SingletonString",
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1701
},
"methods": [
{
"docs": {
"stability": "experimental"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1704
},
"name": "isSingletonString",
"parameters": [
{
"name": "value",
"type": {
"primitive": "string"
}
}
],
"returns": {
"type": {
"primitive": "boolean"
}
}
}
],
"name": "SingletonString"
},
"jsii-calc.SingletonStringEnum": {
"assembly": "jsii-calc",
"docs": {
"stability": "experimental",
"summary": "A singleton string."
},
"fqn": "jsii-calc.SingletonStringEnum",
"kind": "enum",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1709
},
"members": [
{
"docs": {
"stability": "experimental",
"summary": "1337."
},
"name": "SingletonString"
}
],
"name": "SingletonStringEnum"
},
"jsii-calc.StableClass": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -8648,5 +8774,5 @@
}
},
"version": "0.11.2",
"fingerprint": "eQpFH3EHC2GlCSnThymTxnuO9HyZBFvsvddZqu1Fy+8="
"fingerprint": "5TwMNffhxUueZQEAGZG6+JIfS6jcTwCJWc9vixH5aLc="
}
19 changes: 13 additions & 6 deletions packages/jsii/lib/assembler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -735,11 +735,17 @@ export class Assembler implements Emitter {
LOG.trace(`Processing enum: ${colors.gray(ctx.namespace.join('.'))}.${colors.cyan(type.symbol.name)}`);
}

if (_hasInternalJsDocTag(type.symbol)) {
// Forcefully resolving to the EnumDeclaration symbol for single-valued enums
const symbol: ts.Symbol = type.isLiteral() ? (type.symbol as any).parent : type.symbol;
if (!symbol) {
throw new Error(`Unable to resolve enum declaration for ${type.symbol.name}!`);
}

if (_hasInternalJsDocTag(symbol)) {
return undefined;
}

const decl = type.symbol.valueDeclaration;
const decl = symbol.valueDeclaration;
const flags = ts.getCombinedModifierFlags(decl);
// tslint:disable-next-line:no-bitwise
if (flags & ts.ModifierFlags.Const) {
Expand All @@ -748,19 +754,20 @@ export class Assembler implements Emitter {
`Exported enum cannot be declared 'const'`);
}

const docs = this._visitDocumentation(type.symbol, ctx);
const docs = this._visitDocumentation(symbol, ctx);

const typeContext = ctx.replaceStability(docs && docs.stability);
const members = type.isUnion() ? type.types : [type];

const jsiiType: spec.EnumType = {
assembly: this.projectInfo.name,
fqn: `${[this.projectInfo.name, ...ctx.namespace].join('.')}.${type.symbol.name}`,
fqn: `${[this.projectInfo.name, ...ctx.namespace].join('.')}.${symbol.name}`,
kind: spec.TypeKind.Enum,
members: ((type as ts.UnionType).types || []).map(m => ({
members: members.map(m => ({
name: m.symbol.name,
docs: this._visitDocumentation(m.symbol, typeContext),
})),
name: type.symbol.name,
name: symbol.name,
namespace: ctx.namespace.length > 0 ? ctx.namespace.join('.') : undefined,
docs
};
Expand Down

0 comments on commit 907df8f

Please sign in to comment.