Enum defined without package have incorrect class name. (#2988)

Fix the bug by sharing the code for generating class name for both
message and enum.

5 files changed, 54 insertions, 53 deletions

diff --git a/php/tests/generated_class_test.php b/php/tests/generated_class_test.php
index 39e6c6c4..41d63a60 100644
--- a/php/tests/generated_class_test.php
+++ b/php/tests/generated_class_test.php
@@ -1,7 +1,7 @@
 <?php
 
-require_once('generated/NoNameSpaceEnum.php');
-require_once('generated/NoNameSpaceMessage.php');
+require_once('generated/NoNamespaceEnum.php');
+require_once('generated/NoNamespaceMessage.php');
 require_once('test_base.php');
 require_once('test_util.php');
 
@@ -832,12 +832,16 @@ class GeneratedClassTest extends TestBase
 
     public function testMessageWithoutNamespace()
     {
-        $m = new NoNameSpaceMessage();
+        $m = new TestMessage();
+        $m->setOptionalNoNamespaceMessage(new NoNameSpaceMessage());
+        $m->getRepeatedNoNamespaceMessage()[] = new NoNameSpaceMessage();
     }
 
     public function testEnumWithoutNamespace()
     {
-        $m = new NoNameSpaceEnum();
+        $m = new TestMessage();
+        $m->setOptionalNoNamespaceEnum(NoNameSpaceEnum::VALUE_A);
+        $m->getRepeatedNoNamespaceEnum()[] = NoNameSpaceEnum::VALUE_A;
     }
 
     #########################################################
diff --git a/php/tests/memory_leak_test.php b/php/tests/memory_leak_test.php
index 68b6f5be..6b7077f0 100644
--- a/php/tests/memory_leak_test.php
+++ b/php/tests/memory_leak_test.php
@@ -2,6 +2,8 @@
 
 # phpunit has memory leak by itself. Thus, it cannot be used to test memory leak.
 
+require_once('generated/NoNamespaceEnum.php');
+require_once('generated/NoNamespaceMessage.php');
 require_once('generated/PrefixTestPrefix.php');
 require_once('generated/Bar/TestInclude.php');
 require_once('generated/Foo/TestEnum.php');
@@ -13,6 +15,7 @@ require_once('generated/Foo/TestPhpDoc.php');
 require_once('generated/Foo/TestUnpackedMessage.php');
 require_once('generated/GPBMetadata/Proto/Test.php');
 require_once('generated/GPBMetadata/Proto/TestInclude.php');
+require_once('generated/GPBMetadata/Proto/TestNoNamespace.php');
 require_once('generated/GPBMetadata/Proto/TestPrefix.php');
 require_once('test_util.php');
 
diff --git a/php/tests/proto/test.proto b/php/tests/proto/test.proto
index 1a47a3f2..cc9bf8c8 100644
--- a/php/tests/proto/test.proto
+++ b/php/tests/proto/test.proto
@@ -1,6 +1,7 @@
 syntax = "proto3";
 
 import 'proto/test_include.proto';
+import 'proto/test_no_namespace.proto';
 import 'proto/test_prefix.proto';
 
 package foo;
@@ -96,6 +97,11 @@ message TestMessage {
 
   // Reserved for non-existing field test.
   // int32 non_exist = 89;
+
+  NoNamespaceMessage optional_no_namespace_message = 91;
+  NoNamespaceEnum optional_no_namespace_enum = 92;
+  repeated NoNamespaceMessage repeated_no_namespace_message = 93;
+  repeated NoNamespaceEnum repeated_no_namespace_enum = 94;
 }
 
 enum TestEnum {
diff --git a/php/tests/proto/test_no_namespace.proto b/php/tests/proto/test_no_namespace.proto
index b8c4fdf2..b0f66002 100644
--- a/php/tests/proto/test_no_namespace.proto
+++ b/php/tests/proto/test_no_namespace.proto
@@ -1,10 +1,10 @@
 syntax = "proto3";
 
-message NoNameSpaceMessage {
+message NoNamespaceMessage {
   int32 a = 1;
 }
 
-enum NoNameSpaceEnum {
+enum NoNamespaceEnum {
   VALUE_A = 0;
   VALUE_B = 1;
 }
diff --git a/src/google/protobuf/compiler/php/php_generator.cc b/src/google/protobuf/compiler/php/php_generator.cc
index db72ea1a..32f40b2e 100644
--- a/src/google/protobuf/compiler/php/php_generator.cc
+++ b/src/google/protobuf/compiler/php/php_generator.cc
@@ -84,33 +84,6 @@ std::string RenameEmpty(const std::string& name) {
   }
 }
 
-std::string MessagePrefix(const Descriptor* message) {
-  // Empty cannot be php class name.
-  if (message->name() == "Empty" &&
-      message->file()->package() == "google.protobuf") {
-    return "GPB";
-  } else {
-    return (message->file()->options()).php_class_prefix();
-  }
-}
-
-std::string MessageName(const Descriptor* message, bool is_descriptor) {
-  string message_name = message->name();
-  const Descriptor* descriptor = message->containing_type();
-  while (descriptor != NULL) {
-    message_name = descriptor->name() + '_' + message_name;
-    descriptor = descriptor->containing_type();
-  }
-  message_name = MessagePrefix(message) + message_name;
-
-  if (message->file()->package() == "") {
-    return message_name;
-  } else {
-    return PhpName(message->file()->package(), is_descriptor) + '\\' +
-           message_name;
-  }
-}
-
 std::string MessageFullName(const Descriptor* message, bool is_descriptor) {
   if (is_descriptor) {
     return StringReplace(message->full_name(),
@@ -131,19 +104,34 @@ std::string EnumFullName(const EnumDescriptor* envm, bool is_descriptor) {
   }
 }
 
-std::string EnumClassName(const EnumDescriptor* envm) {
-  string enum_class_name = envm->name();
-  const Descriptor* descriptor = envm->containing_type();
-  while (descriptor != NULL) {
-    enum_class_name = descriptor->name() + '_' + enum_class_name;
-    descriptor = descriptor->containing_type();
+template <typename DescriptorType>
+std::string ClassNamePrefix(const DescriptorType* desc) {
+  // Empty cannot be php class name.
+  if (desc->name() == "Empty" &&
+      desc->file()->package() == "google.protobuf") {
+    return "GPB";
+  } else {
+    return (desc->file()->options()).php_class_prefix();
   }
-  return enum_class_name;
 }
 
-std::string EnumName(const EnumDescriptor* envm, bool is_descriptor) {
-  string enum_name = EnumClassName(envm);
-  return PhpName(envm->file()->package(), is_descriptor) + '\\' + enum_name;
+
+template <typename DescriptorType>
+std::string FullClassName(const DescriptorType* desc, bool is_descriptor) {
+  string classname = desc->name();
+  const Descriptor* containing = desc->containing_type();
+  while (containing != NULL) {
+    classname = containing->name() + '_' + classname;
+    containing = containing->containing_type();
+  }
+  classname = ClassNamePrefix(desc) + classname;
+
+  if (desc->file()->package() == "") {
+    return classname;
+  } else {
+    return PhpName(desc->file()->package(), is_descriptor) + '\\' +
+           classname;
+  }
 }
 
 std::string PhpName(const std::string& full_name, bool is_descriptor) {
@@ -231,7 +219,7 @@ std::string GeneratedMetadataFileName(const std::string& proto_file,
 
 std::string GeneratedMessageFileName(const Descriptor* message,
                                      bool is_descriptor) {
-  std::string result = MessageName(message, is_descriptor);
+  std::string result = FullClassName(message, is_descriptor);
   for (int i = 0; i < result.size(); i++) {
     if (result[i] == '\\') {
       result[i] = '/';
@@ -242,7 +230,7 @@ std::string GeneratedMessageFileName(const Descriptor* message,
 
 std::string GeneratedEnumFileName(const EnumDescriptor* en,
                                   bool is_descriptor) {
-  std::string result = EnumName(en, is_descriptor);
+  std::string result = FullClassName(en, is_descriptor);
   for (int i = 0; i < result.size(); i++) {
     if (result[i] == '\\') {
       result[i] = '/';
@@ -456,12 +444,12 @@ void GenerateFieldAccessor(const FieldDescriptor* field, bool is_descriptor,
       printer->Print(
           ", \\^class_name^);\n",
           "class_name",
-          MessageName(value->message_type(), is_descriptor) + "::class");
+          FullClassName(value->message_type(), is_descriptor) + "::class");
     } else if (value->cpp_type() == FieldDescriptor::CPPTYPE_ENUM) {
       printer->Print(
-          ", ^class_name^);\n",
+          ", \\^class_name^);\n",
           "class_name",
-          EnumName(value->enum_type(), is_descriptor) + "::class");
+          FullClassName(value->enum_type(), is_descriptor) + "::class");
     } else {
       printer->Print(");\n");
     }
@@ -474,23 +462,23 @@ void GenerateFieldAccessor(const FieldDescriptor* field, bool is_descriptor,
       printer->Print(
           ", \\^class_name^);\n",
           "class_name",
-          MessageName(field->message_type(), is_descriptor) + "::class");
+          FullClassName(field->message_type(), is_descriptor) + "::class");
     } else if (field->cpp_type() == FieldDescriptor::CPPTYPE_ENUM) {
       printer->Print(
-          ", ^class_name^);\n",
+          ", \\^class_name^);\n",
           "class_name",
-          EnumName(field->enum_type(), is_descriptor) + "::class");
+          FullClassName(field->enum_type(), is_descriptor) + "::class");
     } else {
       printer->Print(");\n");
     }
   } else if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
     printer->Print(
         "GPBUtil::checkMessage($var, \\^class_name^::class);\n",
-        "class_name", MessageName(field->message_type(), is_descriptor));
+        "class_name", FullClassName(field->message_type(), is_descriptor));
   } else if (field->cpp_type() == FieldDescriptor::CPPTYPE_ENUM) {
     printer->Print(
         "GPBUtil::checkEnum($var, \\^class_name^::class);\n",
-        "class_name", EnumName(field->enum_type(), is_descriptor));
+        "class_name", FullClassName(field->enum_type(), is_descriptor));
   } else if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING) {
     printer->Print(
         "GPBUtil::checkString($var, ^utf8^);\n",