buf icon indicating copy to clipboard operation
buf copied to clipboard

`buf breaking` incorrectly detecting a breaking change when increasing a reserved range

Open matteofiordarancio opened this issue 3 years ago • 2 comments

Say we define an enum:

enum FeedsVersion
{
    reserved 1 to 3;
    FEED_IDL_VERSION = 4;
}

if we want to increase the feed version, we'd do:

enum FeedsVersion
{
    reserved 1 to 4;
    FEED_IDL_VERSION = 5;
}

but buf breaking will complain saying: proto/feed.proto:6:1:Previously present reserved range "[1,2]" on enum "FeedsVersion" was deleted.

Instead, if we write:

enum FeedsVersion
{
    reserved 1, 2, 3;
    FEED_IDL_VERSION = 4;
}

And then we want to increase the feed version:

enum FeedsVersion
{
    reserved 1, 2, 3, 4;
    FEEDS_IDL_VERSION = 5;
}

Then buf breaking does not complain. I think the first case is a bug. Semantically it's the same thing.

buf version: 1.5.0 protobuf: proto2

matteofiordarancio avatar Sep 08 '22 09:09 matteofiordarancio

You're correct that buf should deal with ranges better, I believe this limitation is currently documented on https://docs.buf.build/breaking/rules#reserved_message_no_delete

Note that moving from reserved 3 to 6; to reserved 2 to 8;, for example, would technically be fine, however Buf still fails in this case - making sure all ranges are covered is truly a pain, we have no other excuse. We could fix this in the future. For now, just do reserved 3 to 6, 2, 7 to 8; to pass breaking change detection.

I'll mark this as a bug and see if we can get improved semantics here fixed for a future version.

robbertvanginkel avatar Sep 16 '22 13:09 robbertvanginkel

Did miss this one in the documentation, thanks for pointing this out

matteofiordarancio avatar Sep 16 '22 14:09 matteofiordarancio