goprotobuf: Fix handling of empty string defaults for 'string' and 'bytes' fields.

This is a bit of an edge case. All other valid defaults appear as
non-empty strings, but we need to distinguish [default=""] for string/bytes
to uphold the SetDefaults contract.

LGTM=djd
R=djd
CC=golang-codereviews
https://codereview.appspot.com/125100044

6 files changed, 35 insertions, 17 deletions

diff --git a/proto/all_test.go b/proto/all_test.go
index 1ac9587..b2a0191 100644
--- a/proto/all_test.go
+++ b/proto/all_test.go
@@ -1387,10 +1387,11 @@ func TestAllSetDefaults(t *testing.T) {
 		F_Pinf:    Float32(float32(math.Inf(1))),
 		F_Ninf:    Float32(float32(math.Inf(-1))),
 		F_Nan:     Float32(1.7),
+		StrZero:   String(""),
 	}
 	SetDefaults(m)
 	if !Equal(m, expected) {
-		t.Errorf(" got %v\nwant %v", m, expected)
+		t.Errorf("SetDefaults failed\n got %v\nwant %v", m, expected)
 	}
 }
 
diff --git a/proto/lib.go b/proto/lib.go
index e22084e..46a4416 100644
--- a/proto/lib.go
+++ b/proto/lib.go
@@ -667,7 +667,7 @@ func buildDefaultMessage(t reflect.Type) (dm defaultMessage) {
 		}
 
 		// scalar fields without defaults
-		if prop.Default == "" {
+		if !prop.HasDefault {
 			dm.scalars = append(dm.scalars, sf)
 			continue
 		}
diff --git a/proto/properties.go b/proto/properties.go
index da352f4..8513ae7 100644
--- a/proto/properties.go
+++ b/proto/properties.go
@@ -145,17 +145,19 @@ func (sp *StructProperties) Swap(i, j int) { sp.order[i], sp.order[j] = sp.order
 
 // Properties represents the protocol-specific behavior of a single struct field.
 type Properties struct {
-	Name       string // name of the field, for error messages
-	OrigName   string // original name before protocol compiler (always set)
-	Wire       string
-	WireType   int
-	Tag        int
-	Required   bool
-	Optional   bool
-	Repeated   bool
-	Packed     bool   // relevant for repeated primitives only
-	Enum       string // set for enum types only
+	Name     string // name of the field, for error messages
+	OrigName string // original name before protocol compiler (always set)
+	Wire     string
+	WireType int
+	Tag      int
+	Required bool
+	Optional bool
+	Repeated bool
+	Packed   bool   // relevant for repeated primitives only
+	Enum     string // set for enum types only
+
 	Default    string // default value
+	HasDefault bool   // whether an explicit default was provided
 	def_uint64 uint64
 
 	enc           encoder
@@ -201,7 +203,7 @@ func (p *Properties) String() string {
 	if len(p.Enum) > 0 {
 		s += ",enum=" + p.Enum
 	}
-	if len(p.Default) > 0 {
+	if p.HasDefault {
 		s += ",def=" + p.Default
 	}
 	return s
@@ -273,6 +275,7 @@ func (p *Properties) Parse(s string) {
 		case strings.HasPrefix(f, "enum="):
 			p.Enum = f[5:]
 		case strings.HasPrefix(f, "def="):
+			p.HasDefault = true
 			p.Default = f[4:] // rest of string
 			if i+1 < len(fields) {
 				// Commas aren't escaped, and def is always last.
diff --git a/proto/testdata/test.pb.go b/proto/testdata/test.pb.go
index 9ff2d1b..5a9a603 100644
--- a/proto/testdata/test.pb.go
+++ b/proto/testdata/test.pb.go
@@ -1529,8 +1529,10 @@ type Defaults struct {
 	F_Ninf *float32 `protobuf:"fixed32,16,opt,def=-inf" json:"F_Ninf,omitempty"`
 	F_Nan  *float32 `protobuf:"fixed32,17,opt,def=nan" json:"F_Nan,omitempty"`
 	// Sub-message.
-	Sub              *SubDefaults `protobuf:"bytes,18,opt,name=sub" json:"sub,omitempty"`
-	XXX_unrecognized []byte       `json:"-"`
+	Sub *SubDefaults `protobuf:"bytes,18,opt,name=sub" json:"sub,omitempty"`
+	// Redundant but explicit defaults.
+	StrZero          *string `protobuf:"bytes,19,opt,name=str_zero,def=" json:"str_zero,omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
 }
 
 func (m *Defaults) Reset()         { *m = Defaults{} }
@@ -1684,6 +1686,13 @@ func (m *Defaults) GetSub() *SubDefaults {
 	return nil
 }
 
+func (m *Defaults) GetStrZero() string {
+	if m != nil && m.StrZero != nil {
+		return *m.StrZero
+	}
+	return ""
+}
+
 type SubDefaults struct {
 	N                *int64 `protobuf:"varint,1,opt,name=n,def=7" json:"n,omitempty"`
 	XXX_unrecognized []byte `json:"-"`
diff --git a/proto/testdata/test.proto b/proto/testdata/test.proto
index f1fbf2b..9c0c4c8 100644
--- a/proto/testdata/test.proto
+++ b/proto/testdata/test.proto
@@ -381,6 +381,9 @@ message Defaults {
 
   // Sub-message.
   optional SubDefaults sub = 18;
+
+  // Redundant but explicit defaults.
+  optional string str_zero = 19 [default=""];
 }
 
 message SubDefaults {
diff --git a/protoc-gen-go/generator/generator.go b/protoc-gen-go/generator/generator.go
index 0ed6175..4f99191 100644
--- a/protoc-gen-go/generator/generator.go
+++ b/protoc-gen-go/generator/generator.go
@@ -1278,8 +1278,10 @@ func (g *Generator) goTag(field *descriptor.FieldDescriptorProto, wiretype strin
 	case isRepeated(field):
 		optrepreq = "rep"
 	}
-	defaultValue := field.GetDefaultValue()
-	if defaultValue != "" {
+	var defaultValue string
+	if dv := field.DefaultValue; dv != nil { // set means an explicit default
+		defaultValue = *dv
+		// Some types need tweaking.
 		switch *field.Type {
 		case descriptor.FieldDescriptorProto_TYPE_BOOL:
 			if defaultValue == "true" {