Prepend "PB" to generated classes whose name are reserved words. (#2990)

8 files changed, 163 insertions, 61 deletions

diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c
index 50c0350e..8e563a61 100644
--- a/php/ext/google/protobuf/def.c
+++ b/php/ext/google/protobuf/def.c
@@ -30,6 +30,9 @@
 
 #include "protobuf.h"
 
+const char* const kReservedNames[] = {"Empty"};
+const int kReservedNamesSize = 1;
+
 // Forward declare.
 static void descriptor_init_c_instance(Descriptor* intern TSRMLS_DC);
 static void descriptor_free_c(Descriptor* object TSRMLS_DC);
@@ -355,58 +358,90 @@ PHP_METHOD(DescriptorPool, getGeneratedPool) {
 #endif
 }
 
-static void convert_to_class_name_inplace(char *class_name,
-                                          const char* fullname,
-                                          const char* prefix,
-                                          const char* package_name) {
+static void classname_no_prefix(const char *fullname, const char *package_name,
+                                char *class_name) {
   size_t i = 0, j;
-  bool first_char = true;
+  bool first_char = true, is_reserved = false;
   size_t pkg_name_len = package_name == NULL ? 0 : strlen(package_name);
-  size_t prefix_len = prefix == NULL ? 0 : strlen(prefix);
   size_t message_name_start = package_name == NULL ? 0 : pkg_name_len + 1;
   size_t message_len = (strlen(fullname) - message_name_start);
 
-  // In php, class name cannot be Empty.
-  if (strcmp("google.protobuf.Empty", fullname) == 0) {
-    strcpy(class_name, "\\Google\\Protobuf\\GPBEmpty");
-    return;
+  // Submessage is concatenated with its containing messages by '_'.
+  for (j = message_name_start; j < message_name_start + message_len; j++) {
+    if (fullname[j] == '.') {
+      class_name[i++] = '_';
+    } else {
+      class_name[i++] = fullname[j];
+    }
+  }
+}
+
+static const char *classname_prefix(const char *classname,
+                                    const char *prefix_given,
+                                    const char *package_name) {
+  size_t i;
+  bool is_reserved = false;
+
+  if (prefix_given != NULL && strcmp(prefix_given, "") != 0) {
+    return prefix_given;
   }
 
-  if (pkg_name_len != 0) {
-    class_name[i++] = '\\';
-    for (j = 0; j < pkg_name_len; j++) {
+  for (i = 0; i < kReservedNamesSize; i++) {
+    if (strcmp(kReservedNames[i], classname) == 0) {
+      is_reserved = true;
+      break;
+    }
+  }
+
+  if (is_reserved) {
+    if (package_name != NULL && strcmp("google.protobuf", package_name) == 0) {
+      return "GPB";
+    } else {
+      return "PB";
+    }
+  }
+
+  return "";
+}
+
+static void convert_to_class_name_inplace(const char *package,
+                                          const char *prefix, char *classname) {
+  size_t package_len = package == NULL ? 0 : strlen(package);
+  size_t prefix_len = prefix == NULL ? 0 : strlen(prefix);
+  size_t classname_len = strlen(classname);
+  int i = 0, j;
+  bool first_char = true;
+
+  int offset = package_len != 0 ? 2 : 0;
+
+  for (j = 0; j < classname_len; j++) {
+    classname[package_len + prefix_len + classname_len + offset - 1 - j] =
+        classname[classname_len - j - 1];
+  }
+
+  if (package_len != 0) {
+    classname[i++] = '\\';
+    for (j = 0; j < package_len; j++) {
       // php packages are divided by '\'.
-      if (package_name[j] == '.') {
-        class_name[i++] = '\\';
+      if (package[j] == '.') {
+        classname[i++] = '\\';
         first_char = true;
       } else if (first_char) {
         // PHP package uses camel case.
-        if (package_name[j] < 'A' || package_name[j] > 'Z') {
-          class_name[i++] = package_name[j] + 'A' - 'a';
+        if (package[j] < 'A' || package[j] > 'Z') {
+          classname[i++] = package[j] + 'A' - 'a';
         } else {
-          class_name[i++] = package_name[j];
+          classname[i++] = package[j];
         }
         first_char = false;
       } else {
-        class_name[i++] = package_name[j];
+        classname[i++] = package[j];
       }
     }
-    class_name[i++] = '\\';
+    classname[i++] = '\\';
   }
 
-  if (prefix_len > 0) {
-    strcpy(class_name + i, prefix);
-    i += prefix_len;
-  }
-
-  // Submessage is concatenated with its containing messages by '_'.
-  for (j = message_name_start; j < message_name_start + message_len; j++) {
-    if (fullname[j] == '.') {
-      class_name[i++] = '_';
-    } else {
-      class_name[i++] = fullname[j];
-    }
-  }
+  memcpy(classname + i, prefix, prefix_len);
 }
 
 PHP_METHOD(DescriptorPool, internalAddGeneratedFile) {
@@ -455,25 +490,27 @@ PHP_METHOD(DescriptorPool, internalAddGeneratedFile) {
      * bytes allocated, one for '.', one for trailing 0, and 3 for 'GPB' if    \
      * given message is google.protobuf.Empty.*/                               \
     const char *fullname = upb_##def_type_lower##_fullname(def_type_lower);    \
-    const char *prefix = upb_filedef_phpprefix(files[0]);                      \
-    size_t klass_name_len = strlen(fullname) + 5;                              \
-    if (prefix != NULL) {                                                      \
-      klass_name_len += strlen(prefix);                                        \
+    const char *prefix_given = upb_filedef_phpprefix(files[0]);                \
+    size_t classname_len = strlen(fullname) + 5;                               \
+    if (prefix_given != NULL) {                                                \
+      classname_len += strlen(prefix_given);                                   \
     }                                                                          \
-    char *klass_name = ecalloc(sizeof(char), klass_name_len);                  \
-    convert_to_class_name_inplace(klass_name, fullname, prefix,                \
-                                  upb_filedef_package(files[0]));              \
+    char *classname = ecalloc(sizeof(char), classname_len);                    \
+    const char *package = upb_filedef_package(files[0]);                       \
+    classname_no_prefix(fullname, package, classname);                         \
+    const char *prefix = classname_prefix(classname, prefix_given, package);   \
+    convert_to_class_name_inplace(package, prefix, classname);                 \
     PHP_PROTO_CE_DECLARE pce;                                                  \
-    if (php_proto_zend_lookup_class(klass_name, strlen(klass_name), &pce) ==   \
+    if (php_proto_zend_lookup_class(classname, strlen(classname), &pce) ==     \
         FAILURE) {                                                             \
       zend_error(E_ERROR, "Generated message class %s hasn't been defined",    \
-                 klass_name);                                                  \
+                 classname);                                                   \
       return;                                                                  \
     } else {                                                                   \
       desc->klass = PHP_PROTO_CE_UNREF(pce);                                   \
     }                                                                          \
     add_ce_obj(desc->klass, desc_php);                                         \
-    efree(klass_name);                                                         \
+    efree(classname);                                                          \
     break;                                                                     \
   }
 
diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php
index 887c86ca..0fb6cdc0 100644
--- a/php/src/Google/Protobuf/Internal/Message.php
+++ b/php/src/Google/Protobuf/Internal/Message.php
@@ -71,6 +71,7 @@ class Message
             return;
         }
         $pool = DescriptorPool::getGeneratedPool();
+        var_dump(get_class($this));
         $this->desc = $pool->getDescriptorByClassName(get_class($this));
         foreach ($this->desc->getField() as $field) {
             $setter = $field->getSetter();
diff --git a/php/src/Google/Protobuf/descriptor.php b/php/src/Google/Protobuf/descriptor.php
index 2263af6e..9c744a8a 100644
--- a/php/src/Google/Protobuf/descriptor.php
+++ b/php/src/Google/Protobuf/descriptor.php
@@ -220,20 +220,36 @@ class Descriptor
     }
 }
 
+function getClassNamePrefix(
+    $classname,
+    $file_proto)
+{
+    $option = $file_proto->getOptions();
+    $prefix = is_null($option) ? "" : $option->getPhpClassPrefix();
+    if ($prefix !== "") {
+        return $prefix;
+    }
+
+    $reserved_words = array("Empty");
+    foreach ($reserved_words as $reserved_word) {
+        if ($classname === $reserved_word) {
+            if ($file_proto->getPackage() === "google.protobuf") {
+                return "GPB";
+            } else {
+                return "PB";
+            }
+        }
+    }
+
+    return "";
+}
+
 function getClassNameWithoutPackage(
     $name,
     $file_proto)
 {
-    if ($name === "Empty" && $file_proto->getPackage() === "google.protobuf") {
-        return "GPBEmpty";
-    } else {
-        $option = $file_proto->getOptions();
-        $prefix = is_null($option) ? "" : $option->getPhpClassPrefix();
-        // Nested message class names are seperated by '_', and package names
-        // are seperated by '\'.
-        return $prefix . implode('_', array_map('ucwords',
-                               explode('.', $name)));
-    }
+    $classname = implode('_', array_map('ucwords', explode('.', $name)));
+    return getClassNamePrefix($classname, $file_proto) . $classname;
 }
 
 function getFullClassName(
diff --git a/php/tests/generated_class_test.php b/php/tests/generated_class_test.php
index c5dee2d6..554d2bea 100644
--- a/php/tests/generated_class_test.php
+++ b/php/tests/generated_class_test.php
@@ -865,4 +865,15 @@ class GeneratedClassTest extends TestBase
         $m->setPrefixMessage($n);
         $this->assertSame(1, $m->getPrefixMessage()->getA());
     }
+
+    #########################################################
+    # Test prefix for reserved words.
+    #########################################################
+
+    public function testPrefixForReservedWords()
+    {
+        $m = new \Foo\TestMessage_Empty();
+        $m = new \Foo\PBEmpty();
+        $m = new \PrefixEmpty();
+    }
 }
diff --git a/php/tests/memory_leak_test.php b/php/tests/memory_leak_test.php
index ea7a4c96..361982b5 100644
--- a/php/tests/memory_leak_test.php
+++ b/php/tests/memory_leak_test.php
@@ -5,11 +5,14 @@
 require_once('generated/NoNamespaceEnum.php');
 require_once('generated/NoNamespaceMessage.php');
 require_once('generated/NoNamespaceMessage_NestedEnum.php');
+require_once('generated/PrefixEmpty.php');
 require_once('generated/PrefixTestPrefix.php');
 require_once('generated/Bar/TestInclude.php');
+require_once('generated/Foo/PBEmpty.php');
 require_once('generated/Foo/TestEnum.php');
 require_once('generated/Foo/TestIncludePrefixMessage.php');
 require_once('generated/Foo/TestMessage.php');
+require_once('generated/Foo/TestMessage_Empty.php');
 require_once('generated/Foo/TestMessage_NestedEnum.php');
 require_once('generated/Foo/TestMessage_Sub.php');
 require_once('generated/Foo/TestPackedMessage.php');
diff --git a/php/tests/proto/test.proto b/php/tests/proto/test.proto
index b6c14866..f0d009c8 100644
--- a/php/tests/proto/test.proto
+++ b/php/tests/proto/test.proto
@@ -108,6 +108,11 @@ message TestMessage {
   }
 
   NestedEnum optional_nested_enum = 101;
+
+  // Test prefix for reserved words.
+  message Empty {
+    int32 a = 1;
+  }
 }
 
 enum TestEnum {
@@ -116,6 +121,11 @@ enum TestEnum {
   TWO  = 2;
 }
 
+// Test prefix for reserved words.
+message Empty {
+  int32 a = 1;
+}
+
 message TestPackedMessage {
   repeated int32    repeated_int32    = 90  [packed = true];
   repeated int64    repeated_int64    = 91  [packed = true];
diff --git a/php/tests/proto/test_prefix.proto b/php/tests/proto/test_prefix.proto
index 04582121..9bfbad7f 100644
--- a/php/tests/proto/test_prefix.proto
+++ b/php/tests/proto/test_prefix.proto
@@ -5,3 +5,8 @@ option php_class_prefix = "Prefix";
 message TestPrefix {
   int32 a = 1;
 }
+
+// Test prefix for reserved words.
+message Empty {
+  int32 a = 1;
+}
diff --git a/src/google/protobuf/compiler/php/php_generator.cc b/src/google/protobuf/compiler/php/php_generator.cc
index 32f40b2e..d24e1e5e 100644
--- a/src/google/protobuf/compiler/php/php_generator.cc
+++ b/src/google/protobuf/compiler/php/php_generator.cc
@@ -49,6 +49,8 @@ const std::string kDescriptorMetadataFile =
     "GPBMetadata/Google/Protobuf/Internal/Descriptor.php";
 const std::string kDescriptorDirName = "Google/Protobuf/Internal";
 const std::string kDescriptorPackageName = "Google\\Protobuf\\Internal";
+const char* const kReservedNames[] = {"Empty"};
+const int kReservedNamesSize = 1;
 
 namespace google {
 namespace protobuf {
@@ -105,14 +107,31 @@ std::string EnumFullName(const EnumDescriptor* envm, bool is_descriptor) {
 }
 
 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();
+std::string ClassNamePrefix(const string& classname,
+                            const DescriptorType* desc) {
+  const string& prefix = (desc->file()->options()).php_class_prefix();
+  if (prefix != "") {
+    return prefix;
+  }
+
+  bool is_reserved = false;
+
+  for (int i = 0; i < kReservedNamesSize; i++) {
+    if (classname == kReservedNames[i]) {
+      is_reserved = true;
+      break;
+    }
   }
+
+  if (is_reserved) {
+    if (desc->file()->package() == "google.protobuf") {
+      return "GPB";
+    } else {
+      return "PB";
+    }
+  }
+
+  return "";
 }
 
 
@@ -124,7 +143,7 @@ std::string FullClassName(const DescriptorType* desc, bool is_descriptor) {
     classname = containing->name() + '_' + classname;
     containing = containing->containing_type();
   }
-  classname = ClassNamePrefix(desc) + classname;
+  classname = ClassNamePrefix(classname, desc) + classname;
 
   if (desc->file()->package() == "") {
     return classname;