From 7d5311b4e70d90863c2b702c0f063301bfbfdf4c Mon Sep 17 00:00:00 2001 From: Luke Sneeringer Date: Mon, 24 Aug 2020 10:01:33 -0600 Subject: [PATCH 1/2] feat: Resource references should be strings. --- docs/rules/0122/resource-reference-type.md | 82 +++++++++++++++++++ rules/aip0122/resource_reference_type.go | 36 ++++++++ rules/aip0122/resource_reference_type_test.go | 53 ++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 docs/rules/0122/resource-reference-type.md create mode 100644 rules/aip0122/resource_reference_type.go create mode 100644 rules/aip0122/resource_reference_type_test.go diff --git a/docs/rules/0122/resource-reference-type.md b/docs/rules/0122/resource-reference-type.md new file mode 100644 index 000000000..db7971973 --- /dev/null +++ b/docs/rules/0122/resource-reference-type.md @@ -0,0 +1,82 @@ +--- +rule: + aip: 122 + name: [core, '0122', resource-reference-type] + summary: All resource references must be strings. +permalink: /122/resource-reference-type +redirect_from: + - /0122/resource-reference-type +--- + +# Resource reference type + +This rule enforces that all fields with the `google.api.resource_reference` +annotation are strings, as mandated in [AIP-122][]. + +## Details + +This rule complains if it sees a field with a `google.api.resource_reference` +that has a type other than `string`. + +## Examples + +**Incorrect** code for this rule: + +```proto +// Incorrect. +message Book { + string name = 1; + + // Resource references should be strings. + Author author = 2 [(google.api.resource_reference) = { + type: "library.googleapis.com/Author" + }]; +} +``` + +**Correct** code for this rule: + +```proto +// Correct. +message Book { + string name = 1; + + string author = 2 [(google.api.resource_reference) = { + type: "library.googleapis.com/Author" + }]; +} +``` + +```proto +// Correct. +message Book { + string name = 1; + + // If "author" is not a first-class resource, then it may be a composite + // field within the book. + Author author = 2; +} +``` + +## Disabling + +If you need to violate this rule, use a leading comment above the method. +Remember to also include an [aip.dev/not-precedent][] comment explaining why. + +```proto +message Book { + string name = 1; + + // (-- api-linter: core::0122::resource-reference-type=disabled + // aip.dev/not-precedent: We need to do this because reasons. --) + Author author = 2 [(google.api.resource_reference) = { + type: "library.googleapis.com/Author" + }]; +} +``` + +If you need to violate this rule for an entire file, place the comment at the +top of the file. + +[aip-122]: https://aip.dev/122 +[aip.dev/not-precedent]: https://aip.dev/not-precedent diff --git a/rules/aip0122/resource_reference_type.go b/rules/aip0122/resource_reference_type.go new file mode 100644 index 000000000..a42e2d5e1 --- /dev/null +++ b/rules/aip0122/resource_reference_type.go @@ -0,0 +1,36 @@ +// 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 aip0122 + +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 resourceReferenceType = &lint.FieldRule{ + Name: lint.NewRuleName(122, "resource-reference-type"), + LintField: func(f *desc.FieldDescriptor) []lint.Problem { + if utils.GetResourceReference(f) != nil && utils.GetTypeName(f) != "string" { + return []lint.Problem{{ + Message: "The resource_reference annotation should only be used on string fields.", + Descriptor: f, + Location: locations.FieldResourceReference(f), + }} + } + return nil + }, +} diff --git a/rules/aip0122/resource_reference_type_test.go b/rules/aip0122/resource_reference_type_test.go new file mode 100644 index 000000000..6edf6f76a --- /dev/null +++ b/rules/aip0122/resource_reference_type_test.go @@ -0,0 +1,53 @@ +// 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 aip0122 + +import ( + "testing" + + "github.com/googleapis/api-linter/rules/internal/testutils" +) + +func TestResourceReferenceType(t *testing.T) { + ann := ` [(google.api.resource_reference) = { + type: "library.googleapis.com/Author" + }]` + for _, test := range []struct { + name string + Type string + Annotation string + problems testutils.Problems + }{ + {"ValidString", "string", ann, nil}, + {"InvalidMessage", "Author", ann, testutils.Problems{{Message: "string fields"}}}, + {"IrrelevantNoAnnotation", "Author", "", nil}, + } { + t.Run(test.name, func(t *testing.T) { + f := testutils.ParseProto3Tmpl(t, ` + import "google/api/resource.proto"; + + message Book { + {{.Type}} author = 1{{.Annotation}}; + + } + message Author {} + `, test) + field := f.GetMessageTypes()[0].GetFields()[0] + if diff := test.problems.SetDescriptor(field).Diff(resourceReferenceType.Lint(f)); diff != "" { + t.Errorf(diff) + } + }) + } +} From 45cac65d45fcfaf3c49f2b4ca64d6c9c20b8df4a Mon Sep 17 00:00:00 2001 From: Luke Sneeringer Date: Mon, 24 Aug 2020 10:03:09 -0600 Subject: [PATCH 2/2] Add rule to aip0122.AllRules --- rules/aip0122/aip0122.go | 1 + 1 file changed, 1 insertion(+) diff --git a/rules/aip0122/aip0122.go b/rules/aip0122/aip0122.go index 47db3b223..241e79484 100644 --- a/rules/aip0122/aip0122.go +++ b/rules/aip0122/aip0122.go @@ -26,5 +26,6 @@ func AddRules(r lint.RuleRegistry) error { 122, httpURICase, nameSuffix, + resourceReferenceType, ) }