Fix field name collisions with methods on generated structs

actgardner · Jan 2, 2021 · d3fd7f0 · d3fd7f0
1 parent 4a5e651
commit d3fd7f0
Show file tree

Hide file tree

Showing 22 changed files with 644 additions and 45 deletions.
diff --git a/v7/generator/flat/templates/record.go b/v7/generator/flat/templates/record.go
@@ -13,9 +13,9 @@ type {{ .Name }} struct {
 {{ range $i, $field := .Fields -}}
 	{{ if ne $field.Doc "" }}// {{ $field.Doc }}{{ end }}
 	{{ if ne $field.Tags "" -}}
-		{{ $field.SimpleName }} {{ $field.Type.GoType }} ` + "`{{ $field.Tags }}`" + `
+		{{ $field.GoName }} {{ $field.Type.GoType }} ` + "`{{ $field.Tags }}`" + `
 	{{ else -}}
-		{{ $field.SimpleName }} {{ $field.Type.GoType }}
+		{{ $field.GoName }} {{ $field.Type.GoType }}
 	{{ end -}}
 {{ end }}
 }

diff --git a/v7/go.mod b/v7/go.mod
@@ -6,7 +6,7 @@ require (
 	github.com/davecgh/go-spew v1.1.1
 	github.com/golang/snappy v0.0.2
 	github.com/linkedin/goavro v2.1.0+incompatible
-	github.com/linkedin/goavro/v2 v2.9.8
+	github.com/linkedin/goavro/v2 v2.10.0
 	github.com/pmezard/go-difflib v1.0.0
 	github.com/stretchr/testify v1.6.1
 	gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 // indirect

diff --git a/v7/go.sum b/v7/go.sum
@@ -14,6 +14,8 @@ github.com/linkedin/goavro/v2 v2.9.7 h1:Vd++Rb/RKcmNJjM0HP/JJFMEWa21eUBVKPYlKehO
 github.com/linkedin/goavro/v2 v2.9.7/go.mod h1:UgQUb2N/pmueQYH9bfqFioWxzYCZXSfF8Jw03O5sjqA=
 github.com/linkedin/goavro/v2 v2.9.8 h1:jN50elxBsGBDGVDEKqUlDuU1cFwJ11K/yrJCBMe/7Wg=
 github.com/linkedin/goavro/v2 v2.9.8/go.mod h1:UgQUb2N/pmueQYH9bfqFioWxzYCZXSfF8Jw03O5sjqA=
+github.com/linkedin/goavro/v2 v2.10.0 h1:eTBIRoInBM88gITGXYtUSqqxLTFXfOsJBiX8ZMW0o4U=
+github.com/linkedin/goavro/v2 v2.10.0/go.mod h1:UgQUb2N/pmueQYH9bfqFioWxzYCZXSfF8Jw03O5sjqA=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=

diff --git a/v7/schema/array.go b/v7/schema/array.go
@@ -97,10 +97,6 @@ func (s *ArrayField) IsReadableBy(f AvroType, visited map[QualifiedName]interfac
 	return false
 }
 
-func (s *ArrayField) SimpleName() string {
-	return s.Name()
-}
-
 func (s *ArrayField) ItemConstructable() string {
 	if constructor, ok := getConstructableForType(s.itemType); ok {
 		return fmt.Sprintf("v = %v\n", constructor.ConstructorMethod())

diff --git a/v7/schema/definition.go b/v7/schema/definition.go
@@ -10,7 +10,6 @@ type Definition interface {
 
 	// A user-friendly name that can be built into a Go string (for unions, mostly)
 	Name() string
-	SimpleName() string
 
 	GoType() string
 

diff --git a/v7/schema/enum.go b/v7/schema/enum.go
@@ -33,10 +33,6 @@ func (e *EnumDefinition) Doc() string {
 	return e.doc
 }
 
-func (e *EnumDefinition) SimpleName() string {
-	return e.name.Name
-}
-
 func (e *EnumDefinition) AvroName() QualifiedName {
 	return e.name
 }

diff --git a/v7/schema/field.go b/v7/schema/field.go
@@ -7,6 +7,30 @@ import (
 	"github.com/actgardner/gogen-avro/v7/generator"
 )
 
+// invalidFieldNames is a list of field names that conflict with hard-coded method names on
+// generated structs. These are converted to `Field_<name>` in the resulting struct to avoid errors.
+var invalidFieldNames = map[string]interface{}{
+	"Schema":               true,
+	"Serialize":            true,
+	"SchemaName":           true,
+	"MarshalJSON":          true,
+	"UnmarshalJSON":        true,
+	"AvroCRC64Fingerprint": true,
+	"SetBoolean":           true,
+	"SetInt":               true,
+	"SetLong":              true,
+	"SetFloat":             true,
+	"SetDouble":            true,
+	"SetBytes":             true,
+	"SetString":            true,
+	"Get":                  true,
+	"SetDefault":           true,
+	"AppendMap":            true,
+	"AppendArray":          true,
+	"NullField":            true,
+	"Finalize":             true,
+}
+
 type Field struct {
 	avroName   string
 	avroType   AvroType
@@ -37,10 +61,6 @@ func (f *Field) Name() string {
 	return f.avroName
 }
 
-func (f *Field) SimpleName() string {
-	return generator.ToPublicSimpleName(f.avroName)
-}
-
 func (f *Field) Index() int {
 	return f.index
 }
@@ -59,7 +79,11 @@ func (f *Field) Tags() string {
 }
 
 func (f *Field) GoName() string {
-	return generator.ToPublicName(f.avroName)
+	name := generator.ToPublicName(f.avroName)
+	if _, ok := invalidFieldNames[name]; ok {
+		return "Field_" + name
+	}
+	return name
 }
 
 // IsSameField checks whether two fields have the same name or any of their aliases are the same, in which case they're the same for purposes of schema evolution

diff --git a/v7/schema/file_root.go b/v7/schema/file_root.go
@@ -18,10 +18,6 @@ func (f *FileRoot) Name() string {
 	return ""
 }
 
-func (f *FileRoot) SimpleName() string {
-	return ""
-}
-
 func (f *FileRoot) GoType() string {
 	return ""
 }

diff --git a/v7/schema/fixed.go b/v7/schema/fixed.go
@@ -26,10 +26,6 @@ func (s *FixedDefinition) Name() string {
 	return s.GoType()
 }
 
-func (s *FixedDefinition) SimpleName() string {
-	return generator.ToPublicSimpleName(s.name.Name)
-}
-
 func (s *FixedDefinition) AvroName() QualifiedName {
 	return s.name
 }

diff --git a/v7/schema/map.go b/v7/schema/map.go
@@ -91,10 +91,6 @@ func (s *MapField) IsReadableBy(f AvroType, visited map[QualifiedName]interface{
 	return false
 }
 
-func (s *MapField) SimpleName() string {
-	return s.Name()
-}
-
 func (s *MapField) ItemConstructable() string {
 	if constructor, ok := getConstructableForType(s.itemType); ok {
 		return fmt.Sprintf("v = %v\n", constructor.ConstructorMethod())

diff --git a/v7/schema/primitive.go b/v7/schema/primitive.go
@@ -30,10 +30,6 @@ func (s *PrimitiveField) Definition(_ map[QualifiedName]interface{}) (interface{
 	return s.definition, nil
 }
 
-func (s *PrimitiveField) SimpleName() string {
-	return s.name
-}
-
 func (s *PrimitiveField) Children() []AvroType {
 	return []AvroType{}
 }

diff --git a/v7/schema/record.go b/v7/schema/record.go
@@ -33,10 +33,6 @@ func (r *RecordDefinition) Name() string {
 	return generator.ToPublicName(r.name.String())
 }
 
-func (r *RecordDefinition) SimpleName() string {
-	return generator.ToPublicName(r.name.Name)
-}
-
 func (r *RecordDefinition) GoType() string {
 	return fmt.Sprintf("*%v", r.Name())
 }

diff --git a/v7/schema/reference.go b/v7/schema/reference.go
@@ -19,10 +19,6 @@ func (s *Reference) Name() string {
 	return s.Def.Name()
 }
 
-func (s *Reference) SimpleName() string {
-	return s.Def.SimpleName()
-}
-
 func (s *Reference) GoType() string {
 	return s.Def.GoType()
 }

diff --git a/v7/schema/union.go b/v7/schema/union.go
@@ -159,10 +159,6 @@ func (s *UnionField) Equals(reader *UnionField) bool {
 	return true
 }
 
-func (s *UnionField) SimpleName() string {
-	return s.GoType()
-}
-
 func (s *UnionField) Children() []AvroType {
 	return s.itemType
 }

diff --git a/v7/test/name-conflict/evolution.json b/v7/test/name-conflict/evolution.json
@@ -0,0 +1,14 @@
+[
+  {
+    "data": {},
+    "expected": {
+      "NewLongField": 1234,
+      "NewStringField": "testdefault",
+      "NewFloatField": 12.34,
+      "NewBytesField": "\u0000\u00ff\u0001\u00fe",
+      "NewDoubleField": 56.78,
+      "NewIntField": 5678,
+      "NewBoolField": true
+    }
+  }
+]
diff --git a/v7/test/name-conflict/fixtures.json b/v7/test/name-conflict/fixtures.json
@@ -0,0 +1,23 @@
+[
+  {
+	"Schema":               true,
+	"Serialize":            true,
+	"SchemaName":           true,
+	"MarshalJSON":          true,
+	"UnmarshalJSON":        true,
+	"AvroCRC64Fingerprint": true,
+	"SetBoolean":           true,
+	"SetInt":               true,
+	"SetLong":              true,
+	"SetFloat":             true,
+	"SetDouble":            true,
+	"SetBytes":             true,
+	"SetString":            true,
+	"Get":                  true,
+	"SetDefault":           true,
+	"AppendMap":            true,
+	"AppendArray":          true,
+	"NullField":            true,
+	"Finalize":             true
+  }
+]
diff --git a/v7/test/name-conflict/generate.go b/v7/test/name-conflict/generate.go
@@ -0,0 +1,3 @@
+package avro
+
+//go:generate $GOPATH/bin/gogen-avro -containers . schema.avsc