From fe8e1512ac23bd16d0c1db381e8ae74e42a0cc94 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Sun, 16 Jul 2023 13:52:34 -0700 Subject: [PATCH] fix(AIP-203): handle recursive field behavior check (#1199) --- rules/aip0142/time_field_names.go | 2 +- rules/aip0203/field_behavior_required.go | 12 +++++++++--- rules/aip0203/field_behavior_required_test.go | 7 +++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/rules/aip0142/time_field_names.go b/rules/aip0142/time_field_names.go index dd54763a2..854c936d3 100644 --- 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 index bafb05039..0581043b1 100644 --- 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 index ff81bb9ee..339bb7a9c 100644 --- 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;",