Repository: googleapis/api-linter
Pull request #415 (2019-12-05 00:56:32)

[AIP-123] Require a google.api.resource annotation on resources.


Patch:
diff --git a/docs/rules/0123/index.md b/docs/rules/0123/index.md
new file mode 100644
index 0000000..e793567
--- /dev/null
+++ b/docs/rules/0123/index.md
@@ -0,0 +1,10 @@
+---
+aip_listing: 123
+permalink: /123/
+redirect_from:
+  - /0123/
+---
+
+# Unified resource types
+
+{% include linter-aip-listing.md aip=123 %}
diff --git a/docs/rules/0123/resource-annotation.md b/docs/rules/0123/resource-annotation.md
new file mode 100644
index 0000000..1096c72
--- /dev/null
+++ b/docs/rules/0123/resource-annotation.md
@@ -0,0 +1,65 @@
+---
+rule:
+  aip: 123
+  name: [core, '0123', resource-annotation]
+  summary: Resource messages should be annotated with `google.api.resource`.
+permalink: /123/resource-annotation
+redirect_from:
+  - /0123/resource-annotation
+---
+
+# Resource annotation presence
+
+This rule enforces that messages that appear to represent resources have a
+`google.api.resource` annotation, as described in [AIP-123][].
+
+## Details
+
+This rule scans all messages, and assumes that messages with a `string name`
+field are resources unless the message name ends with `Request`. For messages
+that are resources, it complains if the `google.api.resource` annotation is
+missing.
+
+## Examples
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+message Book {
+  // A `google.api.resource` annotation should be here.
+  string name = 1;
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+message Book {
+  option (google.api.resource) = {
+    type: "library.googleapis.com/Book"
+    pattern: "publishers/{publisher}/books/{book}"
+  };
+
+  string name = 1;
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the message.
+
+```proto
+// (-- api-linter: core::0123::resource-annotation=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+message Book {
+  string name = 1;
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-123]: http://aip.dev/123
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/rules/aip0123/aip0123.go b/rules/aip0123/aip0123.go
new file mode 100644
index 0000000..c25da3f
--- /dev/null
+++ b/rules/aip0123/aip0123.go
@@ -0,0 +1,36 @@
+// Copyright 2019 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 aip0123 contains rules defined in https://aip.dev/123.
+package aip0123
+
+import (
+	"strings"
+
+	"github.com/googleapis/api-linter/lint"
+	"github.com/jhump/protoreflect/desc"
+)
+
+// AddRules accepts a register function and registers each of
+// this AIP's rules to it.
+func AddRules(r lint.RuleRegistry) error {
+	return r.Register(
+		123,
+		resourceAnnotation,
+	)
+}
+
+func isResourceMessage(m *desc.MessageDescriptor) bool {
+	return m.FindFieldByName("name") != nil && !strings.HasSuffix(m.GetName(), "Request")
+}
diff --git a/rules/aip0123/resource_annotation.go b/rules/aip0123/resource_annotation.go
new file mode 100644
index 0000000..3f44c6e
--- /dev/null
+++ b/rules/aip0123/resource_annotation.go
@@ -0,0 +1,35 @@
+// Copyright 2019 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 aip0123
+
+import (
+	"github.com/googleapis/api-linter/lint"
+	"github.com/googleapis/api-linter/rules/internal/utils"
+	"github.com/jhump/protoreflect/desc"
+)
+
+var resourceAnnotation = &lint.MessageRule{
+	Name:   lint.NewRuleName(123, "resource-annotation"),
+	OnlyIf: isResourceMessage,
+	LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
+		if utils.GetResource(m) == nil {
+			return []lint.Problem{{
+				Message:    "Resource messages should include a `google.api.resource` annotation.",
+				Descriptor: m,
+			}}
+		}
+		return nil
+	},
+}
diff --git a/rules/rules.go b/rules/rules.go
index 1e76452..82ecdbf 100644
--- a/rules/rules.go
+++ b/rules/rules.go
@@ -52,6 +52,7 @@ package rules
 import (
 	"github.com/googleapis/api-linter/lint"
 	"github.com/googleapis/api-linter/rules/aip0122"
+	"github.com/googleapis/api-linter/rules/aip0123"
 	"github.com/googleapis/api-linter/rules/aip0126"
 	"github.com/googleapis/api-linter/rules/aip0127"
 	"github.com/googleapis/api-linter/rules/aip0131"
@@ -83,6 +84,7 @@ type addRulesFuncType func(lint.RuleRegistry) error
 
 var aipAddRulesFuncs = []addRulesFuncType{
 	aip0122.AddRules,
+	aip0123.AddRules,
 	aip0126.AddRules,
 	aip0127.AddRules,
 	aip0131.AddRules,


Test patch:
diff --git a/rules/aip0123/aip0123_test.go b/rules/aip0123/aip0123_test.go
new file mode 100644
index 0000000..d0d127b
--- /dev/null
+++ b/rules/aip0123/aip0123_test.go
@@ -0,0 +1,27 @@
+// Copyright 2019 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 aip0123
+
+import (
+	"testing"
+
+	"github.com/googleapis/api-linter/lint"
+)
+
+func TestAddRules(t *testing.T) {
+	if err := AddRules(lint.NewRuleRegistry()); err != nil {
+		t.Errorf("AddRules got an error: %v", err)
+	}
+}
diff --git a/rules/aip0123/resource_annotation_test.go b/rules/aip0123/resource_annotation_test.go
new file mode 100644
index 0000000..273423f
--- /dev/null
+++ b/rules/aip0123/resource_annotation_test.go
@@ -0,0 +1,65 @@
+// Copyright 2019 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 aip0123
+
+import (
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+)
+
+func TestResourceAnnotation(t *testing.T) {
+	// The rule should pass if the option is present on a resource message.
+	t.Run("Present", func(t *testing.T) {
+		f := testutils.ParseProto3String(t, `
+			import "google/api/resource.proto";
+			message Book {
+				option (google.api.resource) = {
+					type: "library.googleapis.com/Book"
+					pattern: "publishers/{publisher}/books/{book}"
+				};
+				string name = 1;
+			}
+		`)
+		if diff := (testutils.Problems{}).Diff(resourceAnnotation.Lint(f)); diff != "" {
+			t.Errorf(diff)
+		}
+	})
+
+	// The rule should fail if the option is absent on a resource message,
+	// but pass on messages that are not resource messages.
+	for _, test := range []struct {
+		name        string
+		MessageName string
+		FieldName   string
+		problems    testutils.Problems
+	}{
+		{"ValidNoNameField", "Book", "title", testutils.Problems{}},
+		{"ValidRequestMessage", "GetBookRequest", "name", testutils.Problems{}},
+		{"Invalid", "Book", "name", testutils.Problems{{Message: "google.api.resource"}}},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			f := testutils.ParseProto3Tmpl(t, `
+				message {{.MessageName}} {
+					string {{.FieldName}} = 1;
+				}
+			`, test)
+			m := f.GetMessageTypes()[0]
+			if diff := test.problems.SetDescriptor(m).Diff(resourceAnnotation.Lint(f)); diff != "" {
+				t.Errorf(diff)
+			}
+		})
+	}
+}