Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add rule for AIP-162 Commit request name field #732

Merged
merged 2 commits into from
Jan 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions docs/rules/0162/commit-request-name-field.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
---
rule:
aip: 162
name: [core, '0162', commit-request-name-field]
summary: Commit RPCs must have a `name` field in the request.
permalink: /162/commit-request-name-field
redirect_from:
- /0162/commit-request-name-field
---

# Commit requests: Name field

This rule enforces that all `Commit` methods have a `string name`
field in the request message, as mandated in [AIP-162][].

## Details

This rule looks at any message matching `Commit*Request` and complains if
either the `name` field is missing or it has any type other than `string`.

## Examples

**Incorrect** code for this rule:

```proto
// Incorrect.
// Should include a `string name` field.
message CommitBookRequest {
}
```

```proto
// Incorrect.
message CommitBookRequest {
// Field type should be `string`.
bytes name = 1 [
(google.api.field_behavior) = REQUIRED,
(google.api.resource_reference).type = "library.googleapis.com/Book"
];
}
```

**Correct** code for this rule:

```proto
// Correct.
message CommitBookRequest {
string name = 1 [
(google.api.field_behavior) = REQUIRED,
(google.api.resource_reference).type = "library.googleapis.com/Book"
];
}
```

## Disabling

If you need to violate this rule, use a leading comment above the message (if
the `name` 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
message CommitBookRequest {
// (-- api-linter: core::0162::commit-request-name-field=disabled
// aip.dev/not-precedent: We need to do this because reasons. --)
bytes name = 1 [
(google.api.field_behavior) = REQUIRED,
(google.api.resource_reference).type = "library.googleapis.com/Book"
];
}
```

If you need to violate this rule for an entire file, place the comment at the
top of the file.

[aip-162]: https://aip.dev/162
[aip.dev/not-precedent]: https://aip.dev/not-precedent
7 changes: 7 additions & 0 deletions rules/aip0162/aip0162.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func AddRules(r lint.RuleRegistry) error {
commitHTTPBody,
commitHTTPMethod,
commitHTTPURISuffix,
commitRequestNameField,
commitRequestMessageName,
tagRevisionHTTPBody,
tagRevisionHTTPMethod,
Expand Down Expand Up @@ -59,9 +60,15 @@ func isTagRevisionRequestMessage(m *desc.MessageDescriptor) bool {
}

var commitMethodRegexp = regexp.MustCompile(`^Commit(?:[A-Za-z0-9]+)$`)
var commitReqMessageRegexp = regexp.MustCompile(`^Commit(?:[A-Za-z0-9]+)Request$`)
var commitURINameRegexp = regexp.MustCompile(`:commit$`)

// Returns true if this is an AIP-162 Commit method, false otherwise.
func isCommitMethod(m *desc.MethodDescriptor) bool {
return commitMethodRegexp.MatchString(m.GetName())
}

// Returns true if this is an AIP-162 Commit request message, false otherwise.
func isCommitRequestMessage(m *desc.MessageDescriptor) bool {
return commitReqMessageRegexp.MatchString(m.GetName())
}
52 changes: 52 additions & 0 deletions rules/aip0162/commit_request_name_field.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright 2021 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 aip0162

import (
"fmt"

"github.com/googleapis/api-linter/lint"
"github.com/googleapis/api-linter/locations"
"github.com/jhump/protoreflect/desc"
"github.com/jhump/protoreflect/desc/builder"
)

// The Commit request message should have a name field.
var commitRequestNameField = &lint.MessageRule{
Name: lint.NewRuleName(162, "commit-request-name-field"),
OnlyIf: isCommitRequestMessage,
LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
// Rule check: Establish that a `name` field is present.
nameField := m.FindFieldByName("name")
if nameField == nil {
return []lint.Problem{{
Message: fmt.Sprintf("Message %q has no `name` field.", m.GetName()),
Descriptor: m,
}}
}

// Rule check: Establish that the name field is a string.
if nameField.GetType() != builder.FieldTypeString().GetType() || nameField.IsRepeated() {
return []lint.Problem{{
Message: "`name` field on Commit request message must be a singular string.",
Descriptor: nameField,
Location: locations.FieldType(nameField),
Suggestion: "string",
}}
}

return nil
},
}
51 changes: 51 additions & 0 deletions rules/aip0162/commit_request_name_field_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2021 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 aip0162

import (
"testing"

"github.com/googleapis/api-linter/rules/internal/testutils"
"github.com/jhump/protoreflect/desc"
)

func TestCommitRequestNameField(t *testing.T) {
for _, test := range []struct {
name string
RPC string
Field string
problems testutils.Problems
}{
{"Valid", "CommitBook", "string name = 1;", nil},
{"Missing", "CommitBook", "", testutils.Problems{{Message: "no `name`"}}},
{"InvalidType", "CommitBook", "int32 name = 1;", testutils.Problems{{Suggestion: "string"}}},
{"IrrelevantRPCName", "EnumerateBooks", "", nil},
} {
t.Run(test.name, func(t *testing.T) {
f := testutils.ParseProto3Tmpl(t, `
message {{.RPC}}Request {
{{.Field}}
}
`, test)
var d desc.Descriptor = f.GetMessageTypes()[0]
if test.name == "InvalidType" {
d = f.GetMessageTypes()[0].GetFields()[0]
}
if diff := test.problems.SetDescriptor(d).Diff(commitRequestNameField.Lint(f)); diff != "" {
t.Errorf(diff)
}
})
}
}