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
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 @@
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 @@
}
// 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 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
// 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 @@
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" {