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

feat(jsii): enforce enum names to be UPPER_CASE #541

Merged
merged 18 commits into from
Jun 20, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ like any other class.

> NOTE: Due to performance of the hosted JavaScript engine and marshaling costs,
__jsii__ modules are likely to be used for development and build tools, as
oppose to performance-sensitive runtime behavior.
opposed to performance-sensitive runtime behavior.

From Java:

Expand Down
14 changes: 14 additions & 0 deletions packages/jsii/lib/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export type ValidationFunction = (validator: Validator,
function _defaultValidations(): ValidationFunction[] {
return [
_typeNamesMustUsePascalCase,
_enumNamesMustUserUpperSnakeCase,
_memberNamesMustUseCamelCase,
_staticConstantNamesMustUseUpperSnakeCase,
_memberNamesMustNotLookLikeJavaGettersOrSetters,
Expand All @@ -63,13 +64,26 @@ function _defaultValidations(): ValidationFunction[] {

function _typeNamesMustUsePascalCase(_: Validator, assembly: spec.Assembly, diagnostic: DiagnosticEmitter) {
for (const type of _allTypes(assembly)) {
if (spec.isEnumType(type)) { continue; }
Copy link
Contributor

Choose a reason for hiding this comment

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

The enum /type/ should be pascal case, just members should be capital

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah i see, totally misread that - so enum values then?
i'll get on it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Enum members:

export enum MyEnum {
  MEMBER_NUMBER_ONE,
  MEMBER_FOO,
  MEMBER_XOO
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated


if (type.name !== Case.pascal(type.name)) {
diagnostic(ts.DiagnosticCategory.Error,
`Type names must use PascalCase: ${type.name}`);
}
}
}

function _enumNamesMustUserUpperSnakeCase(_: Validator, assembly: spec.Assembly, diagnostic: DiagnosticEmitter) {
for (const type of _allTypes(assembly)) {
if (!spec.isEnumType(type)) { continue; }

if (type.name && type.name !== Case.constant(type.name)) {
diagnostic(ts.DiagnosticCategory.Error,
`Enum names must use TRUMP_CASE: ${type.name}`);
}
}
}

function _memberNamesMustUseCamelCase(_: Validator, assembly: spec.Assembly, diagnostic: DiagnosticEmitter) {
for (const member of _allMembers(assembly)) {
if (member.static && (member as spec.Property).const) { continue; }
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii/test/negatives/neg.enum-name.1.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
///!MATCH_ERROR: Type names must use PascalCase: myEnum
///!MATCH_ERROR: Enum names must use TRUMP_CASE: myEnum

export enum myEnum {
Foo,
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii/test/negatives/neg.enum-name.2.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
///!MATCH_ERROR: Type names must use PascalCase: My_Enum
///!MATCH_ERROR: Enum names must use TRUMP_CASE: My_Enum
Copy link
Contributor

Choose a reason for hiding this comment

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

Nope! Just the member names should be TRUMP_CASE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think my Java disposition's are showing - enum names vs enum values. I'll get it updated!


export enum My_Enum {
Foo,
Expand Down
6 changes: 6 additions & 0 deletions packages/jsii/test/negatives/neg.enum-name.3.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
///!MATCH_ERROR: Enum names must use TRUMP_CASE: MyEnum

export enum MyEnum {
Foo,
Goo
}
16 changes: 8 additions & 8 deletions packages/jsii/test/test.enums.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@ export = {
// ----------------------------------------------------------------------
async 'test parsing enum with two members and no values'(test: Test) {
const assembly = await sourceToAssemblyHelper(`
export enum Foo {
export enum FOO {
Bar,
Baz
}
`);

test.deepEqual(assembly.types!['testpkg.Foo'] , {
test.deepEqual(assembly.types!['testpkg.FOO'] , {
assembly: 'testpkg',
fqn: 'testpkg.Foo',
fqn: 'testpkg.FOO',
kind: 'enum',
members: [{ name: 'Bar' }, { name: 'Baz' }],
locationInModule: { filename: 'index.ts', line: 2 },
name: 'Foo'
name: 'FOO'
});

test.done();
Expand All @@ -28,19 +28,19 @@ export = {
// ----------------------------------------------------------------------
async 'test parsing enum with two members and assigned values'(test: Test) {
const assembly = await sourceToAssemblyHelper(`
export enum Foo {
export enum FOO {
Bar = 'Bar',
Baz = 'Baz'
}
`);

test.deepEqual(assembly.types!['testpkg.Foo'] , {
test.deepEqual(assembly.types!['testpkg.FOO'] , {
assembly: 'testpkg',
fqn: 'testpkg.Foo',
fqn: 'testpkg.FOO',
kind: 'enum',
members: [{ name: 'Bar' }, { name: 'Baz' }],
locationInModule: { filename: 'index.ts', line: 2 },
name: 'Foo'
name: 'FOO'
});

test.done();
Expand Down