Repository: googleapis/api-linter (go)

Issue:
feat(AIP-202): Enforce field format type restrictions
AIP-202, being added in https://github.com/aip-dev/google.aip.dev/pull/1216 requires that certain field formats are used only on certain proto field types. This needs to be enforced.

All of `UUID4`, `IPV4`, `IPV6` and `IPV4_OR_IPV6` must only be used on `string` fields. The implementation should be flexible enough to support other format<>type mappings.

Pull request description:
feat(AIP-202): add string-only-format rule
Adds a new rule `string-only-format` to start the rule group for AIP-202. Some of the `format` values must only be used on `string` fields, which this enforces.

Fixes #1257 

Patch:
diff --git a/docs/rules/0202/index.md b/docs/rules/0202/index.md
new file mode 100644
index 0000000..18836c9
--- /dev/null
+++ b/docs/rules/0202/index.md
@@ -0,0 +1,10 @@
+---
+aip_listing: 202
+permalink: /202/
+redirect_from:
+  - /0202/
+---
+
+# Field behavior
+
+{% include linter-aip-listing.md aip=202 %}
diff --git a/docs/rules/0202/string-only-format.md b/docs/rules/0202/string-only-format.md
new file mode 100644
index 0000000..e66d085
--- /dev/null
+++ b/docs/rules/0202/string-only-format.md
@@ -0,0 +1,73 @@
+---
+rule:
+  aip: 202
+  name: [core, '0202', string-only-format]
+  summary: Certain field formats can only be applied to a string field.
+permalink: /202/string-only-format
+redirect_from:
+  - /0202/string-only-format
+---
+
+# String only format
+
+This rule enforces that the following format specifiers are used only on fields
+of type `string`, as mandated by [AIP-202][]:
+
+- `UUID4`
+- `IPV4`
+- `IPV6`
+- `IPV4_OR_IPV6`
+
+## Details
+
+This rule looks at every non-string field with a
+`(google.api.field_info).format` and complains if the format specifier is one
+meant only for use on `string` fields.
+
+## Examples
+
+**Incorrect** code for this rule:
+
+```proto
+message Book {
+  string name = 1;
+
+  // Incorrect. Non-string must not be assigned format UUID4.
+  int32 edition = 2 [(google.api.field_info).format = UUID4];
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+message Book {
+  string name = 1;
+
+  int32 edition = 2;
+
+  // This is a correct example of a string-only format
+  // on a string field.
+  string uid = 3 [(google.api.field_info).format = UUID4];
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the field.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+message Book {
+  string name = 1;
+
+  // (-- api-linter: core::0202::string-only-format=disabled
+  //     aip.dev/not-precedent: We need to do this because reasons. --)
+  int32 edition = 2 [(google.api.field_info).format = UUID4];
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-202]: https://aip.dev/202
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
\ No newline at end of file
diff --git a/rules/aip0202/aip0202.go b/rules/aip0202/aip0202.go
new file mode 100644
index 0000000..c207ba5
--- /dev/null
+++ b/rules/aip0202/aip0202.go
@@ -0,0 +1,28 @@
+// 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 aip0202 contains rules defined in https://aip.dev/202.
+package aip0202
+
+import (
+	"github.com/googleapis/api-linter/lint"
+)
+
+// AddRules adds all of the AIP-202 rules to the provided registry.
+func AddRules(r lint.RuleRegistry) error {
+	return r.Register(
+		202,
+		stringOnlyFormat,
+	)
+}
diff --git a/rules/aip0202/string_only_format.go b/rules/aip0202/string_only_format.go
new file mode 100644
index 0000000..f545466
--- /dev/null
+++ b/rules/aip0202/string_only_format.go
@@ -0,0 +1,50 @@
+// 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 aip0202
+
+import (
+	"fmt"
+
+	"bitbucket.org/creachadair/stringset"
+	"github.com/googleapis/api-linter/lint"
+	"github.com/googleapis/api-linter/rules/internal/utils"
+	"github.com/jhump/protoreflect/desc"
+	apb "google.golang.org/genproto/googleapis/api/annotations"
+	"google.golang.org/protobuf/types/descriptorpb"
+)
+
+var stringOnlyFormats = stringset.New(
+	apb.FieldInfo_IPV4.String(),
+	apb.FieldInfo_IPV6.String(),
+	apb.FieldInfo_IPV4_OR_IPV6.String(),
+	apb.FieldInfo_UUID4.String(),
+)
+
+var stringOnlyFormat = &lint.FieldRule{
+	Name: lint.NewRuleName(202, "string-only-format"),
+	OnlyIf: func(fd *desc.FieldDescriptor) bool {
+		return utils.HasFormat(fd) && fd.GetType() != descriptorpb.FieldDescriptorProto_TYPE_STRING
+	},
+	LintField: func(fd *desc.FieldDescriptor) []lint.Problem {
+		// Field being linted is not a string, check that it isn't using a string-only format.
+		if format := utils.GetFormat(fd).String(); stringOnlyFormats.Contains(format) {
+			return []lint.Problem{{
+				Message:    fmt.Sprintf("Format %q must only be used on string fields", format),
+				Descriptor: fd,
+			}}
+		}
+		return nil
+	},
+}
diff --git a/rules/internal/utils/extension.go b/rules/internal/utils/extension.go
index 22ad194..be19948 100644
--- a/rules/internal/utils/extension.go
+++ b/rules/internal/utils/extension.go
@@ -252,3 +252,31 @@ func FindResourceChildren(parent *apb.ResourceDescriptor, file *desc.FileDescrip
 
 	return children
 }
+
+func HasFieldInfo(fd *desc.FieldDescriptor) bool {
+	return fd != nil && proto.HasExtension(fd.GetFieldOptions(), apb.E_FieldInfo)
+}
+
+func GetFieldInfo(fd *desc.FieldDescriptor) *apb.FieldInfo {
+	if !HasFieldInfo(fd) {
+		return nil
+	}
+
+	return proto.GetExtension(fd.GetFieldOptions(), apb.E_FieldInfo).(*apb.FieldInfo)
+}
+
+func HasFormat(fd *desc.FieldDescriptor) bool {
+	if !HasFieldInfo(fd) {
+		return false
+	}
+
+	fi := GetFieldInfo(fd)
+	return fi.GetFormat() != apb.FieldInfo_FORMAT_UNSPECIFIED
+}
+
+func GetFormat(fd *desc.FieldDescriptor) apb.FieldInfo_Format {
+	if !HasFormat(fd) {
+		return apb.FieldInfo_FORMAT_UNSPECIFIED
+	}
+	return GetFieldInfo(fd).GetFormat()
+}
diff --git a/rules/rules.go b/rules/rules.go
index f2a3916..3c290ad 100644
--- a/rules/rules.go
+++ b/rules/rules.go
@@ -84,6 +84,7 @@ import (
 	"github.com/googleapis/api-linter/rules/aip0165"
 	"github.com/googleapis/api-linter/rules/aip0191"
 	"github.com/googleapis/api-linter/rules/aip0192"
+	"github.com/googleapis/api-linter/rules/aip0202"
 	"github.com/googleapis/api-linter/rules/aip0203"
 	"github.com/googleapis/api-linter/rules/aip0214"
 	"github.com/googleapis/api-linter/rules/aip0215"
@@ -132,6 +133,7 @@ var aipAddRulesFuncs = []addRulesFuncType{
 	aip0165.AddRules,
 	aip0191.AddRules,
 	aip0192.AddRules,
+	aip0202.AddRules,
 	aip0203.AddRules,
 	aip0214.AddRules,
 	aip0215.AddRules,


Test patch:
diff --git a/rules/aip0202/aip0202_test.go b/rules/aip0202/aip0202_test.go
new file mode 100644
index 0000000..8438782
--- /dev/null
+++ b/rules/aip0202/aip0202_test.go
@@ -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 aip0202
+
+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)
+	}
+}
diff --git a/rules/aip0202/string_only_format_test.go b/rules/aip0202/string_only_format_test.go
new file mode 100644
index 0000000..71dffb6
--- /dev/null
+++ b/rules/aip0202/string_only_format_test.go
@@ -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 aip0202
+
+import (
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+)
+
+func TestStringOnlyFormat(t *testing.T) {
+	for _, tc := range []struct {
+		name, Type, Format string
+		problems           testutils.Problems
+	}{
+		{
+			name:     "ValidSkipString",
+			Type:     "string",
+			Format:   "format: UUID4",
+			problems: nil,
+		},
+		{
+			name:     "ValidSkipNoFormat",
+			Type:     "int32",
+			problems: nil,
+		},
+		{
+			name:     "InvalidNonString",
+			Type:     "int32",
+			Format:   "format: UUID4",
+			problems: testutils.Problems{{Message: "string fields"}},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+			import "google/api/field_info.proto";
+			
+			message CreateBookRequest {
+				{{.Type}} foo = 1 [(google.api.field_info) = {
+					{{.Format}}
+				}];
+			}
+			`, tc)
+			fd := file.FindMessage("CreateBookRequest").FindFieldByName("foo")
+			problems := stringOnlyFormat.Lint(file)
+			if diff := tc.problems.SetDescriptor(fd).Diff(problems); diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}
diff --git a/rules/internal/utils/extension_test.go b/rules/internal/utils/extension_test.go
index c2c42ab..dea5e4b 100644
--- a/rules/internal/utils/extension_test.go
+++ b/rules/internal/utils/extension_test.go
@@ -622,3 +622,135 @@ func TestFindResourceChildren(t *testing.T) {
 		})
 	}
 }
+
+func TestHasFieldInfo(t *testing.T) {
+	testCases := []struct {
+		name, FieldInfo string
+		want            bool
+	}{
+		{
+			name:      "HasFieldInfo",
+			FieldInfo: "[(google.api.field_info).format = UUID4]",
+			want:      true,
+		},
+		{
+			name: "NoFieldInfo",
+			want: false,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+			import "google/api/field_info.proto";
+			
+			message CreateBookRequest {
+				string foo = 1 {{.FieldInfo}};
+			}
+			`, tc)
+			fd := file.FindMessage("CreateBookRequest").FindFieldByName("foo")
+			if got := HasFieldInfo(fd); got != tc.want {
+				t.Errorf("HasFieldInfo(%+v): expected %v, got %v", fd, tc.want, got)
+			}
+		})
+	}
+}
+
+func TestGetFieldInfo(t *testing.T) {
+	testCases := []struct {
+		name, FieldInfo string
+		want            *apb.FieldInfo
+	}{
+		{
+			name:      "HasFieldInfo",
+			FieldInfo: "[(google.api.field_info).format = UUID4]",
+			want:      &apb.FieldInfo{Format: apb.FieldInfo_UUID4},
+		},
+		{
+			name: "NoFieldInfo",
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+			import "google/api/field_info.proto";
+			
+			message CreateBookRequest {
+				string foo = 1 {{.FieldInfo}};
+			}
+			`, tc)
+			fd := file.FindMessage("CreateBookRequest").FindFieldByName("foo")
+			got := GetFieldInfo(fd)
+			if diff := cmp.Diff(got, tc.want, cmp.Comparer(proto.Equal)); diff != "" {
+				t.Errorf("GetFieldInfo(%+v): got(-),want(+):\n%s", fd, diff)
+			}
+		})
+	}
+}
+
+func TestHasFormat(t *testing.T) {
+	testCases := []struct {
+		name, Format string
+		want         bool
+	}{
+		{
+			name:   "HasFormat",
+			Format: "format: UUID4",
+			want:   true,
+		},
+		{
+			name: "NoFormat",
+			want: false,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+			import "google/api/field_info.proto";
+			
+			message CreateBookRequest {
+				string foo = 1 [(google.api.field_info) = {
+					{{.Format}}
+				}];
+			}
+			`, tc)
+			fd := file.FindMessage("CreateBookRequest").FindFieldByName("foo")
+			if got := HasFormat(fd); got != tc.want {
+				t.Errorf("HasFormat(%+v): expected %v, got %v", fd, tc.want, got)
+			}
+		})
+	}
+}
+
+func TestGetFormat(t *testing.T) {
+	testCases := []struct {
+		name, Format string
+		want         apb.FieldInfo_Format
+	}{
+		{
+			name:   "HasUUID4Format",
+			Format: "format: UUID4",
+			want:   apb.FieldInfo_UUID4,
+		},
+		{
+			name: "NoFormat",
+			want: apb.FieldInfo_FORMAT_UNSPECIFIED,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+			import "google/api/field_info.proto";
+			
+			message CreateBookRequest {
+				string foo = 1 [(google.api.field_info) = {
+					{{.Format}}
+				}];
+			}
+			`, tc)
+			fd := file.FindMessage("CreateBookRequest").FindFieldByName("foo")
+			if got := GetFormat(fd); got != tc.want {
+				t.Errorf("GetFormat(%+v): expected %v, got %v", fd, tc.want, got)
+			}
+		})
+	}
+}