Should this only be warning for FEATURE_PROTO3_OPTIONAL and not for all features (including FEATURE_SUPPORTS_EDITIONS)?

I think from a "safe behaviours" standpoint, that is probably the right thing to do (or to just error on all unsupported features), but given that the previous versions allowed for ignoring features, it's a bit of a tough call to make. Also, if a plugin doesn't support a particular edition, we still get that back as an error.

I am okay with restricting this to FEATURE_PROTO3_OPTIONAL, but I want to make sure that we don't end up still doing our users a disservice (while also allowing optional, despite the fact that maybe it might make sense to error for that too in this case).

Under the hood, proto3 optional fields are put into a synthetic oneof. As a result, the code generated by a plugin that does not support proto3 optional will still serialize correctly.

Code generated by a plugin that does not support editions will most likely behave much worse. In practice, the plugin will likely print an error and exit with an error code (or populate CodeGeneratorResponse.error). But there is no guarantee that it does. It could just as well generate code that looks okay, but crashes at runtime, or serializes in a non-conformant way.

Even if we chose to keep FEATURE_PROTO3_OPTIONAL as a warning for BC, I think we need to error for FEATURE_SUPPORTS_EDITIONS.

It's really unfortunate that we are stuck with the warning behavior for back-compat. I would really prefer to see a config setting to treat the error as a warning for a particular plugin+feature pair. That way we could make it default to being an error for v2 config (and buf migrate could auto-add the opt-out flag when migrating buf.gen.yaml, to preserve back compat).

If that's controversial or we think it would take too long to implement, maybe we can at least change this PR to only warn on proto3-option for pre-v2 configs? Otherwise, our back-compat constraints mean that once we introduce this we can never fix/improve it :(

I would really prefer to see a config setting to treat the error as a warning for a particular plugin+feature pair. That way we could make it default to being an error for v2 config (and buf migrate could auto-add the opt-out flag when migrating buf.gen.yaml, to preserve back compat).

I don't think that's too controversial, tbh, but it might take a little bit to implement, not necessarily for complexity reasons, but we want to make sure we capture a reasonable UX for the configs without breaking what we currently have released.

Given the comments in this thread, I'm going to take the route or warning on only on FEATURE_PROTO3_OPTIONAL for now, since I do think we want to move things closer to a safer model.

This isn't too clear to me - can provide suggestions, but I might just remove for now.

Yeah, I can remove for now -- I just wasn't sure if we wanted a changelog for this, so I put it up.