Skip to content

Commit

Permalink
Validate target type for all option fields, not just features (#279)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jhump authored Apr 16, 2024
1 parent 71b8e4c commit e8799f7
Show file tree
Hide file tree
Showing 2 changed files with 284 additions and 207 deletions.
7 changes: 4 additions & 3 deletions linker/linker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1379,7 +1379,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 @@ -2444,7 +2445,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 @@ -2457,7 +2458,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 e8799f7

Please sign in to comment.