Repository: googleapis/api-linter
Pull request #647 (2020-10-07 03:41:38)

feat: Add IsDeclarativeFriendly utility method.
This will be needed for upcoming declarative-friendly lint checks.


Patch:
diff --git a/go.mod b/go.mod
index d53a7f8..2882e19 100644
--- a/go.mod
+++ b/go.mod
@@ -13,7 +13,7 @@ require (
 	github.com/olekukonko/tablewriter v0.0.4
 	github.com/spf13/pflag v1.0.5
 	github.com/stoewer/go-strcase v1.2.0
-	google.golang.org/genproto v0.0.0-20200831141814-d751682dd103
+	google.golang.org/genproto v0.0.0-20201006033701-bcad7cf615f2
 	google.golang.org/protobuf v1.25.0
 	gopkg.in/yaml.v2 v2.3.0
 )
diff --git a/go.sum b/go.sum
index 2c2d693..eaa0394 100644
--- a/go.sum
+++ b/go.sum
@@ -72,8 +72,10 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
 golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU=
+golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 h1:XQyxROzUlZH+WIQwySDgnISgOivlhjIEwaQaJEJrrN0=
 golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
 golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKGUJ2LatrhH/nqhxcFungHvyanc=
+golang.org/x/mod v0.2.0 h1:KU7oHjnv3XNWfa5COkzUifxZmxp1TyI7ImMXqFxLwvQ=
 golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -100,6 +102,7 @@ golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3
 golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20191130070609-6e064ea0cf2d/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
+golang.org/x/tools v0.0.0-20200522201501-cb1345f3a375 h1:SjQ2+AKWgZLc1xej6WSzL+Dfs5Uyd5xcZH1mGC411IA=
 golang.org/x/tools v0.0.0-20200522201501-cb1345f3a375/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
@@ -113,6 +116,8 @@ google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98
 google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo=
 google.golang.org/genproto v0.0.0-20200831141814-d751682dd103 h1:z46CEPU+LlO0kGGwrH8h5epkkJhRZbAHYWOWD9JhLPI=
 google.golang.org/genproto v0.0.0-20200831141814-d751682dd103/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
+google.golang.org/genproto v0.0.0-20201006033701-bcad7cf615f2 h1:rBG1miiV00OG3NTbgxg42kPWohjVNn1sGhJyt4xyPLQ=
+google.golang.org/genproto v0.0.0-20201006033701-bcad7cf615f2/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
 google.golang.org/grpc v1.8.0/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw=
 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
 google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg=
diff --git a/rules/internal/utils/declarative_friendly.go b/rules/internal/utils/declarative_friendly.go
new file mode 100644
index 0000000..04f98af
--- /dev/null
+++ b/rules/internal/utils/declarative_friendly.go
@@ -0,0 +1,130 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	"strings"
+
+	"github.com/jhump/protoreflect/desc"
+	"github.com/stoewer/go-strcase"
+	apb "google.golang.org/genproto/googleapis/api/annotations"
+)
+
+// IsDeclarativeFriendly returns true if the descriptor is declarative-friendly.
+//
+// For messages, this means the message is annotated with google.api.resource
+// and style: DECLARATIVE_FRIENDLY is set.
+//
+// For methods, the output message is checked and the same logic applied.
+// Additionally, methods beginning with "List" (and "Delete" if returning Empty)
+// have special logic applied to try to find the correct message and run the
+// check.
+func IsDeclarativeFriendly(d desc.Descriptor) bool {
+	switch m := d.(type) {
+	case *desc.MessageDescriptor:
+		// We have a message; get the google.api.resource annotation and
+		// see if it is styled declarative-friendly.
+		if resource := GetResource(m); resource != nil {
+			for _, style := range resource.GetStyle() {
+				if style == apb.ResourceDescriptor_DECLARATIVE_FRIENDLY {
+					return true
+				}
+			}
+		}
+	case *desc.MethodDescriptor:
+		// Get the return type for the method. If the method is an LRO, then
+		// get the response type from the operation_info annotation.
+		response := m.GetOutputType()
+		if response.GetFullyQualifiedName() == "google.longrunning.Operation" {
+			if opInfo := GetOperationInfo(m); opInfo != nil {
+				response = findMessage(m.GetFile(), opInfo.GetResponseType())
+
+				// Sanity check: We may not have found the message.
+				// If that is the case, give up and assume the method is not
+				// declarative-friendly.
+				if response == nil {
+					return false
+				}
+			}
+		}
+
+		// If the return value has a google.api.resource annotation, we can
+		// assume it is the resource and check it.
+		if IsResource(response) {
+			return IsDeclarativeFriendly(response)
+		}
+
+		// If the return value is a List response (AIP-132), we should be able
+		// to find the resource as a field in the response.
+		if n := response.GetName(); strings.HasPrefix(n, "List") && strings.HasSuffix(n, "Response") {
+			for _, field := range response.GetFields() {
+				if field.IsRepeated() && field.GetMessageType() != nil {
+					return IsDeclarativeFriendly(field.GetMessageType())
+				}
+			}
+		}
+
+		// If this is a Delete method (AIP-135) with a return value of Empty,
+		// try to find the resource.
+		if strings.HasPrefix(m.GetName(), "Delete") && m.GetOutputType().GetName() == "Empty" {
+			if resource := findMessage(m.GetFile(), strings.TrimPrefix(m.GetName(), "Delete")); resource != nil {
+				return IsDeclarativeFriendly(resource)
+			}
+		}
+
+		// At this point, we probably have a custom method.
+		// Try to identify a resource by whittling away at the method name and
+		// seeing if there is a match.
+		snakeName := strings.Split(strcase.SnakeCase(m.GetName()), "_")
+		for i := 1; i < len(snakeName); i++ {
+			name := strcase.UpperCamelCase(strings.Join(snakeName[i:], "_"))
+			if resource := findMessage(m.GetFile(), name); resource != nil {
+				return IsDeclarativeFriendly(resource)
+			}
+		}
+	}
+
+	// We found no evidence that this descriptor is supposed to be
+	// declarative-friendly. Return false.
+	return false
+}
+
+// findMessage looks for a message in a file and all imports within the
+// same package.
+func findMessage(f *desc.FileDescriptor, name string) *desc.MessageDescriptor {
+	// FileDescriptor.FindMessage requires fully-qualified message names;
+	// attempt to infer that.
+	if !strings.Contains(name, ".") && f.GetPackage() != "" {
+		name = f.GetPackage() + "." + name
+	}
+
+	// Attempt to find the message in the file provided.
+	if m := f.FindMessage(name); m != nil {
+		return m
+	}
+
+	// Attempt to find the message in any dependency files if they are in the
+	// same package.
+	for _, dep := range f.GetDependencies() {
+		if f.GetPackage() == dep.GetPackage() {
+			if m := findMessage(dep, name); m != nil {
+				return m
+			}
+		}
+	}
+
+	// Whelp, no luck. Too bad.
+	return nil
+}
diff --git a/rules/internal/utils/extension.go b/rules/internal/utils/extension.go
index 3c899d6..ff3646e 100644
--- a/rules/internal/utils/extension.go
+++ b/rules/internal/utils/extension.go
@@ -71,6 +71,15 @@ func GetResource(m *desc.MessageDescriptor) *apb.ResourceDescriptor {
 	return nil
 }
 
+// IsResource returns true if the message has a populated google.api.resource
+// annotation with a non-empty "type" field.
+func IsResource(m *desc.MessageDescriptor) bool {
+	if res := GetResource(m); res != nil {
+		return res.GetType() != ""
+	}
+	return false
+}
+
 // GetResourceDefinitions returns the google.api.resource_definition annotations
 // for a file.
 func GetResourceDefinitions(f *desc.FileDescriptor) []*apb.ResourceDescriptor {


Test patch:
diff --git a/rules/internal/utils/declarative_friendly_test.go b/rules/internal/utils/declarative_friendly_test.go
new file mode 100644
index 0000000..3bd3efd
--- /dev/null
+++ b/rules/internal/utils/declarative_friendly_test.go
@@ -0,0 +1,208 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+)
+
+func TestDeclarativeFriendlyMessage(t *testing.T) {
+	// Test the cases where a google.api.resource annotation is present.
+	for _, test := range []struct {
+		name  string
+		Style string
+		want  bool
+	}{
+		{"True", "style: DECLARATIVE_FRIENDLY", true},
+		{"FalseNoStyle", "", false},
+		{"FalseOtherStyle", "style: STYLE_UNSPECIFIED", false},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			m := testutils.ParseProto3Tmpl(t, `
+				import "google/api/resource.proto";
+
+				message Book {
+					option (google.api.resource) = {
+						type: "library.googleapis.com/Book"
+						{{.Style}}
+					};
+				}
+			`, test).GetMessageTypes()[0]
+			if got := IsDeclarativeFriendly(m); got != test.want {
+				t.Errorf("Got %v, expected %v.", got, test.want)
+			}
+		})
+	}
+
+	// Test the case where the google.api.resource annotation is not present.
+	t.Run("NotResource", func(t *testing.T) {
+		m := testutils.ParseProto3String(t, "message Book {}").GetMessageTypes()[0]
+		if IsDeclarativeFriendly(m) {
+			t.Errorf("Got true, expected false.")
+		}
+	})
+}
+
+func TestDeclarativeFriendlyMethod(t *testing.T) {
+	// We need different templates for different situations.
+	//
+	// Note: The Book resource itself is always present and omitted here to
+	// avoid excess repetition; it is appended to the templates in the body of
+	// the test.
+	tmpls := map[string]string{
+		// The basic template just returns the resource with no frills.
+		"basic": `
+			service Library {
+				rpc GetBook(GetBookRequest) returns (Book);
+			}
+
+			message GetBookRequest {}
+		`,
+
+		// The LRO template returns the resource, but as the result of an LRO
+		// that has to be resolved first.
+		"lro": `
+			import "google/longrunning/operations.proto";
+
+			service Library {
+				rpc GetBook(GetBookRequest) returns (google.longrunning.Operation) {
+					option (google.longrunning.operation_info) = {
+						response_type: "Book"
+					};
+				}
+			}
+
+			message GetBookRequest {}
+		`,
+
+		// The List template returns a normal list response.
+		"list": `
+			service Library {
+				rpc ListBooks(ListBooksRequest) returns (ListBooksResponse);
+			}
+
+			message ListBooksRequest {}
+			message ListBooksResponse {
+				repeated Book books = 1;
+			}
+		`,
+
+		// The Delete template returns a normal delete response.
+		"delete": `
+			import "google/protobuf/empty.proto";
+			service Library {
+				rpc DeleteBook(DeleteBookRequest) returns (google.protobuf.Empty);
+			}
+			message DeleteBookRequest {}
+		`,
+
+		// The custom method template is a straightforward custom method
+		// with no direct reference to Book.
+		"custom": `
+			service Library {
+				rpc ArchiveBook(ArchiveBookRequest) returns (ArchiveBookResponse);
+			}
+
+			message ArchiveBookRequest {}
+			message ArchiveBookResponse {}
+		`,
+	}
+
+	for key, tmpl := range tmpls {
+		t.Run(key, func(t *testing.T) {
+			for _, test := range []struct {
+				name string
+				want bool
+			}{
+				{"true", true},
+				{"false", false},
+			} {
+				t.Run(test.name, func(t *testing.T) {
+					// Set the style of the resource to DECLARATIVE_FRIENDLY if that
+					// is the expected result.
+					s := struct{ Style string }{Style: ""}
+					if test.want == true {
+						s.Style = "style: DECLARATIVE_FRIENDLY"
+					}
+
+					// Parse the template and test the method.
+					f := testutils.ParseProto3Tmpl(t, fmt.Sprintf(`
+						import "google/api/resource.proto";
+
+						%s
+
+						message Book {
+							option (google.api.resource) = {
+								type: "library.googleapis.com/Book"
+								{{.Style}}
+							};
+						}
+					`, tmpl), s)
+					m := f.GetServices()[0].GetMethods()[0]
+					if got := IsDeclarativeFriendly(m); got != test.want {
+						t.Errorf("Got %v, expected %v.", got, test.want)
+					}
+				})
+			}
+		})
+	}
+
+	// Test an edge case where the LRO response is not found.
+	t.Run("lro/not-found", func(t *testing.T) {
+		f := testutils.ParseProto3String(t, `
+			import "google/longrunning/operations.proto";
+			service Library {
+				rpc CreateBook(CreateBookRequest) returns (google.longrunning.Operation) {
+					option (google.longrunning.operation_info) = {
+						response_type: "Shrug"
+					};
+				}
+			}
+			message CreateBookRequest {}
+		`)
+		m := f.GetServices()[0].GetMethods()[0]
+		want := false
+		if got := IsDeclarativeFriendly(m); got != want {
+			t.Errorf("Got %v, expected %v.", got, want)
+		}
+	})
+}
+
+func TestFindMessage(t *testing.T) {
+	files := testutils.ParseProtoStrings(t, map[string]string{
+		"a.proto": `
+			package test;
+			message Book {}
+		`,
+		"b.proto": `
+			package other;
+			message Scroll {}
+		`,
+		"c.proto": `
+			package test;
+			import "a.proto";
+			import "b.proto";
+		`,
+	})
+	if book := findMessage(files["c.proto"], "Book"); book == nil {
+		t.Errorf("Got nil, expected Book message.")
+	}
+	if scroll := findMessage(files["c.proto"], "Scroll"); scroll != nil {
+		t.Errorf("Got Sctoll message, expected nil.")
+	}
+}