fix: exclude etag from field_behavior lints (#1009)

googleapis · Sep 7, 2022 · b521af0 · b521af0
1 parent 8be4f1a
commit b521af0
Show file tree

Hide file tree

Showing 11 changed files with 52 additions and 5 deletions.
diff --git a/rules/aip0203/aip0203.go b/rules/aip0203/aip0203.go
@@ -19,11 +19,14 @@ import (
 	"fmt"
 	"regexp"
 
+	"bitbucket.org/creachadair/stringset"
 	"github.com/googleapis/api-linter/lint"
 	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/jhump/protoreflect/desc"
 )
 
+var standardFields = stringset.New("etag")
+
 // AddRules adds all of the AIP-203 rules to the provided registry.
 func AddRules(r lint.RuleRegistry) error {
 	return r.Register(
@@ -61,5 +64,5 @@ func checkLeadingComments(pattern *regexp.Regexp, annotation string, unless ...*
 }
 
 func withoutFieldBehavior(f *desc.FieldDescriptor) bool {
-	return utils.GetFieldBehavior(f).Len() == 0
+	return utils.GetFieldBehavior(f).Len() == 0 && !standardFields.Contains(f.GetName())
 }
diff --git a/rules/aip0203/immutable.go b/rules/aip0203/immutable.go
@@ -31,5 +31,5 @@ var immutable = &lint.FieldRule{
 var immutableRegexp = regexp.MustCompile("(?i).*immutable.*")
 
 func withoutImmutableFieldBehavior(f *desc.FieldDescriptor) bool {
-	return !utils.GetFieldBehavior(f).Contains("IMMUTABLE")
+	return !utils.GetFieldBehavior(f).Contains("IMMUTABLE") && !standardFields.Contains(f.GetName())
 }
diff --git a/rules/aip0203/immutable_test.go b/rules/aip0203/immutable_test.go
@@ -44,6 +44,12 @@ func TestImmutable(t *testing.T) {
 			field:    fieldPartWithImmtutableBehavior,
 			problems: nil,
 		},
+		{
+			name:     "Valid-exclude-etag",
+			comment:  "Immutable.",
+			field:    "string etag = 1;",
+			problems: nil,
+		},
 		{
 			name:    "Invalid-immutable",
 			comment: "immutable",

diff --git a/rules/aip0203/input_only.go b/rules/aip0203/input_only.go
@@ -31,5 +31,5 @@ var inputOnly = &lint.FieldRule{
 var inputOnlyRegexp = regexp.MustCompile("(?i).*input.?only.*")
 
 func withoutInputOnlyFieldBehavior(f *desc.FieldDescriptor) bool {
-	return !utils.GetFieldBehavior(f).Contains("INPUT_ONLY")
+	return !utils.GetFieldBehavior(f).Contains("INPUT_ONLY") && !standardFields.Contains(f.GetName())
 }
diff --git a/rules/aip0203/input_only_test.go b/rules/aip0203/input_only_test.go
@@ -33,6 +33,12 @@ func TestInputOnly(t *testing.T) {
 			field:    "string secret = 1 [(google.api.field_behavior) = INPUT_ONLY];",
 			problems: nil,
 		},
+		{
+			name:     "valid_exclude_etag",
+			comment:  "Input only.",
+			field:    "string etag = 1;",
+			problems: nil,
+		},
 		{
 			name:    "input_only",
 			comment: "input_only",

diff --git a/rules/aip0203/optional_consistency.go b/rules/aip0203/optional_consistency.go
@@ -27,7 +27,7 @@ var optionalBehaviorConsistency = &lint.MessageRule{
 	OnlyIf: messageHasOptionalFieldBehavior,
 	LintMessage: func(m *desc.MessageDescriptor) (problems []lint.Problem) {
 		for _, f := range m.GetFields() {
-			if utils.GetFieldBehavior(f).Len() == 0 && f.GetOneOf() == nil {
+			if utils.GetFieldBehavior(f).Len() == 0 && !standardFields.Contains(f.GetName()) && f.GetOneOf() == nil {
 				problems = append(problems, lint.Problem{
 					Message:    "Within a single message, either all optional fields should be indicated, or none of them should be.",
 					Descriptor: f,

diff --git a/rules/aip0203/optional_consistency_test.go b/rules/aip0203/optional_consistency_test.go
@@ -86,6 +86,20 @@ oneof other {
 }`,
 			problems: nil,
 		},
+		{
+			name: "Valid-IgnoreEtag",
+			field: `
+string name = 1 [
+	(google.api.field_behavior) = IMMUTABLE,
+	(google.api.field_behavior) = OUTPUT_ONLY];
+
+string title = 2 [(google.api.field_behavior) = REQUIRED];
+
+string summary = 3 [(google.api.field_behavior) = OPTIONAL];
+
+string etag = 4;`,
+			problems: nil,
+		},
 	}
 
 	for _, test := range testCases {

diff --git a/rules/aip0203/optional_test.go b/rules/aip0203/optional_test.go
@@ -44,6 +44,12 @@ func TestOptional(t *testing.T) {
 			field:    titleWithOptionalBehavior,
 			problems: nil,
 		},
+		{
+			name:     "Valid-exclude-etag",
+			comment:  "Optional.",
+			field:    "string etag = 1;",
+			problems: nil,
+		},
 		{
 			name:    "Invalid-optional",
 			comment: "optional",

diff --git a/rules/aip0203/output_only.go b/rules/aip0203/output_only.go
@@ -31,5 +31,5 @@ var outputOnly = &lint.FieldRule{
 var outputOnlyRegexp = regexp.MustCompile("(?i).*output.?only.*")
 
 func withoutOutputOnlyFieldBehavior(f *desc.FieldDescriptor) bool {
-	return !utils.GetFieldBehavior(f).Contains("OUTPUT_ONLY")
+	return !utils.GetFieldBehavior(f).Contains("OUTPUT_ONLY") && !standardFields.Contains(f.GetName())
 }
diff --git a/rules/aip0203/output_only_test.go b/rules/aip0203/output_only_test.go
@@ -45,6 +45,12 @@ func TestOutput(t *testing.T) {
 			field:    generatedURIWithOutputOnlyBehavior,
 			problems: nil,
 		},
+		{
+			name:     "Valid-exclude-etag",
+			comment:  "Output Only.",
+			field:    "string etag = 1;",
+			problems: nil,
+		},
 		{
 			name:    "Invalid-Output Only",
 			comment: "Output Only",

diff --git a/rules/aip0203/required_test.go b/rules/aip0203/required_test.go
@@ -85,6 +85,12 @@ func TestRequired(t *testing.T) {
 			field:    title,
 			problems: nil,
 		},
+		{
+			name:     "Valid-exclude-etag",
+			comment:  "Required",
+			field:    "string etag = 1;",
+			problems: nil,
+		},
 		{
 			name:    "Invalid-required",
 			comment: "required",