buf
buf copied to clipboard
bufbuild
Missing breaking rule for adding into required enum
According to docs adding a value into required enum is a breaking change
A second issue with required fields appears when someone adds a value to an enum. In this case, the unrecognized enum value is treated as if it were missing, which also causes the required value check to fail.
Running buf breaking does not detect it. Could you please extend the rules to include it?
Can you provide a before and after example of what you think buf breaking should detect?
Hi, here is an example:
enum Protocol {
SOME_PROTOCOL = 0;
}
message SomeMessage {
required Protocol protocol = 1;
}
changed to
enum Protocol {
SOME_PROTOCOL = 0;
NEW_PROTOCOL = 1
}
message SomeMessage {
required Protocol protocol = 1;
}
should be detected as a breaking change.
@m-handy That sounds like an interesting problem. Curious: what language are you using?
I've experienced a similar type of problem before with an exhaustive switch statements breaking Kotlin code when a new enum is added. We ended up writing a linter for a default case requirement to ensure the APIs were able to add new enum values.
Is it correct to say that this particular issue is only really relevant in the context of proto2 syntax?
The exhaustiveness checking is another issue. It certainly has merit, but in the exhaustiveness checking case, it would need to be considered a "break" even if the enum was never used as the type of a required field; I believe this is only a source-level break, whereas the required issue is a wire break if I'm understanding the documentation correctly.
Closing this as wontfix for now - this would imply that buf breaking needs to perform a type of checking that is non-transitive. We always want to make sure that buf breaking implies that if going from commit 1 -> 2 is non-breaking, and commit 2 -> 3 is non-breaking, then going from commit 1 -> 3 is non-breaking. However, implementing this feature would require us to take into account the usage of enums in fields, ie is an enum used as a required field. Consider this case:
- commit 1: enum has one value
- commit 2: enum has two values
- commit 3: enum starts being used as a required field
In this scenario, neither 1 -> 2 or 2 -> 3 would result in a breaking change that is detected, but 1 -> 3 would.
Regardless, even if we did this, this is only in the context of a given module and its dependencies, we cannot say that this applies to anyone depending on a module that implements an enum.
@bufdev Thanks for your comment. I haven't thought about it from such perspective, but what you wrote makes sense.