diff --git a/php/tests/GeneratedClassTest.php b/php/tests/GeneratedClassTest.php index 11b11e34c9cb4..7683b75268255 100644 --- a/php/tests/GeneratedClassTest.php +++ b/php/tests/GeneratedClassTest.php @@ -101,6 +101,92 @@ public function testDeprecatedInt32Field() $this->assertSame(4, $deprecationCount); } + public function testDeprecatedFieldGetterDoesNotThrowWarning() + { + // temporarily change error handler to capture the deprecated errors + $deprecationCount = 0; + set_error_handler(function ($errno, $errstr) use (&$deprecationCount) { + if (false !== strpos($errstr, ' is deprecated.')) { + $deprecationCount++; + } + }, E_USER_DEPRECATED); + + // does not throw warning + $message = new TestMessage(); + $message->getDeprecatedInt32(); + $message->getDeprecatedOptionalInt32(); + $message->getDeprecatedInt32ValueUnwrapped(); // wrapped field + $message->getDeprecatedInt32Value(); // wrapped field + $message->getDeprecatedOneofInt32(); // oneof field + $message->getDeprecatedOneof(); // oneof field + $message->getDeprecatedRepeatedInt32(); // repeated field + $message->getDeprecatedMapInt32Int32(); // map field + $message->getDeprecatedAny(); // any field + $message->getDeprecatedMessage(); // message field + $message->getDeprecatedEnum(); // enum field + + restore_error_handler(); + + $this->assertEquals(0, $deprecationCount); + } + + public function testDeprecatedFieldGetterThrowsWarningWithValue() + { + $message = new TestMessage([ + 'deprecated_int32' => 1, + 'deprecated_optional_int32' => 1, + 'deprecated_int32_value' => new \Google\Protobuf\Int32Value(['value' => 1]), + 'deprecated_oneof_int32' => 1, + 'deprecated_repeated_int32' => [1], + 'deprecated_map_int32_int32' => [1 => 1], + 'deprecated_any' => new \Google\Protobuf\Any(['type_url' => 'foo', 'value' => 'bar']), + 'deprecated_message' => new TestMessage(), + 'deprecated_enum' => 1, + ]); + + // temporarily change error handler to capture the deprecated errors + $deprecationCount = 0; + set_error_handler(function ($errno, $errstr) use (&$deprecationCount) { + if (false !== strpos($errstr, ' is deprecated.')) { + $deprecationCount++; + } + }, E_USER_DEPRECATED); + + $message->getDeprecatedInt32(); + $message->getDeprecatedOptionalInt32(); + $message->getDeprecatedInt32ValueUnwrapped(); // wrapped field unwrapped + $message->getDeprecatedInt32Value(); // wrapped field + $message->getDeprecatedOneofInt32(); // oneof field + $message->getDeprecatedRepeatedInt32(); // repeated field + $message->getDeprecatedMapInt32Int32(); // map field + $message->getDeprecatedAny(); // any field + $message->getDeprecatedMessage(); // message field + $message->getDeprecatedEnum(); // enum field + + // oneof field (should never warn) + $message->getDeprecatedOneof(); + + restore_error_handler(); + + $this->assertEquals(10, $deprecationCount); + } + + public function testDeprecatedFieldWarningsOnSerialize() + { + set_error_handler(function ($errno, $errstr) { + if (false !== strpos($errstr, ' is deprecated.')) { + throw new \Exception($errstr); + } + }, E_USER_DEPRECATED); + + $message = new TestMessage(); + $message->serializeToJsonString(); + + restore_error_handler(); + + $this->assertTrue(true, 'No deprecation warning on serialize'); + } + ######################################################### # Test optional int32 field. ######################################################### diff --git a/php/tests/proto/test.proto b/php/tests/proto/test.proto index 4217800507b0a..d385bb7bb39cf 100644 --- a/php/tests/proto/test.proto +++ b/php/tests/proto/test.proto @@ -1,16 +1,16 @@ syntax = "proto3"; +package foo; + import 'google/protobuf/any.proto'; -import 'google/protobuf/wrappers.proto'; import 'google/protobuf/struct.proto'; +import 'google/protobuf/wrappers.proto'; +import 'proto/test_empty_php_namespace.proto'; import 'proto/test_include.proto'; import 'proto/test_no_namespace.proto'; import 'proto/test_php_namespace.proto'; -import 'proto/test_empty_php_namespace.proto'; import 'proto/test_prefix.proto'; -package foo; - message TestMessage { // Singular int32 optional_int32 = 1; @@ -56,63 +56,63 @@ message TestMessage { optional bar.TestInclude true_optional_included_message = 218; // Repeated - repeated int32 repeated_int32 = 31; - repeated int64 repeated_int64 = 32; - repeated uint32 repeated_uint32 = 33; - repeated uint64 repeated_uint64 = 34; - repeated sint32 repeated_sint32 = 35; - repeated sint64 repeated_sint64 = 36; - repeated fixed32 repeated_fixed32 = 37; - repeated fixed64 repeated_fixed64 = 38; + repeated int32 repeated_int32 = 31; + repeated int64 repeated_int64 = 32; + repeated uint32 repeated_uint32 = 33; + repeated uint64 repeated_uint64 = 34; + repeated sint32 repeated_sint32 = 35; + repeated sint64 repeated_sint64 = 36; + repeated fixed32 repeated_fixed32 = 37; + repeated fixed64 repeated_fixed64 = 38; repeated sfixed32 repeated_sfixed32 = 39; repeated sfixed64 repeated_sfixed64 = 40; - repeated float repeated_float = 41; - repeated double repeated_double = 42; - repeated bool repeated_bool = 43; - repeated string repeated_string = 44; - repeated bytes repeated_bytes = 45; + repeated float repeated_float = 41; + repeated double repeated_double = 42; + repeated bool repeated_bool = 43; + repeated string repeated_string = 44; + repeated bytes repeated_bytes = 45; repeated TestEnum repeated_enum = 46; repeated Sub repeated_message = 47; repeated TestMessage repeated_recursive = 48; oneof my_oneof { - int32 oneof_int32 = 51; - int64 oneof_int64 = 52; - uint32 oneof_uint32 = 53; - uint64 oneof_uint64 = 54; - uint32 oneof_sint32 = 55; - uint64 oneof_sint64 = 56; - uint32 oneof_fixed32 = 57; - uint64 oneof_fixed64 = 58; + int32 oneof_int32 = 51; + int64 oneof_int64 = 52; + uint32 oneof_uint32 = 53; + uint64 oneof_uint64 = 54; + uint32 oneof_sint32 = 55; + uint64 oneof_sint64 = 56; + uint32 oneof_fixed32 = 57; + uint64 oneof_fixed64 = 58; uint32 oneof_sfixed32 = 59; uint64 oneof_sfixed64 = 60; - double oneof_double = 61; - float oneof_float = 62; - bool oneof_bool = 63; - string oneof_string = 64; - bytes oneof_bytes = 65; - TestEnum oneof_enum = 66; - Sub oneof_message = 67; + double oneof_double = 61; + float oneof_float = 62; + bool oneof_bool = 63; + string oneof_string = 64; + bytes oneof_bytes = 65; + TestEnum oneof_enum = 66; + Sub oneof_message = 67; } - map map_int32_int32 = 71; - map map_int64_int64 = 72; - map map_uint32_uint32 = 73; - map map_uint64_uint64 = 74; - map map_sint32_sint32 = 75; - map map_sint64_sint64 = 76; - map map_fixed32_fixed32 = 77; - map map_fixed64_fixed64 = 78; + map map_int32_int32 = 71; + map map_int64_int64 = 72; + map map_uint32_uint32 = 73; + map map_uint64_uint64 = 74; + map map_sint32_sint32 = 75; + map map_sint64_sint64 = 76; + map map_fixed32_fixed32 = 77; + map map_fixed64_fixed64 = 78; map map_sfixed32_sfixed32 = 79; map map_sfixed64_sfixed64 = 80; - map map_int32_float = 81; - map map_int32_double = 82; - map map_bool_bool = 83; - map map_string_string = 84; - map map_int32_bytes = 85; - map map_int32_enum = 86; - map map_int32_message = 87; + map map_int32_float = 81; + map map_int32_double = 82; + map map_bool_bool = 83; + map map_string_string = 84; + map map_int32_bytes = 85; + map map_int32_enum = 86; + map map_int32_message = 87; map map_recursive = 88; @@ -149,13 +149,31 @@ message TestMessage { map map_string_struct = 124; // deprecated field - int32 deprecated_optional_int32 = 125 [deprecated=true]; + int32 deprecated_int32 = 125 [deprecated = true]; + // deprecated optional field + optional int32 deprecated_optional_int32 = 126 [deprecated = true]; + // deprecated wrapped field + google.protobuf.Int32Value deprecated_int32_value = 127 [deprecated = true]; + // deprecated oneof + oneof deprecated_oneof { + int32 deprecated_oneof_int32 = 128 [deprecated = true]; + } + // deprecated repeated field + repeated int32 deprecated_repeated_int32 = 129 [deprecated = true]; + // deprecated map + map deprecated_map_int32_int32 = 130 [deprecated = true]; + // deprecated any + google.protobuf.Any deprecated_any = 131 [deprecated = true]; + // deprecated message + TestMessage deprecated_message = 132 [deprecated = true]; + // deprecated enum + NestedEnum deprecated_enum = 133 [deprecated = true]; } enum TestEnum { ZERO = 0; - ONE = 1; - TWO = 2; + ONE = 1; + TWO = 2; ECHO = 3; // Test reserved name. } @@ -173,38 +191,38 @@ message EmptyAnySerialization { } message TestPackedMessage { - repeated int32 repeated_int32 = 90 [packed = true]; - repeated int64 repeated_int64 = 91 [packed = true]; - repeated uint32 repeated_uint32 = 92 [packed = true]; - repeated uint64 repeated_uint64 = 93 [packed = true]; - repeated sint32 repeated_sint32 = 94 [packed = true]; - repeated sint64 repeated_sint64 = 95 [packed = true]; - repeated fixed32 repeated_fixed32 = 96 [packed = true]; - repeated fixed64 repeated_fixed64 = 97 [packed = true]; - repeated sfixed32 repeated_sfixed32 = 98 [packed = true]; - repeated sfixed64 repeated_sfixed64 = 99 [packed = true]; - repeated float repeated_float = 100 [packed = true]; - repeated double repeated_double = 101 [packed = true]; - repeated bool repeated_bool = 102 [packed = true]; - repeated TestEnum repeated_enum = 103 [packed = true]; + repeated int32 repeated_int32 = 90 [packed = true]; + repeated int64 repeated_int64 = 91 [packed = true]; + repeated uint32 repeated_uint32 = 92 [packed = true]; + repeated uint64 repeated_uint64 = 93 [packed = true]; + repeated sint32 repeated_sint32 = 94 [packed = true]; + repeated sint64 repeated_sint64 = 95 [packed = true]; + repeated fixed32 repeated_fixed32 = 96 [packed = true]; + repeated fixed64 repeated_fixed64 = 97 [packed = true]; + repeated sfixed32 repeated_sfixed32 = 98 [packed = true]; + repeated sfixed64 repeated_sfixed64 = 99 [packed = true]; + repeated float repeated_float = 100 [packed = true]; + repeated double repeated_double = 101 [packed = true]; + repeated bool repeated_bool = 102 [packed = true]; + repeated TestEnum repeated_enum = 103 [packed = true]; } // Need to be in sync with TestPackedMessage. message TestUnpackedMessage { - repeated int32 repeated_int32 = 90 [packed = false]; - repeated int64 repeated_int64 = 91 [packed = false]; - repeated uint32 repeated_uint32 = 92 [packed = false]; - repeated uint64 repeated_uint64 = 93 [packed = false]; - repeated sint32 repeated_sint32 = 94 [packed = false]; - repeated sint64 repeated_sint64 = 95 [packed = false]; - repeated fixed32 repeated_fixed32 = 96 [packed = false]; - repeated fixed64 repeated_fixed64 = 97 [packed = false]; - repeated sfixed32 repeated_sfixed32 = 98 [packed = false]; - repeated sfixed64 repeated_sfixed64 = 99 [packed = false]; - repeated float repeated_float = 100 [packed = false]; - repeated double repeated_double = 101 [packed = false]; - repeated bool repeated_bool = 102 [packed = false]; - repeated TestEnum repeated_enum = 103 [packed = false]; + repeated int32 repeated_int32 = 90 [packed = false]; + repeated int64 repeated_int64 = 91 [packed = false]; + repeated uint32 repeated_uint32 = 92 [packed = false]; + repeated uint64 repeated_uint64 = 93 [packed = false]; + repeated sint32 repeated_sint32 = 94 [packed = false]; + repeated sint64 repeated_sint64 = 95 [packed = false]; + repeated fixed32 repeated_fixed32 = 96 [packed = false]; + repeated fixed64 repeated_fixed64 = 97 [packed = false]; + repeated sfixed32 repeated_sfixed32 = 98 [packed = false]; + repeated sfixed64 repeated_sfixed64 = 99 [packed = false]; + repeated float repeated_float = 100 [packed = false]; + repeated double repeated_double = 101 [packed = false]; + repeated bool repeated_bool = 102 [packed = false]; + repeated TestEnum repeated_enum = 103 [packed = false]; } // /**/@<>&\{ @@ -236,8 +254,7 @@ message TestReverseFieldOrder { string b = 1; } -message testLowerCaseMessage { -} +message testLowerCaseMessage {} enum testLowerCaseEnum { VALUE = 0; diff --git a/src/google/protobuf/compiler/php/php_generator.cc b/src/google/protobuf/compiler/php/php_generator.cc index ef8cdcefe5557..d7f5fa65e0b00 100644 --- a/src/google/protobuf/compiler/php/php_generator.cc +++ b/src/google/protobuf/compiler/php/php_generator.cc @@ -219,6 +219,20 @@ std::string DefaultForField(const FieldDescriptor* field) { } } +std::string DeprecatedConditionalForField(const FieldDescriptor* field) { + if (field->is_repeated()) { + return absl::StrCat("$this->", field->name(), "->count() !== 0"); + } + if (field->real_containing_oneof() != nullptr) { + return absl::StrCat("$this->hasOneof(", field->number(), ")"); + } + if (field->has_presence()) { + return absl::StrCat("isset($this->", field->name(), ")"); + } + return absl::StrCat("$this->", field->name(), " !== ", + field->has_presence() ? "null" : DefaultForField(field)); +} + std::string GeneratedMetadataFileName(const FileDescriptor* file, const Options& options) { absl::string_view proto_file = file->name(); @@ -574,6 +588,12 @@ void GenerateFieldAccessor(const FieldDescriptor* field, const Options& options, ? absl::StrCat("@trigger_error('", field->name(), " is deprecated.', E_USER_DEPRECATED);\n ") : ""; + std::string deprecation_trigger_with_conditional = + (field->options().deprecated()) + ? absl::StrCat("if (" + DeprecatedConditionalForField(field), + ") {\n ", deprecation_trigger, + "}\n ") + : ""; // Emit getter. if (oneof != nullptr) { @@ -584,7 +604,7 @@ void GenerateFieldAccessor(const FieldDescriptor* field, const Options& options, "}\n\n", "camel_name", UnderscoresToCamelCase(field->name(), true), "number", IntToString(field->number()), "deprecation_trigger", - deprecation_trigger); + deprecation_trigger_with_conditional); } else if (field->has_presence() && !field->message_type()) { printer->Print( "public function get^camel_name^()\n" @@ -594,7 +614,7 @@ void GenerateFieldAccessor(const FieldDescriptor* field, const Options& options, "}\n\n", "camel_name", UnderscoresToCamelCase(field->name(), true), "name", field->name(), "default_value", DefaultForField(field), - "deprecation_trigger", deprecation_trigger); + "deprecation_trigger", deprecation_trigger_with_conditional); } else { printer->Print( "public function get^camel_name^()\n" @@ -602,7 +622,8 @@ void GenerateFieldAccessor(const FieldDescriptor* field, const Options& options, " ^deprecation_trigger^return $this->^name^;\n" "}\n\n", "camel_name", UnderscoresToCamelCase(field->name(), true), "name", - field->name(), "deprecation_trigger", deprecation_trigger); + field->name(), "deprecation_trigger", + deprecation_trigger_with_conditional); } // Emit hazzers/clear. @@ -614,20 +635,22 @@ void GenerateFieldAccessor(const FieldDescriptor* field, const Options& options, "}\n\n", "camel_name", UnderscoresToCamelCase(field->name(), true), "number", IntToString(field->number()), "deprecation_trigger", - deprecation_trigger); + deprecation_trigger_with_conditional); } else if (field->has_presence()) { printer->Print( "public function has^camel_name^()\n" "{\n" - " ^deprecation_trigger^return isset($this->^name^);\n" - "}\n\n" + " ^deprecation_trigger_with_conditional^return isset($this->^name^);" + "\n}\n\n" "public function clear^camel_name^()\n" "{\n" " ^deprecation_trigger^unset($this->^name^);\n" "}\n\n", "camel_name", UnderscoresToCamelCase(field->name(), true), "name", field->name(), "default_value", DefaultForField(field), - "deprecation_trigger", deprecation_trigger); + "deprecation_trigger", deprecation_trigger, + "deprecation_trigger_with_conditional", + deprecation_trigger_with_conditional); } // For wrapper types, generate an additional getXXXUnwrapped getter @@ -642,7 +665,8 @@ void GenerateFieldAccessor(const FieldDescriptor* field, const Options& options, "$this->readWrapperValue(\"^field_name^\");\n" "}\n\n", "camel_name", UnderscoresToCamelCase(field->name(), true), "field_name", - field->name(), "deprecation_trigger", deprecation_trigger); + field->name(), "deprecation_trigger", + deprecation_trigger_with_conditional); } // Generate setter. @@ -654,7 +678,8 @@ void GenerateFieldAccessor(const FieldDescriptor* field, const Options& options, Indent(printer); - if (field->options().deprecated()) { + if (field->options().deprecated() && !field->is_map() && + !field->is_repeated()) { printer->Print("^deprecation_trigger^", "deprecation_trigger", deprecation_trigger); } @@ -712,6 +737,12 @@ void GenerateFieldAccessor(const FieldDescriptor* field, const Options& options, UnderscoresToCamelCase(field->cpp_type_name(), true)); } + if (field->options().deprecated() && + (field->is_map() || field->is_repeated())) { + printer->Print("if ($arr->count() !== 0) {\n ^deprecation_trigger^}\n", + "deprecation_trigger", deprecation_trigger); + } + if (oneof != nullptr) { printer->Print("$this->writeOneof(^number^, $var);\n", "number", IntToString(field->number()));