Skip to content
This repository has been archived by the owner on Mar 4, 2022. It is now read-only.

Unexpected error when formatting valid proto definition #24

Closed
johanbrandhorst opened this issue Apr 12, 2018 · 5 comments
Closed

Unexpected error when formatting valid proto definition #24

johanbrandhorst opened this issue Apr 12, 2018 · 5 comments

Comments

@johanbrandhorst
Copy link

The following protofile produces a pretty hard-to-debug error when linting (and probably for other stuff as well):

syntax = "proto3";

import "google/api/annotations.proto";

service Test {
    rpc Test(Void) returns (Void) {
        option (google.api.http) = {
            post: "/api/v1/test";
            body: "*";
        };
    }
}

message Void {}

The error:

$ prototool lint test.proto
test.proto:12:1: found "}" but expected [.proto element {comment|option|import|syntax|enum|service|package|message}]

Specifically, it's the semicolons inside the options object that the parser doesn't like.

This is, of course, a perfectly valid protofile definition (as protoc accepts it), though I admit I'm having trouble finding a definition for multi-value options. See https://developers.google.com/protocol-buffers/docs/reference/proto3-spec#option, https://developers.google.com/protocol-buffers/docs/proto#customoptions for more information. The latter lists a single example of a multi-value option where all options are on the same line, without semicolon;

// usage:
message Bar {
  optional int32 a = 1 [(foo_options).opt1 = 123, (foo_options).opt2 = "baz"];
  // alternative aggregate syntax (uses TextFormat):
  optional int32 b = 2 [(foo_options) = { opt1: 123 opt2: "baz" }];
}

So perhaps this could be a linter option? In any case the parser should allow it.

@bufdev
Copy link
Contributor

bufdev commented Apr 12, 2018

This is an issue in emicklei/proto, see #1, we will need to file an issue over there.

@johanbrandhorst
Copy link
Author

johanbrandhorst commented Apr 12, 2018

In fact, even when fixing this (removing the semicolons) and then running prototool format -w twice, the second run fails because the parser can't parse the output of prototool format when using nested options:

  1. Start with a protofile with nested options:
syntax = "proto3";

import "google/api/annotations.proto";

service Test {
    rpc Test(Void) returns (Void) {
        option (google.api.http) = {
            get: "/api/v1/test"
            additional_bindings {
                post: "/api/v1/test"
                body: "*"
            }
        };
    }
}

message Void {}
  1. Run prototool format -w twice:
$ prototool format -w
$ prototool format -w
test.proto:7:44:1:1:Error while parsing option value for "http": Expected "{", found ".".

The formatted file looks like this:

syntax = "proto3";

import "google/api/annotations.proto";

service Test {
	rpc Test(Void) returns (Void) {
		option (google.api.http) = {
			get: "/api/v1/test"
			additional_bindings.post: "/api/v1/test"
			additional_bindings.body: "*"
		};
	}
}

message Void {}

@johanbrandhorst
Copy link
Author

I could raise a separate issue for this case if you wish?

@bufdev
Copy link
Contributor

bufdev commented Apr 12, 2018

I’d prefer the relevant issues be filed in emicklei/proto, no need to create new issues here. A test case in a PR to reproduce the above would be good too.

@johanbrandhorst
Copy link
Author

Raised emicklei/proto#79 and emicklei/proto#80, I also raised #27 so we can make a decision on what should be done for that case. Closing this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants