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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 18 commits into from
Jun 20, 2019
Merged

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

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
11f3aba
feat(jsii): enforce enum names to be UPPER_CASE
shivlaks Jun 17, 2019
13f7604
validate enum members to use TRUMP_CASE and not enum names
shivlaks Jun 17, 2019
4690725
fix jsii-calc-lib
shivlaks Jun 17, 2019
08510f9
fix enum members to be TRUMP_CASE in jsii-calc
shivlaks Jun 17, 2019
17813c6
fix jsii-dotnet-runtime-test
shivlaks Jun 17, 2019
e8cd402
fix test & fingerprint in jsii-calc
shivlaks Jun 17, 2019
69e1934
fix test & fingerprint
shivlaks Jun 17, 2019
075fa5f
fix tests in jsii-kernel
shivlaks Jun 17, 2019
73e131f
fix tests in jsii-reflect
shivlaks Jun 17, 2019
110333d
fix tests in jsii-diff
shivlaks Jun 17, 2019
abcd79b
Merge branch 'master' into shivlaks/upper-case-all-enums
shivlaks Jun 20, 2019
b7331d4
add negative test for testing enum members
shivlaks Jun 20, 2019
b343d4a
update fingerprints after merge
shivlaks Jun 20, 2019
5a857fc
jsii-pacmak UPDATE_DIFF when running test
shivlaks Jun 20, 2019
d86eab6
update enum members in jsii-java-runtime-test
shivlaks Jun 20, 2019
0b1613e
update enum members in jsii-python-runtime
shivlaks Jun 20, 2019
7a20567
missed a file in jsii-python-runtime
shivlaks Jun 20, 2019
ec44403
updating error messaging to use ALL_CAPS as terminology
shivlaks Jun 20, 2019
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
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions packages/jsii-calc-lib/lib/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ export interface StructWithOnlyOptionals {
* See awslabs/jsii#138
*/
export enum EnumFromScopedModule {
Value1,
Value2
VALUE1,
VALUE2
}

/**
Expand Down
6 changes: 3 additions & 3 deletions packages/jsii-calc-lib/test/assembly.jsii
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,13 @@
"docs": {
"stability": "deprecated"
},
"name": "Value1"
"name": "VALUE1"
},
{
"docs": {
"stability": "deprecated"
},
"name": "Value2"
"name": "VALUE2"
}
],
"name": "EnumFromScopedModule"
Expand Down Expand Up @@ -540,5 +540,5 @@
}
},
"version": "0.11.2",
"fingerprint": "VqoqMBfUNmZz4tgHYWRELk7dts3F/YWdWH0uOkpYTXE="
"fingerprint": "Pz4eej1Q5lbYerXF2OyXh0WjES23ntkF3d9MnDRbQc8="
}
10 changes: 5 additions & 5 deletions packages/jsii-calc/lib/calculator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ export namespace composition {
/**
* The .toString() style.
*/
public stringStyle = CompositeOperation.CompositionStringStyle.Normal
public stringStyle = CompositeOperation.CompositionStringStyle.NORMAL

/**
* A set of prefixes to include in a decorated .toString().
Expand All @@ -157,9 +157,9 @@ export namespace composition {

toString() {
switch (this.stringStyle) {
case CompositeOperation.CompositionStringStyle.Normal:
case CompositeOperation.CompositionStringStyle.NORMAL:
return this.expression.toString();
case CompositeOperation.CompositionStringStyle.Decorated:
case CompositeOperation.CompositionStringStyle.DECORATED:
return this.decorationPrefixes.join('') + this.expression.toString() + this.decorationPostfixes.join('');
default:
throw new Error(`Unknown string style: ${this.stringStyle}`);
Expand All @@ -173,10 +173,10 @@ export namespace composition {
*/
export enum CompositionStringStyle {
/** Normal string expression */
Normal,
NORMAL,

/** Decorated string expression */
Decorated
DECORATED
}
}
}
Expand Down
26 changes: 13 additions & 13 deletions packages/jsii-calc/lib/compliance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import base = require('@scope/jsii-calc-base');
const readFile = promisify(fs.readFile);

export enum AllTypesEnum {
MyEnumValue,
YourEnumValue = 100,
ThisIsGreat
MY_ENUM_VALUE,
YOUR_ENUM_VALUE = 100,
THIS_IS_GREAT
}

export enum StringEnum {
Expand Down Expand Up @@ -169,7 +169,7 @@ export class AllTypes {
// enum

public optionalEnumValue?: StringEnum;
private enumValue: AllTypesEnum = AllTypesEnum.ThisIsGreat;
private enumValue: AllTypesEnum = AllTypesEnum.THIS_IS_GREAT;

get enumProperty() {
return this.enumValue;
Expand All @@ -178,9 +178,9 @@ export class AllTypes {
set enumProperty(value: AllTypesEnum) {
this.enumValue = value;
switch (value) {
case AllTypesEnum.MyEnumValue:
case AllTypesEnum.YourEnumValue:
case AllTypesEnum.ThisIsGreat:
case AllTypesEnum.MY_ENUM_VALUE:
case AllTypesEnum.YOUR_ENUM_VALUE:
case AllTypesEnum.THIS_IS_GREAT:
return;
default:
throw new Error('Invalid enum: ' + value);
Expand Down Expand Up @@ -1008,7 +1008,7 @@ export interface ImplictBaseOfBase extends base.BaseProps {
* See awslabs/jsii#138
*/
export class ReferenceEnumFromScopedPackage {
public foo?: EnumFromScopedModule = EnumFromScopedModule.Value2;
public foo?: EnumFromScopedModule = EnumFromScopedModule.VALUE2;

public loadFoo(): EnumFromScopedModule | undefined {
return this.foo;
Expand Down Expand Up @@ -1594,7 +1594,7 @@ export abstract class PartiallyInitializedThisConsumer {

export class ConstructorPassesThisOut {
constructor(consumer: PartiallyInitializedThisConsumer) {
const result = consumer.consumePartiallyInitializedThis(this, new Date(0), AllTypesEnum.ThisIsGreat);
const result = consumer.consumePartiallyInitializedThis(this, new Date(0), AllTypesEnum.THIS_IS_GREAT);
if (result !== 'OK') {
throw new Error(`Expected OK but received ${result}`);
}
Expand Down Expand Up @@ -1702,13 +1702,13 @@ export class SingletonString {
private constructor() { }

public isSingletonString(value: string): boolean {
return value === SingletonStringEnum.SingletonString;
return value === SingletonStringEnum.SINGLETON_STRING;
}
}
/** A singleton string */
export enum SingletonStringEnum {
/** 1337 */
SingletonString = '3L1T3!'
SINGLETON_STRING = '3L1T3!'
}
/**
* Verifies that singleton enums are handled correctly
Expand All @@ -1718,11 +1718,11 @@ export enum SingletonStringEnum {
export class SingletonInt {
private constructor() { }
public isSingletonInt(value: number): boolean {
return value === SingletonIntEnum.SingletonInt;
return value === SingletonIntEnum.SINGLETON_INT;
}
}
/** A singleton integer. */
export enum SingletonIntEnum {
/** Elite! */
SingletonInt = 1337
SINGLETON_INT = 1337
}
12 changes: 6 additions & 6 deletions packages/jsii-calc/lib/stability.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ export class ExperimentalClass {
/** @experimental */
export enum ExperimentalEnum {
/** @experimental */
OptionA,
OPTION_A,
/** @experimental */
OptionB
OPTION_B
}

/** @stable */
Expand Down Expand Up @@ -64,9 +64,9 @@ export class StableClass {
/** @stable */
export enum StableEnum {
/** @stable */
OptionA,
OPTION_A,
/** @stable */
OptionB
OPTION_B
}

/** @deprecated it just wraps a string */
Expand Down Expand Up @@ -98,7 +98,7 @@ export class DeprecatedClass {
/** @deprecated your deprecated selection of bad options */
export enum DeprecatedEnum {
/** @deprecated option A is not great */
OptionA,
OPTION_A,
/** @deprecated option B is kinda bad, too */
OptionB
OPTION_B
}
28 changes: 14 additions & 14 deletions packages/jsii-calc/test/assembly.jsii
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -892,19 +892,19 @@
"docs": {
"stability": "experimental"
},
"name": "MyEnumValue"
"name": "MY_ENUM_VALUE"
},
{
"docs": {
"stability": "experimental"
},
"name": "YourEnumValue"
"name": "YOUR_ENUM_VALUE"
},
{
"docs": {
"stability": "experimental"
},
"name": "ThisIsGreat"
"name": "THIS_IS_GREAT"
}
],
"name": "AllTypesEnum"
Expand Down Expand Up @@ -2262,14 +2262,14 @@
"deprecated": "option A is not great",
"stability": "deprecated"
},
"name": "OptionA"
"name": "OPTION_A"
},
{
"docs": {
"deprecated": "option B is kinda bad, too",
"stability": "deprecated"
},
"name": "OptionB"
"name": "OPTION_B"
}
],
"name": "DeprecatedEnum"
Expand Down Expand Up @@ -2972,13 +2972,13 @@
"docs": {
"stability": "experimental"
},
"name": "OptionA"
"name": "OPTION_A"
},
{
"docs": {
"stability": "experimental"
},
"name": "OptionB"
"name": "OPTION_B"
}
],
"name": "ExperimentalEnum"
Expand Down Expand Up @@ -7179,7 +7179,7 @@
"stability": "experimental",
"summary": "Elite!"
},
"name": "SingletonInt"
"name": "SINGLETON_INT"
}
],
"name": "SingletonIntEnum"
Expand Down Expand Up @@ -7242,7 +7242,7 @@
"stability": "experimental",
"summary": "1337."
},
"name": "SingletonString"
"name": "SINGLETON_STRING"
}
],
"name": "SingletonStringEnum"
Expand Down Expand Up @@ -7338,13 +7338,13 @@
"docs": {
"stability": "stable"
},
"name": "OptionA"
"name": "OPTION_A"
},
{
"docs": {
"stability": "stable"
},
"name": "OptionB"
"name": "OPTION_B"
}
],
"name": "StableEnum"
Expand Down Expand Up @@ -8759,20 +8759,20 @@
"stability": "experimental",
"summary": "Normal string expression."
},
"name": "Normal"
"name": "NORMAL"
},
{
"docs": {
"stability": "experimental",
"summary": "Decorated string expression."
},
"name": "Decorated"
"name": "DECORATED"
}
],
"name": "CompositionStringStyle",
"namespace": "composition.CompositeOperation"
}
},
"version": "0.11.2",
"fingerprint": "5TwMNffhxUueZQEAGZG6+JIfS6jcTwCJWc9vixH5aLc="
"fingerprint": "VQt1+HJ6ExEhE/LVMUx9J9XloyFXmcxHdBt6JTauWX4="
}
2 changes: 1 addition & 1 deletion packages/jsii-calc/test/test.calc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ calc.neg(); assert.equal(-3200000, calc.value);

calc.curr = new Multiply(new calcLib.Number(2), calc.curr);
assert.equal(-6400000, calc.value);
calc.stringStyle = composition.CompositeOperation.CompositionStringStyle.Decorated;
calc.stringStyle = composition.CompositeOperation.CompositionStringStyle.DECORATED;
assert.equal(calc.toString(),
'<<[[{{(2 * -(((((1 * ((0 + 10) * 2)) * ((0 + 10) * 2)) * ((0 + 10) * 2)) * ((0 + 10) * 2)) * ((0 + 10) * 2)))}}]]>>');

Expand Down
18 changes: 9 additions & 9 deletions ...ges/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,21 +260,21 @@ public void GetAndSetEnumValues()

calc.Add(9);
calc.Pow(3);
Assert.Equal(CompositionStringStyle.Normal, calc.StringStyle);
calc.StringStyle = CompositionStringStyle.Decorated;
Assert.Equal(CompositionStringStyle.Decorated, calc.StringStyle);
Assert.Equal(CompositionStringStyle.NORMAL, calc.StringStyle);
calc.StringStyle = CompositionStringStyle.DECORATED;
Assert.Equal(CompositionStringStyle.DECORATED, calc.StringStyle);
Assert.Equal("<<[[{{(((1 * (0 + 9)) * (0 + 9)) * (0 + 9))}}]]>>", calc.ToString());
}

[Fact(DisplayName = Prefix + nameof(EnumFromScopedModule))]
public void UseEnumFromScopedModule()
{
ReferenceEnumFromScopedPackage obj = new ReferenceEnumFromScopedPackage();
Assert.Equal(EnumFromScopedModule.Value2, obj.Foo);
obj.Foo = EnumFromScopedModule.Value1;
Assert.Equal(EnumFromScopedModule.Value1, obj.LoadFoo());
obj.SaveFoo(EnumFromScopedModule.Value2);
Assert.Equal(EnumFromScopedModule.Value2, obj.Foo);
Assert.Equal(EnumFromScopedModule.VALUE2, obj.Foo);
Copy link
Contributor

Choose a reason for hiding this comment

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

wait... what's the convention for .NET? Do we want .NET to also use all-caps?
Same question for Python.... @garnaat is it idiomatic to use all caps for enum values?

Copy link
Contributor

Choose a reason for hiding this comment

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

  • .NET seems to prefer PascalCase.
  • Python looks like ALL_CAPS are good.

So basically, the .NET code generator needs to covert the case for .NET.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's a good question. I thought we wanted to avoid dropping into the language specific mangling... let me check.

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 Pascal casing for both names and values are the idiomatic thing for .NET

Copy link
Contributor

Choose a reason for hiding this comment

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

No. We wanted to normalize the typescript/jsii source to CAPITALS but then, jsii is all about language-specific idiomacy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I'll make the changes to the generation of .NET so we can change enum values into PascalCase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we decided we would make the changes for .NET generation to use enum members in PascalCase outside of this PR.

Should I create an issue for it before resolving this conversation?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, "right click, create issue"

obj.Foo = EnumFromScopedModule.VALUE1;
Assert.Equal(EnumFromScopedModule.VALUE1, obj.LoadFoo());
obj.SaveFoo(EnumFromScopedModule.VALUE2);
Assert.Equal(EnumFromScopedModule.VALUE2, obj.Foo);
}

[Fact(DisplayName = Prefix + nameof(UndefinedAndNull))]
Expand Down Expand Up @@ -929,7 +929,7 @@ public override String ConsumePartiallyInitializedThis(ConstructorPassesThisOut
{
Assert.NotNull(obj);
Assert.Equal(new DateTime(0), dt);
Assert.Equal(AllTypesEnum.ThisIsGreat, ev);
Assert.Equal(AllTypesEnum.THIS_IS_GREAT, ev);

return "OK";
}
Expand Down
Loading