feat: more readable phpdoc escaping (#11208)

The PHPDoc escaping in PHP is aggressive in that it escapes some character sequences that don't need to be escaped (`/*`), and it uses HTML entities to escape others (`*/` and `@`) instead of the recommended PHPDoc escape sequences.

For Example, in [`Google\Api\RoutingParameter`](https://github.com/googleapis/common-protos-php/blob/main/src/Api/RoutingParameter.php#L42):

```
 * path_template: "projects/*/{table_location=instances/*}/tables/*"
```

Should be escaped as:

```
 * path_template: "projects/{@*}{table_location=instances/*}/tables/*"
```

according to [the PHPDoc guide](https://manual.phpdoc.org/HTMLframesConverter/default/phpDocumentor/tutorial_phpDocumentor.howto.pkg.html#basics.desc):

 - For `@`: "if you need an actual "@" in your DocBlock's description parts, you should be careful to either ensure it is not the first character on a line, or else escape it ("\\@") to avoid it being interpreted as a PhpDocumentor tag marker."

 - For `*/`: " If you need to use the closing comment "\*/" in a DocBlock, use the special escape sequence "{@*}."

Closes #11208

COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/11208 from bshaffer:more-readable-phpdoc-escaping a75f9745ea
PiperOrigin-RevId: 603091642
pull/15655/head
Brent Shaffer 1 year ago committed by Copybara-Service
parent f8ea418e6a
commit f929439ccd
  1. 22
      php/tests/GeneratedClassTest.php
  2. 17
      php/tests/proto/test_special_characters.proto
  3. 24
      src/google/protobuf/compiler/php/php_generator.cc

@ -13,6 +13,7 @@ use Foo\Test32Fields;
use Foo\TestEnum;
use Foo\TestIncludeNamespaceMessage;
use Foo\TestIncludePrefixMessage;
use Foo\TestSpecialCharacters;
use Foo\TestMessage;
use Foo\TestMessage\Sub;
use Foo\TestMessage_Sub;
@ -1906,4 +1907,25 @@ class GeneratedClassTest extends TestBase
$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);
}
}

@ -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;
}

@ -1556,38 +1556,30 @@ 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;
// NOTE: "/*" is allowed, do not escape it
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;
}

Loading…
Cancel
Save