CrowdStrike / csproto

CrowdStrike's Protocol Buffers library
MIT License
122 stars 13 forks source link

Rename Size() method #125

Open biosvs opened 1 year ago

biosvs commented 1 year ago

Rename Size() method

Rename Size() to something that has less chance of name collision (like PB_Size()), or add new argument/option to change this name.

Description

The word "Size" too ubiquitous. If proto field is called "Size", plugin generates uncompilable code (because of Size redeclaration).

dylan-bourque commented 1 year ago

Unfortunately, that would be a massively breaking change for us. We actually support this scenario in the same way that gogo/protobuf did, though, by allowing consumers to indicate "special" field names that will get an underscore appended using the specialname generator option.

protoc .... fastmarshal_out=paths=source_relative,specialname=Size:. ....

You can specify the option multiple times if you have more than one "special" name.

There was/is code in github.com/gogo/protobuf that checked for a Size() int method (emitted by protoc-gen-gogofaster if I remember correctly) and, since we were migrating from that runtime having this method be Size() made the most sense at the time for interoperability.

For what it's worth, we have thousands of Protobuf message types defined across our organization and we only had to update a handful (less than 20) because they had a field named Size (by either renaming the field or adding the specialname=Size generator option)

biosvs commented 1 year ago

Thanks for response!

Yeah, I understand that at this point it's already impossible to rename this method.

As for specialname, I tried to use it, it renames field in code generated by csProto plugin, but code generated by protobuf-go remains the same. Maybe I missed something?

Proto file:

syntax = "proto3";

option go_package = "protobuf-benchmarks/idl";

message MyMessage {
  string size = 1;
}

Cmd:

protoc \
                --go_out=./idl/ \
                --fastmarshal_out=apiversion=v2,specialname=Size:./idl \
                --proto_path=./idl/ \
                idl/test.proto

Result:

// test.pb.go

type MyMessage struct {
// [...]

    Size string `protobuf:"bytes,1,opt,name=size,proto3" json:"size,omitempty"`
}

// test.pb.fm.go
func (m *MyMessage) Size() int {
// [...]
    if l = len(m.Size_); l > 0 {
// [...]