Repository: googleapis/api-linter

Issue:
Feature request (AIP-133): add type check for required fields
The existing `core::0133::request-required-fields` rule checks for unexpected required fields in a `Create` request but does not validate the *types* of the allowed required fields. This means that a field could have the correct name (e.g., `parent`) but an incorrect Protobuf type (e.g., `int32` instead of `string`), and the linter would not flag it. This can lead to subtle bugs and non-compliance with AIP-133 regarding expected field types.

**Describe the solution you'd like**
I would like a new rule, `core::0133::request-required-fields-type`, to be created. This rule should specifically validate the types of the standard required fields within `Create*Request` messages. The new rule should enforce the following type constraints:
*   The `parent` field must be of type `string`.
*   The `<resource>_id` field (e.g., `book_id` for a `Book` resource) must be of type `string`.
*   The `<resource>` field (e.g., `book` for a `Book` resource) must be of type `message`.

If a required field is found to have an incorrect type, the rule should report a problem with a clear message indicating the expected type.

**Describe alternatives you've considered**
Initially, the type-checking logic was integrated directly into the `core::0133::request-required-fields` rule. However, this approach combines two distinct types of findings (unexpected fields vs. incorrect field types) into a single rule. Separating them into `request-required-fields` (for unexpected fields) and `request-required-fields-type` (for incorrect types) allows for more granular control over enabling and disabling specific lint checks, as suggested by @noahdietz  feedback in [this](https://github.com/googleapis/api-linter/pull/1578#discussion_r2586184051) PR.

**Additional context**
This new rule complements the existing `core::0133::request-required-fields` rule, making the validation of `Create` request messages more comprehensive and robust without conflating distinct concerns. The resource inference logic (deriving the resource name from the RPC method name if not available from the response type) will be shared with this new type-checking rule to ensure consistency and correct behavior in scenarios with custom response types.


Discussion:

Solution patch:
diff --git a/docs/rules/0133/request-required-fields-types.md b/docs/rules/0133/request-required-fields-types.md
new file mode 100644
index 0000000..534779a
--- /dev/null
+++ b/docs/rules/0133/request-required-fields-types.md
@@ -0,0 +1,67 @@
+---
+rule:
+  aip: 133
+  name: [core, '0133', request-required-fields-types]
+  summary: Create RPCs must have correct types for required fields.
+permalink: /133/request-required-fields-types
+---
+
+# Create methods: Required fields types
+
+This rule enforces that all `Create` standard methods have the correct types for
+required fields, as mandated in [AIP-133][].
+
+## Details
+
+This rule looks at any message matching `Create*Request` and complains if the
+following required fields have the wrong type:
+
+- `string parent` ([AIP-133][])
+- `{Resource} {resource}` (must be a `message`)([AIP-133][])
+- `string {resource}_id` ([AIP-133][])
+
+## Examples
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect: The `book` field should be a message type (Book).
+message CreateBookRequest {
+  string parent = 1;
+  string book = 2;
+  string book_id = 3;
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+message CreateBookRequest {
+  string parent = 1;
+  Book book = 2;
+  string book_id = 3;
+}
+```
+
+## 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 CreateBookRequest {
+  // (-- api-linter: core::0133::request-required-fields-types=disabled
+  //     aip.dev/not-precedent: We need to use bytes for the parent because
+  // reasons. --)
+  bytes parent = 1;
+  Book book = 2;
+  string book_id = 3;
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-133]: https://aip.dev/133
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/rules/aip0133/aip0133.go b/rules/aip0133/aip0133.go
index 3e25b85..95408c0 100644
--- a/rules/aip0133/aip0133.go
+++ b/rules/aip0133/aip0133.go
@@ -40,6 +40,7 @@ func AddRules(r lint.RuleRegistry) error {
 		requestParentReference,
 		requestParentRequired,
 		requestRequiredFields,
+		requestRequiredFieldsTypes,
 		requestResourceBehavior,
 		resourceField,
 		resourceReferenceType,
diff --git a/rules/aip0133/request_required_fields_types.go b/rules/aip0133/request_required_fields_types.go
new file mode 100644
index 0000000..d9b4542
--- /dev/null
+++ b/rules/aip0133/request_required_fields_types.go
@@ -0,0 +1,87 @@
+// 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 aip0133
+
+import (
+	"fmt"
+
+	"github.com/googleapis/api-linter/v2/lint"
+	"github.com/googleapis/api-linter/v2/rules/internal/utils"
+	"github.com/stoewer/go-strcase"
+	"google.golang.org/protobuf/reflect/protoreflect"
+)
+
+// The create request message should have standardized field types for required fields.
+var requestRequiredFieldsTypes = &lint.MethodRule{
+	Name:   lint.NewRuleName(133, "request-required-fields-types"),
+	OnlyIf: utils.IsCreateMethodWithResolvedReturnType,
+	LintMethod: func(m protoreflect.MethodDescriptor) []lint.Problem {
+		ot := utils.GetResponseType(m)
+		var resourceMsgName string
+
+		// Try to get resource from response type
+		if r := utils.GetResource(ot); r != nil {
+			resourceMsgName = utils.GetResourceSingular(r)
+		}
+
+		// If we can't get a resource from the response types, we can try to infer it from the method
+		if resourceMsgName == "" {
+			if noun := utils.GetResourceMessageName(m, "Create"); noun != "" {
+				resourceMsgName = noun
+			}
+		}
+
+		problems := []lint.Problem{}
+		reqFields := m.Input().Fields()
+
+		// We can check for each field explicitly since we can infer what each field should be
+
+		// Check for `parent`.
+		if parentField := reqFields.ByName("parent"); parentField != nil {
+			if parentField.Kind() != protoreflect.StringKind {
+				problems = append(problems, lint.Problem{
+					Message:    `The field "parent" must be of type string.`,
+					Descriptor: parentField,
+				})
+			}
+		}
+
+		snakeResourceName := strcase.SnakeCase(resourceMsgName)
+		resourceIdFieldName := protoreflect.Name(fmt.Sprintf("%s_id", snakeResourceName))
+		resourceFieldName := protoreflect.Name(snakeResourceName)
+
+		// Check for `<resource>_id`.
+		if idField := reqFields.ByName(resourceIdFieldName); idField != nil {
+			if idField.Kind() != protoreflect.StringKind {
+				problems = append(problems, lint.Problem{
+					Message:    fmt.Sprintf("The resource ID field %q must be of type string.", idField.Name()),
+					Descriptor: idField,
+				})
+			}
+		}
+
+		// Check for `<resource>`.
+		if resourceField := reqFields.ByName(resourceFieldName); resourceField != nil {
+			if resourceField.Kind() != protoreflect.MessageKind {
+				problems = append(problems, lint.Problem{
+					Message:    fmt.Sprintf("The resource field %q must be of type message.", resourceField.Name()),
+					Descriptor: resourceField,
+				})
+			}
+		}
+
+		return problems
+	},
+}



Test patch:
diff --git a/rules/aip0133/request_required_fields_types_test.go b/rules/aip0133/request_required_fields_types_test.go
new file mode 100644
index 0000000..455452e
--- /dev/null
+++ b/rules/aip0133/request_required_fields_types_test.go
@@ -0,0 +1,132 @@
+// 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 aip0133
+
+import (
+	"testing"
+
+	"github.com/googleapis/api-linter/v2/rules/internal/testutils"
+	"google.golang.org/protobuf/reflect/protoreflect"
+)
+
+func TestRequestRequiredFieldsType(t *testing.T) {
+	for _, test := range []struct {
+		name                 string
+		Fields               string
+		problematicFieldName string
+		Singular             string
+		ReturnType           string
+		problems             testutils.Problems
+	}{
+		{
+			name: "ValidStandardFields",
+			Fields: `string parent = 1;
+			 BookShelf book_shelf = 2;
+			 string book_shelf_id = 3;`,
+			problematicFieldName: "",
+			Singular:             "bookShelf",
+			ReturnType:           "BookShelf",
+			problems:             nil,
+		},
+		{
+			name: "ValidRpcNameInference",
+			Fields: `string parent = 1;
+			 BookShelf book_shelf = 2;
+			 string book_shelf_id = 3;`,
+			problematicFieldName: "",
+			Singular:             "bookShelf",
+			ReturnType:           "CreateBookShelfResponse",
+			problems:             nil,
+		},
+		{
+			name: "InvalidParentType",
+			Fields: `int32 parent = 1;
+			 BookShelf book_shelf = 2;
+			 string book_shelf_id = 3;`,
+			problematicFieldName: "parent",
+			Singular:             "bookShelf",
+			ReturnType:           "BookShelf",
+			problems: testutils.Problems{
+				{Message: `The field "parent" must be of type string.`},
+			},
+		},
+		{
+			name: "InvalidResourceIdType",
+			Fields: `string parent = 1;
+			 BookShelf book_shelf = 2;
+			 int32 book_shelf_id = 3;`,
+			problematicFieldName: "book_shelf_id",
+			Singular:             "bookShelf",
+			ReturnType:           "BookShelf",
+			problems: testutils.Problems{
+				{Message: `The resource ID field "book_shelf_id" must be of type string.`},
+			},
+		},
+		{
+			name: "InvalidResourceType",
+			Fields: `string parent = 1;
+			 string book_shelf = 2;
+			 string book_shelf_id = 3;`,
+			problematicFieldName: "book_shelf",
+			Singular:             "bookShelf",
+			ReturnType:           "BookShelf",
+			problems: testutils.Problems{
+				{Message: `The resource field "book_shelf" must be of type message.`},
+			},
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			f := testutils.ParseProto3Tmpl(t, `
+				import "google/api/annotations.proto";
+				import "google/api/field_behavior.proto";
+				import "google/api/resource.proto";
+
+				service Library {
+					rpc CreateBookShelf(CreateBookShelfRequest) returns ({{.ReturnType}}) {
+						option (google.api.http) = {
+							post: "/v1/{parent=publishers/*}/bookShelves",
+							body: "book_shelf"
+						};
+					}
+				}
+
+				message BookShelf {
+					option (google.api.resource) = {
+						type: "library.googleapis.com/BookShelf"
+						pattern: "publishers/{publisher}/bookShelves/{book_shelf}"
+						singular: "{{.Singular}}"
+					};
+					string name = 1;
+				}
+
+				message CreateBookShelfRequest {
+					{{.Fields}}
+				}
+
+				message CreateBookShelfResponse {}
+			`, test)
+
+			reqMsg := f.Messages().ByName("CreateBookShelfRequest")
+			var dbr protoreflect.Descriptor = reqMsg
+			if test.problematicFieldName != "" {
+				dbr = reqMsg.Fields().ByName(protoreflect.Name(test.problematicFieldName))
+			}
+
+			if diff := test.problems.SetDescriptor(dbr).Diff(requestRequiredFieldsTypes.Lint(f)); diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}