Skip to content

Commit

Permalink
fix: exit rule if response type cannot be resolved (#1415)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
noahdietz authored Aug 9, 2024
1 parent aa9587b commit 6874dab
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 1 deletion.
3 changes: 3 additions & 0 deletions rules/aip0133/request_required_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
3 changes: 3 additions & 0 deletions rules/aip0134/request_required_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
6 changes: 6 additions & 0 deletions rules/aip0136/response_message_name.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
3 changes: 3 additions & 0 deletions rules/aip0233/resource_reference_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion rules/internal/utils/common_lints.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 6874dab

Please sign in to comment.