Skip to content

Commit

Permalink
fix: do not throw deprecated warning on field getters for default val…
Browse files Browse the repository at this point in the history 
…ues (#17788)

fixes #13428

Wraps deprecated field getters in a conditional to see if the value has been set before throwing the warning. This is because this method is used internally, so users can get deprecated warnings even when they are not using the field.

Closes #17788

COPYBARA_INTEGRATE_REVIEW=#17788 from bshaffer:wrap-deprecation-in-conditional 084c87d
PiperOrigin-RevId: 665346195
  • Loading branch information
@bshaffer @copybara-github
bshaffer authored and copybara-github committed Aug 20, 2024
1 parent b0db5bd commit 6d84da5
Show file tree
Hide file tree
Showing 3 changed files with 223 additions and 89 deletions.
86 changes: 86 additions & 0 deletions php/tests/GeneratedClassTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#########################################################
Expand Down
177 changes: 97 additions & 80 deletions php/tests/proto/test.proto
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<int32, int32> map_int32_int32 = 71;
map<int64, int64> map_int64_int64 = 72;
map<uint32, uint32> map_uint32_uint32 = 73;
map<uint64, uint64> map_uint64_uint64 = 74;
map<sint32, sint32> map_sint32_sint32 = 75;
map<sint64, sint64> map_sint64_sint64 = 76;
map<fixed32, fixed32> map_fixed32_fixed32 = 77;
map<fixed64, fixed64> map_fixed64_fixed64 = 78;
map<int32, int32> map_int32_int32 = 71;
map<int64, int64> map_int64_int64 = 72;
map<uint32, uint32> map_uint32_uint32 = 73;
map<uint64, uint64> map_uint64_uint64 = 74;
map<sint32, sint32> map_sint32_sint32 = 75;
map<sint64, sint64> map_sint64_sint64 = 76;
map<fixed32, fixed32> map_fixed32_fixed32 = 77;
map<fixed64, fixed64> map_fixed64_fixed64 = 78;
map<sfixed32, sfixed32> map_sfixed32_sfixed32 = 79;
map<sfixed64, sfixed64> map_sfixed64_sfixed64 = 80;
map<int32, float> map_int32_float = 81;
map<int32, double> map_int32_double = 82;
map<bool, bool> map_bool_bool = 83;
map<string, string> map_string_string = 84;
map<int32, bytes> map_int32_bytes = 85;
map<int32, TestEnum> map_int32_enum = 86;
map<int32, Sub> map_int32_message = 87;
map<int32, float> map_int32_float = 81;
map<int32, double> map_int32_double = 82;
map<bool, bool> map_bool_bool = 83;
map<string, string> map_string_string = 84;
map<int32, bytes> map_int32_bytes = 85;
map<int32, TestEnum> map_int32_enum = 86;
map<int32, Sub> map_int32_message = 87;

map<int32, TestMessage> map_recursive = 88;

Expand Down Expand Up @@ -149,13 +149,31 @@ message TestMessage {
map<string, google.protobuf.Struct> 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<int32, int32> 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.
}

Expand All @@ -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];
}

// /**/@<>&\{
Expand Down Expand Up @@ -236,8 +254,7 @@ message TestReverseFieldOrder {
string b = 1;
}

message testLowerCaseMessage {
}
message testLowerCaseMessage {}

enum testLowerCaseEnum {
VALUE = 0;
Expand Down
Loading

0 comments on commit 6d84da5

Please sign in to comment.