Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename Size() method #125

Open
biosvs opened this issue Aug 21, 2023 · 2 comments
Open

Rename Size() method #125

biosvs opened this issue Aug 21, 2023 · 2 comments
Labels
enhancement New feature or request triage Issues that need to be reviewed by maintainers

Comments

@biosvs
Copy link
Contributor

biosvs commented Aug 21, 2023

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).

@biosvs biosvs added enhancement New feature or request triage Issues that need to be reviewed by maintainers labels Aug 21, 2023
@dylan-bourque
Copy link

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
Copy link
Contributor Author

biosvs commented Aug 22, 2023

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 {
// [...]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triage Issues that need to be reviewed by maintainers
Projects
None yet
Development

No branches or pull requests

2 participants