From 8984184e5464bbe3cd8046c90f4e1559145aab4b Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Tue, 24 May 2022 21:06:00 +0000 Subject: [PATCH] Revert "fix: reserve "ReadOnly" keyword for PHP 8.1 and add compatibility (#9633)" This reverts commit eb27c201f121b02c990c3665edce5171a8c70192. --- php/ext/google/protobuf/names.c | 12 +-- php/src/Google/Protobuf/Internal/GPBUtil.php | 11 +- php/tests/GeneratedClassTest.php | 18 ---- .../proto/test_reserved_enum_lower.proto | 1 - .../proto/test_reserved_enum_upper.proto | 1 - .../test_reserved_enum_value_lower.proto | 1 - .../test_reserved_enum_value_upper.proto | 1 - .../proto/test_reserved_message_lower.proto | 1 - .../proto/test_reserved_message_upper.proto | 1 - .../protobuf/compiler/php/php_generator.cc | 100 ++++-------------- 10 files changed, 30 insertions(+), 117 deletions(-) diff --git a/php/ext/google/protobuf/names.c b/php/ext/google/protobuf/names.c index a2988816ed0d..5d7b68aaf5dd 100644 --- a/php/ext/google/protobuf/names.c +++ b/php/ext/google/protobuf/names.c @@ -82,12 +82,12 @@ const char *const kReservedNames[] = { "global", "goto", "insteadof", "interface", "isset", "list", "match", "namespace", "new", "object", "or", "parent", "print", "private", "protected", - "public", "readonly", "require", "require_once", "return", - "self", "static", "switch", "throw", "trait", - "try", "unset", "use", "var", "while", - "xor", "yield", "int", "float", "bool", - "string", "true", "false", "null", "void", - "iterable", NULL}; + "public", "require", "require_once", "return", "self", + "static", "switch", "throw", "trait", "try", + "unset", "use", "var", "while", "xor", + "yield", "int", "float", "bool", "string", + "true", "false", "null", "void", "iterable", + NULL}; bool is_reserved_name(const char* name) { int i; diff --git a/php/src/Google/Protobuf/Internal/GPBUtil.php b/php/src/Google/Protobuf/Internal/GPBUtil.php index d7f2faaf6c58..4b152839ec5e 100644 --- a/php/src/Google/Protobuf/Internal/GPBUtil.php +++ b/php/src/Google/Protobuf/Internal/GPBUtil.php @@ -285,12 +285,11 @@ public static function getClassNamePrefix( "include"=>0, "include_once"=>0, "instanceof"=>0, "insteadof"=>0, "interface"=>0, "isset"=>0, "list"=>0, "match"=>0, "namespace"=>0, "new"=>0, "or"=>0, "parent"=>0, "print"=>0, "private"=>0, - "protected"=>0,"public"=>0, "readonly" => 0,"require"=>0, - "require_once"=>0,"return"=>0, "self"=>0, "static"=>0, "switch"=>0, - "throw"=>0,"trait"=>0, "try"=>0,"unset"=>0, "use"=>0, "var"=>0, - "while"=>0,"xor"=>0, "yield"=>0, "int"=>0, "float"=>0, "bool"=>0, - "string"=>0,"true"=>0, "false"=>0, "null"=>0, "void"=>0, - "iterable"=>0 + "protected"=>0,"public"=>0, "require"=>0, "require_once"=>0, + "return"=>0, "self"=>0, "static"=>0, "switch"=>0, "throw"=>0, + "trait"=>0, "try"=>0,"unset"=>0, "use"=>0, "var"=>0, "while"=>0, + "xor"=>0, "yield"=>0, "int"=>0, "float"=>0, "bool"=>0, "string"=>0, + "true"=>0, "false"=>0, "null"=>0, "void"=>0, "iterable"=>0 ); if (array_key_exists(strtolower($classname), $reserved_words)) { diff --git a/php/tests/GeneratedClassTest.php b/php/tests/GeneratedClassTest.php index 37c33dfabec1..8a89973ce804 100644 --- a/php/tests/GeneratedClassTest.php +++ b/php/tests/GeneratedClassTest.php @@ -334,18 +334,6 @@ public function testLegacyTypehintWithNestedEnums() $this->legacyEnum(new TestLegacyMessage\NestedEnum); } - public function testLegacyReadOnlyMessage() - { - $this->assertTrue(class_exists('\Upper\READONLY')); - $this->assertTrue(class_exists('\Lower\readonly')); - } - - public function testLegacyReadOnlyEnum() - { - $this->assertTrue(class_exists('\Upper_enum\READONLY')); - $this->assertTrue(class_exists('\Lower_enum\readonly')); - } - private function legacyEnum(TestLegacyMessage_NestedEnum $enum) { // If we made it here without a PHP Fatal error, the typehint worked @@ -955,7 +943,6 @@ public function testPrefixForReservedWords() $m = new \Lower\PBprivate(); $m = new \Lower\PBprotected(); $m = new \Lower\PBpublic(); - $m = new \Lower\PBreadonly(); $m = new \Lower\PBrequire(); $m = new \Lower\PBrequire_once(); $m = new \Lower\PBreturn(); @@ -1036,7 +1023,6 @@ public function testPrefixForReservedWords() $m = new \Upper\PBPRIVATE(); $m = new \Upper\PBPROTECTED(); $m = new \Upper\PBPUBLIC(); - $m = new \Upper\PBREADONLY(); $m = new \Upper\PBREQUIRE(); $m = new \Upper\PBREQUIRE_ONCE(); $m = new \Upper\PBRETURN(); @@ -1118,7 +1104,6 @@ public function testPrefixForReservedWords() $m = new \Lower_enum\PBprotected(); $m = new \Lower_enum\PBpublic(); $m = new \Lower_enum\PBrequire(); - $m = new \Lower_enum\PBreadonly(); $m = new \Lower_enum\PBrequire_once(); $m = new \Lower_enum\PBreturn(); $m = new \Lower_enum\PBself(); @@ -1198,7 +1183,6 @@ public function testPrefixForReservedWords() $m = new \Upper_enum\PBPRIVATE(); $m = new \Upper_enum\PBPROTECTED(); $m = new \Upper_enum\PBPUBLIC(); - $m = new \Upper_enum\PBREADONLY(); $m = new \Upper_enum\PBREQUIRE(); $m = new \Upper_enum\PBREQUIRE_ONCE(); $m = new \Upper_enum\PBRETURN(); @@ -1303,7 +1287,6 @@ public function testPrefixForReservedWords() $m = \Lower_enum_value\NotAllowed::iterable; $m = \Lower_enum_value\NotAllowed::parent; $m = \Lower_enum_value\NotAllowed::self; - $m = \Lower_enum_value\NotAllowed::readonly; $m = \Upper_enum_value\NotAllowed::PBABSTRACT; $m = \Upper_enum_value\NotAllowed::PBAND; @@ -1384,7 +1367,6 @@ public function testPrefixForReservedWords() $m = \Upper_enum_value\NotAllowed::ITERABLE; $m = \Upper_enum_value\NotAllowed::PARENT; $m = \Upper_enum_value\NotAllowed::SELF; - $m = \Upper_enum_value\NotAllowed::READONLY; $this->assertTrue(true); } diff --git a/php/tests/proto/test_reserved_enum_lower.proto b/php/tests/proto/test_reserved_enum_lower.proto index 1f96ac6fe020..f8557d250fea 100644 --- a/php/tests/proto/test_reserved_enum_lower.proto +++ b/php/tests/proto/test_reserved_enum_lower.proto @@ -57,7 +57,6 @@ enum print { ZERO51 = 0; } enum private { ZERO52 = 0; } enum protected { ZERO53 = 0; } enum public { ZERO54 = 0; } -enum readonly { ZERO80 = 0; } enum require { ZERO55 = 0; } enum require_once { ZERO56 = 0; } enum return { ZERO57 = 0; } diff --git a/php/tests/proto/test_reserved_enum_upper.proto b/php/tests/proto/test_reserved_enum_upper.proto index c5e7e99fd5dd..8d382ab31e5d 100644 --- a/php/tests/proto/test_reserved_enum_upper.proto +++ b/php/tests/proto/test_reserved_enum_upper.proto @@ -57,7 +57,6 @@ enum PRINT { ZERO51 = 0; } enum PRIVATE { ZERO52 = 0; } enum PROTECTED { ZERO53 = 0; } enum PUBLIC { ZERO54 = 0; } -enum READONLY { ZERO80 = 0; } enum REQUIRE { ZERO55 = 0; } enum REQUIRE_ONCE { ZERO56 = 0; } enum RETURN { ZERO57 = 0; } diff --git a/php/tests/proto/test_reserved_enum_value_lower.proto b/php/tests/proto/test_reserved_enum_value_lower.proto index 86c6877f7d88..ca5a7c7352a0 100644 --- a/php/tests/proto/test_reserved_enum_value_lower.proto +++ b/php/tests/proto/test_reserved_enum_value_lower.proto @@ -58,7 +58,6 @@ enum NotAllowed { private = 51; protected = 52; public = 53; - readonly = 79; require = 54; require_once = 55; return = 56; diff --git a/php/tests/proto/test_reserved_enum_value_upper.proto b/php/tests/proto/test_reserved_enum_value_upper.proto index ac0beda7d903..6b4040d5e4ab 100644 --- a/php/tests/proto/test_reserved_enum_value_upper.proto +++ b/php/tests/proto/test_reserved_enum_value_upper.proto @@ -58,7 +58,6 @@ enum NotAllowed { PRIVATE = 51; PROTECTED = 52; PUBLIC = 53; - READONLY = 79; REQUIRE = 54; REQUIRE_ONCE = 55; RETURN = 56; diff --git a/php/tests/proto/test_reserved_message_lower.proto b/php/tests/proto/test_reserved_message_lower.proto index 551ed7a408d4..2390a87dd696 100644 --- a/php/tests/proto/test_reserved_message_lower.proto +++ b/php/tests/proto/test_reserved_message_lower.proto @@ -57,7 +57,6 @@ message print {} message private {} message protected {} message public {} -message readonly {} message require {} message require_once {} message return {} diff --git a/php/tests/proto/test_reserved_message_upper.proto b/php/tests/proto/test_reserved_message_upper.proto index 96995c99178a..9f5533022347 100644 --- a/php/tests/proto/test_reserved_message_upper.proto +++ b/php/tests/proto/test_reserved_message_upper.proto @@ -57,7 +57,6 @@ message PRINT {} message PRIVATE {} message PROTECTED {} message PUBLIC {} -message READONLY {} message REQUIRE {} message REQUIRE_ONCE {} message RETURN {} diff --git a/src/google/protobuf/compiler/php/php_generator.cc b/src/google/protobuf/compiler/php/php_generator.cc index 135a92f6545a..85671f7c9cba 100644 --- a/src/google/protobuf/compiler/php/php_generator.cc +++ b/src/google/protobuf/compiler/php/php_generator.cc @@ -48,29 +48,29 @@ const std::string kDescriptorMetadataFile = const std::string kDescriptorDirName = "Google/Protobuf/Internal"; const std::string kDescriptorPackageName = "Google\\Protobuf\\Internal"; const char* const kReservedNames[] = { - "abstract", "and", "array", "as", "break", - "callable", "case", "catch", "class", "clone", - "const", "continue", "declare", "default", "die", - "do", "echo", "else", "elseif", "empty", - "enddeclare", "endfor", "endforeach", "endif", "endswitch", - "endwhile", "eval", "exit", "extends", "final", - "finally", "fn", "for", "foreach", "function", - "global", "goto", "if", "implements", "include", - "include_once", "instanceof", "insteadof", "interface", "isset", - "list", "match", "namespace", "new", "or", - "parent", "print", "private", "protected", "public", - "readonly", "require", "require_once", "return", "self", - "static", "switch", "throw", "trait", "try", - "unset", "use", "var", "while", "xor", - "yield", "int", "float", "bool", "string", - "true", "false", "null", "void", "iterable"}; + "abstract", "and", "array", "as", "break", + "callable", "case", "catch", "class", "clone", + "const", "continue", "declare", "default", "die", + "do", "echo", "else", "elseif", "empty", + "enddeclare", "endfor", "endforeach", "endif", "endswitch", + "endwhile", "eval", "exit", "extends", "final", + "finally", "fn", "for", "foreach", "function", + "global", "goto", "if", "implements", "include", + "include_once", "instanceof", "insteadof", "interface", "isset", + "list", "match", "namespace", "new", "or", + "parent", "print", "private", "protected", "public", + "require", "require_once", "return", "self", "static", + "switch", "throw", "trait", "try", "unset", + "use", "var", "while", "xor", "yield", + "int", "float", "bool", "string", "true", + "false", "null", "void", "iterable"}; const char* const kValidConstantNames[] = { "int", "float", "bool", "string", "true", "false", "null", "void", "iterable", "parent", - "self", "readonly" + "self" }; -const int kReservedNamesSize = 80; -const int kValidConstantNamesSize = 12; +const int kReservedNamesSize = 79; +const int kValidConstantNamesSize = 11; const int kFieldSetter = 1; const int kFieldGetter = 2; const int kFieldProperty = 3; @@ -420,16 +420,6 @@ std::string LegacyGeneratedClassFileName(const DescriptorType* desc, return result + ".php"; } -template -std::string LegacyReadOnlyGeneratedClassFileName(const DescriptorType* desc, - const Options& options) { - std::string php_namespace = RootPhpNamespace(desc, options); - if (!php_namespace.empty()) { - return php_namespace + "/" + desc->name() + ".php"; - } - return desc->name() + ".php"; -} - std::string GeneratedServiceFileName(const ServiceDescriptor* service, const Options& options) { std::string result = FullClassName(service, options) + "Interface"; @@ -1312,32 +1302,6 @@ void LegacyGenerateClassFile(const FileDescriptor* file, "fullname", newname); } -template -void LegacyReadOnlyGenerateClassFile(const FileDescriptor* file, - const DescriptorType* desc, const Options& options, - GeneratorContext* generator_context) { - std::string filename = LegacyReadOnlyGeneratedClassFileName(desc, options); - std::unique_ptr output( - generator_context->Open(filename)); - io::Printer printer(output.get(), '^'); - - GenerateHead(file, &printer); - - std::string php_namespace = RootPhpNamespace(desc, options); - if (!php_namespace.empty()) { - printer.Print( - "namespace ^name^;\n\n", - "name", php_namespace); - } - std::string newname = FullClassName(desc, options); - printer.Print("class_exists(^new^::class);\n", - "new", GeneratedClassNameImpl(desc)); - printer.Print("@trigger_error(__NAMESPACE__ . '\\^old^ is deprecated and will be removed in " - "the next major release. Use ^fullname^ instead', E_USER_DEPRECATED);\n\n", - "old", desc->name(), - "fullname", newname); -} - void GenerateEnumFile(const FileDescriptor* file, const EnumDescriptor* en, const Options& options, GeneratorContext* generator_context) { @@ -1459,19 +1423,6 @@ void GenerateEnumFile(const FileDescriptor* file, const EnumDescriptor* en, "old", LegacyFullClassName(en, options)); LegacyGenerateClassFile(file, en, options, generator_context); } - - // Write legacy file for backwards compatibility with "readonly" keywword - std::string lower = en->name(); - std::transform(lower.begin(), lower.end(), lower.begin(), ::tolower); - if (lower == "readonly") { - printer.Print( - "// Adding a class alias for backwards compatibility with the \"readonly\" keyword.\n"); - printer.Print( - "class_alias(^new^::class, __NAMESPACE__ . '\\^old^');\n\n", - "new", fullname, - "old", en->name()); - LegacyReadOnlyGenerateClassFile(file, en, options, generator_context); - } } void GenerateMessageFile(const FileDescriptor* file, const Descriptor* message, @@ -1588,19 +1539,6 @@ void GenerateMessageFile(const FileDescriptor* file, const Descriptor* message, LegacyGenerateClassFile(file, message, options, generator_context); } - // Write legacy file for backwards compatibility with "readonly" keywword - std::string lower = message->name(); - std::transform(lower.begin(), lower.end(), lower.begin(), ::tolower); - if (lower == "readonly") { - printer.Print( - "// Adding a class alias for backwards compatibility with the \"readonly\" keyword.\n"); - printer.Print( - "class_alias(^new^::class, __NAMESPACE__ . '\\^old^');\n\n", - "new", fullname, - "old", message->name()); - LegacyReadOnlyGenerateClassFile(file, message, options, generator_context); - } - // Nested messages and enums. for (int i = 0; i < message->nested_type_count(); i++) { GenerateMessageFile(file, message->nested_type(i), options,