From 9f6d51e40288bdcd293695f8bc19d921be4f902b Mon Sep 17 00:00:00 2001 From: Daniel Carbone Date: Fri, 12 Aug 2022 08:29:31 -0500 Subject: [PATCH] Initial pass at #2321 --- codegen/templates/templates.go | 112 ++++++++++++++ codegen/templates/templates_test.go | 63 +++++++- docs/content/reference/name-collision.md | 188 +++++++++++++++++++++++ plugin/modelgen/models.gotpl | 30 ++-- 4 files changed, 377 insertions(+), 16 deletions(-) create mode 100644 docs/content/reference/name-collision.md diff --git a/codegen/templates/templates.go b/codegen/templates/templates.go index ab39c08eecf..802fe243535 100644 --- a/codegen/templates/templates.go +++ b/codegen/templates/templates.go @@ -8,10 +8,12 @@ import ( "os" "path/filepath" "reflect" + "regexp" "runtime" "sort" "strconv" "strings" + "sync" "text/template" "unicode" @@ -202,6 +204,7 @@ func Funcs() template.FuncMap { "lookupImport": CurrentImports.Lookup, "go": ToGo, "goPrivate": ToGoPrivate, + "goModelName": ToGoModelName, "add": func(a, b int) int { return a + b }, @@ -291,6 +294,115 @@ func Call(p *types.Func) string { return pkg + p.Name() } +var ( + modelNamesMu sync.Mutex + modelNames = make(map[string]string, 0) +) + +func resetModelNames() { + modelNamesMu.Lock() + defer modelNamesMu.Unlock() + modelNames = make(map[string]string, 0) +} + +func buildGoModelNameKey(parts []string) string { + const sep = ":" + return strings.Join(parts, sep) +} + +func ToGoModelName(parts ...string) string { + modelNamesMu.Lock() + defer modelNamesMu.Unlock() + + var ( + goNameKey string + partLen int + + nameExists = func(n string) bool { + for _, v := range modelNames { + if n == v { + return true + } + } + return false + } + + applyToGo = func(parts []string) string { + var out string + for _, p := range parts { + out = fmt.Sprintf("%s%s", out, ToGo(p)) + } + return out + } + + applyValidGoName = func(parts []string) string { + var out string + for _, p := range parts { + out = fmt.Sprintf("%s%s", out, ValidGoName(p)) + } + return out + } + ) + + // build key for this entity + goNameKey = buildGoModelNameKey(parts) + + // determine if we've seen this entity before, and reuse if so + if goName, ok := modelNames[goNameKey]; ok { + return goName + } + + // attempt first pass + + // test first pass + if goName := applyToGo(parts); !nameExists(goName) { + modelNames[goNameKey] = goName + return goName + } + + // determine number of parts + partLen = len(parts) + + // if there is only 1 part, append incrementing number until no conflict + if partLen == 1 { + base := applyToGo(parts) + for i := 0; ; i++ { + tmp := fmt.Sprintf("%s%d", base, i) + if !nameExists(tmp) { + modelNames[goNameKey] = tmp + return tmp + } + } + } + + // best effort "pretty" name + for i := partLen - 1; i >= 1; i-- { + tmp := fmt.Sprintf("%s%s", applyToGo(parts[0:i]), applyValidGoName(parts[i:])) + if !nameExists(tmp) { + modelNames[goNameKey] = tmp + return tmp + } + } + + // finally, fallback to just adding an incrementing number + base := applyToGo(parts) + for i := 0; ; i++ { + tmp := fmt.Sprintf("%s%d", base, i) + if !nameExists(tmp) { + modelNames[goNameKey] = tmp + return tmp + } + } +} + +var ( + goNameRe = regexp.MustCompile("[^a-zA-Z0-9_]") +) + +func ValidGoName(in string) string { + return goNameRe.ReplaceAllString(in, "_") +} + func ToGo(name string) string { if name == "_" { return "_" diff --git a/codegen/templates/templates_test.go b/codegen/templates/templates_test.go index ed69b1c7c7b..4fa0eafa12f 100644 --- a/codegen/templates/templates_test.go +++ b/codegen/templates/templates_test.go @@ -2,6 +2,7 @@ package templates import ( "embed" + "fmt" "os" "path/filepath" "testing" @@ -74,9 +75,69 @@ func TestToGoPrivate(t *testing.T) { require.Equal(t, "_", ToGoPrivate("_")) } +func TestToGoModelName(t *testing.T) { + type aTest struct { + input [][]string + expected []string + } + + theTests := []aTest{ + { + input: [][]string{{"MyValue"}}, + expected: []string{"MyValue"}, + }, + { + input: [][]string{{"MyValue"}, {"myValue"}}, + expected: []string{"MyValue", "MyValue0"}, + }, + { + input: [][]string{{"MyValue"}, {"YourValue"}}, + expected: []string{"MyValue", "YourValue"}, + }, + { + input: [][]string{{"MyEnumName", "Value"}}, + expected: []string{"MyEnumNameValue"}, + }, + { + input: [][]string{{"MyEnumName", "Value"}, {"MyEnumName", "value"}}, + expected: []string{"MyEnumNameValue", "MyEnumNamevalue"}, + }, + { + input: [][]string{{"MyEnumName", "value"}, {"MyEnumName", "Value"}}, + expected: []string{"MyEnumNameValue", "MyEnumNameValue0"}, + }, + { + input: [][]string{{"MyEnumName", "Value"}, {"MyEnumName", "value"}, {"MyEnumName", "vALue"}, {"MyEnumName", "VALue"}}, + expected: []string{"MyEnumNameValue", "MyEnumNamevalue", "MyEnumNameVALue", "MyEnumNameVALue0"}, + }, + { + input: [][]string{{"MyEnumName", "TitleValue"}, {"MyEnumName", "title_value"}, {"MyEnumName", "title_Value"}, {"MyEnumName", "Title_Value"}}, + expected: []string{"MyEnumNameTitleValue", "MyEnumNametitle_value", "MyEnumNametitle_Value", "MyEnumNameTitle_Value"}, + }, + { + input: [][]string{{"MyEnumName", "TitleValue", "OtherValue"}}, + expected: []string{"MyEnumNameTitleValueOtherValue"}, + }, + { + input: [][]string{{"MyEnumName", "TitleValue", "OtherValue"}, {"MyEnumName", "title_value", "OtherValue"}}, + expected: []string{"MyEnumNameTitleValueOtherValue", "MyEnumNametitle_valueOtherValue"}, + }, + } + + for ti, at := range theTests { + resetModelNames() + t.Run(fmt.Sprintf("modelname-%d", ti), func(t *testing.T) { + at := at + for i, n := range at.input { + require.Equal(t, at.expected[i], ToGoModelName(n...)) + } + }) + } +} + func Test_wordWalker(t *testing.T) { helper := func(str string) []*wordInfo { - resultList := []*wordInfo{} + var resultList []*wordInfo wordWalker(str, func(info *wordInfo) { resultList = append(resultList, info) }) diff --git a/docs/content/reference/name-collision.md b/docs/content/reference/name-collision.md new file mode 100644 index 00000000000..d9201f20f49 --- /dev/null +++ b/docs/content/reference/name-collision.md @@ -0,0 +1,188 @@ +--- +title: Handling type naming collisions +description: Examples of logic used to avoid type name collision +linkTitle: Name Collision +menu: { main: { parent: 'reference', weight: 10 }} +--- + +While most generated Golang types must have unique names by virtue of being based on their GraphQL `type` counterpart, +which themselves must be unique, there are a few edge scenarios where conflicts can occur. This document describes +how those collisions are handled. + +## Enum Constants + +Enum type generation is a prime example of where naming collisions can occur, as we build the const names per value +as a composite of the Enum name and each individual value. + +### Example Problem + +Currently, enum types are transposed as such: + +```graphql +enum MyEnum { + value1 + value2 + value3 + value4 +} +``` + +Which will result in the following Golang: + +```go +type MyEnum string + +const ( + MyEnumValue1 MyEnum = "value1" + MyEnumValue2 MyEnum = "value2" + MyEnumValue3 MyEnum = "value3" + MyEnumValue4 MyEnum = "value4" +) +``` + +However, those above enum values are just strings. What if you encounter a scenario where the following is +necessary: + +```graphql +enum MyEnum { + value1 + value2 + value3 + value4 + Value4 + Value_4 +} +``` + +The `Value4` and `Value_4` enum values cannot be directly transposed into the same "pretty" naming convention as their +resulting constant names would conflict with the name for `value4`, as so: + +```go +type MyEnum string + +const ( + MyEnumValue1 MyEnum = "value1" + MyEnumValue2 MyEnum = "value2" + MyEnumValue3 MyEnum = "value3" + MyEnumValue4 MyEnum = "value4" + MyEnumValue4 MyEnum = "Value4" + MyEnumValue4 MyEnum = "Value_4" +) +``` + +Which immediately leads to compilation errors as we now have three constants with the same name, but different values. + +### Resolution + +1. Store each name generated as part of a run for later comparison +2. Try pretty approach first. Use if no conflicts +3. If non-composite name, append integer to end of name, starting at 0 and going to `math.MaxInt` +4. If composite name, in reverse order, the pieces of the name have a less opinionated converter applied +5. If all else fails, append integer to end of name, starting at 0 and going to `math.MaxInt` + +The first step to produce a name that does not conflict with an existing name succeeds. + +## Examples + +### Example A +GraphQL: +```graphql +enum MyEnum { + Value + value + TitleValue + title_value +} +``` +Go: +```go +type MyEnum string + +const ( + MyEnumValue MyEnum = "Value" + MyEnumvalue MyEnum = "value" + MyEnumTitleValue MyEnum = "TitleValue" + MyEnumtitle_value MyEnum = "title_value" +) +``` + +### Example B +GraphQL: +```graphql +enum MyEnum { + TitleValue + title_value + title_Value + Title_Value +} +``` +Go: +```go +type MyEnum string +const ( + MyEnumTitleValue MyEnum = "TitleValue" + MyEnumtitle_value MyEnum = "title_value" + MyEnumtitle_Value MyEnum = "title_Value" + MyEnumTitle_Value MyEnum = "Title_Value" +) +``` + +### Example C +GraphQL: +```graphql +enum MyEnum { + value + Value +} +``` +Go: +```go +type MyEnum string +const ( + MyEnumValue = "value" + MyEnumValue0 = "Value" +) +``` + +## Warning + +Name collision resolution is handled per-name, as they come in. If you change the order of an enum, you could very +well end up with the same constant resolving to a different value. + +Lets mutate [Example C](#example-c): + +### Example C - State A +GraphQL: +```graphql +enum MyEnum { + value + Value +} +``` +Go: +```go +type MyEnum string +const ( + MyEnumValue = "value" + MyEnumValue0 = "Value" +) +``` + +### Example C - State B +GraphQL: +```graphql +enum MyEnum { + Value + value +} +``` +Go: +```go +type MyEnum string +const ( + MyEnumValue = "Value" + MyEnumValue0 = "value" +) +``` + +Notice how the constant names are the same, but the value that each applies to has changed. diff --git a/plugin/modelgen/models.gotpl b/plugin/modelgen/models.gotpl index 6966e0af971..7ad43bef1bd 100644 --- a/plugin/modelgen/models.gotpl +++ b/plugin/modelgen/models.gotpl @@ -14,11 +14,11 @@ {{- range $model := .Interfaces }} {{ with .Description }} {{.|prefixLines "// "}} {{ end }} - type {{.Name|go }} interface { + type {{ goModelName .Name }} interface { {{- range $impl := .Implements }} - Is{{ $impl|go }}() + Is{{ goModelName $impl }}() {{- end }} - Is{{.Name|go }}() + Is{{ goModelName .Name }}() {{- range $field := .Fields }} {{- with .Description }} {{.|prefixLines "// "}} @@ -30,7 +30,7 @@ {{ range $model := .Models }} {{with .Description }} {{.|prefixLines "// "}} {{end}} - type {{ .Name|go }} struct { + type {{ goModelName .Name }} struct { {{- range $field := .Fields }} {{- with .Description }} {{.|prefixLines "// "}} @@ -40,7 +40,7 @@ } {{ range .Implements }} - func ({{ $model.Name|go }}) Is{{ .|go }}() {} + func ({{ goModelName $model.Name }}) Is{{ goModelName . }}() {} {{- with getInterfaceByName . }} {{- range .Fields }} {{- with .Description }} @@ -54,48 +54,48 @@ {{ range $enum := .Enums }} {{ with .Description }} {{.|prefixLines "// "}} {{end}} - type {{.Name|go }} string + type {{ goModelName .Name }} string const ( {{- range $value := .Values}} {{- with .Description}} {{.|prefixLines "// "}} {{- end}} - {{ $enum.Name|go }}{{ .Name|go }} {{$enum.Name|go }} = {{.Name|quote}} + {{ goModelName $enum.Name .Name }} {{ goModelName $enum.Name }} = {{ .Name|quote }} {{- end }} ) - var All{{.Name|go }} = []{{ .Name|go }}{ + var All{{ goModelName .Name }} = []{{ goModelName .Name }}{ {{- range $value := .Values}} - {{$enum.Name|go }}{{ .Name|go }}, + {{ goModelName $enum.Name .Name }}, {{- end }} } - func (e {{.Name|go }}) IsValid() bool { + func (e {{ goModelName .Name }}) IsValid() bool { switch e { - case {{ range $index, $element := .Values}}{{if $index}},{{end}}{{ $enum.Name|go }}{{ $element.Name|go }}{{end}}: + case {{ range $index, $element := .Values}}{{if $index}},{{end}}{{ goModelName $enum.Name $element.Name }}{{end}}: return true } return false } - func (e {{.Name|go }}) String() string { + func (e {{ goModelName .Name }}) String() string { return string(e) } - func (e *{{.Name|go }}) UnmarshalGQL(v interface{}) error { + func (e *{{ goModelName .Name }}) UnmarshalGQL(v interface{}) error { str, ok := v.(string) if !ok { return fmt.Errorf("enums must be strings") } - *e = {{ .Name|go }}(str) + *e = {{ goModelName .Name }}(str) if !e.IsValid() { return fmt.Errorf("%s is not a valid {{ .Name }}", str) } return nil } - func (e {{.Name|go }}) MarshalGQL(w io.Writer) { + func (e {{ goModelName .Name }}) MarshalGQL(w io.Writer) { fmt.Fprint(w, strconv.Quote(e.String())) }