diff --git a/docs/rules/0128/resource-reconciling-field.md b/docs/rules/0128/resource-reconciling-field.md new file mode 100644 index 000000000..f766e2cc1 --- /dev/null +++ b/docs/rules/0128/resource-reconciling-field.md @@ -0,0 +1,95 @@ +--- +rule: + aip: 128 + name: [core, '0128', resource-reconciling-field] + summary: Declarative-friendly resources must have a `reconciling` field. +permalink: /128/resource-reconciling-field +redirect_from: + - /0128/resource-reconciling-field +--- + +# Declarative-friendly resources: Reconciling field + +This rule enforces that all declarative-friendly resources have a `bool +reconciling` field, as mandated in [AIP-128][]. + +## Details + +This rule looks at any message with a `google.api.resource` annotation that +includes `style: DECLARATIVE_FRIENDLY`, and complains if the `reconciling` field +is missing or if it has any type other than `bool`. + +## Examples + +**Incorrect** code for this rule: + +```proto +// Incorrect. +// The `reconciling` field is missing. +message Book { + option (google.api.resource) = { + type: "library.googleapis.com/Book" + pattern: "publishers/{publisher}/books/{book}" + style: DECLARATIVE_FRIENDLY + }; + + string name = 1; +} +``` + +```proto +// Incorrect. +message Book { + option (google.api.resource) = { + type: "library.googleapis.com/Book" + pattern: "publishers/{publisher}/books/{book}" + style: DECLARATIVE_FRIENDLY + }; + + string name = 1; + int32 reconciling = 2; // Type should be `bool`. +} +``` + +**Correct** code for this rule: + +```proto +// Correct. +message Book { + option (google.api.resource) = { + type: "library.googleapis.com/Book" + pattern: "publishers/{publisher}/books/{book}" + style: DECLARATIVE_FRIENDLY + }; + + string name = 1; + bool reconciling = 2; +} +``` + +## Disabling + +If you need to violate this rule, use a leading comment above the message (if +the `reconciling` field is missing) or above the field (if it is the wrong +type). Remember to also include an [aip.dev/not-precedent][] comment explaining +why. + +```proto +// (-- api-linter: core::0128::resource-reconciling-field=disabled +// aip.dev/not-precedent: We need to do this because reasons. --) +message Book { + option (google.api.resource) = { + type: "library.googleapis.com/Book" + pattern: "publishers/{publisher}/books/{book}" + style: DECLARATIVE_FRIENDLY + }; + + string name = 1; +} +``` + +If you need to violate this rule for an entire file, place the comment at the +top of the file. + +[aip-128]: https://aip.dev/128 +[aip.dev/not-precedent]: https://aip.dev/not-precedent diff --git a/rules/aip0128/aip0128.go b/rules/aip0128/aip0128.go index 27f96e0bc..6dd615b82 100644 --- a/rules/aip0128/aip0128.go +++ b/rules/aip0128/aip0128.go @@ -25,5 +25,6 @@ func AddRules(r lint.RuleRegistry) error { return r.Register( 128, resourceAnnotationsField, + resourceReconcilingField, ) } diff --git a/rules/aip0128/resource_reconciling_field.go b/rules/aip0128/resource_reconciling_field.go new file mode 100644 index 000000000..abc06650a --- /dev/null +++ b/rules/aip0128/resource_reconciling_field.go @@ -0,0 +1,55 @@ +// 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 aip0128 + +import ( + "fmt" + + "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" + "github.com/jhump/protoreflect/desc/builder" +) + +var resourceReconcilingField = &lint.MessageRule{ + Name: lint.NewRuleName(128, "resource-reconciling-field"), + OnlyIf: func(m *desc.MessageDescriptor) bool { + // IsDeclarativeFriendly returns true for both + // resources and request messages, but we only care about resources. + resource := utils.DeclarativeFriendlyResource(m) + return resource != nil && resource == m + }, + LintMessage: func(m *desc.MessageDescriptor) []lint.Problem { + f := m.FindFieldByName("reconciling") + if f == nil { + return []lint.Problem{{ + Message: fmt.Sprintf("Declarative-friendly %q has no `reconciling` field.", m.GetName()), + Descriptor: m, + }} + } + + if f.GetType() != builder.FieldTypeBool().GetType() || f.IsRepeated() { + return []lint.Problem{{ + Message: "The `reconciling` field should be a singular bool.", + Descriptor: f, + Location: locations.FieldType(f), + Suggestion: "bool", + }} + } + + return nil + }, +} diff --git a/rules/aip0128/resource_reconciling_field_test.go b/rules/aip0128/resource_reconciling_field_test.go new file mode 100644 index 000000000..5f3832276 --- /dev/null +++ b/rules/aip0128/resource_reconciling_field_test.go @@ -0,0 +1,61 @@ +// 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 aip0128 + +import ( + "testing" + + "github.com/googleapis/api-linter/rules/internal/testutils" + "github.com/jhump/protoreflect/desc" +) + +func TestResourceReconcilingField(t *testing.T) { + for _, test := range []struct { + name string + Style string + Reconciling string + problems testutils.Problems + }{ + {"ValidMissingNotDF", "", "", nil}, + {"ValidPresentNotDF", "", "bool reconciling = 2;", nil}, + {"ValidBadTypeNotDF", "", "int32 reconciling = 2;", nil}, + {"ValidPresentDF", "style: DECLARATIVE_FRIENDLY", "bool reconciling = 2;", nil}, + {"InvalidMissingDF", "style: DECLARATIVE_FRIENDLY", "", testutils.Problems{{Message: "reconciling"}}}, + {"InvalidBadTypeDF", "style: DECLARATIVE_FRIENDLY", "int32 reconciling = 2;", testutils.Problems{{Suggestion: "bool"}}}, + {"InvalidRepeatedDF", "style: DECLARATIVE_FRIENDLY", "repeated bool reconciling = 2;", testutils.Problems{{Suggestion: "bool"}}}, + } { + t.Run(test.name, func(t *testing.T) { + f := testutils.ParseProto3Tmpl(t, ` + import "google/api/resource.proto"; + message Book { + option (google.api.resource) = { + type: "library.googleapis.com/Book" + {{.Style}} + }; + string name = 1; + {{.Reconciling}} + } + message DeleteBookRequest {} + `, test) + var d desc.Descriptor = f.GetMessageTypes()[0] + if test.name == "InvalidBadTypeDF" || test.name == "InvalidRepeatedDF" { + d = f.GetMessageTypes()[0].GetFields()[1] + } + if diff := test.problems.SetDescriptor(d).Diff(resourceReconcilingField.Lint(f)); diff != "" { + t.Errorf(diff) + } + }) + } +}