googleapis / api-linter

A linter for APIs defined in protocol buffers.
https://linter.aip.dev/
Apache License 2.0
558 stars 137 forks source link

Fields using proto3 `optional` incorrectly treated as `oneof` #1323

Closed kyle-mccarthy closed 2 months ago

kyle-mccarthy commented 5 months ago

proto3 optional causes the field to be treated as a oneof. From what I can tell this is caused by protoreflect adding synthetic oneofs to descriptors for proto3 optional fields.

Environment details

Steps to reproduce

  1. Use proto3 optional on a field that must not be oneof. page_token and page_size are easy repro cases.
  2. Run linter and observe errors stating that the field must not be a oneof
message ListBooksRequest {
  // The parent, which owns this collection of books.
  // Format: publishers/{publisher}
  string parent = 1 [
    (google.api.field_behavior) = REQUIRED,
    (google.api.resource_reference) = {
      child_type: "library.googleapis.com/Book"
    }];

  // The maximum number of books to return. The service may return fewer than
  // this value.
  // If unspecified, at most 50 books will be returned.
  // The maximum value is 1000; values above 1000 will be coerced to 1000.
  optional int32 page_size = 2 [(google.api.field_behavior) = OPTIONAL];

  // A page token, received from a previous `ListBooks` call.
  // Provide this to retrieve the subsequent page.
  //
  // When paginating, all other parameters provided to `ListBooks` must match
  // the call that provided the page token.
  optional string page_token = 3 [(google.api.field_behavior) = OPTIONAL];
}
> The `page_token` field should not be a oneof field. https://linter.aip.dev/158/request-page-token-field
> The `page_size` field should not be a oneof field. https://linter.aip.dev/158/request-page-size-field
noahdietz commented 2 months ago

Thank you for catching this and I'm sorry for the delay in responding to it. I've got a patch out.