From 907df8fdff9256050b1fa35839542558df7e43e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Thu, 13 Jun 2019 18:06:02 +0200 Subject: [PATCH] fix(jsii): Correctly handle singleton enums 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 --- packages/jsii-calc/lib/compliance.ts | 34 +++++++ packages/jsii-calc/test/assembly.jsii | 128 +++++++++++++++++++++++++- packages/jsii/lib/assembler.ts | 19 ++-- 3 files changed, 174 insertions(+), 7 deletions(-) diff --git a/packages/jsii-calc/lib/compliance.ts b/packages/jsii-calc/lib/compliance.ts index b5a0a7f858..a5194ccd75 100644 --- a/packages/jsii-calc/lib/compliance.ts +++ b/packages/jsii-calc/lib/compliance.ts @@ -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 +} diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii index 5246a4694d..23f1aa9ffe 100644 --- a/packages/jsii-calc/test/assembly.jsii +++ b/packages/jsii-calc/test/assembly.jsii @@ -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": { @@ -8648,5 +8774,5 @@ } }, "version": "0.11.2", - "fingerprint": "eQpFH3EHC2GlCSnThymTxnuO9HyZBFvsvddZqu1Fy+8=" + "fingerprint": "5TwMNffhxUueZQEAGZG6+JIfS6jcTwCJWc9vixH5aLc=" } diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts index 5bf1b68990..860f752d3c 100644 --- a/packages/jsii/lib/assembler.ts +++ b/packages/jsii/lib/assembler.ts @@ -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) { @@ -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 };