TheThingsIndustries / protoc-gen-fieldmask

Generate field mask utilities from proto files
Apache License 2.0
11 stars 3 forks source link

Do not prefix oneof fields with the oneof type names #35

Open bafonins opened 4 years ago

bafonins commented 4 years ago

Summary

Currently, each oneof field is prefixed with the type name. According to https://developers.google.com/protocol-buffers/docs/proto3#using-oneof

In your generated code, oneof fields have the same getters and setters as regular fields.

and from https://developers.google.com/protocol-buffers/docs/reference/csharp/class/google/protobuf/well-known-types/field-mask

Field masks treat fields in oneofs just as regular fields.

Some background discussed in slack from @rvolosatovs:

So the only reason oneof container name is part of the path is to be able to select the oneof container without knowledge of which is set. E.g. to be able to SetFields on a Message and select the Payload regardless of what the "actual" payload it is. I was not aware of this spec point when implementing this, so I just did what made sense. This can be changed, but we should check that we don't have a use case for selecting "either of oneofs"

Why do we need this?

Comply with the spec

What is already there? What do you see now?

Every oneof field is prefixed with the type name, for example generating field masks from:

message Test2 {
    oneof provider {
      bool test_field = 1;
      string test_field2 = 2;
    };
}

results in the following allowed field masks: provider, provider.test_field and provider.test_field2

What is missing? What do you want to see?

Just test_field and test_field2

Environment

v0.3.3

How do you propose to implement this?

I guess this has something to do with these lines

Can you do this yourself and submit a Pull Request?

@rvolosatovs