Add support for 'optional' in proto3 files #317
Conversation
| iTakeAnOptionInt(req.maybeInt) | ||
| iTakeAnOptionString(req.maybeString) |
There was a problem hiding this comment.
this would also pass if maybeInt were of type Some[Int] or None
I think it might be better to assert the fields' types directly (without even needing a service I guess?)
There was a problem hiding this comment.
I added same-type constraints to these methods, so they can only be called by a value of the exact type we expect.
There was a problem hiding this comment.
my FP-scala-foo is a bit rusty. What is the difference to writing private def iTakeAnOptionInt(_: Option[Int]): Unit = ()?
There was a problem hiding this comment.
That's what I originally had, but as Jordan pointed out, if the generated code was of type Some[Int] or None[Int] it would still pass. Using the same-type constraint ensures that the function can really only be called with an expression that has the exact type Option[T].
I'm not sure how much we really need to be concerned about this though, so I'm happy to revert it if you 'd prefer.
| @@ -43,7 +43,8 @@ object ServerClientCodeGenerator extends CodeGenApp { | |||
| for { | |||
| file <- request.filesToGenerate | |||
| serviceFile <- generateServiceFiles(file, implicits) | |||
| } yield serviceFile | |||
| } yield serviceFile, | |||
| supportedFeatures = Set(CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL) | |||
There was a problem hiding this comment.
is this empty by default? If not, maybe
| supportedFeatures = Set(CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL) | |
| supportedFeatures += CodeGeneratorResponse.Feature.FEATURE_PROTO3_OPTIONAL |
There was a problem hiding this comment.
It's empty, so I think it's probably fine to leave as-is?
| iTakeAnOptionInt(req.maybeInt) | ||
| iTakeAnOptionString(req.maybeString) |
There was a problem hiding this comment.
my FP-scala-foo is a bit rusty. What is the difference to writing private def iTakeAnOptionInt(_: Option[Int]): Unit = ()?
Since protoc 3.15, proto3 files have had support for the
optionalkeyword, which can be used with scalar fields to achieve nullability.Our underlying code generator already supported this, we just needed to return the feature in order to indicate support in our
CodeGenApp.