Implement support for proto3 optional (#26)

proto3 optional requires opting into the experimental feature in our code generator. proto3 optional is implemented as a one-off synthetic field so we must treat those as normal objects in our codegenerator. You can find the instructions on how to handle those in the upstream protobuf documentation: https://github.com/protocolbuffers/protobuf/blob/main/docs/implementing_proto3_presence.md The PR adds tests for the logic for both binary and JSON decoding. We are missing reflection support but that seemed reasonable for now.
slackhq · Apr 24, 2024 · 9eb21c3 · 9eb21c3
1 parent eaf0ef0
commit 9eb21c3
Show file tree

Hide file tree

Showing 29 changed files with 1,343 additions and 243 deletions.
diff --git a/.gitattributes b/.gitattributes
@@ -0,0 +1,2 @@
+*.pb.bin -diff
+*.pb.bin.gz -diff
diff --git a/BUILD b/BUILD
@@ -89,7 +89,7 @@ hh_test(
     hh_args = "lib_test/test.php",
 )
 
-INTEGRATION_PHP = glob(["test/**/*.php"])
+INTEGRATION_PHP = glob(["test/**/*.php", "test/**/*.pb.json"])
 
 ALL_PHP += INTEGRATION_PHP
 

diff --git a/gen.sh b/gen.sh
@@ -38,12 +38,15 @@ for SRC in $PBS
 do
   echo source: $SRC
   ARGS="-I external/com_google_protobuf/src -I ./"
-  $PROTOC $ARGS --plugin=$GENHACK --hack_out="plugin=grpc,allow_proto2_dangerous:$TMP" $SRC
+  $PROTOC $ARGS --plugin=$GENHACK --hack_out="plugin=grpc,allow_proto2_dangerous:$TMP" --experimental_allow_proto3_optional $SRC
   echo
 done
 
 ARGS="-I external/com_google_protobuf/src -I ./"
 $PROTOC $ARGS --encode=foo.bar.example1  ./test/example1.proto < ./test/example1.pb.txt > $TMP/test/example1.pb.bin
+$PROTOC $ARGS --encode=baz.optional_proto3  ./test/optional_proto3.proto < ./test/empty_optional_proto3.pb.txt > $TMP/test/empty_optional_proto3.pb.bin
+$PROTOC $ARGS --encode=baz.optional_proto3  ./test/optional_proto3.proto < ./test/default_optional_proto3.pb.txt > $TMP/test/default_optional_proto3.pb.bin
+$PROTOC $ARGS --encode=baz.optional_proto3  ./test/optional_proto3.proto < ./test/custom_optional_proto3.pb.txt > $TMP/test/custom_optional_proto3.pb.bin
 
 if [ $# -gt 0 ]; then
   # Comparison mode; see if there are diffs, if none, exit 0.

diff --git a/generated/external/com_google_protobuf/conformance/conformance_proto.php b/generated/external/com_google_protobuf/conformance/conformance_proto.php
@@ -321,7 +321,9 @@ public function MergeFrom(\Protobuf\Internal\Decoder $d): void {
           $this->test_category = \conformance\TestCategory::FromInt($d->readVarint());
           break;
         case 6:
-          if ($this->jspb_encoding_options == null) $this->jspb_encoding_options = new \conformance\JspbEncodingConfig();
+          if ($this->jspb_encoding_options is null) {
+            $this->jspb_encoding_options = new \conformance\JspbEncodingConfig();
+          }
           $this->jspb_encoding_options->MergeFrom($d->readDecoder());
           break;
         case 7:
@@ -397,8 +399,10 @@ public function MergeJsonFrom(mixed $m): void {
           $this->test_category = \conformance\TestCategory::FromMixed($v);
           break;
         case 'jspb_encoding_options': case 'jspbEncodingOptions':
-          if ($v === null) break;
-          if ($this->jspb_encoding_options == null) $this->jspb_encoding_options = new \conformance\JspbEncodingConfig();
+          if ($v is null) break;
+          if ($this->jspb_encoding_options is null) {
+            $this->jspb_encoding_options = new \conformance\JspbEncodingConfig();
+          }
           $this->jspb_encoding_options->MergeJsonFrom($v);
           break;
         case 'jspb_payload': case 'jspbPayload':
@@ -424,7 +428,7 @@ public function CopyFrom(\Protobuf\Message $o): \Errors\Error {
     $this->message_type = $o->message_type;
     $this->test_category = $o->test_category;
     $tmp = $o->jspb_encoding_options;
-    if ($tmp !== null) {
+    if ($tmp is nonnull) {
       $nv = new \conformance\JspbEncodingConfig();
       $nv->CopyFrom($tmp);
       $this->jspb_encoding_options = $nv;

diff --git a/generated/google/protobuf/api_proto.php b/generated/google/protobuf/api_proto.php
@@ -67,7 +67,9 @@ public function MergeFrom(\Protobuf\Internal\Decoder $d): void {
           $this->version = $d->readString();
           break;
         case 5:
-          if ($this->source_context == null) $this->source_context = new \google\protobuf\SourceContext();
+          if ($this->source_context is null) {
+            $this->source_context = new \google\protobuf\SourceContext();
+          }
           $this->source_context->MergeFrom($d->readDecoder());
           break;
         case 6:
@@ -158,8 +160,10 @@ public function MergeJsonFrom(mixed $m): void {
           $this->version = \Protobuf\Internal\JsonDecoder::readString($v);
           break;
         case 'source_context': case 'sourceContext':
-          if ($v === null) break;
-          if ($this->source_context == null) $this->source_context = new \google\protobuf\SourceContext();
+          if ($v is null) break;
+          if ($this->source_context is null) {
+            $this->source_context = new \google\protobuf\SourceContext();
+          }
           $this->source_context->MergeJsonFrom($v);
           break;
         case 'mixins':
@@ -195,7 +199,7 @@ public function CopyFrom(\Protobuf\Message $o): \Errors\Error {
     }
     $this->version = $o->version;
     $tmp = $o->source_context;
-    if ($tmp !== null) {
+    if ($tmp is nonnull) {
       $nv = new \google\protobuf\SourceContext();
       $nv->CopyFrom($tmp);
       $this->source_context = $nv;

diff --git a/generated/google/protobuf/compiler/plugin_proto.php b/generated/google/protobuf/compiler/plugin_proto.php
@@ -167,7 +167,9 @@ public function MergeFrom(\Protobuf\Internal\Decoder $d): void {
           $this->parameter = $d->readString();
           break;
         case 3:
-          if ($this->compiler_version == null) $this->compiler_version = new \google\protobuf\compiler\Version();
+          if ($this->compiler_version is null) {
+            $this->compiler_version = new \google\protobuf\compiler\Version();
+          }
           $this->compiler_version->MergeFrom($d->readDecoder());
           break;
         case 15:
@@ -226,8 +228,10 @@ public function MergeJsonFrom(mixed $m): void {
           $this->parameter = \Protobuf\Internal\JsonDecoder::readString($v);
           break;
         case 'compiler_version': case 'compilerVersion':
-          if ($v === null) break;
-          if ($this->compiler_version == null) $this->compiler_version = new \google\protobuf\compiler\Version();
+          if ($v is null) break;
+          if ($this->compiler_version is null) {
+            $this->compiler_version = new \google\protobuf\compiler\Version();
+          }
           $this->compiler_version->MergeJsonFrom($v);
           break;
         case 'proto_file': case 'protoFile':
@@ -250,7 +254,7 @@ public function CopyFrom(\Protobuf\Message $o): \Errors\Error {
     $this->file_to_generate = $o->file_to_generate;
     $this->parameter = $o->parameter;
     $tmp = $o->compiler_version;
-    if ($tmp !== null) {
+    if ($tmp is nonnull) {
       $nv = new \google\protobuf\compiler\Version();
       $nv->CopyFrom($tmp);
       $this->compiler_version = $nv;
@@ -337,7 +341,9 @@ public function MergeFrom(\Protobuf\Internal\Decoder $d): void {
           $this->content = $d->readString();
           break;
         case 16:
-          if ($this->generated_code_info == null) $this->generated_code_info = new \google\protobuf\GeneratedCodeInfo();
+          if ($this->generated_code_info is null) {
+            $this->generated_code_info = new \google\protobuf\GeneratedCodeInfo();
+          }
           $this->generated_code_info->MergeFrom($d->readDecoder());
           break;
         default:
@@ -391,8 +397,10 @@ public function MergeJsonFrom(mixed $m): void {
           $this->content = \Protobuf\Internal\JsonDecoder::readString($v);
           break;
         case 'generated_code_info': case 'generatedCodeInfo':
-          if ($v === null) break;
-          if ($this->generated_code_info == null) $this->generated_code_info = new \google\protobuf\GeneratedCodeInfo();
+          if ($v is null) break;
+          if ($this->generated_code_info is null) {
+            $this->generated_code_info = new \google\protobuf\GeneratedCodeInfo();
+          }
           $this->generated_code_info->MergeJsonFrom($v);
           break;
         default:
@@ -409,7 +417,7 @@ public function CopyFrom(\Protobuf\Message $o): \Errors\Error {
     $this->insertion_point = $o->insertion_point;
     $this->content = $o->content;
     $tmp = $o->generated_code_info;
-    if ($tmp !== null) {
+    if ($tmp is nonnull) {
       $nv = new \google\protobuf\GeneratedCodeInfo();
       $nv->CopyFrom($tmp);
       $this->generated_code_info = $nv;