From 6874dabb4f0d3503f267bb0ab970d62785d12727 Mon Sep 17 00:00:00 2001 From: Noah Dietz Date: Thu, 8 Aug 2024 19:22:20 -0700 Subject: [PATCH] fix: exit rule if response type cannot be resolved (#1415) The utility method that resolves the response type descriptor, handling LROs in the process, can return `nil` if either the response type is not resolvable in the protos (very bad) OR the RPC is an LRO that isn't annotated as required by AIP-151 with `google.longrunning.operation_info`. In these cases, anywhere the helper is used should just exit - if the type is unresolvable there are other issues, and if the LRO RPC is unannotated, then AIP-151 rules will warn on that missing annotation. We definitely shouldn't be going into a `panic` Updates #1399 --- rules/aip0133/request_required_fields.go | 3 +++ rules/aip0134/request_required_fields.go | 3 +++ rules/aip0136/response_message_name.go | 6 ++++++ rules/aip0233/resource_reference_type.go | 3 +++ rules/internal/utils/common_lints.go | 6 +++++- 5 files changed, 20 insertions(+), 1 deletion(-) diff --git a/rules/aip0133/request_required_fields.go b/rules/aip0133/request_required_fields.go index 04111a3dd..c547aae5c 100644 --- a/rules/aip0133/request_required_fields.go +++ b/rules/aip0133/request_required_fields.go @@ -31,6 +31,9 @@ var requestRequiredFields = &lint.MethodRule{ OnlyIf: utils.IsCreateMethodWithResolvedReturnType, LintMethod: func(m *desc.MethodDescriptor) []lint.Problem { ot := utils.GetResponseType(m) + if ot == nil { + return nil + } r := utils.GetResource(ot) resourceMsgName := utils.GetResourceSingular(r) diff --git a/rules/aip0134/request_required_fields.go b/rules/aip0134/request_required_fields.go index f4cb3c75f..033b51d63 100644 --- a/rules/aip0134/request_required_fields.go +++ b/rules/aip0134/request_required_fields.go @@ -29,6 +29,9 @@ var requestRequiredFields = &lint.MethodRule{ OnlyIf: utils.IsUpdateMethod, LintMethod: func(m *desc.MethodDescriptor) (problems []lint.Problem) { ot := utils.GetResponseType(m) + if ot == nil { + return nil + } it := m.GetInputType() allowedRequiredFields := stringset.New("update_mask") diff --git a/rules/aip0136/response_message_name.go b/rules/aip0136/response_message_name.go index 25453b475..ab10dcc30 100644 --- a/rules/aip0136/response_message_name.go +++ b/rules/aip0136/response_message_name.go @@ -47,6 +47,12 @@ var responseMessageName = &lint.MethodRule{ } response := utils.GetResponseType(m) + if response == nil { + // If the return type is not resolveable (bad) or if an LRO and + // missing the operation_info annotation (covered by AIP-151 rules), + // just exit. + return nil + } requestResourceType := utils.GetResourceReference(m.GetInputType().FindFieldByName("name")).GetType() responseResourceType := utils.GetResource(response).GetType() diff --git a/rules/aip0233/resource_reference_type.go b/rules/aip0233/resource_reference_type.go index 87f1aaafe..7e268692a 100644 --- a/rules/aip0233/resource_reference_type.go +++ b/rules/aip0233/resource_reference_type.go @@ -28,6 +28,9 @@ var resourceReferenceType = &lint.MethodRule{ OnlyIf: func(m *desc.MethodDescriptor) bool { // Return type of the RPC. ot := utils.GetResponseType(m) + if ot == nil { + return false + } // First repeated message field must be annotated with google.api.resource. repeated := utils.GetRepeatedMessageFields(ot) diff --git a/rules/internal/utils/common_lints.go b/rules/internal/utils/common_lints.go index 69a8ddac9..14a82ba9b 100644 --- a/rules/internal/utils/common_lints.go +++ b/rules/internal/utils/common_lints.go @@ -182,7 +182,11 @@ func LintMethodHasMatchingRequestName(m *desc.MethodDescriptor) []lint.Problem { // have a name matching the method's, with a "Response" suffix. func LintMethodHasMatchingResponseName(m *desc.MethodDescriptor) []lint.Problem { // GetResponseType handles the LRO case. - if got, want := GetResponseType(m).GetName(), m.GetName()+"Response"; got != want { + rt := GetResponseType(m) + if rt == nil { + return nil + } + if got, want := rt.GetName(), m.GetName()+"Response"; got != want { loc := locations.MethodResponseType(m) suggestion := want