From a259d59624bf1a02bd134d9f649a3629efb1908b Mon Sep 17 00:00:00 2001 From: Komal Date: Mon, 9 Nov 2020 11:33:22 -0800 Subject: [PATCH] [codegen/dotnet,nodejs] - Improve makeSafeEnumName (#5711) * Improve makeSafeEnumName * Improve nodejs enumName generator * Fix comment * PR feedback --- pkg/codegen/dotnet/gen.go | 23 ++++++++++------- pkg/codegen/dotnet/gen_test.go | 30 ++++++++++++++++++++++ pkg/codegen/dotnet/utilities.go | 45 +++++++++++++++++++++++++++++++++ pkg/codegen/nodejs/gen.go | 28 +++++++++++++------- pkg/codegen/nodejs/gen_test.go | 33 ++++++++++++++++-------- pkg/codegen/nodejs/utilities.go | 40 +++++++++++++++++++++++++++++ 6 files changed, 170 insertions(+), 29 deletions(-) diff --git a/pkg/codegen/dotnet/gen.go b/pkg/codegen/dotnet/gen.go index 8e72bb293..6342f943c 100644 --- a/pkg/codegen/dotnet/gen.go +++ b/pkg/codegen/dotnet/gen.go @@ -1008,17 +1008,24 @@ func printObsoleteAttribute(w io.Writer, deprecationMessage, indent string) { } } -func makeSafeEnumName(enum *schema.Enum) string { - if enum.Name != "" { - return enum.Name - } - return strings.Title(makeValidIdentifier(enum.Value.(string))) -} - func (mod *modContext) genEnum(w io.Writer, enum *schema.EnumType) error { indent := " " enumName := tokenToName(enum.Token) + // Fix up identifiers for each enum value. + for _, e := range enum.Elements { + // If the enum doesn't have a name, set the value as the name. + if e.Name == "" { + e.Name = fmt.Sprintf("%v", e.Value) + } + + safeName, err := makeSafeEnumName(e.Name) + if err != nil { + return err + } + e.Name = safeName + } + // Print documentation comment printComment(w, enum.Comment, indent) @@ -1050,7 +1057,6 @@ func (mod *modContext) genEnum(w io.Writer, enum *schema.EnumType) error { for _, e := range enum.Elements { printComment(w, e.Comment, indent) printObsoleteAttribute(w, e.DeprecationMessage, indent) - e.Name = makeSafeEnumName(e) fmt.Fprintf(w, "%[1]spublic static %[2]s %[3]s { get; } = new %[2]s(", indent, enumName, e.Name) if enum.ElementType == schema.StringType { fmt.Fprintf(w, "%q", e.Value) @@ -1109,7 +1115,6 @@ func (mod *modContext) genEnum(w io.Writer, enum *schema.EnumType) error { indent := strings.Repeat(indent, 2) printComment(w, e.Comment, indent) printObsoleteAttribute(w, e.DeprecationMessage, indent) - e.Name = makeSafeEnumName(e) fmt.Fprintf(w, "%s%s = %v,\n", indent, e.Name, e.Value) } default: diff --git a/pkg/codegen/dotnet/gen_test.go b/pkg/codegen/dotnet/gen_test.go index 5d4c5b783..62ac92b05 100644 --- a/pkg/codegen/dotnet/gen_test.go +++ b/pkg/codegen/dotnet/gen_test.go @@ -49,3 +49,33 @@ func TestGeneratePackage(t *testing.T) { }) } } + +func TestMakeSafeEnumName(t *testing.T) { + tests := []struct { + input string + expected string + wantErr bool + }{ + {"+", "", true}, + {"*", "Asterisk", false}, + {"0", "Zero", false}, + {"Microsoft-Windows-Shell-Startup", "Microsoft_Windows_Shell_Startup", false}, + {"Microsoft.Batch", "Microsoft_Batch", false}, + {"readonly", "@Readonly", false}, + {"SystemAssigned, UserAssigned", "SystemAssigned_UserAssigned", false}, + {"Dev(NoSLA)_Standard_D11_v2", "Dev_NoSLA_Standard_D11_v2", false}, + {"Standard_E8as_v4+1TB_PS", "Standard_E8as_v4_1TB_PS", false}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + got, err := makeSafeEnumName(tt.input) + if (err != nil) != tt.wantErr { + t.Errorf("makeSafeEnumName() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.expected { + t.Errorf("makeSafeEnumName() got = %v, want %v", got, tt.expected) + } + }) + } +} diff --git a/pkg/codegen/dotnet/utilities.go b/pkg/codegen/dotnet/utilities.go index 7644bcd23..dc3f74170 100644 --- a/pkg/codegen/dotnet/utilities.go +++ b/pkg/codegen/dotnet/utilities.go @@ -15,8 +15,11 @@ package dotnet import ( + "regexp" "strings" "unicode" + + "github.com/pkg/errors" ) // isReservedWord returns true if s is a C# reserved word as per @@ -78,3 +81,45 @@ func makeValidIdentifier(name string) string { func propertyName(name string) string { return makeValidIdentifier(Title(name)) } + +func makeSafeEnumName(name string) (string, error) { + safeName := name + + // If the name is one illegal character, replace it. + if len(safeName) == 1 && !isLegalIdentifierStart(rune(safeName[0])) { + enumReplacer := strings.NewReplacer( + "0", "Zero", + "1", "One", + "2", "Two", + "3", "Three", + "4", "Four", + "5", "Five", + "6", "Six", + "7", "Seven", + "8", "Eight", + "9", "Nine", + "*", "Asterisk", + ) + + safeName = enumReplacer.Replace(safeName) + + // If it's still an illegal character (we weren't able to find a replacement), return an error. + if !isLegalIdentifierStart(rune(safeName[0])) { + return "", errors.Errorf("enum name %s is not a valid identifier", safeName) + } + } + + // Capitalize and make a valid identifier. + safeName = strings.Title(makeValidIdentifier(safeName)) + + // If there are multiple underscores in a row, replace with one. + regex := regexp.MustCompile(`_+`) + safeName = regex.ReplaceAllString(safeName, "_") + + // "Equals" conflicts with a method on the EnumType struct, change it to EqualsValue. + if safeName == "Equals" { + safeName = "EqualsValue" + } + + return safeName, nil +} diff --git a/pkg/codegen/nodejs/gen.go b/pkg/codegen/nodejs/gen.go index 230dac29f..05cdee5dc 100644 --- a/pkg/codegen/nodejs/gen.go +++ b/pkg/codegen/nodejs/gen.go @@ -1111,19 +1111,21 @@ func (mod *modContext) genNamespace(w io.Writer, ns *namespace, input bool, leve } } -func makeSafeEnumName(name string) string { - return makeValidIdentifier(title(name)) -} - -func (mod *modContext) genEnum(w io.Writer, enum *schema.EnumType) { +func (mod *modContext) genEnum(w io.Writer, enum *schema.EnumType) error { indent := " " enumName := tokenToName(enum.Token) fmt.Fprintf(w, "export const %s = {\n", enumName) for _, e := range enum.Elements { + // If the enum doesn't have a name, set the value as the name. if e.Name == "" { e.Name = fmt.Sprintf("%v", e.Value) } - e.Name = makeSafeEnumName(e.Name) + safeName, err := makeSafeEnumName(e.Name) + if err != nil { + return err + } + e.Name = safeName + printComment(w, e.Comment, e.DeprecationMessage, indent) fmt.Fprintf(w, "%s%s: ", indent, e.Name) if val, ok := e.Value.(string); ok { @@ -1137,6 +1139,7 @@ func (mod *modContext) genEnum(w io.Writer, enum *schema.EnumType) { printComment(w, enum.Comment, "", "") fmt.Fprintf(w, "export type %[1]s = (typeof %[1]s)[keyof typeof %[1]s];\n", enumName) + return nil } type fs map[string][]byte @@ -1249,7 +1252,10 @@ func (mod *modContext) gen(fs fs) error { buffer := &bytes.Buffer{} mod.genHeader(buffer, []string{}, nil) - mod.genEnums(buffer, mod.enums) + err := mod.genEnums(buffer, mod.enums) + if err != nil { + return err + } var fileName string if modDir == "" { @@ -1378,7 +1384,7 @@ func (mod *modContext) hasEnums() bool { return false } -func (mod *modContext) genEnums(buffer *bytes.Buffer, enums []*schema.EnumType) { +func (mod *modContext) genEnums(buffer *bytes.Buffer, enums []*schema.EnumType) error { if len(mod.children) > 0 { children := codegen.NewStringSet() @@ -1402,12 +1408,16 @@ func (mod *modContext) genEnums(buffer *bytes.Buffer, enums []*schema.EnumType) if len(enums) > 0 { fmt.Fprintf(buffer, "\n") for i, enum := range enums { - mod.genEnum(buffer, enum) + err := mod.genEnum(buffer, enum) + if err != nil { + return err + } if i != len(enums)-1 { fmt.Fprintf(buffer, "\n") } } } + return nil } // genPackageMetadata generates all the non-code metadata required by a Pulumi package. diff --git a/pkg/codegen/nodejs/gen_test.go b/pkg/codegen/nodejs/gen_test.go index 06766b47f..c74e292f2 100644 --- a/pkg/codegen/nodejs/gen_test.go +++ b/pkg/codegen/nodejs/gen_test.go @@ -59,19 +59,30 @@ func TestMakeSafeEnumName(t *testing.T) { tests := []struct { input string expected string + wantErr bool }{ - {"red", "Red"}, - {"snake_cased_name", "Snake_cased_name"}, - {"8", "_8"}, - {"Microsoft-Windows-Shell-Startup", "Microsoft_Windows_Shell_Startup"}, - {"Microsoft.Batch", "Microsoft_Batch"}, - {"readonly", "Readonly"}, - {"SystemAssigned, UserAssigned", "SystemAssigned__UserAssigned"}, - {"Dev(NoSLA)_Standard_D11_v2", "Dev_NoSLA__Standard_D11_v2"}, - {"Standard_E8as_v4+1TB_PS", "Standard_E8as_v4_1TB_PS"}, + {"red", "Red", false}, + {"snake_cased_name", "Snake_cased_name", false}, + {"+", "", true}, + {"*", "Asterisk", false}, + {"0", "Zero", false}, + {"Microsoft-Windows-Shell-Startup", "Microsoft_Windows_Shell_Startup", false}, + {"Microsoft.Batch", "Microsoft_Batch", false}, + {"readonly", "Readonly", false}, + {"SystemAssigned, UserAssigned", "SystemAssigned_UserAssigned", false}, + {"Dev(NoSLA)_Standard_D11_v2", "Dev_NoSLA_Standard_D11_v2", false}, + {"Standard_E8as_v4+1TB_PS", "Standard_E8as_v4_1TB_PS", false}, } for _, tt := range tests { - result := makeSafeEnumName(tt.input) - assert.Equal(t, tt.expected, result) + t.Run(tt.input, func(t *testing.T) { + got, err := makeSafeEnumName(tt.input) + if (err != nil) != tt.wantErr { + t.Errorf("makeSafeEnumName() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.expected { + t.Errorf("makeSafeEnumName() got = %v, want %v", got, tt.expected) + } + }) } } diff --git a/pkg/codegen/nodejs/utilities.go b/pkg/codegen/nodejs/utilities.go index 21deb4fea..d72b4dcb9 100644 --- a/pkg/codegen/nodejs/utilities.go +++ b/pkg/codegen/nodejs/utilities.go @@ -16,8 +16,11 @@ package nodejs import ( "io" + "regexp" "strings" "unicode" + + "github.com/pkg/errors" ) // isReservedWord returns true if s is a reserved word as per ECMA-262. @@ -98,3 +101,40 @@ func makeValidIdentifier(name string) string { } return name } + +func makeSafeEnumName(name string) (string, error) { + safeName := name + + // If the name is one illegal character, replace it. + if len(safeName) == 1 && !isLegalIdentifierStart(rune(safeName[0])) { + enumReplacer := strings.NewReplacer( + "0", "Zero", + "1", "One", + "2", "Two", + "3", "Three", + "4", "Four", + "5", "Five", + "6", "Six", + "7", "Seven", + "8", "Eight", + "9", "Nine", + "*", "Asterisk", + ) + + safeName = enumReplacer.Replace(safeName) + + // If it's still an illegal character (we weren't able to find a replacement), return an error. + if !isLegalIdentifierStart(rune(safeName[0])) { + return "", errors.Errorf("enum name %s is not a valid identifier", safeName) + } + } + + // Capitalize and make a valid identifier. + safeName = makeValidIdentifier(title(safeName)) + + // If there are multiple underscores in a row, replace with one. + regex := regexp.MustCompile(`_+`) + safeName = regex.ReplaceAllString(safeName, "_") + + return safeName, nil +}