ksprojects / protobuf-jetbrains-plugin

Protobuf Support for JetBrains IDEs
https://plugins.jetbrains.com/plugin/8277
Apache License 2.0
259 stars 45 forks source link

Support nested annotation syntax #143

Closed F21 closed 4 years ago

F21 commented 5 years ago

Describe the bug I am using https://github.com/envoyproxy/protoc-gen-validate to generate validators for my protobuf using annotations.

Both of the following are valid syntax and compiles properly, but is considered to be invalid by the protobuf plugin:

Formatted by https://github.com/uber/prototool

message SomeMessage {
  repeated string ids = 1 [
    (validate.rules).repeated = {
      items: {
        string: {
          len: 36
        }
      }
      min_items: 1
    }
  ];
}

In a single line:

message SomeMessage {
  repeated string ids = 1 [(validate.rules).repeated = {items {string {len: 36}}, min_items: 1}];
}

To Reproduce Steps to reproduce the behavior:

  1. Paste the above snippets into the editor and see errors being highlighted.

Expected behavior The above snippets are valid and should not be considered errors

Screenshots err

Plugin (please complete the following information):

them0ntem commented 5 years ago

+1

bjaglin commented 5 years ago

This does not mention arrays (to express values of repeated fields in options), but support is also lacking.

The current parser only supports scalar literal for option values, but a literal can be a map or an array of literals (recursively).

Another example, using the same options:

syntax = "proto2";

// https://raw.githubusercontent.com/envoyproxy/protoc-gen-validate/dbe039e/validate/validate.proto
import "validate/validate.proto";

message Foo {
    optional int64 bar = 1 [(validate.rules).int64 = { gte: 42 }]; //OK
    optional int64 baz = 2 [(validate.rules) = { int64: { gte: 42 } }]; //KO
    optional int64 qux = 3 [(validate.rules).int64 = { in: [1, 2, 3] }]; //KO
}
bjaglin commented 5 years ago

I am not familiar with antlr, but looking at https://github.com/protostuff/protostuff-compiler/blob/61b53fa/protostuff-parser/src/main/antlr4/io/protostuff/compiler/parser/ProtoParser.g4#L210-L235, I think we can see that the text format support is partial:

jvolkman commented 5 years ago

The whole text format is supported. Text format is not well-specified publicly, but the canonical parser can be found here, and a distilled grammar can be found here.

In addition, map fields in proto generate implicit structures with key and value fields that can be used in text format. So the following would be valid:

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

package foo.bar;

message TestMessage {
  message Option {
    map<string, int32> map = 1;
  }
  extend google.protobuf.FieldOptions {
    Option option = 2000;
  }

  string test_field = 10 [ (option) = {
    map [
      { key: 'foo', value: 1 },
      { key: 'bar', value: 2 }
    ]
  }];
}
bjaglin commented 5 years ago

Thanks for the context @jvolkman, and interesting to see https://github.com/google/intellij-protocol-buffer-editor, although I understand this is currently for educational purpose.

The whole text format is supported

You mean that text format is fully supported to express option values in the language? When I said support was partial, I meant the support of text format in the protostuff compiler that is used by this plugin.

In addition, map fields in proto generate implicit structures with key and value fields

My bad, by "map" in my previous message, I meant the curly braces, { key1: value, key2: value } syntax in text format, not the map type from protobuf.

jvolkman commented 5 years ago

You mean that text format is fully supported to express option values in the language? When I said support was partial, I meant the support of text format in the protostuff compiler that is used by this plugin.

Yeah. This part of the proto language isn't well-specified, but protoc just defers to the text format parser for these long form options (relevant code in descriptor.cc).

nickcaballero-mm commented 4 years ago

+1

jvolkman commented 4 years ago

Shameless plug for anyone that needs to use the types of long-form options described in this issue: https://plugins.jetbrains.com/plugin/14004

kshchepanovskyi commented 4 years ago

This issue is closed as plugin is not supported anymore. Please switch to https://github.com/jvolkman/intellij-protobuf-editor.