Rust-specific `.proto` field option (optionally: comment) to be turned into an attribute

[vriesk]

Idea/request:

The unfortunate problem with using the generated code is that it is not possible to add custom type/field attributes; it can be achieved with .type_attribute() entries in the config, but that results in decoupled attributes, making things like proper extensive validator annotations quite unreadable.

The solution could be to use a custom protobuf field option to contain such attribute line(s) which then would result in them appearing in generated Rust code.

WDYT?

[vriesk]

This could look like this:

message Foo {
  option rust.type_attribute = "#[derive(::validator::Validate)]";

  string email = 1 [ rust.field_attribute = "#[validate(email)]" ];
  string name = 2 [ rust.field_attribute = "#[validate(custom = "::my_crate::check_name")]";
}

... being turned into:

#[derive(Clone, PartialEq, ::prost::Message)]
#[derive(::validator::Validate)]
struct Foo {
  #[validate(email)]
  email: String,
  #[validate(custom = "::my_crate::check_name")]
  name: String,
}

Alternatively, comments could be used for that, but that's not very ProtoBuf-idiomatic.

[vriesk]

Another (alternative) idea:

@LucioFranco, would you accept a PR that adds a (feature-guarded) dependency on validator crate and then generates validation instrumentation (will need a few custom functions, but nothing fancy) in the generated protobufs based on PGV semantics (see https://github.com/envoyproxy/protoc-gen-validate/blob/main/validate/validate.proto)? WDYT?

[LucioFranco]

Could you explain more about the design you're proposing? I am cautious to add more dependencies even if they are behind a feature gate.

[vriesk]

OK, so the way I imagine it working would be the following:

• prost declares to natively support the PGV validation schema.
• The PGV proto(s) will need to be pulled into prost, probably just copied and updated every now and then. The prost ecosystem will have to "see" them automatically.
• When validation option is present on a field, the generated code will then contain a #[derive(::validator::Validate)] trait attribute and a relevant #[validate(....)] on the relevant field(s)
• Even when a particular message does not have any annotated fields, it still should have a validate() method (either through the derive or explicit impl Validate for it) and use nested validation on any message fields it has
• validator does not fully support everything the PGV does out of the box, so the implementation will need to include a number of custom validation functions that probably should live in the prost crate
• There are special semantics for Duration and Timestamp WKTs in PGV, so instead of nested validation, they will receive direct treatment.
• This means that apart from the two above, every message in the generated code will implement Validate trait, even if no validation logic is present in it.

All this (both the validator dependency and the code generation) can be guarded by crate feature, so unless explicitly requested, the default installation will not notice anything at all.

WDYT?

[LucioFranco]

So I think this sounds interesting but the main concerns I have is I'd not really like to add this as a dependency even if its behind a feature flag. From the looks at PGV its a part of a plugin ecosystem that allows them to decouple things which we don't have yet for prost. So maybe part of the larger work here we could look into codegen plugins potentially that would enable many other types of features as well. What do you think?

[vriesk]

Codegen plugins API sound very nice, but it also sounds like a substantially larger and more complex effort. While I could possibly contribute the PGV implementation in some freeish time, the latter sounds somewhat prohibitive time-wise.

BTW, why are feature-guarded dependencies a concern?

[LucioFranco]

My concern by adding something like this is that another user might want a different decorator library and supporting all of them is out of scope of this project. Even though you would be able to maintain I believe the effort in adding something like this is better spent on figuring out how to support other types of decorators like this.

If something is behind a feature flag it is still in the dependency closure of the project and therefore we are responsible for it (even if you maintain it or not). This also means if we want to ship to 1.0 we need to also understand what that looks like for a crate like this. In reality, the goal of prost will be to reduce dependencies to improve stability/security posture.

[Victor-N-Suadicani]

I think using custom field attributes would be really interesting as a much better alternative than .type_attribute.

I don't think prost should use PGV though, at least not yet. Firstly, it's currently in alpha and unstable. But more importantly, I think it'd be better to just use some kind of custom message attribute which would tell prost to use another type that implements message, or something along those lines, see #427 or #699 .

[Ganitzsh]

Hey there! I was wondering if anything validation related was still on the table maybe in another issue that I couldn't find with the validation keyword?