Skip to content

Commit

Permalink
Remove checking not_contains containing prefix/suffix/contains in buf…
Browse files Browse the repository at this point in the history
… lint for protovalidate (#2597)

This PR updates `PROTOVALIDATE` rule to stop checking whether
`not_contains` contains `prefix`/`suffix`/`contains`, which does not
imply invalid rules.

For example, `not_contains: "foo_bar", contains: "foo"` is valid because
string `foo_` passes this check.
  • Loading branch information
oliversun9 authored Nov 16, 2023
1 parent 8b898f1 commit e7eef03
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 14 deletions.
1 change: 0 additions & 1 deletion private/bufpkg/bufcheck/buflint/buflint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,6 @@ func TestRunProtovalidateRules(t *testing.T) {
bufanalysistesting.NewFileAnnotation(t, "string.proto", 122, 5, 122, 47, "PROTOVALIDATE"),
bufanalysistesting.NewFileAnnotation(t, "string.proto", 130, 5, 130, 46, "PROTOVALIDATE"),
bufanalysistesting.NewFileAnnotation(t, "string.proto", 133, 5, 133, 45, "PROTOVALIDATE"),
bufanalysistesting.NewFileAnnotation(t, "string.proto", 142, 5, 142, 47, "PROTOVALIDATE"),
bufanalysistesting.NewFileAnnotation(t, "timestamp.proto", 57, 5, 60, 6, "PROTOVALIDATE"),
bufanalysistesting.NewFileAnnotation(t, "timestamp.proto", 61, 5, 64, 6, "PROTOVALIDATE"),
bufanalysistesting.NewFileAnnotation(t, "timestamp.proto", 68, 5, 71, 6, "PROTOVALIDATE"),
Expand Down
13 changes: 0 additions & 13 deletions private/bufpkg/bufcheck/buflint/internal/buflintvalidate/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,19 +517,6 @@ func checkStringRules(adder *adder, stringRules *validate.StringRules) error {
*stringRules.NotContains,
)
}
if stringRules.NotContains != nil && strings.Contains(*stringRules.NotContains, substring) {
adder.addForPathf(
[]int32{stringRulesFieldNumber, substringFieldNumber},
"Field %q has a %s (%q) containing its %s (%q). It is impossible for a string to contain %q without containing %q.",
adder.fieldName(),
adder.getFieldRuleName(stringRulesFieldNumber, notContainsFieldNumberInStringRules),
*stringRules.NotContains,
substringField.name,
substring,
substring,
*stringRules.NotContains,
)
}
}
if stringRules.Pattern != nil {
if _, err := regexp.Compile(*stringRules.Pattern); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ message StringTest {
(buf.validate.field).string.prefix = "foo",
(buf.validate.field).string.not_contains = "bar"
];
// this is valid: for example, "foo_" contains "foo" without containing "bar_foo".
string not_contains_conatins_prefix = 31 [
(buf.validate.field).string.not_contains = "bar_foo",
(buf.validate.field).string.prefix = "foo"
Expand Down

0 comments on commit e7eef03

Please sign in to comment.