feat: more readable phpdoc escaping

php/tests/GeneratedClassTest.php

@@ -13,6 +13,7 @@
 use Foo\TestEnum;
 use Foo\TestIncludeNamespaceMessage;
 use Foo\TestIncludePrefixMessage;
+use Foo\TestSpecialCharacters;
 use Foo\TestMessage;
 use Foo\TestMessage\Sub;
 use Foo\TestMessage_Sub;
@@ -1894,4 +1895,25 @@ public function testIssue9440()
         $m->setVersion('1');
         $this->assertEquals(8, $m->getId());
     }
+
+    public function testSpecialCharacters()
+    {
+        $reflectionMethod = new \ReflectionMethod(TestSpecialCharacters::class, 'getA');
+        $docComment = $reflectionMethod->getDocComment();
+        $commentLines = explode("\n", $docComment);
+        $this->assertEquals('/**', array_shift($commentLines));
+        $this->assertEquals('     */', array_pop($commentLines));
+        $docComment = implode("\n", $commentLines);
+        // test special characters
+        $this->assertContains(";,/?:&=+$-_.!~*'()", $docComment);
+        // test open doc comment
+        $this->assertContains('/*', $docComment);
+        // test escaped closed doc comment
+        $this->assertNotContains('*/', $docComment);
+        $this->assertContains('{@*}', $docComment);
+        // test escaped at-sign
+        $this->assertContains('\@foo', $docComment);
+        // test forwardslash on new line
+        $this->assertContains("* /\n", $docComment);
+    }
 }

php/tests/proto/test_special_characters.proto

@@ -0,0 +1,17 @@
+syntax = "proto3";
+
+package foo;
+
+message TestSpecialCharacters {
+  // test special characters (shouldn't escape): ;,/?:&=+$-_.!~*'()
+  // test open comment (shouldn't escape): /*
+  // test close comment (should escape): */
+  // test at-sign (should escape): @foo
+  // test forward slash as first character on a newline:
+  ///
+  string a = 1;
+
+  ///
+  // test forward slash as first character on first line
+  string b = 2;
+}

src/google/protobuf/compiler/php/php_generator.cc

@@ -1610,38 +1610,37 @@ static std::string EscapePhpdoc(absl::string_view input) {
   std::string result;
   result.reserve(input.size() * 2);
 
-  char prev = '*';
+  char prev = '\0';
 
   for (std::string::size_type i = 0; i < input.size(); i++) {
     char c = input[i];
     switch (c) {
-      case '*':
-        // Avoid "/*".
-        if (prev == '/') {
-          result.append("&#42;");
-        } else {
-          result.push_back(c);
-        }
-        break;
+      // "/*" is allowed, do nothing
+      // case '*':
+      //   if (prev == '/') {
+      //     result.append("&#42;");
+      //   } else {
+      //     result.push_back(c);
+      //   }
+      //   break;
       case '/':
-        // Avoid "*/".
+        // Escape "*/" with "{@*}".
         if (prev == '*') {
-          result.append("&#47;");
+          result.pop_back();
+          result.append("{@*}");
         } else {
           result.push_back(c);
         }
         break;
       case '@':
-        // '@' starts phpdoc tags including the @deprecated tag, which will
-        // cause a compile-time error if inserted before a declaration that
-        // does not have a corresponding @Deprecated annotation.
-        result.append("&#64;");
+        // '@' starts phpdoc tags. Play it safe and escape it.
+        result.append("\\");
+        result.push_back(c);
         break;
       default:
         result.push_back(c);
         break;
     }
-
     prev = c;
   }