diff --git a/rules/aip0203/aip0203.go b/rules/aip0203/aip0203.go index 9b3659066..a2f9ec3db 100644 --- a/rules/aip0203/aip0203.go +++ b/rules/aip0203/aip0203.go @@ -42,15 +42,22 @@ func AddRules(r lint.RuleRegistry) error { // check leading comments of a field and produce a problem // if the comments match the give pattern. -func checkLeadingComments(f *desc.FieldDescriptor, pattern *regexp.Regexp, annotation string) []lint.Problem { - leadingComments := f.GetSourceInfo().GetLeadingComments() - if pattern.MatchString(leadingComments) { - return []lint.Problem{{ - Message: fmt.Sprintf("Use the `google.api.field_behavior` annotation instead of %q in the leading comments. For example, `string name = 1 [(google.api.field_behavior) = %s];`.", pattern.FindString(leadingComments), annotation), - Descriptor: f, - }} +func checkLeadingComments(pattern *regexp.Regexp, annotation string, unless ...*regexp.Regexp) func(*desc.FieldDescriptor) []lint.Problem { + return func(f *desc.FieldDescriptor) []lint.Problem { + leadingComments := f.GetSourceInfo().GetLeadingComments() + for _, ul := range unless { + if ul.MatchString(leadingComments) { + return nil + } + } + if pattern.MatchString(leadingComments) { + return []lint.Problem{{ + Message: fmt.Sprintf("Use the `google.api.field_behavior` annotation instead of %q in the leading comments. For example, `string name = 1 [(google.api.field_behavior) = %s];`.", pattern.FindString(leadingComments), annotation), + Descriptor: f, + }} + } + return nil } - return nil } func withoutFieldBehavior(f *desc.FieldDescriptor) bool { diff --git a/rules/aip0203/immutable.go b/rules/aip0203/immutable.go index 76560e3bc..3882d2da3 100644 --- a/rules/aip0203/immutable.go +++ b/rules/aip0203/immutable.go @@ -23,11 +23,9 @@ import ( ) var immutable = &lint.FieldRule{ - Name: lint.NewRuleName(203, "immutable"), - OnlyIf: withoutImmutableFieldBehavior, - LintField: func(f *desc.FieldDescriptor) []lint.Problem { - return checkLeadingComments(f, immutableRegexp, "IMMUTABLE") - }, + Name: lint.NewRuleName(203, "immutable"), + OnlyIf: withoutImmutableFieldBehavior, + LintField: checkLeadingComments(immutableRegexp, "IMMUTABLE"), } var immutableRegexp = regexp.MustCompile("(?i).*immutable.*") diff --git a/rules/aip0203/input_only.go b/rules/aip0203/input_only.go index f41d5e011..f773ed82b 100644 --- a/rules/aip0203/input_only.go +++ b/rules/aip0203/input_only.go @@ -23,11 +23,9 @@ import ( ) var inputOnly = &lint.FieldRule{ - Name: lint.NewRuleName(203, "input-only"), - OnlyIf: withoutInputOnlyFieldBehavior, - LintField: func(f *desc.FieldDescriptor) []lint.Problem { - return checkLeadingComments(f, inputOnlyRegexp, "INPUT_ONLY") - }, + Name: lint.NewRuleName(203, "input-only"), + OnlyIf: withoutInputOnlyFieldBehavior, + LintField: checkLeadingComments(inputOnlyRegexp, "INPUT_ONLY"), } var inputOnlyRegexp = regexp.MustCompile("(?i).*input.?only.*") diff --git a/rules/aip0203/optional.go b/rules/aip0203/optional.go index e4e6505e8..0bd0bc74a 100644 --- a/rules/aip0203/optional.go +++ b/rules/aip0203/optional.go @@ -23,11 +23,9 @@ import ( ) var optional = &lint.FieldRule{ - Name: lint.NewRuleName(203, "optional"), - OnlyIf: withoutFieldBehavior, - LintField: func(f *desc.FieldDescriptor) []lint.Problem { - return checkLeadingComments(f, optionalRegexp, "OPTIONAL") - }, + Name: lint.NewRuleName(203, "optional"), + OnlyIf: withoutFieldBehavior, + LintField: checkLeadingComments(optionalRegexp, "OPTIONAL", requiredRegexp), } var optionalRegexp = regexp.MustCompile("(?i).*optional.*") diff --git a/rules/aip0203/optional_test.go b/rules/aip0203/optional_test.go index 50b12ba32..623b132e7 100644 --- a/rules/aip0203/optional_test.go +++ b/rules/aip0203/optional_test.go @@ -50,6 +50,12 @@ func TestOptional(t *testing.T) { Message: "google.api.field_behavior", }}, }, + { + name: "valid-required-and-optional", + comment: "optional required", + field: titleField, + problems: nil, + }, { name: "Invalid-Optional", comment: "Optional", diff --git a/rules/aip0203/output_only.go b/rules/aip0203/output_only.go index c18c571f9..8d34a8c4e 100644 --- a/rules/aip0203/output_only.go +++ b/rules/aip0203/output_only.go @@ -23,11 +23,9 @@ import ( ) var outputOnly = &lint.FieldRule{ - Name: lint.NewRuleName(203, "output-only"), - OnlyIf: withoutOutputOnlyFieldBehavior, - LintField: func(f *desc.FieldDescriptor) []lint.Problem { - return checkLeadingComments(f, outputOnlyRegexp, "OUTPUT_ONLY") - }, + Name: lint.NewRuleName(203, "output-only"), + OnlyIf: withoutOutputOnlyFieldBehavior, + LintField: checkLeadingComments(outputOnlyRegexp, "OUTPUT_ONLY"), } var outputOnlyRegexp = regexp.MustCompile("(?i).*output.?only.*") diff --git a/rules/aip0203/required.go b/rules/aip0203/required.go index cffbfda50..ff4038c41 100644 --- a/rules/aip0203/required.go +++ b/rules/aip0203/required.go @@ -18,15 +18,12 @@ import ( "regexp" "github.com/googleapis/api-linter/lint" - "github.com/jhump/protoreflect/desc" ) var required = &lint.FieldRule{ - Name: lint.NewRuleName(203, "required"), - OnlyIf: withoutFieldBehavior, - LintField: func(f *desc.FieldDescriptor) []lint.Problem { - return checkLeadingComments(f, requiredRegexp, "REQUIRED") - }, + Name: lint.NewRuleName(203, "required"), + OnlyIf: withoutFieldBehavior, + LintField: checkLeadingComments(requiredRegexp, "REQUIRED", optionalRegexp), } var requiredRegexp = regexp.MustCompile("(?i).*required.*")