Skip to content

Commit

Permalink
fix(jsii): fail compilation when two or more enum members have same v…
Browse files Browse the repository at this point in the history
…al (#3412)

If two enum members have the same value, only the first one will be retained.

This is a bit of an issue as we are renaming enum members: the named version will never appear in the assembly, and so not work over jsii.

What's worse, if we deprecate-and-strip the original one, neither of the enum members will appear.

Addressing the issue by failing the compilation by adding validation for enum values

Related change in CDK aws/aws-cdk#19320
Fixes #2782.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
yuth committed Jun 29, 2022
1 parent 6d9dda5 commit f64dace
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 1 deletion.
54 changes: 54 additions & 0 deletions packages/jsii/lib/assembler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1738,6 +1738,9 @@ export class Assembler implements Emitter {
return undefined;
}

// check the enum to see if there are duplicate enum values
this.assertNoDuplicateEnumValues(decl);

this._warnAboutReservedWords(symbol);

const flags = ts.getCombinedModifierFlags(decl);
Expand Down Expand Up @@ -1791,6 +1794,57 @@ export class Assembler implements Emitter {
return jsiiType;
}

private assertNoDuplicateEnumValues(decl: ts.EnumDeclaration): void {
type EnumValue = {
name: string;
value: string;
decl: ts.DeclarationName | undefined;
};

const enumValues = decl.members
.filter((m) => m.initializer)
.map((member): EnumValue => {
return {
value: member.initializer!.getText(),
name: member.name.getText(),
decl: ts.getNameOfDeclaration(member),
};
});

const hasDuplicateEnumValues = enumValues.some(
(val, _, arr) => arr.filter((e) => val.value === e.value).length > 1,
);

if (hasDuplicateEnumValues) {
const enumValueMap = enumValues.reduce<Record<string, EnumValue[]>>(
(acc, val) => {
if (!acc[val.value]) {
acc[val.value] = [];
}
acc[val.value].push(val);
return acc;
},
{},
);
for (const duplicateValue of Object.keys(enumValueMap)) {
if (enumValueMap[duplicateValue].length > 1) {
const err = JsiiDiagnostic.JSII_1004_DUPLICATE_ENUM_VALUE.create(
enumValueMap[duplicateValue][0].decl!,
duplicateValue,
enumValueMap[duplicateValue].map((v) => v.name),
);
for (let i = 1; i < enumValueMap[duplicateValue].length; i++) {
err.addRelatedInformation(
enumValueMap[duplicateValue][i].decl!,
'The conflicting declaration is here',
);
}
this._diagnostics.push(err);
}
}
}
}

/**
* Return docs for a symbol
*/
Expand Down
11 changes: 10 additions & 1 deletion packages/jsii/lib/jsii-diagnostic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class Code<
*
* @param code the numeric code for the diagnostic
* @param name the symbolic name for the diagnostic
* @param defaultCategory the default category this diagnostic ransk in
* @param defaultCategory the default category this diagnostic ranks in
* @param formatter a message formatter for easy creation of diagnostics
*/
private constructor(
Expand Down Expand Up @@ -292,6 +292,15 @@ export class JsiiDiagnostic implements ts.Diagnostic {
name: 'typescript-restrictions/unsupported-type',
});

public static readonly JSII_1004_DUPLICATE_ENUM_VALUE = Code.error({
code: 1004,
formatter: (enumValue: string, enumMemberNames: string[]) =>
`Value ${enumValue} is used for multiple enum values: ${enumMemberNames.join(
', ',
)}`,
name: 'typescript-restrictions/duplicate-enum-value',
});

//////////////////////////////////////////////////////////////////////////////
// 2000 => 2999 -- RESERVED

Expand Down
22 changes: 22 additions & 0 deletions packages/jsii/test/__snapshots__/negatives.test.js.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions packages/jsii/test/enums.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,16 @@ test('enums can have a mix of letters and number', () => {
{ name: 'IB3M' },
]);
});

test('enums with the same assigned value should fail', () => {
expect(() =>
sourceToAssemblyHelper(`
export enum Foo {
BAR = 'Bar',
BAR_DUPE = 'Bar',
BAZ = 'Baz',
BAZ_DUPE = 'Baz',
}
`),
).toThrow(/There were compiler errors/);
});
6 changes: 6 additions & 0 deletions packages/jsii/test/negatives/neg.enum-duplicate-values.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export enum Foo {
FOO = 'foo',
FOO_DUPLICATE = 'foo',
BAR = 'bar',
BAR_COPY = 'bar',
}

0 comments on commit f64dace

Please sign in to comment.