Skip to content

Commit

Permalink
Validate target type for all option fields, not just features (bufbui…
Browse files Browse the repository at this point in the history
…ld#279)

The code to verify that a feature field was only used on particular kind
of element was features-specific. However, in `protoc`, it is not
features-specific and applies to _all_ option fields that have a
`targets` option defined. So this updates protocompile to behave the
same way.

This PR is a bit complicated because I had to change the error handling
of the `*interpreter.fieldValue` method (and the `setOptionField` method
that called it, as well as numerous methods that it used). It previously
either returned a value or an error. If it returned an error, it would
be reported by the caller. But now, these places handle the errors, to
allow reporting of multiple errors for the same option value. This change
in behavior is the reason for most of the churn/change in the code.

The change to just the target type validation is actually a net simplification.

(cherry picked from commit e8799f7)
  • Loading branch information
jhump authored and kralicky committed Jun 8, 2024
1 parent 7a798fa commit 4da3ba8
Show file tree
Hide file tree
Showing 2 changed files with 286 additions and 209 deletions.
7 changes: 4 additions & 3 deletions linker/linker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1294,7 +1294,8 @@ func TestLinkerValidation(t *testing.T) {
};
}`,
},
expectedErr: `foo.proto:9:6: message foo.bar.Baz: option (foo.bar.any): any type references cannot be repeated or mixed with other fields`,
expectedErr: `foo.proto:9:6: message foo.bar.Baz: option (foo.bar.any): any type references cannot be repeated or mixed with other fields` +
` && foo.proto:13:6: message foo.bar.Baz: option (foo.bar.any): any type references cannot be repeated or mixed with other fields`,
},
"failure_scope_type_name": {
input: map[string]string{
Expand Down Expand Up @@ -2310,7 +2311,7 @@ func TestLinkerValidation(t *testing.T) {
}
`,
},
expectedErr: `test.proto:3:27: feature "enum_type" is allowed on [enum,file], not on field`,
expectedErr: `test.proto:3:27: field "google.protobuf.FeatureSet.enum_type" is allowed on [enum,file], not on field`,
},
"failure_editions_feature_on_wrong_target_type_msg_literal": {
input: map[string]string{
Expand All @@ -2323,7 +2324,7 @@ func TestLinkerValidation(t *testing.T) {
}
`,
},
expectedErr: `test.proto:4:5: feature "enum_type" is allowed on [enum,file], not on field`,
expectedErr: `test.proto:4:5: field "google.protobuf.FeatureSet.enum_type" is allowed on [enum,file], not on field`,
},
"failure_proto3_enum_zero_value": {
input: map[string]string{
Expand Down
Loading

0 comments on commit 4da3ba8

Please sign in to comment.