fix(AIP-133): skip method sig for non-standard create (googleapis#1521)

When what looks like a Standard Create in fact isn't one because it doesn't actually operate on something annotated as a resource, skip the rule. Also, if after all of the munching of descriptors, there is nothing to suggest for a signature, just exit early, we don't want to suggest an empty signature (which this currently is capable of doing)

Internal bug http://b/438826214

Author: noahdietz

rules/aip0133/method_signature.go

@@ -27,8 +27,10 @@ import (
 )
 
 var methodSignature = &lint.MethodRule{
-	Name:   lint.NewRuleName(133, "method-signature"),
-	OnlyIf: utils.IsCreateMethod,
+	Name: lint.NewRuleName(133, "method-signature"),
+	OnlyIf: func(m *desc.MethodDescriptor) bool {
+		return utils.IsCreateMethod(m) && utils.IsResource(utils.GetResponseType(m))
+	},
 	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
 		signatures := utils.GetMethodSignatures(m)
 
@@ -44,12 +46,19 @@ var methodSignature = &lint.MethodRule{
 			}
 		}
 		// The {resource}_id is desired if and only if the field exists on the
-		// request.
+		// request and the request targets a resource.
 		expectedResourceIDField := strcase.SnakeCase(utils.GetResourceMessageName(m, "Create"))
 		if idField := expectedResourceIDField + "_id"; m.GetInputType().FindFieldByName(idField) != nil {
 			want = append(want, idField)
 		}
 
+		// The Standard Create is not standard and has nothing to suggest.
+		// There are likely other rules warning about the non-standard nature
+		// so just silently move on.
+		if len(want) == 0 {
+			return nil
+		}
+
 		// Check if the signature is missing.
 		if len(signatures) == 0 {
 			return []lint.Problem{{

rules/aip0133/method_signature_test.go

@@ -115,6 +115,27 @@ func TestMethodSignature(t *testing.T) {
 			t.Error(diff)
 		}
 	})
+
+	// Ensure that this isn't producing a wonky finding if the "standard create"
+	// doesn't actually interact with a resource. Other rules would capture that
+	// errant aspect.
+	t.Run("SkipNonResource", func(t *testing.T) {
+		file := testutils.ParseProto3String(t, `
+			import "google/api/client.proto";
+			service Library {
+				rpc CreateBook(CreateBookRequest) returns (Book) {}
+			}
+			message CreateBookRequest {
+				Book book = 1;
+				string book_id = 2;
+			}
+			message Book {}
+		`)
+		if diff := (testutils.Problems{}).Diff(methodSignature.Lint(file)); diff != "" {
+			t.Error(diff)
+		}
+	})
+
 	// Add a separate test for the LRO case rather than introducing yet
 	// another knob on the above test.
 	t.Run("Longrunning", func(t *testing.T) {