From 4c57e8475f78ccac80407f03c2d23d30014785f9 Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Thu, 20 Apr 2017 01:19:03 -0700 Subject: Prepend "PB" to generated classes whose name are reserved words. (#2990) --- php/ext/google/protobuf/def.c | 123 ++++++++++++++-------- php/src/Google/Protobuf/Internal/Message.php | 1 + php/src/Google/Protobuf/descriptor.php | 36 +++++-- php/tests/generated_class_test.php | 11 ++ php/tests/memory_leak_test.php | 3 + php/tests/proto/test.proto | 10 ++ php/tests/proto/test_prefix.proto | 5 + src/google/protobuf/compiler/php/php_generator.cc | 35 ++++-- 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 -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; -- cgit v1.2.3