search

yoheimuta / go-protoparser

Yet another Go package which parses a Protocol Buffer file (proto2+proto3)
MIT License
179 stars 20 forks source link

Multiline string literals error #35

Closed pleask closed 4 years ago

pleask commented 5 years ago

String literals that are split between multiple lines in proto annotations cause an error in go-protoparser. Running the parser on _testdata/simple.proto with the following modification

enum EnumAllowingAlias {
    option allow_alias = true;
    UNKNOWN = 0;
    STARTED = 1;
    RUNNING = 2 [(custom_option) = "this is a "
                                   "string on two lines"
                ];
}

results in this error 

failed to parse, err found "\""(Token=11, Pos=simple.proto:12:36) but expected []] at //go/src/github.com/yoheimuta/go-protoparser/parser/enum.go:293:found "\"" but expected [;]

Despite this syntax being accepted by the compiler. It's not clear from the language spec what the proper syntax for multiline literals is, however.

yoheimuta commented 5 years ago

Thank you for reporting the issue and providing a failing example. The go-protoparser should support this case. I will take a look.

yoheimuta commented 5 years ago

@pleask I have cut a release to support this case. Please let me know if you can still experience the problem.

pleask commented 5 years ago

@yoheimuta thanks for the quick response. This fixes this issue for enum values, however the issue is exists for string literals in general (so annotations on all other proto objects are also affected). Based on your changes for the enum I can make similar changes for other objects, if you like.

yoheimuta commented 5 years ago

@pleask Thank you for the additional comment. Actually, I was not sure about the specific scopes at that moment.

Based on your changes for the enum I can make similar changes for other objects, if you like.

I appreciate it. It will help me a lot.

pleask commented 4 years ago

I've opened a pull request that merges together consecutive string literals when they are constant in an option. This solves our problem, I think, without negative consequences.

yoheimuta commented 4 years ago

@pleask I merged the PR and pushed it to releases. Plus, I also published a protolint release by updating this parser. Great thanks!