diff --git a/docs/rules/0148/declarative-friendly-fields.md b/docs/rules/0148/declarative-friendly-fields.md new file mode 100644 index 000000000..464774cda --- /dev/null +++ b/docs/rules/0148/declarative-friendly-fields.md @@ -0,0 +1,99 @@ +--- +rule: + aip: 148 + name: [core, '0148', declarative-friendly-fields] + summary: Declarative-friendly resources must include some standard fields. +permalink: /148/declarative-friendly-fields +redirect_from: + - /0148/declarative-friendly-fields +--- + +# Declarative-friendly fields + +This rule requires certain standard fields on declarative-friendly resources, +as mandated in [AIP-148][]. + +## Details + +This rule looks at any resource with a `google.api.resource` annotation that +includes `style: DECLARATIVE_FRIENDLY`, and complains if it does not include +all of the following fields: + +- `string name` +- `string uid` +- `string display_name` +- `google.protobuf.Timestamp create_time` +- `google.protobuf.Timestamp update_time` +- `google.protobuf.Timestamp delete_time` + +## Examples + +**Incorrect** code for this rule: + +```proto +// Incorrect. +message Book { + option (google.api.resource) = { + type: "library.googleapis.com/Book" + pattern: "publishers/{publisher}/books/{book} + style: DECLARATIVE_FRIENDLY + }; + + string name = 1; + // string uid should be included! + string display_name = 2; + google.protobuf.Timestamp create_time = 3; + google.protobuf.Timestamp update_time = 4; + // google.protobuf.Timestamp delete_time should be included! +} +``` + +**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; + string uid = 2; + string display_name = 3; + google.protobuf.Timestamp create_time = 4; + google.protobuf.Timestamp update_time = 5; + google.protobuf.TImestamp delete_time = 6; +} +``` + +## Disabling + +If you need to violate this rule, use a leading comment above the message. +Remember to also include an [aip.dev/not-precedent][] comment explaining why. + +```proto +// (-- api-linter: core::0148::declarative-friendly-fields=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; + // string uid should be included! + string display_name = 2; + google.protobuf.Timestamp create_time = 3; + google.protobuf.Timestamp update_time = 4; + // google.protobuf.Timestamp delete_time should be included! +} +``` + +If you need to violate this rule for an entire file, place the comment at the +top of the file. + +[aip-148]: https://aip.dev/148 +[aip.dev/not-precedent]: https://aip.dev/not-precedent diff --git a/docs/rules/0148/human-names.md b/docs/rules/0148/human-names.md index 4770fafda..aabcc37fb 100644 --- a/docs/rules/0148/human-names.md +++ b/docs/rules/0148/human-names.md @@ -3,9 +3,9 @@ rule: aip: 148 name: [core, '0148', human-names] summary: Avoid imprecise terms for human names. -permalink: /148/standardized-codes +permalink: /148/human-names redirect_from: - - /0148/standardized-codes + - /0148/human-names --- # Human names diff --git a/rules/aip0148/aip0148.go b/rules/aip0148/aip0148.go index 1fa06bf0d..8234d687a 100644 --- a/rules/aip0148/aip0148.go +++ b/rules/aip0148/aip0148.go @@ -23,6 +23,7 @@ import ( func AddRules(r lint.RuleRegistry) error { return r.Register( 148, + declarativeFriendlyRequired, humanNames, ) } diff --git a/rules/aip0148/declarative_friendly_fields.go b/rules/aip0148/declarative_friendly_fields.go new file mode 100644 index 000000000..360831249 --- /dev/null +++ b/rules/aip0148/declarative_friendly_fields.go @@ -0,0 +1,72 @@ +// 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 aip0148 + +import ( + "fmt" + "strings" + + "bitbucket.org/creachadair/stringset" + "github.com/googleapis/api-linter/lint" + "github.com/googleapis/api-linter/rules/internal/utils" + "github.com/jhump/protoreflect/desc" +) + +var declarativeFriendlyRequired = &lint.MessageRule{ + Name: lint.NewRuleName(148, "declarative-friendly-fields"), + OnlyIf: func(m *desc.MessageDescriptor) bool { + if resource := utils.DeclarativeFriendlyResource(m); resource == m { + return true + } + return false + }, + LintMessage: func(m *desc.MessageDescriptor) []lint.Problem { + // Define the fields that are expected. + missingFields := stringset.New() + for name, typ := range reqFields { + f := m.FindFieldByName(name) + if f == nil || utils.GetTypeName(f) != typ { + missingFields.Add(fmt.Sprintf("%s %s", typ, name)) + } + } + if missingFields.Len() > 0 { + msg := "Declarative-friendly resources must include the following fields:\n" + if missingFields.Len() == 1 { + msg = fmt.Sprintf( + "Declarative-friendly resources must include the `%s` field.", + missingFields.Unordered()[0], + ) + } else { + for _, field := range missingFields.Elements() { + msg += fmt.Sprintf(" - `%s`\n", field) + } + } + return []lint.Problem{{ + Message: strings.TrimSuffix(msg, "\n"), + Descriptor: m, + }} + } + return nil + }, +} + +var reqFields = map[string]string{ + "name": "string", + "uid": "string", + "display_name": "string", + "create_time": "google.protobuf.Timestamp", + "update_time": "google.protobuf.Timestamp", + "delete_time": "google.protobuf.Timestamp", +} diff --git a/rules/aip0148/declarative_friendly_fields_test.go b/rules/aip0148/declarative_friendly_fields_test.go new file mode 100644 index 000000000..d110f7112 --- /dev/null +++ b/rules/aip0148/declarative_friendly_fields_test.go @@ -0,0 +1,105 @@ +// 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 aip0148 + +import ( + "fmt" + "strings" + "testing" + + "bitbucket.org/creachadair/stringset" + "github.com/googleapis/api-linter/rules/internal/testutils" +) + +func TestDeclarativeFriendlyFields(t *testing.T) { + for _, test := range []struct { + name string + skipped stringset.Set + }{ + {"Valid", stringset.New()}, + {"Name", stringset.New("name")}, + {"UID", stringset.New("uid")}, + {"DisplayName", stringset.New("display_name")}, + {"CreateTime", stringset.New("create_time")}, + {"UpdateTime", stringset.New("update_time")}, + {"DeleteTime", stringset.New("delete_time")}, + {"AllTimes", stringset.New("create_time", "update_time", "delete_time")}, + {"Randos", stringset.New("uid", "display_name")}, + } { + t.Run(test.name, func(t *testing.T) { + // Set up the string with the fields we will include. + fields := "" + cursor := 1 + for fieldName, fieldType := range reqFields { + if !test.skipped.Contains(fieldName) { + fields += fmt.Sprintf(" %s %s = %d;\n", fieldType, fieldName, cursor) + cursor++ + } + } + + // Create the potential problem object for the missing fields. + var problems testutils.Problems + if test.skipped.Len() == 1 { + f := test.skipped.Unordered()[0] + problems = testutils.Problems{{ + Message: fmt.Sprintf("must include the `%s %s` field", reqFields[f], f), + }} + } else if test.skipped.Len() > 1 { + missingFields := stringset.New() + for _, f := range test.skipped.Unordered() { + missingFields.Add(fmt.Sprintf("%s %s", reqFields[f], f)) + } + msg := "" + for _, f := range missingFields.Elements() { + msg += fmt.Sprintf(" - `%s`\n", f) + } + problems = testutils.Problems{{Message: strings.TrimSuffix(msg, "\n")}} + } + + // Test against declarative-friendly and standard styles. + for _, subtest := range []struct { + name string + style string + problems testutils.Problems + }{ + {"DeclFriendly", "style: DECLARATIVE_FRIENDLY", problems}, + {"NotDeclFriendly", "", nil}, + } { + t.Run(subtest.name, func(t *testing.T) { + f := testutils.ParseProto3Tmpl(t, ` + import "google/api/resource.proto"; + import "google/protobuf/timestamp.proto"; + message Book { + option (google.api.resource) = { + type: "library.googleapis.com/Book" + pattern: "publishers/{publisher}/books/{book}" + {{.Style}} + }; + {{.Fields}} + } + `, struct { + Fields string + Style string + }{Fields: fields, Style: subtest.style}) + m := f.GetMessageTypes()[0] + got := declarativeFriendlyRequired.Lint(f) + if diff := subtest.problems.SetDescriptor(m).Diff(got); diff != "" { + t.Errorf(diff) + } + }) + } + }) + } +}