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

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

Open
bafonins opened this issue Feb 7, 2020 · 0 comments
Open

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

bafonins opened this issue Feb 7, 2020 · 0 comments

Comments

@bafonins
Copy link

bafonins commented Feb 7, 2020

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

@bafonins bafonins added this to the Next Up milestone Feb 7, 2020
@htdvisser htdvisser removed this from the Next Up milestone Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants