Repository: googleapis/api-linter
Pull request #381 (2019-11-22 23:05:47)

[AIP-217] Enforce the unreachable field.


Patch:
diff --git a/docs/rules/0217/index.md b/docs/rules/0217/index.md
new file mode 100644
index 0000000..f0f6fd9
--- /dev/null
+++ b/docs/rules/0217/index.md
@@ -0,0 +1,10 @@
+---
+aip_listing: 217
+permalink: /217/
+redirect_from:
+  - /0217/
+---
+
+# States
+
+{% include linter-aip-listing.md aip=217 %}
diff --git a/docs/rules/0217/synonyms.md b/docs/rules/0217/synonyms.md
new file mode 100644
index 0000000..f7003de
--- /dev/null
+++ b/docs/rules/0217/synonyms.md
@@ -0,0 +1,66 @@
+---
+rule:
+  aip: 217
+  name: [core, '0217', synonyms]
+  summary: The field for unreachable resources should be named `unreachable`.
+permalink: /217/synonyms
+redirect_from:
+  - /0217/synonyms
+---
+
+# States
+
+This rule enforces that the field describing unreachable resources in response
+messages is named `unreachable` rather than `unreachable_locations`, as
+mandated in [AIP-217][].
+
+## Details
+
+This rule iterates over paginated response messages and looks for a field named
+`unreachable_locations`, and complains if it finds one, suggesting the use of
+`unreachable` instead.
+
+## Examples
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+message ListBooksResponse {
+  repeated Book books = 1;
+  string next_page_token = 2;
+  repeated string unreachable_locations = 3;
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+message ListBooksResponse {
+  repeated Book books = 1;
+  string next_page_token = 2;
+  repeated string unreachable = 3;
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the field.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+message ListBooksResponse {
+  repeated Book books = 1;
+  string next_page_token = 2;
+  // (-- api-linter: core::0217::synonyms=disabled
+  //     aip.dev/not-precedent: We need to do this because reasons. --)
+  repeated string unreachable_locations = 3;
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-217]: https://aip.dev/217
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/docs/rules/0217/unreachable-field-type.md b/docs/rules/0217/unreachable-field-type.md
new file mode 100644
index 0000000..9261b40
--- /dev/null
+++ b/docs/rules/0217/unreachable-field-type.md
@@ -0,0 +1,73 @@
+---
+rule:
+  aip: 217
+  name: [core, '0217', unreachable-field-type]
+  summary: The unreachable field should be a repeated string.
+permalink: /217/unreachable-field-type
+redirect_from:
+  - /0217/unreachable-field-type
+---
+
+# States
+
+This rule enforces that the `unreachable` field is a `repeated string`, as
+mandated in [AIP-217][].
+
+## Details
+
+This rule looks at fields named `unreachable`, and complains if they are
+anything other than a `repeated string`.
+
+## Examples
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+message ListBooksResponse {
+  repeated Book books = 1;
+  string next_page_token = 2;
+  string unreachable = 3;  // Should be repeated.
+}
+```
+
+```proto
+// Incorrect.
+message ListBooksResponse {
+  repeated Book books = 1;
+  string next_page_token = 2;
+  repeated ErrorInfo unreachable = 3;  // Should be a string.
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+message ListBooksResponse {
+  repeated Book books = 1;
+  string next_page_token = 2;
+  repeated string unreachable = 3;
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the field.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+message ListBooksResponse {
+  repeated Book books = 1;
+  string next_page_token = 2;
+  // (-- api-linter: core::0217::unreachable-field-type=disabled
+  //     aip.dev/not-precedent: We need to do this because reasons. --)
+  repeated string unreachable_locations = 3;
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-217]: https://aip.dev/217
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/rules/aip0217/aip0217.go b/rules/aip0217/aip0217.go
new file mode 100644
index 0000000..deb40ff
--- /dev/null
+++ b/rules/aip0217/aip0217.go
@@ -0,0 +1,28 @@
+// 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 aip0217 contains rules defined in https://aip.dev/217.
+package aip0217
+
+import "github.com/googleapis/api-linter/lint"
+
+// AddRules accepts a register function and registers each of
+// this AIP's rules to it.
+func AddRules(r lint.RuleRegistry) error {
+	return r.Register(
+		217,
+		synonyms,
+		unreachableFieldType,
+	)
+}
diff --git a/rules/aip0217/synonyms.go b/rules/aip0217/synonyms.go
new file mode 100644
index 0000000..ebb45ef
--- /dev/null
+++ b/rules/aip0217/synonyms.go
@@ -0,0 +1,44 @@
+// 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 aip0217
+
+import (
+	"bitbucket.org/creachadair/stringset"
+	"github.com/googleapis/api-linter/lint"
+	"github.com/googleapis/api-linter/locations"
+	"github.com/jhump/protoreflect/desc"
+)
+
+var synSet = stringset.New(
+	"unreachable_locations",
+)
+
+var synonyms = &lint.FieldRule{
+	Name: lint.NewRuleName(217, "synonyms"),
+	OnlyIf: func(f *desc.FieldDescriptor) bool {
+		return f.GetOwner().FindFieldByName("next_page_token") != nil
+	},
+	LintField: func(f *desc.FieldDescriptor) []lint.Problem {
+		if synSet.Contains(f.GetName()) {
+			return []lint.Problem{{
+				Message:    "Use `unreachable` to express unreachable resources when listing.",
+				Suggestion: "unreachable",
+				Descriptor: f,
+				Location:   locations.DescriptorName(f),
+			}}
+		}
+		return nil
+	},
+}
diff --git a/rules/aip0217/unreachable_field_type.go b/rules/aip0217/unreachable_field_type.go
new file mode 100644
index 0000000..7d83065
--- /dev/null
+++ b/rules/aip0217/unreachable_field_type.go
@@ -0,0 +1,46 @@
+// 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 aip0217
+
+import (
+	"github.com/googleapis/api-linter/lint"
+	"github.com/googleapis/api-linter/locations"
+	"github.com/googleapis/api-linter/rules/internal/utils"
+	"github.com/jhump/protoreflect/desc"
+)
+
+var unreachableFieldType = &lint.FieldRule{
+	Name: lint.NewRuleName(217, "unreachable-field-type"),
+	OnlyIf: func(f *desc.FieldDescriptor) bool {
+		return f.GetName() == "unreachable"
+	},
+	LintField: func(f *desc.FieldDescriptor) (problems []lint.Problem) {
+		if !f.IsRepeated() {
+			problems = append(problems, lint.Problem{
+				Message:    "unreachable field should be repeated.",
+				Descriptor: f,
+			})
+		}
+		if utils.GetScalarTypeName(f) != "string" {
+			problems = append(problems, lint.Problem{
+				Message:    "unreachable field should be a string.",
+				Suggestion: "string",
+				Descriptor: f,
+				Location:   locations.FieldType(f),
+			})
+		}
+		return
+	},
+}
diff --git a/rules/rules.go b/rules/rules.go
index bc98b15..e0ad5a1 100644
--- a/rules/rules.go
+++ b/rules/rules.go
@@ -72,6 +72,7 @@ import (
 	"github.com/googleapis/api-linter/rules/aip0192"
 	"github.com/googleapis/api-linter/rules/aip0203"
 	"github.com/googleapis/api-linter/rules/aip0216"
+	"github.com/googleapis/api-linter/rules/aip0217"
 	"github.com/googleapis/api-linter/rules/aip0231"
 	"github.com/googleapis/api-linter/rules/aip0233"
 	"github.com/googleapis/api-linter/rules/aip0234"
@@ -101,6 +102,7 @@ var aipAddRulesFuncs = []addRulesFuncType{
 	aip0192.AddRules,
 	aip0203.AddRules,
 	aip0216.AddRules,
+	aip0217.AddRules,
 	aip0231.AddRules,
 	aip0233.AddRules,
 	aip0234.AddRules,


Test patch:
diff --git a/rules/aip0217/aip0217_test.go b/rules/aip0217/aip0217_test.go
new file mode 100644
index 0000000..04c7ffa
--- /dev/null
+++ b/rules/aip0217/aip0217_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 aip0217
+
+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/aip0217/synonyms_test.go b/rules/aip0217/synonyms_test.go
new file mode 100644
index 0000000..f3b2c00
--- /dev/null
+++ b/rules/aip0217/synonyms_test.go
@@ -0,0 +1,52 @@
+// 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 aip0217
+
+import (
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+	"github.com/stoewer/go-strcase"
+)
+
+func TestSynonyms(t *testing.T) {
+	for _, test := range []struct {
+		FieldName string
+		problems  testutils.Problems
+	}{
+		{"unreachable", testutils.Problems{}},
+		{"unreachable_locations", testutils.Problems{{Suggestion: "unreachable"}}},
+	} {
+		t.Run(strcase.UpperCamelCase(test.FieldName), func(t *testing.T) {
+			f := testutils.ParseProto3Tmpl(t, `
+				message ListBooksResponse {
+					repeated Book books = 1;
+					string next_page_token = 2;
+					repeated string {{.FieldName}} = 3;
+				}
+				message Book {}
+
+				// Ignored because next_page_token is not present.
+				message Irrelevant {
+					repeated string unreachable_locations = 1;
+				}
+			`, test)
+			field := f.GetMessageTypes()[0].GetFields()[2]
+			if diff := test.problems.SetDescriptor(field).Diff(synonyms.Lint(f)); diff != "" {
+				t.Errorf(diff)
+			}
+		})
+	}
+}
diff --git a/rules/aip0217/unreachable_field_type_test.go b/rules/aip0217/unreachable_field_type_test.go
new file mode 100644
index 0000000..68549fe
--- /dev/null
+++ b/rules/aip0217/unreachable_field_type_test.go
@@ -0,0 +1,50 @@
+// 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 aip0217
+
+import (
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+)
+
+func TestUnreachableFieldType(t *testing.T) {
+	for _, test := range []struct {
+		name     string
+		Prefix   string
+		problems testutils.Problems
+	}{
+		{"Valid", "repeated string", testutils.Problems{}},
+		{"InvalidSingle", "string", testutils.Problems{{Message: "repeated"}}},
+		{"InvalidType", "repeated ErrorMetadata", testutils.Problems{{Suggestion: "string"}}},
+		{"InvalidBoth", "ErrorMetadata", testutils.Problems{{Message: "repeated"}, {Suggestion: "string"}}},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			f := testutils.ParseProto3Tmpl(t, `
+				message ListBooksResponse {
+					repeated Book books = 1;
+					string next_page_token = 2;
+					{{.Prefix}} unreachable = 3;
+				}
+				message ErrorMetadata {}
+				message Book {}
+			`, test)
+			field := f.GetMessageTypes()[0].GetFields()[2]
+			if diff := test.problems.SetDescriptor(field).Diff(unreachableFieldType.Lint(f)); diff != "" {
+				t.Errorf(diff)
+			}
+		})
+	}
+}