yoheimuta / go-protoparser

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

Support option aggregate syntax #68

Closed perrydunn closed 2 years ago

perrydunn commented 2 years ago

Thanks for producing protolint!

Please could the aggregate syntax be supported for (custom) options?

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

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

// 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" }];
  // for completeness, split with (optional) commas:
  optional int32 c = 3 [(foo_options) = {
    opt1: 123,
    opt2: "baz",
  }];
}

Unfortunately this is not part of the language spec but is implemented in protoc...

yoheimuta commented 2 years ago

@perrydunn Thank you for reaching out! I'm looking into the syntax.

yoheimuta commented 2 years ago

@perrydunn Basically, the parser supports the aggregate syntax like https://github.com/yoheimuta/go-protoparser/blob/master/_testdata/grpc-gateway_a_bit_of_everything.proto#L16.

But, a trailing period in opt2: "baz". is the unexpected syntax. I couldn't find this split with (optional) commas example in any documents.

image

Where did you pick it up?

perrydunn commented 2 years ago

Ah, sorry, that should have been a trailing comma. I've updated the original message with the comma.

Thanks for clarifying that it is supported. Unfortunately at work we use the comma-separated version. I appreciate that there is no such example in the docs (I couldn't find one either), but it is permitted by protoc. At work we use protobuf extensively and have many such examples which compile fine.

perrydunn commented 2 years ago

Just going to mention these issues here to link: https://github.com/protocolbuffers/protobuf/issues/3755 https://github.com/protocolbuffers/protobuf/issues/1148 It's frustrating that the spec is not fully documented by Google.

yoheimuta commented 2 years ago

Please could the aggregate syntax be supported for (custom) options?

@perrydunn Have you experienced any errors? Because I confirmed it parsed your example well locally.

(base) ~/w/p/g/s/g/y/go-protoparser ❯❯❯ gwd _testdata
diff --git a/_testdata/simple.proto b/_testdata/simple.proto
index f372a79..081f480 100644
--- a/_testdata/simple.proto
+++ b/_testdata/simple.proto
@@ -1,26 +1,22 @@
 syntax = "proto3";
-// An example of the official reference
-// See https://developers.google.com/protocol-buffers/docs/reference/proto3-spec#proto_file
-package examplepb;
-import public "other.proto";
-option java_package = "com.example.foo";
-enum EnumAllowingAlias {
-    option allow_alias = true;
-    UNKNOWN = 0;
-    STARTED = 1;
-    RUNNING = 2 [(custom_option) = "this is a "
-                                   "string on two lines"
-                ];
+
+message FooOptions {
+  optional int32 opt1 = 1;
+  optional string opt2 = 2;
 }
-message outer {
-    option (my_option).a = true;
-    message inner {   // Level 2
-      int64 ival = 1;
-    }
-    repeated inner inner_message = 2;
-    EnumAllowingAlias enum_field =3;
-    map<int32, string> my_map = 4;
+
+extend google.protobuf.FieldOptions {
+  optional FooOptions foo_options = 1234;
+}
+
+// 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" }];
+  // for completeness, split with (optional) commas:
+  optional int32 c = 3 [(foo_options) = {
+    opt1: 123,
+    opt2: "baz",
+  }];
 }
-service HelloService {
-  rpc SayHello (HelloRequest) returns (HelloResponse) {};
-}
\ No newline at end of file
(base) ~/w/p/g/s/g/y/go-protoparser ❯❯❯ make run/dump/example
go run _example/dump/main.go -debug=false -permissive=true -unordered=false
{
  "Syntax": {
    "ProtobufVersion": "proto3",
    "ProtobufVersionQuote": "\"proto3\"",
    "Comments": null,
    "InlineComment": null,
    "Meta": {
      "Pos": {
        "Filename": "simple.proto",
        "Offset": 0,
        "Line": 1,
        "Column": 1
      },
      "LastPos": {
        "Filename": "simple.proto",
        "Offset": 17,
        "Line": 1,
        "Column": 18
      }
    }
  },
  "ProtoBody": [
    {
      "MessageName": "FooOptions",
      "MessageBody": [
        {
          "IsRepeated": false,
          "IsRequired": false,
          "IsOptional": true,
          "Type": "int32",
          "FieldName": "opt1",
          "FieldNumber": "1",
          "FieldOptions": null,
          "Comments": null,
          "InlineComment": null,
          "Meta": {
            "Pos": {
              "Filename": "simple.proto",
              "Offset": 43,
              "Line": 4,
              "Column": 3
            },
            "LastPos": {
              "Filename": "",
              "Offset": 0,
              "Line": 0,
              "Column": 0
            }
          }
        },
        {
          "IsRepeated": false,
          "IsRequired": false,
          "IsOptional": true,
          "Type": "string",
          "FieldName": "opt2",
          "FieldNumber": "2",
          "FieldOptions": null,
          "Comments": null,
          "InlineComment": null,
          "Meta": {
            "Pos": {
              "Filename": "simple.proto",
              "Offset": 70,
              "Line": 5,
              "Column": 3
            },
            "LastPos": {
              "Filename": "",
              "Offset": 0,
              "Line": 0,
              "Column": 0
            }
          }
        }
      ],
      "Comments": null,
      "InlineComment": null,
      "InlineCommentBehindLeftCurly": null,
      "Meta": {
        "Pos": {
          "Filename": "simple.proto",
          "Offset": 20,
          "Line": 3,
          "Column": 1
        },
        "LastPos": {
          "Filename": "simple.proto",
          "Offset": 96,
          "Line": 6,
          "Column": 1
        }
      }
    },
    {
      "MessageType": "google.protobuf.FieldOptions",
      "ExtendBody": [
        {
          "IsRepeated": false,
          "IsRequired": false,
          "IsOptional": true,
          "Type": "FooOptions",
          "FieldName": "foo_options",
          "FieldNumber": "1234",
          "FieldOptions": null,
          "Comments": null,
          "InlineComment": null,
          "Meta": {
            "Pos": {
              "Filename": "simple.proto",
              "Offset": 139,
              "Line": 9,
              "Column": 3
            },
            "LastPos": {
              "Filename": "",
              "Offset": 0,
              "Line": 0,
              "Column": 0
            }
          }
        }
      ],
      "Comments": null,
      "InlineComment": null,
      "InlineCommentBehindLeftCurly": null,
      "Meta": {
        "Pos": {
          "Filename": "simple.proto",
          "Offset": 99,
          "Line": 8,
          "Column": 1
        },
        "LastPos": {
          "Filename": "simple.proto",
          "Offset": 179,
          "Line": 10,
          "Column": 1
        }
      }
    },
    {
      "MessageName": "Bar",
      "MessageBody": [
        {
          "IsRepeated": false,
          "IsRequired": false,
          "IsOptional": true,
          "Type": "int32",
          "FieldName": "a",
          "FieldNumber": "1",
          "FieldOptions": [
            {
              "OptionName": "(foo_options).opt1",
              "Constant": "123"
            },
            {
              "OptionName": "(foo_options).opt2",
              "Constant": "\"baz\""
            }
          ],
          "Comments": null,
          "InlineComment": null,
          "Meta": {
            "Pos": {
              "Filename": "simple.proto",
              "Offset": 208,
              "Line": 14,
              "Column": 3
            },
            "LastPos": {
              "Filename": "",
              "Offset": 0,
              "Line": 0,
              "Column": 0
            }
          }
        },
        {
          "IsRepeated": false,
          "IsRequired": false,
          "IsOptional": true,
          "Type": "int32",
          "FieldName": "b",
          "FieldNumber": "2",
          "FieldOptions": [
            {
              "OptionName": "(foo_options)",
              "Constant": "{opt1:123\nopt2:\"baz\"}"
            }
          ],
          "Comments": [
            {
              "Raw": "// alternative aggregate syntax (uses TextFormat):",
              "Meta": {
                "Pos": {
                  "Filename": "simple.proto",
                  "Offset": 287,
                  "Line": 15,
                  "Column": 3
                },
                "LastPos": {
                  "Filename": "simple.proto",
                  "Offset": 336,
                  "Line": 15,
                  "Column": 52
                }
              }
            }
          ],
          "InlineComment": null,
          "Meta": {
            "Pos": {
              "Filename": "simple.proto",
              "Offset": 340,
              "Line": 16,
              "Column": 3
            },
            "LastPos": {
              "Filename": "",
              "Offset": 0,
              "Line": 0,
              "Column": 0
            }
          }
        },
        {
          "IsRepeated": false,
          "IsRequired": false,
          "IsOptional": true,
          "Type": "int32",
          "FieldName": "c",
          "FieldNumber": "3",
          "FieldOptions": [
            {
              "OptionName": "(foo_options)",
              "Constant": "{opt1:123,opt2:\"baz\",}"
            }
          ],
          "Comments": [
            {
              "Raw": "// for completeness, split with (optional) commas:",
              "Meta": {
                "Pos": {
                  "Filename": "simple.proto",
                  "Offset": 408,
                  "Line": 17,
                  "Column": 3
                },
                "LastPos": {
                  "Filename": "simple.proto",
                  "Offset": 457,
                  "Line": 17,
                  "Column": 52
                }
              }
            }
          ],
          "InlineComment": null,
          "Meta": {
            "Pos": {
              "Filename": "simple.proto",
              "Offset": 461,
              "Line": 18,
              "Column": 3
            },
            "LastPos": {
              "Filename": "",
              "Offset": 0,
              "Line": 0,
              "Column": 0
            }
          }
        }
      ],
      "Comments": [
        {
          "Raw": "// usage:",
          "Meta": {
            "Pos": {
              "Filename": "simple.proto",
              "Offset": 182,
              "Line": 12,
              "Column": 1
            },
            "LastPos": {
              "Filename": "simple.proto",
              "Offset": 190,
              "Line": 12,
              "Column": 9
            }
          }
        }
      ],
      "InlineComment": null,
      "InlineCommentBehindLeftCurly": null,
      "Meta": {
        "Pos": {
          "Filename": "simple.proto",
          "Offset": 192,
          "Line": 13,
          "Column": 1
        },
        "LastPos": {
          "Filename": "simple.proto",
          "Offset": 539,
          "Line": 22,
          "Column": 1
        }
      }
    }
  ],
  "Meta": {
    "Filename": "simple.proto"
  }
perrydunn commented 2 years ago

Thanks for the response. FieldOptions look happy, but at least EnumValueOptions are not (I haven't checked the others yet).

e.g. with simple.proto as

syntax = "proto3";

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

extend google.protobuf.EnumValueOptions {
  FooOptions foo_options = 1235;
}

enum Status {
  UNKNOWN = 0;
  COMPLETE = 1 [(foo_options) = {
    opt1: 123,
    opt2: "baz",
  }];
}

make run/dump/example results in

go run _example/dump/main.go -debug=false -permissive=true -unordered=false
failed to parse, err found "\"{\"(Token=14, Pos=simple.proto:14:33)" but expected [enumValueOption] at /home/peregrine/go-protoparser/parser/enum.go:280:found "{" but expected [;]
exit status 1
Makefile:47: recipe for target 'run/dump/example' failed
make: *** [run/dump/example] Error 1
yoheimuta commented 2 years ago

@perrydunn I've cut a release here: https://github.com/yoheimuta/go-protoparser/releases/tag/v4.5.4

perrydunn commented 2 years ago

FYI I opened https://github.com/yoheimuta/protolint/pull/221 to upgrade protolint's go-protoparser version to the one you've cut.

I'll run protolint on further protobuf files in our repo at work to see if there are any further issues wrt aggregate syntax and//or options before closing this issue.

Thanks!

perrydunn commented 2 years ago

I've run on our repo and all is well!

Thanks @yoheimuta