buf icon indicating copy to clipboard operation
buf copied to clipboard

buf breaking - Better UX around proto3 optional fields

Open rodaine opened this issue 2 years ago • 1 comments

Running breaking change detection with FILE rules, if a proto3 optional field is moved into a oneof, buf breaking correctly identifies the incompatible change, however the error messages can be a bit confusing unless someone is aware of the implementation of optionals (i.e., via a synthetic oneof).

For a message:

message Msg {
  optional int32 foo = 1;
}

... changing it to:

message Msg {
  oneof bar {
    int32 foo = 1;
  }
}

... results in the following error messages:

Error: Previously present oneof "_foo" on message "Msg" was deleted.
Error: Field "1" on message "Msg" moved from oneof "_foo" to oneof "bar".

A better message could combine the two and should probably mention the optional keyword rather than the synthetic _foo oneof:

Error: Field "1" on message "Msg" changed from optional to a member of oneof "bar"

Related: https://github.com/bufbuild/buf/issues/1906

rodaine avatar Aug 22 '23 17:08 rodaine

When we do get to this, make sure that https://github.com/bufbuild/buf/issues/1906 is addressed as well.

bufdev avatar Jun 13 '24 21:06 bufdev