diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a6f5319..aaab4cf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,31 +12,22 @@ jobs: steps: - uses: actions/setup-go@v2 with: - go-version: '^1.18' + go-version: '^1.19' - uses: actions/checkout@v2 - name: Cache protobuf build id: protocache - uses: actions/cache@v2 + uses: actions/cache@v3 with: - path: _vendor/protobuf-3.20.0 - key: protobuf-3.20.0 + path: _vendor/protobuf-21.12 + key: protobuf-21.12 - name: Compile protobuf if: steps.protocache.outputs.cache-hit != 'true' run: | sudo apt-get install -y autoconf automake libtool curl make g++ unzip - - mkdir -p _vendor - curl -#fsSL https://github.com/protocolbuffers/protobuf/releases/download/v3.20.0/protobuf-all-3.20.0.tar.gz | tar -xzvf - -C _vendor - - cd _vendor/protobuf-3.20.0 - ./configure - make - make -C conformance - ls -l src/ - ls -l conformance/ + ./protobuf.sh - run: make install && go mod tidy && go mod verify - run: git --no-pager diff --exit-code diff --git a/features/marshal/marshalto.go b/features/marshal/marshalto.go index 362b996..245f3a5 100644 --- a/features/marshal/marshalto.go +++ b/features/marshal/marshalto.go @@ -46,9 +46,8 @@ type marshal struct { var _ generator.FeatureGenerator = (*marshal)(nil) func (p *marshal) GenerateFile(file *protogen.File) bool { - proto3 := file.Desc.Syntax() == protoreflect.Proto3 for _, message := range file.Messages { - p.message(proto3, message) + p.message(message) } return p.once } @@ -130,9 +129,9 @@ func (p *marshal) mapField(kvField *protogen.Field, varName string) { } } -func (p *marshal) field(proto3, oneof bool, numGen *counter, field *protogen.Field) { +func (p *marshal) field(oneof bool, numGen *counter, field *protogen.Field) { fieldname := field.GoName - nullable := field.Message != nil || (field.Oneof != nil && field.Oneof.Desc.IsSynthetic()) || (!proto3 && !oneof) + nullable := field.Message != nil || (!oneof && field.Desc.HasPresence()) repeated := field.Desc.Cardinality() == protoreflect.Repeated if repeated { p.P(`if len(m.`, fieldname, `) > 0 {`) @@ -433,7 +432,7 @@ func (p *marshal) field(proto3, oneof bool, numGen *counter, field *protogen.Fie p.encodeVarint(`len(`, val, `)`) p.encodeKey(fieldNumber, wireType) p.P(`}`) - } else if !oneof && proto3 { + } else if !oneof && !field.Desc.HasPresence() { p.P(`if len(m.`, fieldname, `) > 0 {`) p.P(`i -= len(m.`, fieldname, `)`) p.P(`copy(dAtA[i:], m.`, fieldname, `)`) @@ -569,9 +568,9 @@ func (p *marshal) methodMarshal() string { } } -func (p *marshal) message(proto3 bool, message *protogen.Message) { +func (p *marshal) message(message *protogen.Message) { for _, nested := range message.Messages { - p.message(proto3, nested) + p.message(nested) } if message.Desc.IsMapEntry() { @@ -631,7 +630,7 @@ func (p *marshal) message(proto3 bool, message *protogen.Message) { field := message.Fields[i] oneof := field.Oneof != nil && !field.Oneof.Desc.IsSynthetic() if !oneof { - p.field(proto3, false, &numGen, field) + p.field(false, &numGen, field) } else { p.P(`if msg, ok := m.`, field.Oneof.GoName, `.(*`, field.GoIdent.GoName, `); ok {`) marshalForwardOneOf("msg") @@ -663,7 +662,7 @@ func (p *marshal) message(proto3 bool, message *protogen.Message) { field := message.Fields[i] oneof := field.Oneof != nil && !field.Oneof.Desc.IsSynthetic() if !oneof { - p.field(proto3, false, &numGen, field) + p.field(false, &numGen, field) } } } @@ -685,7 +684,7 @@ func (p *marshal) message(proto3 bool, message *protogen.Message) { p.P(``) p.P(`func (m *`, ccTypeName, `) `, p.methodMarshalToSizedBuffer(), `(dAtA []byte) (int, error) {`) p.P(`i := len(dAtA)`) - p.field(proto3, true, &numGen, field) + p.field(true, &numGen, field) p.P(`return len(dAtA) - i, nil`) p.P(`}`) } diff --git a/features/size/size.go b/features/size/size.go index 44d0fa1..21fa307 100644 --- a/features/size/size.go +++ b/features/size/size.go @@ -34,9 +34,8 @@ func (p *size) Name() string { } func (p *size) GenerateFile(file *protogen.File) bool { - proto3 := file.Desc.Syntax() == protoreflect.Proto3 for _, message := range file.Messages { - p.message(proto3, message) + p.message(message) } return p.once @@ -72,9 +71,9 @@ func (p *size) messageSize(varName, sizeName string, message *protogen.Message) } } -func (p *size) field(proto3, oneof bool, field *protogen.Field, sizeName string) { +func (p *size) field(oneof bool, field *protogen.Field, sizeName string) { fieldname := field.GoName - nullable := field.Message != nil || (field.Oneof != nil && field.Oneof.Desc.IsSynthetic()) || (!proto3 && !oneof) + nullable := field.Message != nil || (!oneof && field.Desc.HasPresence()) repeated := field.Desc.Cardinality() == protoreflect.Repeated if repeated { p.P(`if len(m.`, fieldname, `) > 0 {`) @@ -239,7 +238,7 @@ func (p *size) field(proto3, oneof bool, field *protogen.Field, sizeName string) p.P(`l = len(b)`) p.P(`n+=`, strconv.Itoa(key), `+l+sov(uint64(l))`) p.P(`}`) - } else if !oneof && proto3 { + } else if !oneof && !field.Desc.HasPresence() { p.P(`l=len(m.`, fieldname, `)`) p.P(`if l > 0 {`) p.P(`n+=`, strconv.Itoa(key), `+l+sov(uint64(l))`) @@ -276,9 +275,9 @@ func (p *size) field(proto3, oneof bool, field *protogen.Field, sizeName string) } } -func (p *size) message(proto3 bool, message *protogen.Message) { +func (p *size) message(message *protogen.Message) { for _, nested := range message.Messages { - p.message(proto3, nested) + p.message(nested) } if message.Desc.IsMapEntry() { @@ -300,7 +299,7 @@ func (p *size) message(proto3 bool, message *protogen.Message) { for _, field := range message.Fields { oneof := field.Oneof != nil && !field.Oneof.Desc.IsSynthetic() if !oneof { - p.field(proto3, false, field, sizeName) + p.field(false, field, sizeName) } else { fieldname := field.Oneof.GoName if _, ok := oneofs[fieldname]; !ok { @@ -327,7 +326,7 @@ func (p *size) message(proto3 bool, message *protogen.Message) { p.P(`}`) p.P(`var l int`) p.P(`_ = l`) - p.field(proto3, true, field, sizeName) + p.field(true, field, sizeName) p.P(`return n`) p.P(`}`) } diff --git a/protobuf.sh b/protobuf.sh index ca0afee..5f843f7 100755 --- a/protobuf.sh +++ b/protobuf.sh @@ -11,14 +11,17 @@ if [ -f "$PROTOBUF_PATH/protoc" ]; then exit 0 fi +mkdir -p _vendor curl -sS -L -o "$ROOT/_vendor/pb.tar.gz" http://github.com/protocolbuffers/protobuf/releases/download/v${PROTOBUF_VERSION}/protobuf-all-${PROTOBUF_VERSION}.tar.gz cd "$ROOT/_vendor" tar zxf pb.tar.gz cd protobuf-${PROTOBUF_VERSION} -./configure --quiet +./configure make -cd conformance/ && make +make -C conformance echo "Dowloaded and compiled protobuf $PROTOBUF_VERSION to $PROTOBUF_PATH" +ls -l src/ +ls -l conformance/ diff --git a/testproto/proto3opt/opt_vtproto.pb.go b/testproto/proto3opt/opt_vtproto.pb.go index 9fd1382..06c1fa5 100644 --- a/testproto/proto3opt/opt_vtproto.pb.go +++ b/testproto/proto3opt/opt_vtproto.pb.go @@ -204,13 +204,11 @@ func (m *OptionalFieldInProto3) MarshalToSizedBufferVT(dAtA []byte) (int, error) dAtA[i] = 0x80 } if m.OptionalBytes != nil { - if len(m.OptionalBytes) > 0 { - i -= len(m.OptionalBytes) - copy(dAtA[i:], m.OptionalBytes) - i = encodeVarint(dAtA, i, uint64(len(m.OptionalBytes))) - i-- - dAtA[i] = 0x7a - } + i -= len(m.OptionalBytes) + copy(dAtA[i:], m.OptionalBytes) + i = encodeVarint(dAtA, i, uint64(len(m.OptionalBytes))) + i-- + dAtA[i] = 0x7a } if m.OptionalString != nil { i -= len(*m.OptionalString) @@ -347,13 +345,11 @@ func (m *OptionalFieldInProto3) MarshalToSizedBufferVTStrict(dAtA []byte) (int, dAtA[i] = 0x80 } if m.OptionalBytes != nil { - if len(m.OptionalBytes) > 0 { - i -= len(m.OptionalBytes) - copy(dAtA[i:], m.OptionalBytes) - i = encodeVarint(dAtA, i, uint64(len(m.OptionalBytes))) - i-- - dAtA[i] = 0x7a - } + i -= len(m.OptionalBytes) + copy(dAtA[i:], m.OptionalBytes) + i = encodeVarint(dAtA, i, uint64(len(m.OptionalBytes))) + i-- + dAtA[i] = 0x7a } if m.OptionalString != nil { i -= len(*m.OptionalString) @@ -492,9 +488,7 @@ func (m *OptionalFieldInProto3) SizeVT() (n int) { } if m.OptionalBytes != nil { l = len(m.OptionalBytes) - if l > 0 { - n += 1 + l + sov(uint64(l)) - } + n += 1 + l + sov(uint64(l)) } if m.OptionalEnum != nil { n += 2 + sov(uint64(*m.OptionalEnum)) diff --git a/testproto/proto3opt/proto3opt_test.go b/testproto/proto3opt/proto3opt_test.go new file mode 100644 index 0000000..90e058b --- /dev/null +++ b/testproto/proto3opt/proto3opt_test.go @@ -0,0 +1,30 @@ +package proto3opt + +import ( + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" +) + +func TestEmptyBytesMarshalling(t *testing.T) { + a := &OptionalFieldInProto3{ + OptionalBytes: nil, + } + b := &OptionalFieldInProto3{ + OptionalBytes: []byte{}, + } + + type Message interface { + proto.Message + MarshalVT() ([]byte, error) + } + + for _, msg := range []Message{a, b} { + vt, err := msg.MarshalVT() + require.NoError(t, err) + goog, err := proto.Marshal(msg) + require.NoError(t, err) + require.Equal(t, goog, vt) + } +}