feat: [PHP] add previous classname to descriptor pool

php/composer.json

@@ -20,7 +20,10 @@
   "autoload-dev": {
     "psr-4": {
       "": "tmp"
-    }
+    },
+    "classmap": [
+      "tests/generated_previous"
+    ]
   },
   "scripts": {
     "test_c": "./generate_test_protos.sh && ./tests/compile_extension.sh && php -dextension=ext/google/protobuf/modules/protobuf.so vendor/bin/phpunit --bootstrap tests/force_c_ext.php tests",

php/ext/google/protobuf/def.c

@@ -162,7 +162,7 @@ static void EnumDescriptor_FromEnumDef(zval *val, const upb_EnumDef *m) {
     ZVAL_NULL(val);
   } else {
     char *classname =
-        GetPhpClassname(upb_EnumDef_File(m), upb_EnumDef_FullName(m));
+        GetPhpClassname(upb_EnumDef_File(m), upb_EnumDef_FullName(m), false);
     zend_string *str = zend_string_init(classname, strlen(classname), 0);
     zend_class_entry *ce = zend_lookup_class(str);  // May autoload the class.
 
@@ -500,14 +500,22 @@ static void Descriptor_destructor(zend_object* obj) {
 
 static zend_class_entry *Descriptor_GetGeneratedClass(const upb_MessageDef *m) {
   char *classname =
-      GetPhpClassname(upb_MessageDef_File(m), upb_MessageDef_FullName(m));
+      GetPhpClassname(upb_MessageDef_File(m), upb_MessageDef_FullName(m), false);
   zend_string *str = zend_string_init(classname, strlen(classname), 0);
   zend_class_entry *ce = zend_lookup_class(str);  // May autoload the class.
 
-  zend_string_release (str);
-
   if (!ce) {
-    zend_error(E_ERROR, "Couldn't load generated class %s", classname);
+    char *classname2 =
+      GetPhpClassname(upb_MessageDef_File(m), upb_MessageDef_FullName(m), true);
+    str = zend_string_init(classname2, strlen(classname2), 0);
+    ce = zend_lookup_class(str);  // May autoload the class.
+
+    zend_string_release (str);
+    free(classname2);
+
+    if (!ce) {
+      zend_error(E_ERROR, "Couldn't load generated class %s", classname);
+    }
   }
 
   free(classname);

php/ext/google/protobuf/names.c

@@ -89,6 +89,9 @@ const char *const kReservedNames[] = {
     "string",     "true",        "false",        "null",         "void",
     "iterable",   NULL};
 
+const char *const kPreviouslyUnreservedNames[] = {
+    "readonly", NULL};
+
 bool is_reserved_name(const char* name) {
   int i;
   for (i = 0; kReservedNames[i]; i++) {
@@ -99,6 +102,16 @@ bool is_reserved_name(const char* name) {
   return false;
 }
 
+bool is_previously_unreserved_name(const char* name) {
+  int i;
+  for (i = 0; kPreviouslyUnreservedNames[i]; i++) {
+    if (strcmp(kPreviouslyUnreservedNames[i], name) == 0) {
+      return true;
+    }
+  }
+  return false;
+}
+
 static char nolocale_tolower(char ch) {
   if (ch >= 'A' && ch <= 'Z') {
     return ch - ('A' - 'a');
@@ -115,7 +128,7 @@ static char nolocale_toupper(char ch) {
   }
 }
 
-static bool is_reserved(const char *segment, int length) {
+static bool is_reserved(const char *segment, int length, bool previous) {
   bool result;
   char* lower = calloc(1, length + 1);
   memcpy(lower, segment, length);
@@ -126,6 +139,9 @@ static bool is_reserved(const char *segment, int length) {
   }
   lower[length] = 0;
   result = is_reserved_name(lower);
+  if (result && previous && is_previously_unreserved_name(lower)) {
+    result = false;
+  }
   free(lower);
   return result;
 }
@@ -133,11 +149,12 @@ static bool is_reserved(const char *segment, int length) {
 static void fill_prefix(const char *segment, int length,
                         const char *prefix_given,
                         const char *package_name,
-                        stringsink *classname) {
+                        stringsink *classname,
+                        bool previous) {
   if (prefix_given != NULL && strcmp(prefix_given, "") != 0) {
     stringsink_string(classname, prefix_given, strlen(prefix_given));
   } else {
-    if (is_reserved(segment, length)) {
+    if (is_reserved(segment, length, previous)) {
       if (package_name != NULL &&
           strcmp("google.protobuf", package_name) == 0) {
         stringsink_string(classname, "GPB", 3);
@@ -160,7 +177,7 @@ static void fill_segment(const char *segment, int length,
 }
 
 static void fill_namespace(const char *package, const char *php_namespace,
-                           stringsink *classname) {
+                           stringsink *classname, bool previous) {
   if (php_namespace != NULL) {
     if (strlen(php_namespace) != 0) {
       stringsink_string(classname, php_namespace, strlen(php_namespace));
@@ -174,7 +191,7 @@ static void fill_namespace(const char *package, const char *php_namespace,
       while (j < package_len && package[j] != '.') {
         j++;
       }
-      fill_prefix(package + i, j - i, "", package, classname);
+      fill_prefix(package + i, j - i, "", package, classname, previous);
       fill_segment(package + i, j - i, classname, true);
       stringsink_string(classname, "\\", 1);
       i = j + 1;
@@ -185,7 +202,8 @@ static void fill_namespace(const char *package, const char *php_namespace,
 static void fill_classname(const char *fullname,
                            const char *package,
                            const char *prefix,
-                           stringsink *classname) {
+                           stringsink *classname,
+                           bool previous) {
   int classname_start = 0;
   if (package != NULL) {
     size_t package_len = strlen(package);
@@ -199,7 +217,7 @@ static void fill_classname(const char *fullname,
     while (j < fullname_len && fullname[j] != '.') {
       j++;
     }
-    fill_prefix(fullname + i, j - i, prefix, package, classname);
+    fill_prefix(fullname + i, j - i, prefix, package, classname, previous);
     fill_segment(fullname + i, j - i, classname, false);
     if (j != fullname_len) {
       stringsink_string(classname, "\\", 1);
@@ -215,7 +233,7 @@ char *str_view_dup(upb_StringView str) {
   return ret;
 }
 
-char *GetPhpClassname(const upb_FileDef *file, const char *fullname) {
+char *GetPhpClassname(const upb_FileDef *file, const char *fullname, bool previous) {
   // Prepend '.' to package name to make it absolute. In the 5 additional
   // bytes allocated, one for '.', one for trailing 0, and 3 for 'GPB' if
   // given message is google.protobuf.Empty.
@@ -234,8 +252,8 @@ char *GetPhpClassname(const upb_FileDef *file, const char *fullname) {
   stringsink namesink;
   stringsink_init(&namesink);
 
-  fill_namespace(package, php_namespace, &namesink);
+  fill_namespace(package, php_namespace, &namesink, previous);
-  fill_classname(fullname, package, prefix, &namesink);
+  fill_classname(fullname, package, prefix, &namesink, previous);
   stringsink_string(&namesink, "\0", 1);
   ret = strdup(namesink.ptr);
   stringsink_uninit(&namesink);

php/ext/google/protobuf/names.h

@@ -35,6 +35,6 @@
 
 // Translates a protobuf symbol name (eg. foo.bar.Baz) into a PHP class name
 // (eg. \Foo\Bar\Baz).
-char *GetPhpClassname(const upb_FileDef *file, const char *fullname);
+char *GetPhpClassname(const upb_FileDef *file, const char *fullname, bool previous);
 
 #endif  // PHP_PROTOBUF_NAMES_H_

php/ext/google/protobuf/protobuf.c

@@ -242,13 +242,18 @@ bool ObjCache_Get(const void *upb_obj, zval *val) {
 // -----------------------------------------------------------------------------
 
 void NameMap_AddMessage(const upb_MessageDef *m) {
-  char *k = GetPhpClassname(upb_MessageDef_File(m), upb_MessageDef_FullName(m));
+  char *k = GetPhpClassname(upb_MessageDef_File(m), upb_MessageDef_FullName(m), false);
   zend_hash_str_add_ptr(&PROTOBUF_G(name_msg_cache), k, strlen(k), (void*)m);
+  char *k2 = GetPhpClassname(upb_MessageDef_File(m), upb_MessageDef_FullName(m), true);
+  if (strcmp(k, k2) != 0) {
+    zend_hash_str_add_ptr(&PROTOBUF_G(name_msg_cache), k2, strlen(k2), (void*)m);
+  }
   free(k);
+  free(k2);
 }
 
 void NameMap_AddEnum(const upb_EnumDef *e) {
-  char *k = GetPhpClassname(upb_EnumDef_File(e), upb_EnumDef_FullName(e));
+  char *k = GetPhpClassname(upb_EnumDef_File(e), upb_EnumDef_FullName(e), false);
   zend_hash_str_add_ptr(&PROTOBUF_G(name_enum_cache), k, strlen(k), (void*)e);
   free(k);
 }

php/src/Google/Protobuf/Internal/Descriptor.php

@@ -45,6 +45,7 @@ class Descriptor
     private $enum_type = [];
     private $klass;
     private $legacy_klass;
+    private $previous_klass;
     private $options;
     private $oneof_decl = [];
 
@@ -162,6 +163,16 @@ class Descriptor
         return $this->legacy_klass;
     }
 
+    public function setPreviouslyReservedClass($klass)
+    {
+        $this->previous_klass = $klass;
+    }
+
+    public function getPreviouslyReservedClass()
+    {
+        return $this->previous_klass;
+    }
+
     public function setOptions($options)
     {
         $this->options = $options;
@@ -179,6 +190,7 @@ class Descriptor
         $message_name_without_package  = "";
         $classname = "";
         $legacy_classname = "";
+        $previous_classname = "";
         $fullname = "";
         GPBUtil::getFullClassName(
             $proto,
@@ -187,10 +199,12 @@ class Descriptor
             $message_name_without_package,
             $classname,
             $legacy_classname,
-            $fullname);
+            $fullname,
+            $previous_classname);
         $desc->setFullName($fullname);
         $desc->setClass($classname);
         $desc->setLegacyClass($legacy_classname);
+        $desc->setPreviouslyReservedClass($previous_classname);
         $desc->setOptions($proto->getOptions());
 
         foreach ($proto->getField() as $field_proto) {

php/src/Google/Protobuf/Internal/DescriptorPool.php

@@ -96,6 +96,7 @@ class DescriptorPool
             $descriptor->getClass();
         $this->class_to_desc[$descriptor->getClass()] = $descriptor;
         $this->class_to_desc[$descriptor->getLegacyClass()] = $descriptor;
+        $this->class_to_desc[$descriptor->getPreviouslyReservedClass()] = $descriptor;
         foreach ($descriptor->getNestedType() as $nested_type) {
             $this->addDescriptor($nested_type);
         }

php/src/Google/Protobuf/Internal/EnumDescriptor.php

@@ -101,7 +101,8 @@ class EnumDescriptor
             $enum_name_without_package,
             $classname,
             $legacy_classname,
-            $fullname);
+            $fullname,
+            $unused_previous_classname);
         $desc->setFullName($fullname);
         $desc->setClass($classname);
         $desc->setLegacyClass($legacy_classname);

php/src/Google/Protobuf/Internal/GPBUtil.php

@@ -304,6 +304,27 @@ class GPBUtil
         return "";
     }
 
+    public static function getPreviouslyUnreservedClassNamePrefix(
+        $classname,
+        $file_proto)
+    {
+        $previously_unreserved_words = array(
+            "readonly"=>0
+        );
+
+        if (array_key_exists(strtolower($classname), $previously_unreserved_words)) {
+            $option = $file_proto->getOptions();
+            $prefix = is_null($option) ? "" : $option->getPhpClassPrefix();
+            if ($prefix !== "") {
+                return $prefix;
+            }
+
+            return "";
+        }
+
+        return self::getClassNamePrefix($classname, $file_proto);
+    }
+
     public static function getLegacyClassNameWithoutPackage(
         $name,
         $file_proto)
@@ -323,6 +344,17 @@ class GPBUtil
         return implode('\\', $parts);
     }
 
+    public static function getPreviouslyUnreservedClassNameWithoutPackage(
+        $name,
+        $file_proto)
+    {
+        $parts = explode('.', $name);
+        foreach ($parts as $i => $part) {
+            $parts[$i] = static::getPreviouslyUnreservedClassNamePrefix($parts[$i], $file_proto) . $parts[$i];
+        }
+        return implode('\\', $parts);
+    }
+
     public static function getFullClassName(
         $proto,
         $containing,
@@ -330,7 +362,8 @@ class GPBUtil
         &$message_name_without_package,
         &$classname,
         &$legacy_classname,
-        &$fullname)
+        &$fullname,
+        &$previous_classname)
     {
         // Full name needs to start with '.'.
         $message_name_without_package = $proto->getName();
@@ -351,6 +384,9 @@ class GPBUtil
         $legacy_class_name_without_package =
             static::getLegacyClassNameWithoutPackage(
                 $message_name_without_package, $file_proto);
+        $previous_class_name_without_package =
+            static::getPreviouslyUnreservedClassNameWithoutPackage(
+                $message_name_without_package, $file_proto);
 
         $option = $file_proto->getOptions();
         if (!is_null($option) && $option->hasPhpNamespace()) {
@@ -359,10 +395,13 @@ class GPBUtil
                 $classname = $namespace . "\\" . $class_name_without_package;
                 $legacy_classname =
                     $namespace . "\\" . $legacy_class_name_without_package;
+                $previous_classname =
+                    $namespace . "\\" . $previous_class_name_without_package;
                 return;
             } else {
                 $classname = $class_name_without_package;
                 $legacy_classname = $legacy_class_name_without_package;
+                $previous_classname = $previous_class_name_without_package;
                 return;
             }
         }
@@ -370,6 +409,7 @@ class GPBUtil
         if ($package === "") {
             $classname = $class_name_without_package;
             $legacy_classname = $legacy_class_name_without_package;
+            $previous_classname = $previous_class_name_without_package;
         } else {
             $parts = array_map('ucwords', explode('.', $package));
             foreach ($parts as $i => $part) {
@@ -382,6 +422,11 @@ class GPBUtil
             $legacy_classname =
                 implode('\\', array_map('ucwords', explode('.', $package))).
                 "\\".$legacy_class_name_without_package;
+            $previous_classname =
+                implode('\\', array_map('ucwords', explode('.', $package))).
+                "\\".self::getPreviouslyUnreservedClassNamePrefix(
+                    $previous_class_name_without_package, $file_proto).
+                    $previous_class_name_without_package;
         }
     }
 

php/tests/PreviouslyGeneratedClassTest.php

@@ -0,0 +1,18 @@
+<?php
+
+require_once('test_base.php');
+require_once('test_util.php');
+
+class PreviouslyGeneratedClassTest extends TestBase
+{
+    #########################################################
+    # Test compatibility for previously unreserved words.
+    #########################################################
+
+    public function testPrefixForReservedWords()
+    {
+        $m = new \Previous\readonly();
+
+        $this->assertTrue(true);
+    }
+}

php/tests/generated_previous/GPBMetadata/ProtoPrevious/TestPreviouslyUnreservedMessage.php

@@ -0,0 +1,28 @@
+<?php
+# Generated by the protocol buffer compiler.  DO NOT EDIT!
+# source: proto_previous/test_previously_unreserved_message.proto
+
+namespace GPBMetadata\ProtoPrevious;
+
+class TestPreviouslyUnreservedMessage
+{
+    public static $is_initialized = false;
+
+    public static function initOnce() {
+        $pool = \Google\Protobuf\Internal\DescriptorPool::getGeneratedPool();
+
+        if (static::$is_initialized == true) {
+          return;
+        }
+        $pool->internalAddGeneratedFile(
+            '
+W
+7proto_previous/test_previously_unreserved_message.protoprevious"
+
+readonlybproto3'
+        , true);
+
+        static::$is_initialized = true;
+    }
+}
+

php/tests/generated_previous/Previous/readonly.php

@@ -0,0 +1,31 @@
+<?php
+# Generated by the protocol buffer compiler.  DO NOT EDIT!
+# source: proto_previous/test_previously_unreserved_message.proto
+
+namespace Previous;
+
+use Google\Protobuf\Internal\GPBType;
+use Google\Protobuf\Internal\RepeatedField;
+use Google\Protobuf\Internal\GPBUtil;
+
+/**
+ * Generated from protobuf message <code>previous.readonly</code>
+ */
+class readonly extends \Google\Protobuf\Internal\Message
+{
+
+    /**
+     * Constructor.
+     *
+     * @param array $data {
+     *     Optional. Data for populating the Message object.
+     *
+     * }
+     */
+    public function __construct($data = NULL) {
+        \GPBMetadata\ProtoPrevious\TestPreviouslyUnreservedMessage::initOnce();
+        parent::__construct($data);
+    }
+
+}
+

php/tests/proto_previous/test_previously_unreserved_message.proto

@@ -0,0 +1,5 @@
+syntax = "proto3";
+
+package previous;
+
+message readonly {}