PHP: Add support for primitive types in setters (#5126)

* Add support for primitive types in setters

* Update to address PR feedback

* Add tests and fixes for repeated fields

* Remove repeated field code, add getters

* Cleanup, test getters and oneofs

* Move boxing logic into separate class

* Add tests for wrapper type constructor args

* Update to add new setXXXValue methods

* Fix tests for invalid values

* Fix c extension for wrapper accessors

* Fix the bug that well known types didn't call Message_construct

* Address PR comments

* Refactoring init message with array logic

* Add include path to protoc

* Add missing TSRM_LS defintion

* Fix TSRM_LS

* Fix dist check

.gitignore

@@ -194,3 +194,7 @@ cmake/cmake-build-debug/
 
 # Visual Studio 2017
 .vs
+
+# IntelliJ
+.idea
+*.iml

Makefile.am

@@ -749,11 +749,13 @@ php_EXTRA_DIST=                                                       \
   php/tests/proto/test_reserved_message_upper.proto                   \
   php/tests/proto/test_service.proto                                  \
   php/tests/proto/test_service_namespace.proto                        \
+  php/tests/proto/test_wrapper_type_setters.proto                     \
   php/tests/test.sh                                                   \
   php/tests/test_base.php                                             \
   php/tests/test_util.php                                             \
   php/tests/undefined_test.php                                        \
-  php/tests/well_known_test.php
+  php/tests/well_known_test.php                                       \
+  php/tests/wrapper_type_setters_test.php
 
 python_EXTRA_DIST=                                                           \
   python/MANIFEST.in                                                         \

php/composer.json

@@ -23,6 +23,6 @@
     }
   },
   "scripts": {
-    "test": "(cd tests && rm -rf generated && mkdir -p generated && ../../src/protoc --php_out=generated proto/empty/echo.proto proto/test.proto proto/test_include.proto proto/test_no_namespace.proto proto/test_prefix.proto proto/test_php_namespace.proto proto/test_empty_php_namespace.proto proto/test_reserved_enum_lower.proto proto/test_reserved_enum_upper.proto proto/test_reserved_enum_value_lower.proto proto/test_reserved_enum_value_upper.proto proto/test_reserved_message_lower.proto proto/test_reserved_message_upper.proto proto/test_service.proto proto/test_service_namespace.proto proto/test_descriptors.proto) && (cd ../src && ./protoc --php_out=../php/tests/generated -I../php/tests -I. ../php/tests/proto/test_import_descriptor_proto.proto) && vendor/bin/phpunit"
+    "test": "(cd tests && rm -rf generated && mkdir -p generated && ../../src/protoc --php_out=generated -I../../src -I. proto/empty/echo.proto proto/test.proto proto/test_include.proto proto/test_no_namespace.proto proto/test_prefix.proto proto/test_php_namespace.proto proto/test_empty_php_namespace.proto proto/test_reserved_enum_lower.proto proto/test_reserved_enum_upper.proto proto/test_reserved_enum_value_lower.proto proto/test_reserved_enum_value_upper.proto proto/test_reserved_message_lower.proto proto/test_reserved_message_upper.proto proto/test_service.proto proto/test_service_namespace.proto proto/test_wrapper_type_setters.proto proto/test_descriptors.proto) && (cd ../src && ./protoc --php_out=../php/tests/generated -I../php/tests -I. ../php/tests/proto/test_import_descriptor_proto.proto) && vendor/bin/phpunit"
   }
 }

php/phpunit.xml

@@ -12,6 +12,7 @@
       <file>tests/well_known_test.php</file>
       <file>tests/descriptors_test.php</file>
       <file>tests/generated_service_test.php</file>
+      <file>tests/wrapper_type_setters_test.php</file>
     </testsuite>
   </testsuites>
 </phpunit>

php/src/Google/Protobuf/Internal/FieldDescriptor.php

@@ -187,6 +187,25 @@ class FieldDescriptor
             $this->getMessageType()->getClass() === "Google\Protobuf\Timestamp";
     }
 
+    public function isWrapperType()
+    {
+        if ($this->getType() == GPBType::MESSAGE) {
+            $class = $this->getMessageType()->getClass();
+            return in_array($class, [
+                "Google\Protobuf\DoubleValue",
+                "Google\Protobuf\FloatValue",
+                "Google\Protobuf\Int64Value",
+                "Google\Protobuf\UInt64Value",
+                "Google\Protobuf\Int32Value",
+                "Google\Protobuf\UInt32Value",
+                "Google\Protobuf\BoolValue",
+                "Google\Protobuf\StringValue",
+                "Google\Protobuf\BytesValue",
+            ]);
+        }
+        return false;
+    }
+
     private static function isTypePackable($field_type)
     {
         return ($field_type !== GPBType::STRING  &&

php/src/Google/Protobuf/Internal/Message.php

@@ -975,7 +975,7 @@ class Message
      *
      * @param array $array An array containing message properties and values.
      * @return null.
-     * @throws Exception Invalid data.
+     * @throws \Exception Invalid data.
      */
     protected function mergeFromArray(array $array)
     {
@@ -987,10 +987,47 @@ class Message
                     'Invalid message property: ' . $key);
             }
             $setter = $field->getSetter();
+            if ($field->isWrapperType()) {
+                self::normalizeToMessageType($value, $field->getMessageType()->getClass());
+            }
             $this->$setter($value);
         }
     }
 
+    /**
+     * Tries to normalize $value into a provided protobuf wrapper type $class.
+     * If $value is any type other than an object, we attempt to construct an
+     * instance of $class and assign $value to it using the setValue method
+     * shared by all wrapper types.
+     *
+     * @param mixed $value The value to normalize.
+     * @param string $class The expected wrapper class name
+     * @throws \Exception If $value cannot be converted to a wrapper type
+     */
+    private static function normalizeToMessageType(&$value, $class)
+    {
+        if (is_null($value) || is_object($value)) {
+            // This handles the case that $value is an instance of $class. We
+            // choose not to do any more strict checking here, relying on the
+            // existing type checking done by GPBUtil.
+            return;
+        } else {
+            // Try to instantiate $class and set the value
+            try {
+                $msg = new $class;
+                $msg->setValue($value);
+                $value = $msg;
+                return;
+            } catch (\Exception $exception) {
+                throw new \Exception(
+                    "Error normalizing value to type '$class': " . $exception->getMessage(),
+                    $exception->getCode(),
+                    $exception
+                );
+            }
+        }
+    }
+
     protected function mergeFromJsonArray($array)
     {
         if (is_a($this, "Google\Protobuf\Any")) {

php/tests/proto/test_wrapper_type_setters.proto

+syntax = "proto3";
+
+import "google/protobuf/wrappers.proto";
+
+package foo;
+
+message TestWrapperSetters {
+  google.protobuf.DoubleValue double_value = 1;
+  google.protobuf.FloatValue float_value = 2;
+  google.protobuf.Int64Value int64_value = 3;
+  google.protobuf.UInt64Value uint64_value = 4;
+  google.protobuf.Int32Value int32_value = 5;
+  google.protobuf.UInt32Value uint32_value = 6;
+  google.protobuf.BoolValue bool_value = 7;
+  google.protobuf.StringValue string_value = 8;
+  google.protobuf.BytesValue bytes_value = 9;
+
+  oneof wrapped_oneofs {
+    google.protobuf.DoubleValue double_value_oneof = 10;
+    google.protobuf.StringValue string_value_oneof = 11;
+  }
+}

php/tests/test.sh

@@ -14,7 +14,7 @@ set -e
 phpize && ./configure CFLAGS='-g -O0' && make
 popd
 
-tests=( array_test.php encode_decode_test.php generated_class_test.php map_field_test.php well_known_test.php descriptors_test.php )
+tests=( array_test.php encode_decode_test.php generated_class_test.php map_field_test.php well_known_test.php descriptors_test.php wrapper_type_setters_test.php)
 
 for t in "${tests[@]}"
 do

src/google/protobuf/compiler/php/php_generator.cc

@@ -99,6 +99,10 @@ void GenerateMessageConstructorDocComment(io::Printer* printer,
                                           int is_descriptor);
 void GenerateFieldDocComment(io::Printer* printer, const FieldDescriptor* field,
                              int is_descriptor, int function_type);
+void GenerateWrapperFieldGetterDocComment(io::Printer* printer,
+                                          const FieldDescriptor* field);
+void GenerateWrapperFieldSetterDocComment(io::Printer* printer,
+                                          const FieldDescriptor* field);
 void GenerateEnumDocComment(io::Printer* printer, const EnumDescriptor* enum_,
                             int is_descriptor);
 void GenerateEnumValueDocComment(io::Printer* printer,
@@ -674,6 +678,21 @@ void GenerateFieldAccessor(const FieldDescriptor* field, bool is_descriptor,
         field->name());
   }
 
+  // For wrapper types, generate an additional getXXXValue getter
+  if (!field->is_map() &&
+      !field->is_repeated() &&
+      field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE &&
+      IsWrapperType(field)) {
+    GenerateWrapperFieldGetterDocComment(printer, field);
+    printer->Print(
+        "public function get^camel_name^Value()\n"
+        "{\n"
+        "    $wrapper = $this->get^camel_name^();\n"
+        "    return is_null($wrapper) ? null : $wrapper->getValue();\n"
+        "}\n\n",
+        "camel_name", UnderscoresToCamelCase(field->name(), true));
+  }
+
   // Generate setter.
   GenerateFieldDocComment(printer, field, is_descriptor, kFieldSetter);
   printer->Print(
@@ -772,6 +791,22 @@ void GenerateFieldAccessor(const FieldDescriptor* field, bool is_descriptor,
   printer->Print(
       "}\n\n");
 
+  // For wrapper types, generate an additional setXXXValue getter
+  if (!field->is_map() &&
+      !field->is_repeated() &&
+      field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE &&
+      IsWrapperType(field)) {
+    GenerateWrapperFieldSetterDocComment(printer, field);
+    printer->Print(
+        "public function set^camel_name^Value($var)\n"
+        "{\n"
+        "    $wrappedVar = is_null($var) ? null : new \\^wrapper_type^(['value' => $var]);\n"
+        "    return $this->set^camel_name^($wrappedVar);\n"
+        "}\n\n",
+        "camel_name", UnderscoresToCamelCase(field->name(), true),
+        "wrapper_type", LegacyFullClassName(field->message_type(), is_descriptor));
+  }
+
   // Generate has method for proto2 only.
   if (is_descriptor) {
     printer->Print(
@@ -988,14 +1023,16 @@ void GenerateUseDeclaration(bool is_descriptor, io::Printer* printer) {
     printer->Print(
         "use Google\\Protobuf\\Internal\\GPBType;\n"
         "use Google\\Protobuf\\Internal\\RepeatedField;\n"
-        "use Google\\Protobuf\\Internal\\GPBUtil;\n\n");
+        "use Google\\Protobuf\\Internal\\GPBUtil;\n"
+        "use Google\\Protobuf\\Internal\\GPBWrapperUtils;\n\n");
   } else {
     printer->Print(
         "use Google\\Protobuf\\Internal\\GPBType;\n"
         "use Google\\Protobuf\\Internal\\GPBWire;\n"
         "use Google\\Protobuf\\Internal\\RepeatedField;\n"
         "use Google\\Protobuf\\Internal\\InputStream;\n"
-        "use Google\\Protobuf\\Internal\\GPBUtil;\n\n");
+        "use Google\\Protobuf\\Internal\\GPBUtil;\n"
+        "use Google\\Protobuf\\Internal\\GPBWrapperUtils;\n\n");
   }
 }
 
@@ -1497,6 +1534,41 @@ void GenerateFieldDocComment(io::Printer* printer, const FieldDescriptor* field,
   printer->Print(" */\n");
 }
 
+void GenerateWrapperFieldGetterDocComment(io::Printer* printer, const FieldDescriptor* field) {
+  // Generate a doc comment for the special getXXXValue methods that are
+  // generated for wrapper types.
+  const FieldDescriptor* primitiveField = field->message_type()->FindFieldByName("value");
+  printer->Print("/**\n");
+  printer->Print(
+      " * Returns the unboxed value from <code>get^camel_name^()</code>\n\n",
+      "camel_name", UnderscoresToCamelCase(field->name(), true));
+  GenerateDocCommentBody(printer, field);
+  printer->Print(
+    " * Generated from protobuf field <code>^def^</code>\n",
+    "def", EscapePhpdoc(FirstLineOf(field->DebugString())));
+  printer->Print(" * @return ^php_type^|null\n",
+        "php_type", PhpGetterTypeName(primitiveField, false));
+  printer->Print(" */\n");
+}
+
+void GenerateWrapperFieldSetterDocComment(io::Printer* printer, const FieldDescriptor* field) {
+  // Generate a doc comment for the special setXXXValue methods that are
+  // generated for wrapper types.
+  const FieldDescriptor* primitiveField = field->message_type()->FindFieldByName("value");
+  printer->Print("/**\n");
+  printer->Print(
+      " * Sets the field by wrapping a primitive type in a ^message_name^ object.\n\n",
+      "message_name", LegacyFullClassName(field->message_type(), false));
+  GenerateDocCommentBody(printer, field);
+  printer->Print(
+    " * Generated from protobuf field <code>^def^</code>\n",
+    "def", EscapePhpdoc(FirstLineOf(field->DebugString())));
+  printer->Print(" * @param ^php_type^|null $var\n",
+        "php_type", PhpSetterTypeName(primitiveField, false));
+  printer->Print(" * @return $this\n");
+  printer->Print(" */\n");
+}
+
 void GenerateEnumDocComment(io::Printer* printer, const EnumDescriptor* enum_,
                             int is_descriptor) {
   printer->Print("/**\n");

src/google/protobuf/compiler/php/php_generator.h

@@ -62,6 +62,11 @@ PROTOC_EXPORT std::string GeneratedClassName(
 PROTOC_EXPORT std::string GeneratedClassName(
     const google::protobuf::ServiceDescriptor* desc);
 
+inline bool IsWrapperType(const FieldDescriptor* descriptor) {
+  return descriptor->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE &&
+      descriptor->message_type()->file()->name() == "google/protobuf/wrappers.proto";
+}
+
 }  // namespace php
 }  // namespace compiler
 }  // namespace protobuf

tests.sh

@@ -285,6 +285,7 @@ generate_php_test_proto() {
   rm -rf generated
   mkdir generated
   ../../src/protoc --php_out=generated         \
+    -I../../src -I.                            \
     proto/empty/echo.proto                     \
     proto/test.proto                           \
     proto/test_include.proto                   \
@@ -300,6 +301,7 @@ generate_php_test_proto() {
     proto/test_reserved_message_upper.proto    \
     proto/test_service.proto                   \
     proto/test_service_namespace.proto         \
+    proto/test_wrapper_type_setters.proto      \
     proto/test_descriptors.proto
   pushd ../../src
   ./protoc --php_out=../php/tests/generated -I../php/tests -I. \