Switch to addEnumType to fix fatal error (#3225)

* Switch to addEnumType to fix fatal error

* Fixing more cases of HHVM incompatibility

* Updating tests to be hhvm compatible

* Fixing tests

* Fixing merge

* Don't use call_user_func, should hopefully fix tests

* Fixing spelling

* Fixing another misspelling in a test

* Fixing placement of append and kvUpdate functions

* Actually fix function helpers

* Remove double addEnumType. How did this ever work?

* Fixing a couple more tests

* Only use the setter if the return value isn't an object

php/src/Google/Protobuf/Internal/FileDescriptor.php

@@ -78,12 +78,11 @@ class FileDescriptor
                 $message_proto, $proto, ""));
         }
         foreach ($proto->getEnumType() as $enum_proto) {
-            $file->getEnumType()[] =
-                $file->addEnumType(
-                    EnumDescriptor::buildFromProto(
-                        $enum_proto,
-                        $proto,
-                        ""));
+            $file->addEnumType(
+                EnumDescriptor::buildFromProto(
+                    $enum_proto,
+                    $proto,
+                    ""));
         }
         return $file;
     }

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

@@ -373,7 +373,7 @@ class Message
             $getter = $field->getGetter();
             while ($input->bytesUntilLimit() > 0) {
                 self::parseFieldFromStreamNoTag($input, $field, $value);
-                $this->$getter()[] = $value;
+                $this->appendHelper($field, $value);
             }
             $input->popLimit($limit);
             return;
@@ -382,11 +382,9 @@ class Message
         }
 
         if ($field->isMap()) {
-            $getter = $field->getGetter();
-            $this->$getter()[$value->getKey()] = $value->getValue();
+            $this->kvUpdateHelper($field, $value->getKey(), $value->getValue());
         } else if ($field->isRepeated()) {
-            $getter = $field->getGetter();
-            $this->$getter()[] = $value;
+            $this->appendHelper($field, $value);
         } else {
             $setter = $field->getSetter();
             $this->$setter($value);
@@ -533,9 +531,10 @@ class Message
                           $klass = $value_field->getMessageType()->getClass();
                           $copy = new $klass;
                           $copy->mergeFrom($value);
-                          $this->$getter()[$key] = $copy;
+
+                          $this->kvUpdateHelper($field, $key, $copy);
                       } else {
-                          $this->$getter()[$key] = $value;
+                          $this->kvUpdateHelper($field, $key, $value);
                       }
                   }
               }
@@ -546,9 +545,9 @@ class Message
                           $klass = $field->getMessageType()->getClass();
                           $copy = new $klass;
                           $copy->mergeFrom($tmp);
-                          $this->$getter()[] = $copy;
+                          $this->appendHelper($field, $copy);
                       } else {
-                          $this->$getter()[] = $tmp;
+                          $this->appendHelper($field, $tmp);
                       }
                   }
               }
@@ -896,4 +895,30 @@ class Message
         }
         return $size;
     }
+
+    private function appendHelper($field, $append_value)
+    {
+        $getter = $field->getGetter();
+        $setter = $field->getSetter();
+
+        $field_arr_value = $this->$getter();
+        $field_arr_value[] = $append_value;
+
+        if (!is_object($field_arr_value)) {
+            $this->$setter($field_arr_value);
+        }
+    }
+
+    private function kvUpdateHelper($field, $update_key, $update_value)
+    {
+        $getter = $field->getGetter();
+        $setter = $field->getSetter();
+
+        $field_arr_value = $this->$getter();
+        $field_arr_value[$update_key] = $update_value;
+
+        if (!is_object($field_arr_value)) {
+            $this->$setter($field_arr_value);
+        }
+    }
 }

php/tests/generated_class_test.php

@@ -743,24 +743,44 @@ class GeneratedClassTest extends TestBase
         $n->setOptionalInt32(100);
         $sub1 = new TestMessage_Sub();
         $sub1->setA(101);
-        $sub1->getB()[] = 102;
+
+        $b = $sub1->getB();
+        $b[] = 102;
+        $sub1->setB($b);
+
         $n->setOptionalMessage($sub1);
 
         // Repeated
-        $n->getRepeatedInt32()[] = 200;
-        $n->getRepeatedString()[] = 'abc';
+        $repeatedInt32 = $n->getRepeatedInt32();
+        $repeatedInt32[] = 200;
+        $n->setRepeatedInt32($repeatedInt32);
+
+        $repeatedString = $n->getRepeatedString();
+        $repeatedString[] = 'abc';
+        $n->setRepeatedString($repeatedString);
+
         $sub2 = new TestMessage_Sub();
         $sub2->setA(201);
-        $n->getRepeatedMessage()[] = $sub2;
+        $repeatedMessage = $n->getRepeatedMessage();
+        $repeatedMessage[] = $sub2;
+        $n->setRepeatedMessage($repeatedMessage);
 
         // Map
-        $n->getMapInt32Int32()[1] = 300;
-        $n->getMapInt32Int32()[-62] = 301;
-        $n->getMapStringString()['def'] = 'def';
-        $n->getMapInt32Message()[1] = new TestMessage_Sub();
-        $n->getMapInt32Message()[1]->setA(302);
-        $n->getMapInt32Message()[2] = new TestMessage_Sub();
-        $n->getMapInt32Message()[2]->setA(303);
+        $mapInt32Int32 = $n->getMapInt32Int32();
+        $mapInt32Int32[1] = 300;
+        $mapInt32Int32[-62] = 301;
+        $n->setMapInt32Int32($mapInt32Int32);
+
+        $mapStringString = $n->getMapStringString();
+        $mapStringString['def'] = 'def';
+        $n->setMapStringString($mapStringString);
+
+        $mapInt32Message = $n->getMapInt32Message();
+        $mapInt32Message[1] = new TestMessage_Sub();
+        $mapInt32Message[1]->setA(302);
+        $mapInt32Message[2] = new TestMessage_Sub();
+        $mapInt32Message[2]->setA(303);
+        $n->setMapInt32Message($mapInt32Message);
 
         $m->mergeFrom($n);
 
@@ -795,9 +815,16 @@ class GeneratedClassTest extends TestBase
         // Check sub-messages are copied by value.
         $n->getOptionalMessage()->setA(-101);
         $this->assertSame(101, $m->getOptionalMessage()->getA());
-        $n->getRepeatedMessage()[0]->setA(-201);
+
+        $repeatedMessage = $n->getRepeatedMessage();
+        $repeatedMessage[0]->setA(-201);
+        $n->setRepeatedMessage($repeatedMessage);
         $this->assertSame(201, $m->getRepeatedMessage()[2]->getA());
-        $n->getMapInt32Message()[1]->setA(-302);
+
+        $mapInt32Message = $n->getMapInt32Message();
+        $mapInt32Message[1]->setA(-302);
+        $n->setMapInt32Message($mapInt32Message);
+
         $this->assertSame(302, $m->getMapInt32Message()[1]->getA());
 
         // Test merge oneof.
@@ -843,7 +870,9 @@ class GeneratedClassTest extends TestBase
         $m = new TestMessage();
         $sub = new NoNameSpaceMessage();
         $m->setOptionalNoNamespaceMessage($sub);
-        $m->getRepeatedNoNamespaceMessage()[] = new NoNameSpaceMessage();
+        $repeatedNoNamespaceMessage = $m->getRepeatedNoNamespaceMessage();
+        $repeatedNoNamespaceMessage[] = new NoNameSpaceMessage();
+        $m->setRepeatedNoNamespaceMessage($repeatedNoNamespaceMessage);
 
         $n = new NoNamespaceMessage();
         $n->setB(NoNamespaceMessage_NestedEnum::ZERO);
@@ -853,7 +882,9 @@ class GeneratedClassTest extends TestBase
     {
         $m = new TestMessage();
         $m->setOptionalNoNamespaceEnum(NoNameSpaceEnum::VALUE_A);
-        $m->getRepeatedNoNamespaceEnum()[] = NoNameSpaceEnum::VALUE_A;
+        $repeatedNoNamespaceEnum = $m->getRepeatedNoNamespaceEnum();
+        $repeatedNoNamespaceEnum[] = NoNameSpaceEnum::VALUE_A;
+        $m->setRepeatedNoNamespaceEnum($repeatedNoNamespaceEnum);
     }
 
     #########################################################