Merge pull request #10361 from haberman/cherry-pick-php-nested

fix: fix PHP readonly legacy files for nested messages
protocolbuffers · Aug 4, 2022 · 23097c4 · 23097c4
2 parents de48e9d + d8f882b
commit 23097c4
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 8 deletions.
diff --git a/php/tests/GeneratedClassTest.php b/php/tests/GeneratedClassTest.php
@@ -338,6 +338,7 @@ public function testLegacyReadOnlyMessage()
     {
         $this->assertTrue(class_exists('\Upper\READONLY'));
         $this->assertTrue(class_exists('\Lower\readonly'));
+        $this->assertTrue(class_exists('\Php\Test\TestNamespace\PBEmpty\ReadOnly'));
     }
 
     public function testLegacyReadOnlyEnum()

diff --git a/php/tests/proto/test_php_namespace.proto b/php/tests/proto/test_php_namespace.proto
@@ -27,5 +27,7 @@ message TestNamespace {
     enum NestedEnum {
       ZERO = 0;
     };
+    // Test previously unreserved name
+    message ReadOnly {}
   }
 }
diff --git a/src/google/protobuf/compiler/php/php_generator.cc b/src/google/protobuf/compiler/php/php_generator.cc
@@ -421,12 +421,17 @@ std::string LegacyGeneratedClassFileName(const DescriptorType* desc,
 }
 
 template <typename DescriptorType>
-std::string LegacyReadOnlyGeneratedClassFileName(const DescriptorType* desc,
-                                                 const Options& options) {
-  std::string php_namespace = RootPhpNamespace(desc, options);
+std::string LegacyReadOnlyGeneratedClassFileName(std::string php_namespace,
+                                                 const DescriptorType* desc) {
   if (!php_namespace.empty()) {
+    for (int i = 0; i < php_namespace.size(); i++) {
+      if (php_namespace[i] == '\\') {
+        php_namespace[i] = '/';
+      }
+    }
     return php_namespace + "/" + desc->name() + ".php";
   }
+
   return desc->name() + ".php";
 }
 
@@ -1316,27 +1321,39 @@ template <typename DescriptorType>
 void LegacyReadOnlyGenerateClassFile(const FileDescriptor* file,
                              const DescriptorType* desc, const Options& options,
                              GeneratorContext* generator_context) {
-  std::string filename = LegacyReadOnlyGeneratedClassFileName(desc, options);
+  std::string fullname = FullClassName(desc, options);
+  std::string php_namespace;
+  std::string classname;
+  int lastindex = fullname.find_last_of("\\");
+
+  if (lastindex != std::string::npos) {
+    php_namespace = fullname.substr(0, lastindex);
+    classname = fullname.substr(lastindex + 1);
+  } else {
+    php_namespace = "";
+    classname = fullname;
+  }
+
+  std::string filename = LegacyReadOnlyGeneratedClassFileName(php_namespace, desc);
   std::unique_ptr<io::ZeroCopyOutputStream> output(
       generator_context->Open(filename));
   io::Printer printer(output.get(), '^');
 
   GenerateHead(file, &printer);
 
-  std::string php_namespace = RootPhpNamespace(desc, options);
   if (!php_namespace.empty()) {
     printer.Print(
         "namespace ^name^;\n\n",
         "name", php_namespace);
   }
-  std::string newname = FullClassName(desc, options);
+
   printer.Print("class_exists(^new^::class); // autoload the new class, which "
       "will also create an alias to the deprecated class\n",
-      "new", GeneratedClassNameImpl(desc));
+      "new", classname);
   printer.Print("@trigger_error(__NAMESPACE__ . '\\^old^ is deprecated and will be removed in "
       "the next major release. Use ^fullname^ instead', E_USER_DEPRECATED);\n\n",
       "old", desc->name(),
-      "fullname", newname);
+      "fullname", classname);
 }
 
 void GenerateEnumFile(const FileDescriptor* file, const EnumDescriptor* en,