[feature request] proto3 optional

[elvizlai]

any plan to support proto3 optional?

ref:
proto3 field presence
proto3 optional impl

Need modify persister.go's Persist method adding or reading --experimental_allow_proto3_optional flag from command-line?:

resp.SupportedFeatures = proto.Uint64(uint64(plugin_go.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL))

[longxia0324]

please merge commits

[snqk]

I think #85 potentially missed out on re-defining how an optional field is detected according to the new definitions:

The following explanation is no longer entirely accurate in proto.go:

	// Proto3 syntax only allows for optional fields, but defaults to the zero
	// value of that particular type. Most of the field types in the generated go
	// structs are value types.
	// See: https://developers.google.com/protocol-buffers/docs/proto3
	Proto3 Syntax = "proto3"

Hence shouldn't the following snippet in field_type.go

func (s *scalarT) IsOptional() bool {
	return !s.fld.Syntax().SupportsRequiredPrefix() || s.ProtoLabel() == Optional
}

be updated to just:

func (s *scalarT) IsOptional() bool {
	return s.ProtoLabel() == Optional
}

\cc @dkunitsk @glerchundi @akonradi

[glerchundi]

Good catch @sarthak40, I think so. I also think that the check is a little bit convoluted...checking if optional is supported by testing if required is not supported before? 🤔

Anyway, yeah I think that it should be removed but we need to careful notifying as people relying on this boolean test would now suddenly see processes launching that weren't triggering before.

WDYT @dkunitsk @akonradi?

[snqk]

Anyway, yeah I think that it should be removed but we need to careful notifying as people relying on this boolean test would now suddenly see processes launching that weren't triggering before.

I guess its a good thing that we advertise this as an unstable api spec in the readme then 😄

[snqk]

@dkunitsk @akonradi if you're okay with the proposal above, I can prepare a new PR?