fix: handle unresolvable lro response types AIP-133 and AIP-233

rules/aip0133/resource_reference_type.go

@@ -26,16 +26,24 @@ import (
 var resourceReferenceType = &lint.MethodRule{
 	Name: lint.NewRuleName(133, "resource-reference-type"),
 	OnlyIf: func(m *desc.MethodDescriptor) bool {
+		// Return type of the RPC.
+		ot := m.GetOutputType()
+		if ot.GetName() == "Operation" {
+			ot = utils.GetResponseType(m)
+		}
+
+		// Unresolvable response_type for an Operation results in nil here.
+		resource := utils.GetResource(ot)
 		p := m.GetInputType().FindFieldByName("parent")
-		return isCreateMethod(m) && p != nil && utils.GetResourceReference(p) != nil
+		return isCreateMethod(m) && p != nil && utils.GetResourceReference(p) != nil && resource != nil
 	},
 	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
-		out := m.GetOutputType()
-		if out.GetName() == "Operation" {
-			info := utils.GetOperationInfo(m)
-			out = utils.FindMessage(m.GetFile(), info.GetResponseType())
+		// Return type of the RPC.
+		ot := m.GetOutputType()
+		if ot.GetName() == "Operation" {
+			ot = utils.GetResponseType(m)
 		}
-		resource := utils.GetResource(out)
+		resource := utils.GetResource(ot)
 		parent := m.GetInputType().FindFieldByName("parent")
 		ref := utils.GetResourceReference(parent)
 

rules/aip0133/resource_reference_type_test.go

@@ -66,16 +66,18 @@ func TestResourceReferenceType(t *testing.T) {
 func TestResourceReferenceTypeLRO(t *testing.T) {
 	// Set up testing permutations.
 	tests := []struct {
-		testName string
-		TypeName string
-		RefType  string
-		problems testutils.Problems
+		testName     string
+		TypeName     string
+		RefType      string
+		ResponseType string
+		problems     testutils.Problems
 	}{
-		{"ValidChildType", "library.googleapis.com/Book", "child_type", nil},
-		{"ValidChildTypeLRO", "library.googleapis.com/Book", "child_type", nil},
-		{"ValidType", "library.googleapis.com/Shelf", "type", nil},
-		{"InvalidType", "library.googleapis.com/Book", "type", testutils.Problems{{Message: "not a `type`"}}},
-		{"InvalidChildType", "library.googleapis.com/Shelf", "child_type", testutils.Problems{{Message: "`child_type`"}}},
+		{"ValidChildType", "library.googleapis.com/Book", "child_type", "Book", nil},
+		{"ValidChildTypeLRO", "library.googleapis.com/Book", "child_type", "Book", nil},
+		{"ValidType", "library.googleapis.com/Shelf", "type", "Book", nil},
+		{"InvalidType", "library.googleapis.com/Book", "type", "Book", testutils.Problems{{Message: "not a `type`"}}},
+		{"InvalidChildType", "library.googleapis.com/Shelf", "child_type", "Book", testutils.Problems{{Message: "`child_type`"}}},
+		{"SkipInvalidUnresolvableResponseType", "library.googleapis.com/Shelf", "child_type", "Foo", nil},
 	}
 
 	// Run each test.
@@ -87,7 +89,7 @@ func TestResourceReferenceTypeLRO(t *testing.T) {
 				service Library {
 					rpc CreateBook(CreateBookRequest) returns (google.longrunning.Operation) {
 						option (google.longrunning.operation_info) = {
-							response_type: "Book"
+							response_type: "{{ .ResponseType }}"
 							metadata_type: "Book"
 						};
 					}

rules/aip0233/resource_reference_type.go

@@ -26,14 +26,14 @@ import (
 var resourceReferenceType = &lint.MethodRule{
 	Name: lint.NewRuleName(233, "resource-reference-type"),
 	OnlyIf: func(m *desc.MethodDescriptor) bool {
-		out := m.GetOutputType()
-		if out.GetName() == "Operation" {
-			info := utils.GetOperationInfo(m)
-			out = utils.FindMessage(m.GetFile(), info.GetResponseType())
+		// Return type of the RPC.
+		ot := m.GetOutputType()
+		if ot.GetName() == "Operation" {
+			ot = utils.GetResponseType(m)
 		}
 
 		// First repeated message field must be annotated with google.api.resource.
-		repeated := utils.GetRepeatedMessageFields(out)
+		repeated := utils.GetRepeatedMessageFields(ot)
 		if len(repeated) == 0 {
 			return false
 		}
@@ -44,12 +44,12 @@ var resourceReferenceType = &lint.MethodRule{
 		return isBatchCreateMethod(m) && parent != nil && utils.GetResourceReference(parent) != nil && resource != nil
 	},
 	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
-		msg := m.GetOutputType()
-		if msg.GetName() == "Operation" {
-			info := utils.GetOperationInfo(m)
-			msg = utils.FindMessage(m.GetFile(), info.GetResponseType())
+		// Return type of the RPC.
+		ot := m.GetOutputType()
+		if ot.GetName() == "Operation" {
+			ot = utils.GetResponseType(m)
 		}
-		repeated := utils.GetRepeatedMessageFields(msg)
+		repeated := utils.GetRepeatedMessageFields(ot)
 		resMsg := repeated[0].GetMessageType()
 
 		resource := utils.GetResource(resMsg)

rules/aip0233/resource_reference_type_test.go

@@ -74,15 +74,17 @@ func TestResourceReferenceType(t *testing.T) {
 func TestResourceReferenceTypeLRO(t *testing.T) {
 	// Set up testing permutations.
 	tests := []struct {
-		testName string
-		TypeName string
-		RefType  string
-		problems testutils.Problems
+		testName     string
+		TypeName     string
+		RefType      string
+		ResponseType string
+		problems     testutils.Problems
 	}{
-		{"ValidChildType", "library.googleapis.com/Book", "child_type", nil},
-		{"ValidType", "library.googleapis.com/Shelf", "type", nil},
-		{"InvalidType", "library.googleapis.com/Book", "type", testutils.Problems{{Message: "not a `type`"}}},
-		{"InvalidChildType", "library.googleapis.com/Shelf", "child_type", testutils.Problems{{Message: "child_type"}}},
+		{"ValidChildType", "library.googleapis.com/Book", "child_type", "BatchCreateBooksResponse", nil},
+		{"ValidType", "library.googleapis.com/Shelf", "type", "BatchCreateBooksResponse", nil},
+		{"InvalidType", "library.googleapis.com/Book", "type", "BatchCreateBooksResponse", testutils.Problems{{Message: "not a `type`"}}},
+		{"InvalidChildType", "library.googleapis.com/Shelf", "child_type", "BatchCreateBooksResponse", testutils.Problems{{Message: "child_type"}}},
+		{"SkipInvalidUnresolvableResponseType", "library.googleapis.com/Shelf", "child_type", "Foo", nil},
 	}
 
 	// Run each test.
@@ -94,7 +96,7 @@ func TestResourceReferenceTypeLRO(t *testing.T) {
 				service Library {
 					rpc BatchCreateBooks(BatchCreateBooksRequest) returns (google.longrunning.Operation) {
 						option (google.longrunning.operation_info) = {
-							response_type: "BatchCreateBooksResponse"
+							response_type: "{{ .ResponseType }}"
 							metadata_type: "BatchCreateBooksResponse"
 						};
 					}

rules/internal/utils/extension.go

@@ -40,13 +40,46 @@ func GetFieldBehavior(f *desc.FieldDescriptor) stringset.Set {
 
 // GetOperationInfo returns the google.longrunning.operation_info annotation.
 func GetOperationInfo(m *desc.MethodDescriptor) *lrpb.OperationInfo {
+	if m == nil {
+		return nil
+	}
 	opts := m.GetMethodOptions()
 	if x := proto.GetExtension(opts, lrpb.E_OperationInfo); x != nil {
 		return x.(*lrpb.OperationInfo)
 	}
 	return nil
 }
 
+// GetResponseType returns the message referred to by the
+// (google.longrunning.operation_info).response_type annotation.
+func GetResponseType(m *desc.MethodDescriptor) *desc.MessageDescriptor {
+	if m == nil {
+		return nil
+	}
+	info := GetOperationInfo(m)
+	if info == nil {
+		return nil
+	}
+	typ := FindMessage(m.GetFile(), info.GetResponseType())
+
+	return typ
+}
+
+// GetMetadataType returns the message referred to by the
+// (google.longrunning.operation_info).metadata_type annotation.
+func GetMetadataType(m *desc.MethodDescriptor) *desc.MessageDescriptor {
+	if m == nil {
+		return nil
+	}
+	info := GetOperationInfo(m)
+	if info == nil {
+		return nil
+	}
+	typ := FindMessage(m.GetFile(), info.GetMetadataType())
+
+	return typ
+}
+
 // GetMethodSignatures returns the `google.api.method_signature` annotations.
 func GetMethodSignatures(m *desc.MethodDescriptor) [][]string {
 	answer := [][]string{}

rules/internal/utils/extension_test.go

@@ -124,6 +124,84 @@ func TestGetOperationInfoNone(t *testing.T) {
 	}
 }
 
+func TestGetOperationInfoResponseType(t *testing.T) {
+	// Set up testing permutations.
+	tests := []struct {
+		testName     string
+		ResponseType string
+		invalid      bool
+	}{
+		{"Valid", "WriteBookResponse", false},
+		{"Invalid", "Foo", true},
+	}
+	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			fd := testutils.ParseProto3Tmpl(t, `
+				import "google/longrunning/operations.proto";
+				service Library {
+					rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+						option (google.longrunning.operation_info) = {
+							response_type: "{{ .ResponseType }}"
+							metadata_type: "WriteBookMetadata"
+						};
+					}
+				}
+				message WriteBookRequest {}
+				message WriteBookResponse {}
+			`, test)
+			typ := GetResponseType(fd.GetServices()[0].GetMethods()[0])
+			if (typ == nil) != test.invalid {
+				t.Fatalf("Expected invalid(%v) response_type message", test.invalid)
+			}
+			if test.invalid {
+				return
+			}
+			if got, want := typ.GetName(), test.ResponseType; got != want {
+				t.Errorf("Response type - got %q, want %q.", got, want)
+			}
+		})
+	}
+}
+
+func TestGetOperationInfoMetadataType(t *testing.T) {
+	// Set up testing permutations.
+	tests := []struct {
+		testName     string
+		MetadataType string
+		invalid      bool
+	}{
+		{"Valid", "WriteBookMetadata", false},
+		{"Invalid", "Foo", true},
+	}
+	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			fd := testutils.ParseProto3Tmpl(t, `
+				import "google/longrunning/operations.proto";
+				service Library {
+					rpc WriteBook(WriteBookRequest) returns (google.longrunning.Operation) {
+						option (google.longrunning.operation_info) = {
+							response_type: "WriteBookResponse"
+							metadata_type: "{{ .MetadataType }}"
+						};
+					}
+				}
+				message WriteBookRequest {}
+				message WriteBookMetadata {}
+			`, test)
+			typ := GetMetadataType(fd.GetServices()[0].GetMethods()[0])
+			if (typ == nil) != test.invalid {
+				t.Fatalf("Expected invalid(%v) metadata_type message", test.invalid)
+			}
+			if test.invalid {
+				return
+			}
+			if got, want := typ.GetName(), test.MetadataType; got != want {
+				t.Errorf("Metadata type - got %q, want %q.", got, want)
+			}
+		})
+	}
+}
+
 func TestGetResource(t *testing.T) {
 	t.Run("Present", func(t *testing.T) {
 		f := testutils.ParseProto3String(t, `

rules/internal/utils/find.go

@@ -111,6 +111,11 @@ func (f fieldSorter) Less(i, j int) bool {
 func GetRepeatedMessageFields(m *desc.MessageDescriptor) []*desc.FieldDescriptor {
 	var fields fieldSorter
 
+	// If an unresolable message is fed into this helper, return empty slice.
+	if m == nil {
+		return fields
+	}
+
 	for _, f := range m.GetFields() {
 		if f.IsRepeated() && f.GetType() == descriptorpb.FieldDescriptorProto_TYPE_MESSAGE {
 			fields = append(fields, f)