Introspect message fields if the field type changes to see if the actual message types are breaking for WIRE, WIRE_JSON

[ParkMyCar]

When detecting breaking changes the linter emits a false positive when changing the type of a mesasge. The format of our buf.yaml is:

version: v1
breaking:
  use:
    - WIRE

Example

Say you have the protobuf message:

message Error {
  oneof kind {
    google.protobuf.Empty overflow = 1;
  }
}

and you change it to:

message Error {
  message OverflowValue {
    string value = 1;
  }

  oneof kind {
    OverflowValue overflow = 1;
  }
}

this is a wire compatible change, but buf indicates that it is not.

google.protobuf.Empty is a message with zero fields, and OverflowValue is a message with one optional field. For a type originally serialized with the first version, and deserialized with the second, you'll get an OverflowValue message where the inner value field is an empty string.

Apologies if this is a duplicate issue, I searched but could not find anything 🙂

[srikrsna-buf]

The rule doesn't mention anything regarding messages: https://buf.build/docs/breaking/rules/#field_wire_compatible_type. But I don't see why this shouldn't be possible for WIRE compatibility. Thank you for reporting!

[bufdev]

buf does not do introspection on actual message field types - buf assumes that changing the type of a field is a breaking change. We could in theory go further than this specifically for WIRE and WIRE_JSON, by saying if a given field changes between message types, running breaking change detection between them, but to be candid I don't know if we'll get to this anytime soon - generally, changing your core message type for a field is not advisable.

[ParkMyCar]

Thanks for the quick responses! 🙂

...but to be candid I don't know if we'll get to this anytime soon - generally, changing your core message type for a field is not advisable.

That's totally fair! I just wanted to call this out because it was a change that is technically wire compatible, but buf was telling me it's not

[bufdev]

We're going to close this as stale, as I don't think this is an update we're going to be able to staff unfortunately. Apologies.

[bufdev]

Actually on second thought, this might be something we want to do. But going to keep this issue closed in favor of #2318.