Skip to content

Commit 08df2b3

Browse files
Update PROTOVALIDATE lint rule to check unenforceable required rules (#4309)
These are unenforceable, because `repeated` and `map` fields do not track field presence. Ref: https://protovalidate.com/schemas/standard-rules/#field-presence Fixes #3812.
1 parent 4899175 commit 08df2b3

File tree

5 files changed

+38
-1
lines changed

5 files changed

+38
-1
lines changed

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44

55
- Add `buf registry policy {commit,create,delete,info,label,settings}` commands to manage BSR policies.
66
- Add LSP deprecate code action to add the deprecated option on types and symbols.
7-
- Fix import LSP document link to not include `import` keyword
7+
- Fix import LSP document link to not include `import` keyword.
8+
- Update `PROTOVALIDATE` lint rule to support checking unenforceable `required` rules on `repeated.items`, `map.keys` and `map.values`.
89

910
## [v1.64.0] - 2026-01-19
1011

private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/field.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,15 @@ func checkRepeatedRules(
453453
)
454454
}
455455
itemAdder := baseAdder.cloneWithNewBasePath(repeatedRulesFieldNumber, itemsFieldNumberInRepeatedRules)
456+
if repeatedRules.Items != nil && repeatedRules.Items.GetRequired() {
457+
itemAdder.addForPathf(
458+
[]int32{requiredFieldNumber},
459+
"Field %q has %s on repeated item rules, which is unenforceable. The %s constraint cannot be applied to individual items in a repeated field.",
460+
baseAdder.fieldName(),
461+
itemAdder.getFieldRuleName(requiredFieldNumber),
462+
itemAdder.getFieldRuleName(requiredFieldNumber),
463+
)
464+
}
456465
return checkRulesForField(itemAdder, repeatedRules.Items, containingMessageDescriptor, nil, fieldDescriptor, false, extensionTypeResolver)
457466
}
458467

@@ -493,11 +502,29 @@ func checkMapRules(
493502
)
494503
}
495504
keyAdder := baseAdder.cloneWithNewBasePath(mapRulesFieldNumber, keysFieldNumberInMapRules)
505+
if mapRules.Keys != nil && mapRules.Keys.GetRequired() {
506+
keyAdder.addForPathf(
507+
[]int32{requiredFieldNumber},
508+
"Field %q has %s on map key rules, which is unenforceable. The %s constraint cannot be applied to map keys.",
509+
baseAdder.fieldName(),
510+
keyAdder.getFieldRuleName(requiredFieldNumber),
511+
keyAdder.getFieldRuleName(requiredFieldNumber),
512+
)
513+
}
496514
err := checkRulesForField(keyAdder, mapRules.Keys, containingMessageDescriptor, fieldDescriptor, fieldDescriptor.MapKey(), false, extensionTypeResolver)
497515
if err != nil {
498516
return err
499517
}
500518
valueAdder := baseAdder.cloneWithNewBasePath(mapRulesFieldNumber, valuesFieldNumberInMapRules)
519+
if mapRules.Values != nil && mapRules.Values.GetRequired() {
520+
valueAdder.addForPathf(
521+
[]int32{requiredFieldNumber},
522+
"Field %q has %s on map value rules, which is unenforceable. The %s constraint cannot be applied to map values.",
523+
baseAdder.fieldName(),
524+
valueAdder.getFieldRuleName(requiredFieldNumber),
525+
valueAdder.getFieldRuleName(requiredFieldNumber),
526+
)
527+
}
501528
return checkRulesForField(valueAdder, mapRules.Values, containingMessageDescriptor, fieldDescriptor, fieldDescriptor.MapValue(), false, extensionTypeResolver)
502529
}
503530

private/bufpkg/bufcheck/lint_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -682,6 +682,8 @@ func TestRunProtovalidate(t *testing.T) {
682682
bufanalysistesting.NewFileAnnotation(t, "map.proto", 83, 5, 83, 55, "PROTOVALIDATE"),
683683
bufanalysistesting.NewFileAnnotation(t, "map.proto", 86, 5, 86, 57, "PROTOVALIDATE"),
684684
bufanalysistesting.NewFileAnnotation(t, "map.proto", 87, 5, 87, 55, "PROTOVALIDATE"),
685+
bufanalysistesting.NewFileAnnotation(t, "map.proto", 90, 53, 90, 98, "PROTOVALIDATE"),
686+
bufanalysistesting.NewFileAnnotation(t, "map.proto", 92, 55, 92, 102, "PROTOVALIDATE"),
685687
bufanalysistesting.NewFileAnnotation(t, "number.proto", 20, 5, 20, 42, "PROTOVALIDATE"),
686688
bufanalysistesting.NewFileAnnotation(t, "number.proto", 25, 5, 25, 43, "PROTOVALIDATE"),
687689
bufanalysistesting.NewFileAnnotation(t, "number.proto", 28, 5, 28, 43, "PROTOVALIDATE"),
@@ -740,6 +742,7 @@ func TestRunProtovalidate(t *testing.T) {
740742
bufanalysistesting.NewFileAnnotation(t, "repeated.proto", 55, 42, 55, 76, "PROTOVALIDATE"),
741743
bufanalysistesting.NewFileAnnotation(t, "repeated.proto", 65, 5, 65, 62, "PROTOVALIDATE"),
742744
bufanalysistesting.NewFileAnnotation(t, "repeated.proto", 68, 55, 68, 110, "PROTOVALIDATE"),
745+
bufanalysistesting.NewFileAnnotation(t, "repeated.proto", 70, 51, 70, 102, "PROTOVALIDATE"),
743746
bufanalysistesting.NewFileAnnotation(t, "string.proto", 31, 5, 31, 46, "PROTOVALIDATE"),
744747
bufanalysistesting.NewFileAnnotation(t, "string.proto", 36, 5, 36, 44, "PROTOVALIDATE"),
745748
bufanalysistesting.NewFileAnnotation(t, "string.proto", 41, 5, 41, 44, "PROTOVALIDATE"),

private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/map.proto

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,8 @@ message MapTest {
8686
(buf.validate.field).map.keys.string.example = "bad",
8787
(buf.validate.field).map.values.int64.example = -1
8888
];
89+
// invalid: required cannot be set on map keys
90+
map<string, int32> invalid_required_on_keys = 19 [(buf.validate.field).map.keys.required = true];
91+
// invalid: required cannot be set on map values
92+
map<int32, string> invalid_required_on_values = 20 [(buf.validate.field).map.values.required = true];
8993
}

private/bufpkg/bufcheck/testdata/lint/protovalidate/proto/repeated.proto

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,6 @@ message RepeatedTest {
6666
];
6767
repeated string valid_no_constraint_example = 21 [(buf.validate.field).repeated.items.string.example = "proto"];
6868
repeated string invalid_no_constraint_example = 22 [(buf.validate.field).repeated.items.bool.example = true];
69+
// invalid: required cannot be set on repeated items
70+
repeated string invalid_required_on_items = 23 [(buf.validate.field).repeated.items.required = true];
6971
}

0 commit comments

Comments
 (0)