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

Fields using proto3 optional incorrectly treated as oneof #1323

Closed
kyle-mccarthy opened this issue Jan 22, 2024 · 1 comment · Fixed by #1372
Closed

Fields using proto3 optional incorrectly treated as oneof #1323

kyle-mccarthy opened this issue Jan 22, 2024 · 1 comment · Fixed by #1372

Comments

@kyle-mccarthy
Copy link

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

  • Programming language: protobuf
  • OS: macos
  • Language runtime version: 3
  • Package version: 1.59.1

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
Copy link
Collaborator

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

gcf-merge-on-green bot pushed a commit that referenced this issue Apr 12, 2024
When linting if a field is a oneof, ignore those fields that are proto3_optional, which are wrapped in a synthetic oneof.

Updates #1323
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants