Skip to content

Commit

Permalink
Tweak configuration of JSON format rules for buf breaking (#2978)
Browse files Browse the repository at this point in the history 
While writing docs up for these I realized a couple of things:
1. They really needed to be included in the `WIRE_JSON` category (for
likely obvious reasons).
2. It is backwards-compatible to go from "best effort" to "allow", just
not vice versa.
  • Loading branch information
@jhump
jhump authored May 13, 2024
1 parent fb0e9ac commit fc1ec89
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 18 deletions.
12 changes: 6 additions & 6 deletions private/buf/cmd/buf/buf_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,6 @@ ENUM_NO_DELETE FILE
FILE_NO_DELETE FILE Checks that files are not deleted.
MESSAGE_NO_DELETE FILE Checks that messages are not deleted from a given file.
SERVICE_NO_DELETE FILE Checks that services are not deleted from a given file.
ENUM_SAME_JSON_FORMAT FILE, PACKAGE Checks that enums have the same JSON format support.
ENUM_SAME_TYPE FILE, PACKAGE Checks that enums have the same type (open vs closed).
ENUM_VALUE_NO_DELETE FILE, PACKAGE Checks that enum values are not deleted from a given enum.
EXTENSION_MESSAGE_NO_DELETE FILE, PACKAGE Checks that extension ranges are not deleted from a given message.
Expand Down Expand Up @@ -732,12 +731,13 @@ FILE_SAME_RUBY_PACKAGE FILE, PACKAGE
FILE_SAME_SWIFT_PREFIX FILE, PACKAGE Checks that files have the same value for the swift_prefix option.
FILE_SAME_SYNTAX FILE, PACKAGE Checks that files have the same syntax.
MESSAGE_NO_REMOVE_STANDARD_DESCRIPTOR_ACCESSOR FILE, PACKAGE Checks that messages do not change the no_standard_descriptor_accessor option from false or unset to true.
MESSAGE_SAME_JSON_FORMAT FILE, PACKAGE Checks that messages have the same JSON format support.
ONEOF_NO_DELETE FILE, PACKAGE Checks that oneofs are not deleted from a given message.
RPC_NO_DELETE FILE, PACKAGE Checks that rpcs are not deleted from a given service.
ENUM_SAME_JSON_FORMAT FILE, PACKAGE, WIRE_JSON Checks that enums have the same JSON format support.
ENUM_VALUE_SAME_NAME FILE, PACKAGE, WIRE_JSON Checks that enum values have the same name.
FIELD_SAME_JSON_NAME FILE, PACKAGE, WIRE_JSON Checks that fields have the same value for the json_name option.
FIELD_SAME_NAME FILE, PACKAGE, WIRE_JSON Checks that fields have the same names in a given message.
MESSAGE_SAME_JSON_FORMAT FILE, PACKAGE, WIRE_JSON Checks that messages have the same JSON format support.
FIELD_SAME_ONEOF FILE, PACKAGE, WIRE_JSON, WIRE Checks that fields have the same oneofs in a given message.
FILE_SAME_PACKAGE FILE, PACKAGE, WIRE_JSON, WIRE Checks that files have the same package.
MESSAGE_SAME_MESSAGE_SET_WIRE_FORMAT FILE, PACKAGE, WIRE_JSON, WIRE Checks that messages have the same value for the message_set_wire_format option.
Expand Down Expand Up @@ -783,7 +783,6 @@ FILE_NO_DELETE FILE
FILE_SAME_PACKAGE FILE Checks that files have the same package.
MESSAGE_NO_DELETE FILE Checks that messages are not deleted from a given file.
SERVICE_NO_DELETE FILE Checks that services are not deleted from a given file.
ENUM_SAME_JSON_FORMAT FILE, PACKAGE Checks that enums have the same JSON format support.
ENUM_SAME_TYPE FILE, PACKAGE Checks that enums have the same type (open vs closed).
ENUM_VALUE_NO_DELETE FILE, PACKAGE Checks that enum values are not deleted from a given enum.
EXTENSION_MESSAGE_NO_DELETE FILE, PACKAGE Checks that extension ranges are not deleted from a given message.
Expand All @@ -810,12 +809,13 @@ FILE_SAME_RUBY_PACKAGE FILE, PACKAGE
FILE_SAME_SWIFT_PREFIX FILE, PACKAGE Checks that files have the same value for the swift_prefix option.
FILE_SAME_SYNTAX FILE, PACKAGE Checks that files have the same syntax.
MESSAGE_NO_REMOVE_STANDARD_DESCRIPTOR_ACCESSOR FILE, PACKAGE Checks that messages do not change the no_standard_descriptor_accessor option from false or unset to true.
MESSAGE_SAME_JSON_FORMAT FILE, PACKAGE Checks that messages have the same JSON format support.
ONEOF_NO_DELETE FILE, PACKAGE Checks that oneofs are not deleted from a given message.
RPC_NO_DELETE FILE, PACKAGE Checks that rpcs are not deleted from a given service.
ENUM_SAME_JSON_FORMAT FILE, PACKAGE, WIRE_JSON Checks that enums have the same JSON format support.
ENUM_VALUE_SAME_NAME FILE, PACKAGE, WIRE_JSON Checks that enum values have the same name.
FIELD_SAME_JSON_NAME FILE, PACKAGE, WIRE_JSON Checks that fields have the same value for the json_name option.
FIELD_SAME_NAME FILE, PACKAGE, WIRE_JSON Checks that fields have the same names in a given message.
MESSAGE_SAME_JSON_FORMAT FILE, PACKAGE, WIRE_JSON Checks that messages have the same JSON format support.
FIELD_SAME_CARDINALITY FILE, PACKAGE, WIRE_JSON, WIRE Checks that fields have the same cardinalities in a given message.
FIELD_SAME_ONEOF FILE, PACKAGE, WIRE_JSON, WIRE Checks that fields have the same oneofs in a given message.
FIELD_SAME_TYPE FILE, PACKAGE, WIRE_JSON, WIRE Checks that fields have the same types in a given message.
Expand Down Expand Up @@ -858,7 +858,6 @@ EXTENSION_NO_DELETE FILE
FILE_NO_DELETE FILE Checks that files are not deleted.
MESSAGE_NO_DELETE FILE Checks that messages are not deleted from a given file.
SERVICE_NO_DELETE FILE Checks that services are not deleted from a given file.
ENUM_SAME_JSON_FORMAT FILE, PACKAGE Checks that enums have the same JSON format support.
ENUM_SAME_TYPE FILE, PACKAGE Checks that enums have the same type (open vs closed).
ENUM_VALUE_NO_DELETE FILE, PACKAGE Checks that enum values are not deleted from a given enum.
EXTENSION_MESSAGE_NO_DELETE FILE, PACKAGE Checks that extension ranges are not deleted from a given message.
Expand Down Expand Up @@ -887,12 +886,13 @@ FILE_SAME_RUBY_PACKAGE FILE, PACKAGE
FILE_SAME_SWIFT_PREFIX FILE, PACKAGE Checks that files have the same value for the swift_prefix option.
FILE_SAME_SYNTAX FILE, PACKAGE Checks that files have the same syntax.
MESSAGE_NO_REMOVE_STANDARD_DESCRIPTOR_ACCESSOR FILE, PACKAGE Checks that messages do not change the no_standard_descriptor_accessor option from false or unset to true.
MESSAGE_SAME_JSON_FORMAT FILE, PACKAGE Checks that messages have the same JSON format support.
ONEOF_NO_DELETE FILE, PACKAGE Checks that oneofs are not deleted from a given message.
RPC_NO_DELETE FILE, PACKAGE Checks that rpcs are not deleted from a given service.
ENUM_SAME_JSON_FORMAT FILE, PACKAGE, WIRE_JSON Checks that enums have the same JSON format support.
ENUM_VALUE_SAME_NAME FILE, PACKAGE, WIRE_JSON Checks that enum values have the same name.
FIELD_SAME_JSON_NAME FILE, PACKAGE, WIRE_JSON Checks that fields have the same value for the json_name option.
FIELD_SAME_NAME FILE, PACKAGE, WIRE_JSON Checks that fields have the same names in a given message.
MESSAGE_SAME_JSON_FORMAT FILE, PACKAGE, WIRE_JSON Checks that messages have the same JSON format support.
FIELD_SAME_DEFAULT FILE, PACKAGE, WIRE_JSON, WIRE Checks that fields have the same default value, if a default is specified.
FIELD_SAME_ONEOF FILE, PACKAGE, WIRE_JSON, WIRE Checks that fields have the same oneofs in a given message.
FILE_SAME_PACKAGE FILE, PACKAGE, WIRE_JSON, WIRE Checks that files have the same package.
Expand Down
10 changes: 0 additions & 10 deletions private/bufpkg/bufcheck/bufbreaking/bufbreaking_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,8 @@ func TestRunBreakingEnumSameJSONFormat(t *testing.T) {
"breaking_enum_same_json_format",
bufanalysistesting.NewFileAnnotation(t, "1.proto", 9, 1, 11, 2, "ENUM_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "1.proto", 13, 1, 15, 2, "ENUM_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "2.proto", 5, 1, 7, 2, "ENUM_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "2.proto", 17, 1, 19, 2, "ENUM_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "3.proto", 5, 1, 7, 2, "ENUM_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "3.proto", 17, 1, 19, 2, "ENUM_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "3.proto", 26, 3, 26, 52, "ENUM_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "4.proto", 11, 1, 13, 2, "ENUM_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "4.proto", 29, 3, 29, 52, "ENUM_SAME_JSON_FORMAT"),
)
}

Expand Down Expand Up @@ -963,13 +958,8 @@ func TestRunBreakingMessageSameJSONFormat(t *testing.T) {
"breaking_message_same_json_format",
bufanalysistesting.NewFileAnnotation(t, "1.proto", 9, 1, 11, 2, "MESSAGE_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "1.proto", 13, 1, 15, 2, "MESSAGE_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "2.proto", 5, 1, 7, 2, "MESSAGE_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "2.proto", 17, 1, 19, 2, "MESSAGE_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "3.proto", 5, 1, 7, 2, "MESSAGE_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "3.proto", 17, 1, 19, 2, "MESSAGE_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "3.proto", 26, 3, 26, 52, "MESSAGE_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "4.proto", 11, 1, 13, 2, "MESSAGE_SAME_JSON_FORMAT"),
bufanalysistesting.NewFileAnnotation(t, "4.proto", 29, 3, 29, 52, "MESSAGE_SAME_JSON_FORMAT"),
)
}

Expand Down
4 changes: 2 additions & 2 deletions private/bufpkg/bufcheck/bufbreaking/internal/bufbreakingcheck/bufbreakingcheck.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func checkEnumSameJSONFormat(
return fmt.Errorf("unable to resolve value of %s feature: %w", featureField.Name(), err)
}
jsonFormat := descriptorpb.FeatureSet_JsonFormat(val.Enum())
if previousJSONFormat != jsonFormat {
if previousJSONFormat == descriptorpb.FeatureSet_ALLOW && jsonFormat != descriptorpb.FeatureSet_ALLOW {
add(
enum,
nil,
Expand Down Expand Up @@ -1162,7 +1162,7 @@ func checkMessageSameJSONFormat(
return fmt.Errorf("unable to resolve value of %s feature: %w", featureField.Name(), err)
}
jsonFormat := descriptorpb.FeatureSet_JsonFormat(val.Enum())
if previousJSONFormat != jsonFormat {
if previousJSONFormat == descriptorpb.FeatureSet_ALLOW && jsonFormat != descriptorpb.FeatureSet_ALLOW {
add(
message,
nil,
Expand Down
2 changes: 2 additions & 0 deletions private/bufpkg/bufcheck/bufbreaking/internal/bufbreakingv1/vars.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ var (
"ENUM_SAME_JSON_FORMAT": {
"FILE",
"PACKAGE",
"WIRE_JSON",
},
"ENUM_SAME_TYPE": {
"FILE",
Expand Down Expand Up @@ -281,6 +282,7 @@ var (
"MESSAGE_SAME_JSON_FORMAT": {
"FILE",
"PACKAGE",
"WIRE_JSON",
},
"MESSAGE_SAME_MESSAGE_SET_WIRE_FORMAT": {
"FILE",
Expand Down
2 changes: 2 additions & 0 deletions private/bufpkg/bufcheck/bufbreaking/internal/bufbreakingv1beta1/vars.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ var (
"ENUM_SAME_JSON_FORMAT": {
"FILE",
"PACKAGE",
"WIRE_JSON",
},
"ENUM_SAME_TYPE": {
"FILE",
Expand Down Expand Up @@ -266,6 +267,7 @@ var (
"MESSAGE_SAME_JSON_FORMAT": {
"FILE",
"PACKAGE",
"WIRE_JSON",
},
"MESSAGE_SAME_MESSAGE_SET_WIRE_FORMAT": {
"FILE",
Expand Down
2 changes: 2 additions & 0 deletions private/bufpkg/bufcheck/bufbreaking/internal/bufbreakingv2/vars.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ var (
"ENUM_SAME_JSON_FORMAT": {
"FILE",
"PACKAGE",
"WIRE_JSON",
},
"ENUM_SAME_TYPE": {
"FILE",
Expand Down Expand Up @@ -285,6 +286,7 @@ var (
"MESSAGE_SAME_JSON_FORMAT": {
"FILE",
"PACKAGE",
"WIRE_JSON",
},
"MESSAGE_SAME_MESSAGE_SET_WIRE_FORMAT": {
"FILE",
Expand Down

0 comments on commit fc1ec89

Please sign in to comment.