emicklei / proto Goto Github PK

View Code? Open in Web Editor NEW

579.0 12.0 63.0 1.35 MB

parser for Google ProtocolBuffers definition

License: MIT License

Go 98.95% Makefile 1.05%

protobuf parser proto3 formatter proto2 golang-package protocol-buffers protobuf-parser

proto's Introduction

proto

Package in Go for parsing Google Protocol Buffers .proto files version 2 + 3

install

go get -u -v github.com/emicklei/proto

usage

package main

import (
	"fmt"
	"os"

	"github.com/emicklei/proto"
)

func main() {
	reader, _ := os.Open("test.proto")
	defer reader.Close()

	parser := proto.NewParser(reader)
	definition, _ := parser.Parse()

	proto.Walk(definition,
		proto.WithService(handleService),
		proto.WithMessage(handleMessage))
}

func handleService(s *proto.Service) {
	fmt.Println(s.Name)
}

func handleMessage(m *proto.Message) {
	lister := new(optionLister)
	for _, each := range m.Elements {
		each.Accept(lister)
	}
	fmt.Println(m.Name)
}

type optionLister struct {
	proto.NoopVisitor
}

func (l optionLister) VisitOption(o *proto.Option) {
	fmt.Println(o.Name)
}

validation

Current parser implementation is not completely validating .proto definitions. In many but not all cases, the parser will report syntax errors when reading unexpected charaters or tokens. Use some linting tools (e.g. https://github.com/uber/prototool) or protoc for full validation.

contributions

See proto-contrib for other contributions on top of this package such as protofmt, proto2xsd and proto2gql. protobuf2map is a small package for inspecting serialized protobuf messages using its .proto definition.

proto's People

Contributors

Stargazers

Watchers

proto's Issues

Associate comment (leading and inline) to each AST node

currently, comment nodes are separate elements per AST node.
This means that getting the comment for a particular AST node requires complex element retrieval.
The parser should assign the comment to the succeeding AST node if possible.
Same holds for inline comment.

At the time of adding this issue , I am working on a big change/refactoring.

Comments affect right justification of options.

import  "google/protobuf/timestamp.proto";
import  "github.com/gogo/protobuf/gogoproto/gogo.proto";

option       (gogoproto.marshaler_all) =  true;
option           (gogoproto.sizer_all) =  true;
option     (gogoproto.unmarshaler_all) =  true;
option (gogoproto.goproto_getters_all) = false;
option        (gogoproto.populate_all) =  true;
option           (gogoproto.equal_all) =  true;

message Msg {
                     string source    = 1; //            This affects the option on #3 ]
                     string dest      = 2; // Foo
  google.protobuf.Timestamp deadline  = 3 [(gogoproto.stdtime) = true]; // Bar
}

Formatting this produces a large space between the = and true in the third line, like so:

  google.protobuf.Timestamp deadline = 3 [(gogoproto.stdtime) =                                         true]; // Bar

It appears to be considering the comments on in other fields when choosing how to align the constant in the option on the field. I think it should only consider options in other fields, and not comments.

In particular, it only seems to do this if not every field has an option. If all the fields have options, it does the right thing.

Un-checkin the .vscode directory?

Just as a cleanup :-) if you want it locally, we could just add it to .gitignore.

No recursive calls if Elements struct Present

This also gets confusing in terms of when there are options slices etc, but as an example, here's a visitor I wrote for making sure there are no c-style comments. Note the calls that have to be individually made. This was probably a design decision but food for thought.

type commentsNoCStyleVisitor struct {
	baseAddVisitor
}

func (v commentsNoCStyleVisitor) VisitMessage(element *proto.Message) {
	v.checkComments(element.Position, element.Comment)
	for _, child := range element.Elements {
		child.Accept(v)
	}
}

func (v commentsNoCStyleVisitor) VisitService(element *proto.Service) {
	v.checkComments(element.Position, element.Comment)
	for _, child := range element.Elements {
		child.Accept(v)
	}
}

func (v commentsNoCStyleVisitor) VisitSyntax(element *proto.Syntax) {
	v.checkComments(element.Position, element.Comment, element.InlineComment)
}

func (v commentsNoCStyleVisitor) VisitPackage(element *proto.Package) {
	v.checkComments(element.Position, element.Comment, element.InlineComment)
}

func (v commentsNoCStyleVisitor) VisitOption(element *proto.Option) {
	v.checkComments(element.Position, element.Comment, element.InlineComment)
}

func (v commentsNoCStyleVisitor) VisitImport(element *proto.Import) {
	v.checkComments(element.Position, element.Comment, element.InlineComment)
}

func (v commentsNoCStyleVisitor) VisitNormalField(element *proto.NormalField) {
	v.checkComments(element.Position, element.Comment, element.InlineComment)
	for _, child := range element.Options {
		child.Accept(v)
	}
}

func (v commentsNoCStyleVisitor) VisitEnumField(element *proto.EnumField) {
	v.checkComments(element.Position, element.Comment, element.InlineComment)
	if element.ValueOption != nil {
		element.ValueOption.Accept(v)
	}
}

func (v commentsNoCStyleVisitor) VisitEnum(element *proto.Enum) {
	v.checkComments(element.Position, element.Comment)
	for _, child := range element.Elements {
		child.Accept(v)
	}
}

func (v commentsNoCStyleVisitor) VisitComment(element *proto.Comment) {
	v.checkComments(element.Position, element)
}

func (v commentsNoCStyleVisitor) VisitOneof(element *proto.Oneof) {
	v.checkComments(element.Position, element.Comment)
	for _, child := range element.Elements {
		child.Accept(v)
	}
}

func (v commentsNoCStyleVisitor) VisitOneofField(element *proto.OneOfField) {
	v.checkComments(element.Position, element.Comment, element.InlineComment)
	for _, child := range element.Options {
		child.Accept(v)
	}
}

func (v commentsNoCStyleVisitor) VisitReserved(element *proto.Reserved) {
	v.checkComments(element.Position, element.Comment, element.InlineComment)
}

func (v commentsNoCStyleVisitor) VisitRPC(element *proto.RPC) {
	v.checkComments(element.Position, element.Comment, element.InlineComment)
	for _, child := range element.Options {
		child.Accept(v)
	}
}

func (v commentsNoCStyleVisitor) VisitMapField(element *proto.MapField) {
	v.checkComments(element.Position, element.Comment, element.InlineComment)
	for _, child := range element.Options {
		child.Accept(v)
	}
}

func (v commentsNoCStyleVisitor) VisitGroup(element *proto.Group) {
	v.checkComments(element.Position, element.Comment)
	for _, child := range element.Elements {
		child.Accept(v)
	}
}

func (v commentsNoCStyleVisitor) VisitExtensions(element *proto.Extensions) {
	v.checkComments(element.Position, element.Comment, element.InlineComment)
}

func (v commentsNoCStyleVisitor) checkComments(position scanner.Position, comments ...*proto.Comment) {
	for _, comment := range comments {
		if comment != nil {
			if comment.Cstyle {
				v.AddFailuref(position, "c-style comments are not allowed")
			}
		}
	}
}

Parsing error when using nested option with selector

As exposed by uber/prototool#24, the following structure is unparsable by emicklei/proto:

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 {}

The semicolons in the option body is causing the error:

test.proto:7:44:1:1:Error while parsing option value for "http": Expected "{", found ".".

Oneof and service options not supported

They are valid:

https://github.com/google/protobuf/blob/master/src/google/protobuf/descriptor.proto#L589
https://github.com/google/protobuf/blob/master/src/google/protobuf/unittest_custom_options.proto#L108

There may be other option types not handled, will check as I go.

Update - service options also not handled https://github.com/google/protobuf/blob/master/src/google/protobuf/descriptor.proto#L632

NormalField has three bools for Repeated/Optional/Required, should be one enum

A NormalField with both required and optional set, for example, has no meaning. It would be great if (especially on a 2.0 release) there was something like type FieldModifier int that had values REPEATED, OPTIONAL, REQUIRED, and this was set to either REPEATED or OPTIONAL for proto3.

Non-primitive field option followed by any option cannot be parsed

func TestFieldCustomOptions(t *testing.T) {
	proto := `foo.bar lots = 1 [foo={hello:1}, bar=2];`
	p := newParserOn(proto)
	f := newNormalField()
	err := f.parse(p)
	if err != nil {
		t.Fatal(err)
	}
	if got, want := f.Type, "foo.bar"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
	if got, want := f.Name, "lots"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
	if got, want := len(f.Options), 2; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	if got, want := f.Options[0].Name, "foo"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
	if got, want := f.Options[1].Name, "bar"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
	if got, want := f.Options[1].Constant.Source, "2"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
}

This returns:

<input>:1:37: found "=" but expected [option aggregate key colon :]

This fails if bar is a non-primitive as well. If foo is a primitive and bar is a non-primitive, this passes.

Primitive method options cannot be parsed

func TestRPCWithOptionPrimitive(t *testing.T) {
	proto := `service AccountService {
		// CreateAccount
		rpc CreateAccount (CreateAccount) returns (ServiceFault){
			option (test_ident) = true;
		}
	}`
	pr, err := newParserOn(proto).Parse()
	if err != nil {
		t.Fatal(err)
	}
	srv := collect(pr).Services()[0]
	if got, want := len(srv.Elements), 1; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	rpc1 := srv.Elements[0].(*RPC)
	if got, want := len(rpc1.Options), 1; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
	opt := rpc1.Options[0]
	if got, want := opt.Name, "(test_ident)"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
	if got, want := len(opt.AggregatedConstants), 0; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	if got, want := opt.Constant.SourceRepresentation(), "true"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
}

This produces found ";" but expected [rpc option]

Comments inside Oneof field break parser

The following valid proto breaks the parser:

syntax = "proto3";

message MyMessage {
    oneof type {
        // Represents a null value.
        Null null = 1;
    }
}

With the warning: found " Represents a null value." on line 6, expected [oneof closing }]

Option with repeated (array) values not parsed correctly

I think this one will be hard to fix...

Options can have "repeated" fields, which in the option value is represented by [], like:

int64 in     = 14 [(validate.rules).int64 = {in: [30, 35]}];

But currently each "," is parsed as a separate aggregate.
I think the corrent way in the current implementation would be to return an string with "[30, 35]".

Here is a protobuf file that has this declaration:

https://github.com/lyft/protoc-gen-validate/blob/master/tests/kitchensink/int64.proto

Add `Parent Visitee` to key Visitees (Option, EnumField, etc)?

For Option specifically, if I'm writing a formatter, I need to know what type of option it is (message option, rpc method option, etc). I could have a completely new visitor for this but this seems overkill.

One-line C-Style comments not parsed correctly

See test in #52:

func TestParseCStyleOneLineComment(t *testing.T) {
	proto := `/* comment 1 */`
	p := newParserOn(proto)
	def, err := p.Parse()
	if err != nil {
		t.Fatal(err)
	}
	if got, want := len(def.Elements), 1; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}

	if got, want := len(def.Elements[0].(*Comment).Lines), 1; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	if got, want := def.Elements[0].(*Comment).Lines[0], "/* comment 1 */"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
	if got, want := def.Elements[0].(*Comment).Cstyle, true; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
}

This returns:

--- FAIL: TestParseCStyleOneLineComment (0.00s)
	comment_test.go:176: got [* comment 1 */] want [/* comment 1 */]
	comment_test.go:179: got [false] want [true]

Aggregate option syntax not supported

According official protobuf documentation, options can be written in aggregate syntax, so repeated field in option's message can be supported:

Input (test.proto)

syntax = "proto3";
import "google/protobuf/descriptor.proto";

message FooOption {
  optional int32 opt1 = 1;
  optional string opt2 = 2;
}

message FooOptions {
  repeated FooOption options = 1;
}

extend google.protobuf.FieldOptions {
  optional FooOptions foo_options = 1234;
}

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

TestCode

package main

import (
	"os"

	"github.com/emicklei/proto"
)

func main() {
	reader, _ := os.Open("test.proto")
	defer reader.Close()
	parser := proto.NewParser(reader)
	definition, _ := parser.Parse()
	formatter := proto.NewFormatter(os.Stdout, "   ")
	formatter.Format(definition)
}

Output

syntax = "proto3";
import "google/protobuf/descriptor.proto";

message FooOption {
   optional  int32 opt1 = 1;
   optional string opt2 = 2;
}
message FooOptions {
   repeated FooOption options = 1;
}
extend google.protobuf.FieldOptions {
   optional FooOptions foo_options = 1234;
}

inline comments are misplaced by formatter

message ConnectRequest {
  string clientID       = 1;  // Client name/identifier.
  string heartbeatInbox = 2;  // Inbox for server initiated heartbeats.
}

results in

message ConnectRequest {
  string clientID       = 1;  
 
  // Client name/identifier.
  string heartbeatInbox = 2; 

  // Inbox for server initiated heartbeats.
}

Get golint, go vet, staticcheck, errcheck passing and add to Travis build

I'll try to make a pass at this soon.

$ golint ./...
field.go:167:1: receiver name n should be consistent with previous receiver name f for NormalField
cmd/proto2gql/converter.go:5:6: exported type Converter should have comment or be unexported
cmd/proto2gql/converter.go:10:1: exported method Converter.NewTypeName should have comment or be unexported
cmd/proto2gql/converter.go:14:1: exported method Converter.OriginalTypeName should have comment or be unexported
cmd/proto2gql/converter.go:25:1: exported method Converter.PackageName should have comment or be unexported
cmd/proto2gql/file.go:9:6: exported type FileWriter should have comment or be unexported
cmd/proto2gql/file.go:16:1: exported function NewFileWriter should have comment or be unexported
cmd/proto2gql/file.go:28:1: exported method FileWriter.IsClosed should have comment or be unexported
cmd/proto2gql/file.go:36:1: exported method FileWriter.Save should have comment or be unexported
cmd/proto2gql/file.go:56:1: exported method FileWriter.Remove should have comment or be unexported
cmd/proto2gql/file.go:70:1: exported method FileWriter.Close should have comment or be unexported
cmd/proto2gql/main.go:13:2: exported type StringMap should have comment or be unexported
cmd/proto2gql/main.go:26:1: exported method StringMap.Set should have comment or be unexported
cmd/proto2gql/main.go:246:2: redundant if ...; err != nil check, just return error instead.
cmd/proto2gql/scope.go:9:2: exported type Type should have comment or be unexported
cmd/proto2gql/scope.go:14:2: exported type Scope should have comment or be unexported
cmd/proto2gql/scope.go:24:1: exported function NewScope should have comment or be unexported
cmd/proto2gql/scope.go:34:1: exported method Scope.Fork should have comment or be unexported
cmd/proto2gql/scope.go:52:1: exported method Scope.SetPackageName should have comment or be unexported
cmd/proto2gql/scope.go:56:1: exported method Scope.AddLocalType should have comment or be unexported
cmd/proto2gql/scope.go:69:1: exported method Scope.AddImportedType should have comment or be unexported
cmd/proto2gql/scope.go:85:1: exported method Scope.ResolveTypeName should have comment or be unexported
cmd/proto2gql/transformer.go:11:2: exported type ExternalPackage should have comment or be unexported
cmd/proto2gql/transformer.go:16:2: exported type Transformer should have comment or be unexported
cmd/proto2gql/transformer.go:24:1: exported function NewTransformer should have comment or be unexported
cmd/proto2gql/transformer.go:39:1: exported method Transformer.DisablePrefix should have comment or be unexported
cmd/proto2gql/transformer.go:43:1: exported method Transformer.Import should have comment or be unexported
cmd/proto2gql/transformer.go:53:1: exported method Transformer.SetPackageAlias should have comment or be unexported
cmd/proto2gql/transformer.go:57:1: exported method Transformer.Transform should have comment or be unexported
cmd/proto2gql/visitor.go:9:5: exported var BUILTINS should have comment or be unexported
cmd/proto2gql/visitor.go:28:2: exported type Visitor should have comment or be unexported
cmd/proto2gql/visitor.go:35:1: exported function NewVisitor should have comment or be unexported
cmd/proto2gql/visitor.go:43:1: exported method Visitor.Fork should have comment or be unexported
cmd/proto2gql/visitor.go:55:1: exported method Visitor.Flush should have comment or be unexported
cmd/proto2gql/visitor.go:65:1: exported method Visitor.VisitMessage should have comment or be unexported
cmd/proto2gql/visitor.go:100:1: exported method Visitor.VisitService should have comment or be unexported
cmd/proto2gql/visitor.go:101:1: exported method Visitor.VisitSyntax should have comment or be unexported
cmd/proto2gql/visitor.go:102:1: exported method Visitor.VisitPackage should have comment or be unexported
cmd/proto2gql/visitor.go:105:1: exported method Visitor.VisitOption should have comment or be unexported
cmd/proto2gql/visitor.go:106:1: exported method Visitor.VisitImport should have comment or be unexported
cmd/proto2gql/visitor.go:109:1: exported method Visitor.VisitNormalField should have comment or be unexported
cmd/proto2gql/visitor.go:126:1: exported method Visitor.VisitEnumField should have comment or be unexported
cmd/proto2gql/visitor.go:129:1: exported method Visitor.VisitEnum should have comment or be unexported
cmd/proto2gql/visitor.go:142:1: exported method Visitor.VisitComment should have comment or be unexported
cmd/proto2gql/visitor.go:143:1: exported method Visitor.VisitOneof should have comment or be unexported
cmd/proto2gql/visitor.go:144:1: exported method Visitor.VisitOneofField should have comment or be unexported
cmd/proto2gql/visitor.go:145:1: exported method Visitor.VisitReserved should have comment or be unexported
cmd/proto2gql/visitor.go:146:1: exported method Visitor.VisitRPC should have comment or be unexported
cmd/proto2gql/visitor.go:147:1: exported method Visitor.VisitMapField should have comment or be unexported
cmd/proto2gql/visitor.go:149:1: comment on exported method Visitor.VisitGroup should be of the form "VisitGroup ..."
cmd/proto2gql/visitor.go:151:1: exported method Visitor.VisitExtensions should have comment or be unexported

$ staticcheck ./...
cmd/proto2gql/transformer_test.go:131:2: self-assignment of expected to expected (SA4018)
field.go:141:12: this value of lit is never used (SA4006)

$ errcheck ./...
cmd/proto2gql/file.go:23:12:	file.Write([]byte(openTag))
cmd/proto2gql/file.go:43:15:	fw.file.Write([]byte(fw.closeTag))
cmd/proto2gql/main.go:215:14:	fw.Remove()
cmd/proto2gql/main.go:228:13:	fw.Remove()
cmd/proto2gql/main.go:244:18:	defer file.Close()
cmd/proto2gql/visitor.go:56:11:	out.Write(v.buff.Bytes())
cmd/proto2xsd/main.go:106:14:	output.Write(data)
formatter.go:68:16:	io.WriteString(f.w, "}\n")
formatter.go:97:16:	io.WriteString(f.w, "}\n")
formatter.go:141:16:	io.WriteString(f.w, "}\n")
formatter.go:162:16:	io.WriteString(f.w, "}\n")
formatter.go:174:16:	io.WriteString(f.w, "reserved ")
formatter.go:178:19:	io.WriteString(f.w, ", ")
formatter.go:185:19:	io.WriteString(f.w, ", ")
formatter.go:212:17:	io.WriteString(f.w, "optional ")
formatter.go:221:16:	io.WriteString(f.w, "}\n")
formatter.go:228:16:	io.WriteString(f.w, "extensions ")
formatter.go:231:18:	io.WriteString(f.w, ", ")
formatter_utils.go:87:17:	io.WriteString(f.w, f.indentSeparator)
formatter_utils.go:128:19:	io.WriteString(f.w, each[c].formatted(f.indentSeparator, f.indentLevel, pw))
formatter_utils.go:137:16:	io.WriteString(f.w, "\n")
formatter_utils.go:190:16:	io.WriteString(f.w, ";")
formatter_utils.go:193:18:	io.WriteString(f.w, " ///")
formatter_utils.go:195:18:	io.WriteString(f.w, " //")
formatter_utils.go:197:17:	io.WriteString(f.w, commentOrNil.Message())
formatter_utils.go:199:16:	io.WriteString(f.w, "\n")
proto_test.go:51:15:	defer f.Close()
protobuf_test.go:22:23:	defer resp.Body.Close()
range.go:78:18:	p.unexpected(lit, "integer", n)
service.go:162:17:	io.WriteString(buf, " {\n")
service.go:167:18:	io.WriteString(buf, "\n")
service.go:170:17:	io.WriteString(buf, "}")

$ go vet ./...
cmd/proto2gql/transformer_test.go:131: self-assignment of expected to expected

Options with only a part parenthesized isn't parsed

This envoyproxy .proto option gives an error when I parsed. I couldn't find whether this is allowed or but, but envoyproxy is a big project, I used their proto files are used a lot.

https://github.com/envoyproxy/data-plane-api/blob/master/envoy/api/v2/auth/cert.proto#L36

TlsProtocol tls_minimum_protocol_version = 1 [(validate.rules).enum.defined_only = true];

2018/03/14 16:34:34 Error parsing file envoy/api/v2/auth/cert.proto: :36:66: found "enum" but expected [option postfix identifier]

Why do some Visitees have String() methods and others do not?

comment.go:165:func (c *Comment) String() string { return fmt.Sprintf("<comment %s>", c.Message()) }
extensions.go:60:func (e *Extensions) String() string { return fmt.Sprintf("<extensions %v>", e.Ranges) }
field.go:167:func (n *NormalField) String() string { return fmt.Sprintf("<field %s=%d>", n.Name, n.Sequence) }
group.go:95:func (g *Group) String() string { return fmt.Sprintf("<group %s>", g.Name) }
message.go:229:func (m *Message) String() string { return fmt.Sprintf("<message %s>", m.Name) }
option.go:149:func (l Literal) String() string {
range.go:38:func (r Range) String() string {

I think in the case of Literal, there's a purpose, but ProtoRepresentation might be a better method name. In the other cases, having a String method messes with debug logging and I argue they should be removed.

Multi-line comments within RPC options brackets do not parse

See tests in #69.

Example: https://github.com/peter-edge/openflights-go/blob/master/openflights.proto#L130

Parser parses non-valid Protobuf files

For example, if I leave the ; off the end of a statement (like a package declaration, a file option, or a field), the Parser still returns a validly parsed definition. This means writing a formatter (like protofmt) or a linter will output something valid for non-valid Protobuf, which (especially in the linter case) means something can "pass lint" without being valid.

Example:

syntax = "proto3";

package foo

Note the pack of a ; after the package declaration.

$ protofmt tmp.proto
syntax = "proto3";

package foo;

Escape sequences are being parsed inside strings

Another problem I found, given this definition:

string name  = 3 [(validate.rules).string = {
                  pattern:   "^[^\d\s]+( [^\d\s]+)*$",
                  max_bytes: 256,
               }];

The parser is trying to escape the "\d" sequence in "pattern", but I don't think it should interpret the string, shouldn't it leave this to the library user?
If I added another "" after the other ones, then it works, but the output string has the two "", it even doesn't remove them.

2018/03/16 09:08:55 go scanner error at <input>:11:34 = illegal char escape
go scanner error at <input>:11:34 = illegal char escape
go scanner error at <input>:11:34 = illegal char escape
go scanner error at <input>:11:34 = illegal char escape

Inline comments not parsed on RPCs if they end with {}

TestService in service_test.go edited as such:

func TestService(t *testing.T) {
	proto := `service AccountService {
		// comment
		rpc CreateAccount (CreateAccount) returns (ServiceFault); // inline comment
		rpc GetAccounts   (stream Int64)  returns (Account) {} // inline comment2
		rpc Health(google.protobuf.Empty) returns (google.protobuf.Empty) {} // inline comment3
	}`
	pr, err := newParserOn(proto).Parse()
	if err != nil {
		t.Fatal(err)
	}
	srv := collect(pr).Services()[0]
	if got, want := len(srv.Elements), 3; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	if got, want := srv.Position.String(), "<input>:1:1"; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	rpc1 := srv.Elements[0].(*RPC)
	if got, want := rpc1.Name, "CreateAccount"; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	if got, want := rpc1.Doc().Message(), " comment"; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	if got, want := rpc1.InlineComment.Message(), " inline comment"; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	if got, want := rpc1.Position.Line, 3; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	rpc2 := srv.Elements[1].(*RPC)
	if got, want := rpc2.Name, "GetAccounts"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
	rpc3 := srv.Elements[2].(*RPC)
	if got, want := rpc3.Name, "Health"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
	if got, want := rpc2.InlineComment.Message(), " inline comment2"; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	if got, want := rpc3.InlineComment.Message(), " inline comment3"; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
}

inline comments 2 and 3 are not associated with the relevant RPCs.

Add line numbers to all Visitees?

I'd like to write a linter based on this library, and line numbers for all Visitees would be great. I can put together a PR to this effect if this is something you'd be open to.

Required and repeated groups not supported

https://developers.google.com/protocol-buffers/docs/proto#groups

Both are valid, I verified with compiling with protoc.

Field inline comments are parsed as regular comments

message Hello {
  // comment
  bool foo = 1; // inline comment
}

The Comment on foo will be:

// comment
// inline comment

Inconsistent addition of Options field to Visitees

Example:

message Foo {
  option deprecated = true;
  int64 hello = 1;
}

The deprecated option is included in the Elements []Visitee slice on Message meaning you need to visit it, while in other Visitee structs such as Field, there is an Options []*Option slice. Related to #27.

Confusing comment parsing

When I look at newComment, the logic seems like it can produce inconsistent results depending on how the user specifies the comment.

What I want:

Lines: The lines of the comment, without any //, no matter what
CStyle: whether the user specified this in c-style
ExtraSlash: remove this

Thoughts?

Option aggregate name that is a keyword is giving parsing error

Keywords in option aggregate names are being treated as keywords when they shouldn't, for example:

message User {
    string email = 3 [(validate.field) = {required: true}];
}

This "required" gives an error as if this was a keyword.

Elements []Visitee and Comment/InlineComment on a given Visitee?

This may just need to be documented, but a Comment is a Visitee, so on Enum for example, you have Elements []Visitee and Comment *Comment - will the Comment not be in the Elements slice? This would make sense, and either way, we should have it documented as to the expected behavior. I may be missing something, just wanted to file this while it is in my head.

the collector in `visitor_test` would be useful as a public object

https://github.com/emicklei/proto/blob/f046ec0808810d6e2de21ed16dc0fb05d821a10d/visitor_test.go

I'm finding, in order to use this library, I basically have to implement the collector from visitor_test because I need to scan through all of the elements and do type checks.

Could these be exposed to make this library more user-friendly?

Move Formatter to subpackage?

I don't think you can do this if you are following SemVer (which you should!!!), since you already did a 1.0 release (was this intentional), but Formatter is a specific implementation on top of the core API, and it would be nice if the root package was just the core API and nothing else. Even nicer would be if the binaries moved outside of this repository into a proto-contrib or similar. Nicer on top of that would be if this was named something other than proto since that overlaps with github.com/gogo/protobuf/proto, but hey, can't get greedy :-)

All just suggestions.

error with included types and service definition

hi I would like to use your library.
But when i parse a file like this one:

syntax = "proto3";

package bla.protobuf.services;

import "google/protobuf/empty.proto";

service blaService {
    rpc Health(google.protobuf.Empty) returns (google.protobuf.Empty) {}
}

I have an error:

<input>:8:22: Found "." but expected [rpc type closing )].

Am i doing something wrong? (the file works fine with protoc)
If the issue is on your side could you fix it?
Thanks

Complex options cannot be parsed

I think the logic in Option.parseAggregate is not complicated enough - it assumes simple key/value for options. However, the following is a valid option:

syntax = "proto3";

import "google/protobuf/descriptor.proto";

package foo;

message Foo {
  int64 hello = 1;
}

message Bar {
  int64 woot = 1;
  Foo foo = 2;
}

extend google.protobuf.FileOptions {
  Bar bar = 80001;
}

Then you can have:

syntax = "proto3";

package baz;

option (foo.bar) = {
  woot: 100
  foo: {
    hello: 200
  }
};

This is completely valid, but due to the lack of recursion in parseAggregate, and the issue that it only stops on a tSEMICOLON (see #49), this will not be parsed.

Update readme with descriptions

Hey bud - is there any chance you can add some samples to the readme.md on the stylistic choices taken wrt comments, spacing, alignment.

Whether you have plans to remove optional/required when syntax = "proto3"; would be nice to add as well.

Parser errors result in error lines that will are not compatible with linters

For things like https://github.com/vim-syntastic/syntastic for example, they need something like FILE:LINE:COLUMN:ERROR, which protoc gives you. Say I have:

syntax = "proto3";

pkg foo

Note pkg not package. If I do protofmt:

$ protofmt tmp.proto
tmp.proto found "pkg" on <input>:3:1, expected [.proto element {comment|option|import|syntax|enum|service|package|message}]

If I do protoc, which produces something valid:

$ protoc --java_out=/tmp tmp.proto
tmp.proto:3:1: Expected top-level statement (e.g. "message").

If I'm building a linter, I could "solve" this by shelling out to protoc first to make sure the file compiles, and then running the linter based on the parser here - and maybe I'll have to do that anyways, as compiling Protobuf is a more complicated task especially when you get into imports - but it would be nicer to not have to do this and for this to produce valid errors.

Related to #14.

Oneof fields cannot be parsed if imported

Example:

message Value {
  oneof value {
    foo.bar value = 1;
  }
}

When this is parsed, the error Found "." but expected [field identifier]. will be returned.

Test to add to field_test.go (if you change "foo.bar" to "foo" this will pass):

func TestFieldOneofImported(t *testing.T) {
	fieldType := "foo.bar"
	proto := `message Value {
		oneof value {
			` + fieldType + ` value = 1;
		}
	}`
	p := newParserOn(proto)
	def := new(Proto)
	err := def.parse(p)
	if err != nil {
		t.Fatal(err)
	}
	m := def.Elements[0].(*Message)
	if len(m.Elements) != 1 {
		t.Errorf("expected one element but got %d", len(m.Elements))
	}
	o := m.Elements[0].(*Oneof)
	if len(o.Elements) != 1 {
		t.Errorf("expected one element but got %d", len(o.Elements))
	}
	f := o.Elements[0].(*OneOfField)
	if got, want := f.Type, fieldType; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
	if got, want := f.Name, "value"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
}

format option with hash constant for rpc definitions

currently , the following definition is not parseable

       rpc Find (  Finder  ) returns ( stream Result ) { // Find
            option (google.api.http) = {
                post: "/v1/finders/1"
                body: "*"
            };
       }

see https://developers.google.com/protocol-buffers/docs/proto#options

RPC option values can be multiple lines

Example: https://github.com/googleapis/googleapis/blob/master/google/cloud/bigquery/datatransfer/v1/datatransfer.proto#L139

This isn't parsed correctly

Parsing error when using nested option messages with semicolons

As exposed by uber/prototool#24, the following structure is unparsable by emicklei/proto:

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 semicolons in the option body is causing the error:

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

Valid package name "google.rpc" cannot be parsed

syntax = "proto3";

package google.rpc;

If you parse this, you will get:

Found "rpc" but expected [.proto element {comment|option|import|syntax|enum|service|package|message}].

Add special test that verifies everything in googleapis/google is parseable

https://github.com/googleapis/googleapis

I'm finding these errors by parsing everything in the google directory. We could add a special test that runs under a different make target (and is run in CI) that clones googleapis and tries to parse every file.

Parse error for message option that has a repeated field of messages

func TestOptionWithRepeatedMessageValues(t *testing.T) {
	src := `message Foo {
		int64 a = 1 [b = {repeated_message_field: [{hello: 1}, {hello: 2}]}];
	}`
	p := newParserOn(src)
	_, err := p.Parse()
	if err != nil {
		t.Errorf("expected no error but got %v", err)
	}
}

In this case, option b is a message that has a field repeated_message_field that itself is a message, that has field hello.

This came up in uber/prototool#68:

	// TODO: add back when fixed in emicklei/proto
	//(bar.repeated_field_dep_option) = { hello: 1, repeated_dep: [
	//   { hello: 1, repeated_bar: [1, 2] },
	//   { hello: 3, repeated_bar: [3, 4] } ] }

C-style comments that are indented take extra indents with them

See #52, specifically commit uber-go@1a09f23.

Package name that starts with keyword cannot be parsed

func TestNextIdentifierStartsWithKeyword(t *testing.T) {
	ident := " rpc.mies.enum ="
	p := newParserOn(ident)
	_, tok, lit := p.nextIdentifier()
	if got, want := tok, tIDENT; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
	if got, want := lit, "rpc.mies.enum"; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
	_, tok, _ = p.next()
	if got, want := tok, tEQUALS; got != want {
		t.Errorf("got [%v] want [%v]", got, want)
	}
}

This produces:

$ go test .
--- FAIL: TestNextIdentifierStartsWithKeyword (0.00s)
	parser_test.go:112: got [23] want [3]
	parser_test.go:115: got [rpc] want [rpc.mies.enum]
	parser_test.go:119: got [19] want [6]

This test is just the WithKeyword test, but starts with a keyword instead of a keyword being in the middle or at the end.

Related to #37.
See uber/prototool#103.

Inline comments on non-primitive options are not parsed

Test to reproduce:

// assume Help is a message such as:
//
// message HelpMessage {
//   string string_field = 1;
// }

func TestNonPrimitiveOptionComment(t *testing.T) {
	proto := `
// comment
option Help = { string_field: "value" }; // inline`
	p := newParserOn(proto)
	pr, err := p.Parse()
	if err != nil {
		t.Fatal(err)
	}
	o := pr.Elements[0].(*Option)
	if got, want := o.Comment != nil, true; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	if got, want := o.Comment.Lines[0], " comment"; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	if got, want := o.InlineComment != nil, true; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
	if got, want := o.InlineComment.Lines[0], " inline"; got != want {
		t.Fatalf("got [%v] want [%v]", got, want)
	}
}

Enum value should accept hex numbers

I see a number of examples that uses hex numbers on enum values, like this post:

https://groups.google.com/forum/#!topic/protobuf/L105Q4NIk0U

The google parser acccepts it, so I think this is a supported thing.

<input>:27:18: found "=" but expected [enum field integer]

The new map of Literals makes iteration non-deterministic

I ran into this with testing.

Before, withe the AggregatedConstants, iterating over the slice would produce a deterministic ordering. Now, with the map[string]*Literal, the key:value ordering in the file is not preserved. The Literal.Map in #81 needs to be ordered.

Filename not being properly set on Position

If I have:

syntax = "proto3";

pkg foo

I get this when running protofmt:

$ protofmt tmp.proto
tmp.proto found "pkg" on <input>:3:1, expected [.proto element {comment|option|import|syntax|enum|service|package|message}]

Note <input> instead of tmp.proto, which is the default value if there is no Filename https://golang.org/src/text/scanner/scanner.go?s=1331:1366#L28

Comments associated with Visitess when newlines between the comment and Visitee

Example:

// foo

// bar

syntax = "proto3";

// baz

package bat;

// bar is the Comment on the Syntax object, and // baz is the Comment on the Package object. // foo correctly is associated with just the Proto object and will be hit when you get to VisitComment on Proto.

Multiple enum value options not allowed

This is valid per the Protobuf spec, but there is only one ValueOption *Option allowed on EnumFields. If you have this:

enum Something {
  SOMETHING_FOO = 0 [
    (bar.enum_value_option) = true,
    (bar.enum_value_dep_option) = { hello: 1 }
  ];
}

The error found "," but expected [option closing ]] is returned.

emicklei / proto Goto Github PK

proto's Introduction

proto

install

usage

validation

contributions

proto's People

Contributors

Stargazers

Watchers

Forkers

proto's Issues

Input (test.proto)

TestCode

Output

Recommend Projects

Recommend Topics

Recommend Org