Fix PhpDoc comments for message accessors to include "|null". (#8136)

Message accessors will return null when when the field is not
set, so this should be reflected in the PhpDoc.

Also updated the code generator for the well-known types to reflect
the edits made in https://github.com/protocolbuffers/protobuf/pull/8105.

Also explicitly check for upb_msg_has() in the oneof accessor, so
we are not implicitly relying on unset message fields returning NULL
at the upb level.
pull/8195/head
Joshua Haberman 4 years ago committed by GitHub
parent 9647a7c235
commit 7089ff0ef2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 4
      php/ext/google/protobuf/message.c
  2. 2
      php/src/Google/Protobuf/Api.php
  3. 2
      php/src/Google/Protobuf/Enum.php
  4. 2
      php/src/Google/Protobuf/Internal/DescriptorProto.php
  5. 2
      php/src/Google/Protobuf/Internal/DescriptorProto/ExtensionRange.php
  6. 2
      php/src/Google/Protobuf/Internal/EnumDescriptorProto.php
  7. 2
      php/src/Google/Protobuf/Internal/EnumValueDescriptorProto.php
  8. 2
      php/src/Google/Protobuf/Internal/FieldDescriptorProto.php
  9. 4
      php/src/Google/Protobuf/Internal/FileDescriptorProto.php
  10. 2
      php/src/Google/Protobuf/Internal/MethodDescriptorProto.php
  11. 2
      php/src/Google/Protobuf/Internal/OneofDescriptorProto.php
  12. 2
      php/src/Google/Protobuf/Internal/ServiceDescriptorProto.php
  13. 2
      php/src/Google/Protobuf/Option.php
  14. 2
      php/src/Google/Protobuf/Type.php
  15. 4
      php/src/Google/Protobuf/Value.php
  16. 78
      src/google/protobuf/compiler/php/php_generator.cc

@ -968,6 +968,10 @@ PHP_METHOD(Message, readOneof) {
(int)field_num); (int)field_num);
} }
if (upb_fielddef_issubmsg(f) && !upb_msg_has(intern->msg, f)) {
RETURN_NULL();
}
{ {
upb_msgval msgval = upb_msg_get(intern->msg, f); upb_msgval msgval = upb_msg_get(intern->msg, f);
const Descriptor *subdesc = Descriptor_GetFromFieldDef(f); const Descriptor *subdesc = Descriptor_GetFromFieldDef(f);

@ -271,7 +271,7 @@ class Api extends \Google\Protobuf\Internal\Message
* message. * message.
* *
* Generated from protobuf field <code>.google.protobuf.SourceContext source_context = 5;</code> * Generated from protobuf field <code>.google.protobuf.SourceContext source_context = 5;</code>
* @return \Google\Protobuf\SourceContext * @return \Google\Protobuf\SourceContext|null
*/ */
public function getSourceContext() public function getSourceContext()
{ {

@ -151,7 +151,7 @@ class Enum extends \Google\Protobuf\Internal\Message
* The source context. * The source context.
* *
* Generated from protobuf field <code>.google.protobuf.SourceContext source_context = 4;</code> * Generated from protobuf field <code>.google.protobuf.SourceContext source_context = 4;</code>
* @return \Google\Protobuf\SourceContext * @return \Google\Protobuf\SourceContext|null
*/ */
public function getSourceContext() public function getSourceContext()
{ {

@ -252,7 +252,7 @@ class DescriptorProto extends \Google\Protobuf\Internal\Message
/** /**
* Generated from protobuf field <code>optional .google.protobuf.MessageOptions options = 7;</code> * Generated from protobuf field <code>optional .google.protobuf.MessageOptions options = 7;</code>
* @return \Google\Protobuf\Internal\MessageOptions * @return \Google\Protobuf\Internal\MessageOptions|null
*/ */
public function getOptions() public function getOptions()
{ {

@ -124,7 +124,7 @@ class ExtensionRange extends \Google\Protobuf\Internal\Message
/** /**
* Generated from protobuf field <code>optional .google.protobuf.ExtensionRangeOptions options = 3;</code> * Generated from protobuf field <code>optional .google.protobuf.ExtensionRangeOptions options = 3;</code>
* @return \Google\Protobuf\Internal\ExtensionRangeOptions * @return \Google\Protobuf\Internal\ExtensionRangeOptions|null
*/ */
public function getOptions() public function getOptions()
{ {

@ -124,7 +124,7 @@ class EnumDescriptorProto extends \Google\Protobuf\Internal\Message
/** /**
* Generated from protobuf field <code>optional .google.protobuf.EnumOptions options = 3;</code> * Generated from protobuf field <code>optional .google.protobuf.EnumOptions options = 3;</code>
* @return \Google\Protobuf\Internal\EnumOptions * @return \Google\Protobuf\Internal\EnumOptions|null
*/ */
public function getOptions() public function getOptions()
{ {

@ -112,7 +112,7 @@ class EnumValueDescriptorProto extends \Google\Protobuf\Internal\Message
/** /**
* Generated from protobuf field <code>optional .google.protobuf.EnumValueOptions options = 3;</code> * Generated from protobuf field <code>optional .google.protobuf.EnumValueOptions options = 3;</code>
* @return \Google\Protobuf\Internal\EnumValueOptions * @return \Google\Protobuf\Internal\EnumValueOptions|null
*/ */
public function getOptions() public function getOptions()
{ {

@ -511,7 +511,7 @@ class FieldDescriptorProto extends \Google\Protobuf\Internal\Message
/** /**
* Generated from protobuf field <code>optional .google.protobuf.FieldOptions options = 8;</code> * Generated from protobuf field <code>optional .google.protobuf.FieldOptions options = 8;</code>
* @return \Google\Protobuf\Internal\FieldOptions * @return \Google\Protobuf\Internal\FieldOptions|null
*/ */
public function getOptions() public function getOptions()
{ {

@ -371,7 +371,7 @@ class FileDescriptorProto extends \Google\Protobuf\Internal\Message
/** /**
* Generated from protobuf field <code>optional .google.protobuf.FileOptions options = 8;</code> * Generated from protobuf field <code>optional .google.protobuf.FileOptions options = 8;</code>
* @return \Google\Protobuf\Internal\FileOptions * @return \Google\Protobuf\Internal\FileOptions|null
*/ */
public function getOptions() public function getOptions()
{ {
@ -408,7 +408,7 @@ class FileDescriptorProto extends \Google\Protobuf\Internal\Message
* development tools. * development tools.
* *
* Generated from protobuf field <code>optional .google.protobuf.SourceCodeInfo source_code_info = 9;</code> * Generated from protobuf field <code>optional .google.protobuf.SourceCodeInfo source_code_info = 9;</code>
* @return \Google\Protobuf\Internal\SourceCodeInfo * @return \Google\Protobuf\Internal\SourceCodeInfo|null
*/ */
public function getSourceCodeInfo() public function getSourceCodeInfo()
{ {

@ -176,7 +176,7 @@ class MethodDescriptorProto extends \Google\Protobuf\Internal\Message
/** /**
* Generated from protobuf field <code>optional .google.protobuf.MethodOptions options = 4;</code> * Generated from protobuf field <code>optional .google.protobuf.MethodOptions options = 4;</code>
* @return \Google\Protobuf\Internal\MethodOptions * @return \Google\Protobuf\Internal\MethodOptions|null
*/ */
public function getOptions() public function getOptions()
{ {

@ -75,7 +75,7 @@ class OneofDescriptorProto extends \Google\Protobuf\Internal\Message
/** /**
* Generated from protobuf field <code>optional .google.protobuf.OneofOptions options = 2;</code> * Generated from protobuf field <code>optional .google.protobuf.OneofOptions options = 2;</code>
* @return \Google\Protobuf\Internal\OneofOptions * @return \Google\Protobuf\Internal\OneofOptions|null
*/ */
public function getOptions() public function getOptions()
{ {

@ -102,7 +102,7 @@ class ServiceDescriptorProto extends \Google\Protobuf\Internal\Message
/** /**
* Generated from protobuf field <code>optional .google.protobuf.ServiceOptions options = 3;</code> * Generated from protobuf field <code>optional .google.protobuf.ServiceOptions options = 3;</code>
* @return \Google\Protobuf\Internal\ServiceOptions * @return \Google\Protobuf\Internal\ServiceOptions|null
*/ */
public function getOptions() public function getOptions()
{ {

@ -97,7 +97,7 @@ class Option extends \Google\Protobuf\Internal\Message
* value using the google.protobuf.Int32Value type. * value using the google.protobuf.Int32Value type.
* *
* Generated from protobuf field <code>.google.protobuf.Any value = 2;</code> * Generated from protobuf field <code>.google.protobuf.Any value = 2;</code>
* @return \Google\Protobuf\Any * @return \Google\Protobuf\Any|null
*/ */
public function getValue() public function getValue()
{ {

@ -185,7 +185,7 @@ class Type extends \Google\Protobuf\Internal\Message
* The source context. * The source context.
* *
* Generated from protobuf field <code>.google.protobuf.SourceContext source_context = 5;</code> * Generated from protobuf field <code>.google.protobuf.SourceContext source_context = 5;</code>
* @return \Google\Protobuf\SourceContext * @return \Google\Protobuf\SourceContext|null
*/ */
public function getSourceContext() public function getSourceContext()
{ {

@ -174,7 +174,7 @@ class Value extends \Google\Protobuf\Internal\Message
* Represents a structured value. * Represents a structured value.
* *
* Generated from protobuf field <code>.google.protobuf.Struct struct_value = 5;</code> * Generated from protobuf field <code>.google.protobuf.Struct struct_value = 5;</code>
* @return \Google\Protobuf\Struct * @return \Google\Protobuf\Struct|null
*/ */
public function getStructValue() public function getStructValue()
{ {
@ -205,7 +205,7 @@ class Value extends \Google\Protobuf\Internal\Message
* Represents a repeated `Value`. * Represents a repeated `Value`.
* *
* Generated from protobuf field <code>.google.protobuf.ListValue list_value = 6;</code> * Generated from protobuf field <code>.google.protobuf.ListValue list_value = 6;</code>
* @return \Google\Protobuf\ListValue * @return \Google\Protobuf\ListValue|null
*/ */
public function getListValue() public function getListValue()
{ {

@ -95,7 +95,6 @@ std::string GeneratedMetadataFileName(const FileDescriptor* file,
const Options& options); const Options& options);
std::string UnderscoresToCamelCase(const std::string& name, std::string UnderscoresToCamelCase(const std::string& name,
bool cap_first_letter); bool cap_first_letter);
std::string BinaryToHex(const std::string& binary);
void Indent(io::Printer* printer); void Indent(io::Printer* printer);
void Outdent(io::Printer* printer); void Outdent(io::Printer* printer);
void GenerateAddFilesToPool(const FileDescriptor* file, const Options& options, void GenerateAddFilesToPool(const FileDescriptor* file, const Options& options,
@ -600,27 +599,6 @@ std::string UnderscoresToCamelCase(const std::string& name,
return result; return result;
} }
std::string BinaryToHex(const std::string& binary) {
std::string dest;
size_t i;
unsigned char symbol[16] = {
'0', '1', '2', '3',
'4', '5', '6', '7',
'8', '9', 'a', 'b',
'c', 'd', 'e', 'f',
};
dest.resize(binary.size() * 2);
char* append_ptr = &dest[0];
for (i = 0; i < binary.size(); i++) {
*append_ptr++ = symbol[(binary[i] & 0xf0) >> 4];
*append_ptr++ = symbol[binary[i] & 0x0f];
}
return dest;
}
void Indent(io::Printer* printer) { void Indent(io::Printer* printer) {
printer->Indent(); printer->Indent();
printer->Indent(); printer->Indent();
@ -1757,8 +1735,11 @@ void GenerateFieldDocComment(io::Printer* printer, const FieldDescriptor* field,
"php_type", PhpSetterTypeName(field, options)); "php_type", PhpSetterTypeName(field, options));
printer->Print(" * @return $this\n"); printer->Print(" * @return $this\n");
} else if (function_type == kFieldGetter) { } else if (function_type == kFieldGetter) {
printer->Print(" * @return ^php_type^\n", bool can_return_null = field->has_presence() &&
"php_type", PhpGetterTypeName(field, options)); field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE;
printer->Print(" * @return ^php_type^^maybe_null^\n",
"php_type", PhpGetterTypeName(field, options),
"maybe_null", can_return_null ? "|null" : "");
} }
printer->Print(" */\n"); printer->Print(" */\n");
} }
@ -1858,7 +1839,7 @@ void GenerateCEnum(const EnumDescriptor* desc, io::Printer* printer) {
" const upb_enumdef *e = upb_symtab_lookupenum(symtab, \"$name$\");\n" " const upb_enumdef *e = upb_symtab_lookupenum(symtab, \"$name$\");\n"
" const char *name;\n" " const char *name;\n"
" zend_long value;\n" " zend_long value;\n"
" if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, \"l\", &value) ==\n" " if (zend_parse_parameters(ZEND_NUM_ARGS(), \"l\", &value) ==\n"
" FAILURE) {\n" " FAILURE) {\n"
" return;\n" " return;\n"
" }\n" " }\n"
@ -1880,7 +1861,7 @@ void GenerateCEnum(const EnumDescriptor* desc, io::Printer* printer) {
" char *name = NULL;\n" " char *name = NULL;\n"
" size_t name_len;\n" " size_t name_len;\n"
" int32_t num;\n" " int32_t num;\n"
" if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, \"s\", &name,\n" " if (zend_parse_parameters(ZEND_NUM_ARGS(), \"s\", &name,\n"
" &name_len) == FAILURE) {\n" " &name_len) == FAILURE) {\n"
" return;\n" " return;\n"
" }\n" " }\n"
@ -1895,8 +1876,8 @@ void GenerateCEnum(const EnumDescriptor* desc, io::Printer* printer) {
"}\n" "}\n"
"\n" "\n"
"static zend_function_entry $c_name$_phpmethods[] = {\n" "static zend_function_entry $c_name$_phpmethods[] = {\n"
" PHP_ME($c_name$, name, NULL, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC)\n" " PHP_ME($c_name$, name, arginfo_void, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC)\n"
" PHP_ME($c_name$, value, NULL, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC)\n" " PHP_ME($c_name$, value, arginfo_void, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC)\n"
" ZEND_FE_END\n" " ZEND_FE_END\n"
"};\n" "};\n"
"\n" "\n"
@ -1990,16 +1971,37 @@ void GenerateCMessage(const Descriptor* message, io::Printer* printer) {
"camel_name", UnderscoresToCamelCase(oneof->name(), true)); "camel_name", UnderscoresToCamelCase(oneof->name(), true));
} }
switch (message->well_known_type()) {
case Descriptor::WELLKNOWNTYPE_ANY:
printer->Print(
"ZEND_BEGIN_ARG_INFO_EX(arginfo_is, 0, 0, 1)\n"
" ZEND_ARG_INFO(0, proto)\n"
"ZEND_END_ARG_INFO()\n"
"\n"
);
break;
case Descriptor::WELLKNOWNTYPE_TIMESTAMP:
printer->Print(
"ZEND_BEGIN_ARG_INFO_EX(arginfo_timestamp_fromdatetime, 0, 0, 1)\n"
" ZEND_ARG_INFO(0, datetime)\n"
"ZEND_END_ARG_INFO()\n"
"\n"
);
break;
default:
break;
}
printer->Print( printer->Print(
"static zend_function_entry $c_name$_phpmethods[] = {\n" "static zend_function_entry $c_name$_phpmethods[] = {\n"
" PHP_ME($c_name$, __construct, NULL, ZEND_ACC_PUBLIC)\n", " PHP_ME($c_name$, __construct, arginfo_void, ZEND_ACC_PUBLIC)\n",
"c_name", c_name); "c_name", c_name);
for (int i = 0; i < message->field_count(); i++) { for (int i = 0; i < message->field_count(); i++) {
auto field = message->field(i); auto field = message->field(i);
printer->Print( printer->Print(
" PHP_ME($c_name$, get$camel_name$, NULL, ZEND_ACC_PUBLIC)\n" " PHP_ME($c_name$, get$camel_name$, arginfo_void, ZEND_ACC_PUBLIC)\n"
" PHP_ME($c_name$, set$camel_name$, NULL, ZEND_ACC_PUBLIC)\n", " PHP_ME($c_name$, set$camel_name$, arginfo_setter, ZEND_ACC_PUBLIC)\n",
"c_name", c_name, "c_name", c_name,
"camel_name", UnderscoresToCamelCase(field->name(), true)); "camel_name", UnderscoresToCamelCase(field->name(), true));
} }
@ -2007,7 +2009,7 @@ void GenerateCMessage(const Descriptor* message, io::Printer* printer) {
for (int i = 0; i < message->real_oneof_decl_count(); i++) { for (int i = 0; i < message->real_oneof_decl_count(); i++) {
auto oneof = message->oneof_decl(i); auto oneof = message->oneof_decl(i);
printer->Print( printer->Print(
" PHP_ME($c_name$, get$camel_name$, NULL, ZEND_ACC_PUBLIC)\n", " PHP_ME($c_name$, get$camel_name$, arginfo_void, ZEND_ACC_PUBLIC)\n",
"c_name", c_name, "c_name", c_name,
"camel_name", UnderscoresToCamelCase(oneof->name(), true)); "camel_name", UnderscoresToCamelCase(oneof->name(), true));
} }
@ -2016,15 +2018,15 @@ void GenerateCMessage(const Descriptor* message, io::Printer* printer) {
switch (message->well_known_type()) { switch (message->well_known_type()) {
case Descriptor::WELLKNOWNTYPE_ANY: case Descriptor::WELLKNOWNTYPE_ANY:
printer->Print( printer->Print(
" PHP_ME($c_name$, is, NULL, ZEND_ACC_PUBLIC)\n" " PHP_ME($c_name$, is, arginfo_is, ZEND_ACC_PUBLIC)\n"
" PHP_ME($c_name$, pack, NULL, ZEND_ACC_PUBLIC)\n" " PHP_ME($c_name$, pack, arginfo_setter, ZEND_ACC_PUBLIC)\n"
" PHP_ME($c_name$, unpack, NULL, ZEND_ACC_PUBLIC)\n", " PHP_ME($c_name$, unpack, arginfo_void, ZEND_ACC_PUBLIC)\n",
"c_name", c_name); "c_name", c_name);
break; break;
case Descriptor::WELLKNOWNTYPE_TIMESTAMP: case Descriptor::WELLKNOWNTYPE_TIMESTAMP:
printer->Print( printer->Print(
" PHP_ME($c_name$, fromDateTime, NULL, ZEND_ACC_PUBLIC)\n" " PHP_ME($c_name$, fromDateTime, arginfo_timestamp_fromdatetime, ZEND_ACC_PUBLIC)\n"
" PHP_ME($c_name$, toDateTime, NULL, ZEND_ACC_PUBLIC)\n", " PHP_ME($c_name$, toDateTime, arginfo_void, ZEND_ACC_PUBLIC)\n",
"c_name", c_name); "c_name", c_name);
break; break;
default: default:
@ -2154,7 +2156,7 @@ void GenerateCWellKnownTypes(const std::vector<const FileDescriptor*>& files,
"}\n" "}\n"
"\n" "\n"
"static zend_function_entry $metadata_c_name$_methods[] = {\n" "static zend_function_entry $metadata_c_name$_methods[] = {\n"
" PHP_ME($metadata_c_name$, initOnce, NULL, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC)\n" " PHP_ME($metadata_c_name$, initOnce, arginfo_void, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC)\n"
" ZEND_FE_END\n" " ZEND_FE_END\n"
"};\n" "};\n"
"\n" "\n"

Loading…
Cancel
Save