fix(AIP-203): handle recursive field behavior check (#1199)

googleapis · Jul 16, 2023 · fe8e151 · fe8e151
1 parent 9e002e6
commit fe8e151
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 4 deletions.
diff --git a/rules/aip0142/time_field_names.go b/rules/aip0142/time_field_names.go
@@ -32,7 +32,7 @@ var fieldNames = &lint.FieldRule{
 			"expired":  "expire_time",
 			"modified": "update_time",
 			"updated":  "update_time",
-			"purged":   "purged_time",
+			"purged":   "purge_time",
 		}
 		for got, want := range mistakes {
 			if strings.Contains(f.GetName(), got) {

diff --git a/rules/aip0203/field_behavior_required.go b/rules/aip0203/field_behavior_required.go
@@ -32,7 +32,7 @@ var fieldBehaviorRequired = &lint.MethodRule{
 	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
 		req := m.GetInputType()
 		p := m.GetFile().GetPackage()
-		ps := problems(req, p)
+		ps := problems(req, p, map[desc.Descriptor]bool{})
 		if len(ps) == 0 {
 			return nil
 		}
@@ -41,10 +41,16 @@ var fieldBehaviorRequired = &lint.MethodRule{
 	},
 }
 
-func problems(m *desc.MessageDescriptor, pkg string) []lint.Problem {
+func problems(m *desc.MessageDescriptor, pkg string, visited map[desc.Descriptor]bool) []lint.Problem {
 	var ps []lint.Problem
 
 	for _, f := range m.GetFields() {
+		// ignore the field if it was already visited
+		if ok := visited[f]; ok {
+			continue
+		}
+		visited[f] = true
+
 		if utils.IsResource(m) && f.GetName() == "name" {
 			continue
 		}
@@ -58,7 +64,7 @@ func problems(m *desc.MessageDescriptor, pkg string) []lint.Problem {
 		}
 
 		if mt := f.GetMessageType(); mt != nil && !mt.IsMapEntry() && mt.GetFile().GetPackage() == pkg {
-			ps = append(ps, problems(mt, pkg)...)
+			ps = append(ps, problems(mt, pkg, visited)...)
 		}
 	}
 

diff --git a/rules/aip0203/field_behavior_required_test.go b/rules/aip0203/field_behavior_required_test.go
@@ -72,6 +72,13 @@ func TestFieldBehaviorRequired_SingleFile_SingleMessage(t *testing.T) {
 			];`,
 			nil,
 		},
+		{
+			"ValidRecursiveMessage",
+			`message Foo { Foo foo = 1 [(google.api.field_behavior) = OPTIONAL]; }
+			 Foo foo = 1 [(google.api.field_behavior) = OPTIONAL];
+			`,
+			nil,
+		},
 		{
 			"InvalidEmpty",
 			"int32 page_count = 1;",