Skip to content

Commit

Permalink
feat: Add rule for required standard fields.
Browse files Browse the repository at this point in the history
Fixes #659.
  • Loading branch information
Luke Sneeringer committed Oct 14, 2020
1 parent e89f988 commit 1704b19
Show file tree
Hide file tree
Showing 5 changed files with 279 additions and 2 deletions.
99 changes: 99 additions & 0 deletions docs/rules/0148/declarative-friendly-fields.md
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions docs/rules/0148/human-names.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions rules/aip0148/aip0148.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
func AddRules(r lint.RuleRegistry) error {
return r.Register(
148,
declarativeFriendlyRequired,
humanNames,
)
}
72 changes: 72 additions & 0 deletions rules/aip0148/declarative_friendly_fields.go
Original file line number Diff line number Diff line change
@@ -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-required"),
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",
}
105 changes: 105 additions & 0 deletions rules/aip0148/declarative_friendly_fields_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
})
}
}

0 comments on commit 1704b19

Please sign in to comment.