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

Buf format deletes inline comments in some cases #3322

Closed
anzboi opened this issue Sep 15, 2024 · 4 comments · Fixed by #3325
Closed

Buf format deletes inline comments in some cases #3322

anzboi opened this issue Sep 15, 2024 · 4 comments · Fixed by #3325
Labels
Bug Something isn't working

Comments

@anzboi
Copy link

anzboi commented Sep 15, 2024

GitHub repository with your minimal reproducible example (do not fill out this field with "github.com/bufbuild/buf" or we will automatically close your issue, see the instructions above!)

https://github.com/anzboi/buf-bugs

Commands

buf format --path module/format.proto

Output

syntax = 'proto3';

package module;

import "google/api/annotations.proto";

service Service {
  rpc Method(Message) returns (Message) {
    option (google.api.http) = {
      additional_bindings: []
      additional_bindings: [] /* This comment is safe */
      additional_bindings: []
      body: "*" // but this one is
    };
  }
}

message Message {}

Expected Output

syntax = 'proto3';

package module;

import "google/api/annotations.proto";

service Service {
  rpc Method(Message) returns (Message) {
    option (google.api.http) = {
      additional_bindings: [] /* This comment will get deleted */
      additional_bindings: [] /* This comment is safe */
      additional_bindings: [] /* this one is not */
      body: "*" // but this one is
    };
  }
}

message Message {}

Anything else?

This issue seems to occur only under specific circumstances. We noticed it only after empty repeated options with a trailing comma, like my_option: [], // this comment gets deleted. If you remove the trailing comma, or the comment is after a different type (string, int), the comment is safe.

We have not tested much further, like what if the repeated field is populated, or after a nested message, or if the option occurs in a message options, field options, service options, file options.

@anzboi anzboi added the Bug Something isn't working label Sep 15, 2024
@bufdev
Copy link
Member

bufdev commented Sep 16, 2024

Thanks for the detailed report - we appreciate it. We'll look into this first thing tomorrow - cc @doriable @emcfarlane

@doriable
Copy link
Member

Hi @anzboi, really appreciate the repro, it was extremely helpful and we were able to figure out the issue and a fix is on the way. There are a couple of details related to the fix that we need to iron out, but I'll keep this issue updated!

@bufdev
Copy link
Member

bufdev commented Sep 18, 2024

This has been fixed and will go out in the next release.

@anzboi
Copy link
Author

anzboi commented Sep 19, 2024

Awesome, Thanks @bufdev, @doriable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
3 participants