-
Notifications
You must be signed in to change notification settings - Fork 144
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(AIP-213): Lint message fields that should use common types
- Loading branch information
Showing
10 changed files
with
457 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
--- | ||
rule: | ||
aip: 213 | ||
name: [core, '0213', common-types-fields] | ||
summary: Message fields with certain names should use a common type. | ||
permalink: /213/common-types-fields | ||
redirect_from: | ||
- /0213/common-types-fields | ||
--- | ||
|
||
# Common types fields | ||
|
||
This rule enforces that message fields with specific names use a common type, as | ||
recommended in [AIP-213][]. | ||
|
||
## Details | ||
|
||
This rule looks at the fields in a message, and complains if it finds a field | ||
with a specific name that doesn't have the corresponding common type. | ||
|
||
Some example pairings of common types and field names that are checked are: | ||
|
||
* `google.protobuf.Duration`: "duration" | ||
* `google.type.Color`: "color", "colour" | ||
* `google.type.PhoneNumber`: "mobile_number", "phone", "phone_number" | ||
|
||
## Examples | ||
|
||
**Incorrect** code for this rule: | ||
|
||
```proto | ||
// Incorrect. | ||
message Book { | ||
string name = 1; | ||
// Should use `google.type.Color`. | ||
string color = 2; | ||
} | ||
``` | ||
|
||
```proto | ||
// Incorrect. | ||
message Book { | ||
string name = 1; | ||
// Should use `google.type.PhoneNumber`. | ||
string phone_number = 2; | ||
} | ||
``` | ||
|
||
**Correct** code for this rule: | ||
|
||
```proto | ||
// Correct. | ||
message Book { | ||
string name = 1; | ||
google.type.Color color = 2; | ||
} | ||
``` | ||
|
||
```proto | ||
// Correct. | ||
message Book { | ||
string name = 1; | ||
google.type.PhoneNumber phone_number = 2; | ||
} | ||
``` | ||
|
||
## Disabling | ||
|
||
If you need to violate this rule, use a leading comment above the enum. | ||
Remember to also include an [aip.dev/not-precedent][] comment explaining why. | ||
|
||
```proto | ||
message Book { | ||
string name = 1; | ||
// (-- api-linter: core::0213::common-types-fields=disabled | ||
// aip.dev/not-precedent: We need to do this because reasons. --) | ||
google.protobuf.Timestamp expire_time = 2 | ||
[(google.api.field_behavior) = OUTPUT_ONLY]; | ||
} | ||
``` | ||
|
||
If you need to violate this rule for an entire file, place the comment at the | ||
top of the file. | ||
|
||
[aip-213]: https://aip.dev/213 | ||
[aip.dev/not-precedent]: https://aip.dev/not-precedent |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
--- | ||
rule: | ||
aip: 213 | ||
name: [core, '0213', common-types-messages] | ||
summary: Messages containing fields of a common type should use the common type. | ||
permalink: /213/common-types-messages | ||
redirect_from: | ||
- /0213/common-types-messages | ||
--- | ||
|
||
# Common types messages | ||
|
||
This rule enforces that messages that contain the fields of a common type use the | ||
common type itself, as recommended in [AIP-213][]. | ||
|
||
## Details | ||
|
||
This rule looks at the fields in a message, and complains if it finds a set of | ||
field names that all belong to a common type. | ||
|
||
## Examples | ||
|
||
**Incorrect** code for this rule: | ||
|
||
```proto | ||
// Incorrect. | ||
message Book { | ||
string name = 1; | ||
// Should use `google.type.Interval`. | ||
google.protobuf.Timestamp start_time = 2; | ||
google.protobuf.Timestamp end_time = 3; | ||
} | ||
``` | ||
|
||
**Correct** code for this rule: | ||
|
||
```proto | ||
// Correct. | ||
message Book { | ||
string name = 1; | ||
// Should use `google.type.Interval`. | ||
google.type.Interval interval = 2; | ||
} | ||
``` | ||
|
||
## Disabling | ||
|
||
If you need to violate this rule, use a leading comment above the enum. | ||
Remember to also include an [aip.dev/not-precedent][] comment explaining why. | ||
|
||
```proto | ||
message Book { | ||
string name = 1; | ||
// (-- api-linter: core::0213::common-types-messages=disabled | ||
// aip.dev/not-precedent: We need to do this because reasons. --) | ||
google.protobuf.Timestamp expire_time = 2 | ||
[(google.api.field_behavior) = OUTPUT_ONLY]; | ||
} | ||
``` | ||
|
||
If you need to violate this rule for an entire file, place the comment at the | ||
top of the file. | ||
|
||
[aip-213]: https://aip.dev/213 | ||
[aip.dev/not-precedent]: https://aip.dev/not-precedent |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
--- | ||
aip_listing: 213 | ||
permalink: /213/ | ||
redirect_from: | ||
- /0213/ | ||
--- | ||
|
||
# Common components | ||
|
||
{% include linter-aip-listing.md aip=213 %} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// Copyright 2023 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 aip0213 contains rules defined in https://aip.dev/213. | ||
package aip0213 | ||
|
||
import ( | ||
"github.com/googleapis/api-linter/lint" | ||
) | ||
|
||
// AddRules adds all of the AIP-213 rules to the provided registry. | ||
func AddRules(r lint.RuleRegistry) error { | ||
return r.Register( | ||
213, | ||
commonTypesFields, | ||
commonTypesMessages, | ||
) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
// Copyright 2023 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 aip0213 | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/googleapis/api-linter/lint" | ||
) | ||
|
||
func TestAddRules(t *testing.T) { | ||
if err := AddRules(lint.NewRuleRegistry()); err != nil { | ||
t.Errorf("AddRules got an error: %v", err) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
// Copyright 2023 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 aip0213 | ||
|
||
import ( | ||
"fmt" | ||
|
||
"github.com/googleapis/api-linter/lint" | ||
"github.com/jhump/protoreflect/desc" | ||
) | ||
|
||
var fieldNameToCommonType = map[string]string{ | ||
"duration": "google.protobuf.Duration", | ||
|
||
"color": "google.type.Color", | ||
"colour": "google.type.Color", | ||
|
||
"dollars": "google.type.Money", | ||
"euros": "google.type.Money", | ||
"money": "google.type.Money", | ||
"pounds": "google.type.Money", | ||
"yen": "google.type.Money", | ||
"yuan": "google.type.Money", | ||
|
||
"mobile_number": "google.type.PhoneNumber", | ||
"phone": "google.type.PhoneNumber", | ||
"phone_number": "google.type.PhoneNumber", | ||
|
||
"clock_time": "google.type.TimeOfDay", | ||
"time_of_day": "google.type.TimeOfDay", | ||
|
||
"timezone": "google.type.TimeZone", | ||
"time_zone": "google.type.TimeZone", | ||
} | ||
|
||
var commonTypesFields = &lint.FieldRule{ | ||
Name: lint.NewRuleName(213, "common-types-fields"), | ||
LintField: func(f *desc.FieldDescriptor) []lint.Problem { | ||
// Flag this field if it has a name in `fieldNameToCommonType` but | ||
// doesn't have its corresponding type. | ||
if messageType, ok := fieldNameToCommonType[f.GetName()]; ok { | ||
if f.GetMessageType() == nil || f.GetMessageType().GetFullyQualifiedName() != messageType { | ||
return []lint.Problem{{ | ||
Message: fmt.Sprintf("Consider using the common type %q.", messageType), | ||
Descriptor: f, | ||
}} | ||
} | ||
} | ||
return nil | ||
}, | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
// Copyright 2023 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 aip0213 | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/googleapis/api-linter/rules/internal/testutils" | ||
) | ||
|
||
func TestCommonTypesFields(t *testing.T) { | ||
for _, test := range []struct { | ||
name string | ||
Field string | ||
problems testutils.Problems | ||
}{ | ||
{"ValidCommonType", "google.protobuf.Duration duration", nil}, | ||
{"ValidOtherType", "string bar", nil}, | ||
{"FieldDoesNotUseMessageType", "map<int32, int32> duration", testutils.Problems{{Message: "common type"}}}, | ||
{"FieldDoesNotUseCommonType", "int32 duration", testutils.Problems{{Message: "common type"}}}, | ||
} { | ||
t.Run(test.name, func(t *testing.T) { | ||
f := testutils.ParseProto3Tmpl(t, ` | ||
import "google/protobuf/duration.proto"; | ||
message Foo { | ||
{{.Field}} = 1; | ||
} | ||
`, test) | ||
field := f.GetMessageTypes()[0].GetFields()[0] | ||
if diff := test.problems.SetDescriptor(field).Diff(commonTypesFields.Lint(f)); diff != "" { | ||
t.Errorf(diff) | ||
} | ||
}) | ||
} | ||
} |
Oops, something went wrong.