From b603fb69bf0c1aef24dfe5470c9d4ae99b9031b1 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Tue, 5 Dec 2023 09:09:09 -0800 Subject: [PATCH] Expand PHP generator to support overlapping subset of proto2. This will allow us to support editions without adding support for proto2 concepts such as closed enums, required fields, and groups. The generator will now only ban unsupported features, meaning that some types of proto2 files will be allowed. The PHP runtime does not yet support editions. PiperOrigin-RevId: 588091790 --- src/google/protobuf/compiler/php/BUILD.bazel | 13 +- .../compiler/php/generator_unittest.cc | 112 ++++++++++++++++++ .../protobuf/compiler/php/php_generator.cc | 90 +++++++++----- .../protobuf/compiler/php/php_generator.h | 7 +- 4 files changed, 186 insertions(+), 36 deletions(-) create mode 100644 src/google/protobuf/compiler/php/generator_unittest.cc diff --git a/src/google/protobuf/compiler/php/BUILD.bazel b/src/google/protobuf/compiler/php/BUILD.bazel index 2a9746d808447..5bddf38779b88 100644 --- a/src/google/protobuf/compiler/php/BUILD.bazel +++ b/src/google/protobuf/compiler/php/BUILD.bazel @@ -32,7 +32,6 @@ cc_library( ], deps = [ ":names", - "//src/google/protobuf:descriptor_legacy", "//src/google/protobuf:protobuf_nowkt", "//src/google/protobuf/compiler:code_generator", "//src/google/protobuf/compiler:retention", @@ -40,6 +39,18 @@ cc_library( ], ) +cc_test( + name = "generator_unittest", + srcs = ["generator_unittest.cc"], + deps = [ + ":php", + "//:protobuf", + "//src/google/protobuf/compiler:command_line_interface_tester", + "@com_google_googletest//:gtest", + "@com_google_googletest//:gtest_main", + ], +) + ################################################################################ # Distribution packaging ################################################################################ diff --git a/src/google/protobuf/compiler/php/generator_unittest.cc b/src/google/protobuf/compiler/php/generator_unittest.cc new file mode 100644 index 0000000000000..e53e1d6d94cd7 --- /dev/null +++ b/src/google/protobuf/compiler/php/generator_unittest.cc @@ -0,0 +1,112 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2023 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +#include + +#include "google/protobuf/descriptor.pb.h" +#include +#include "google/protobuf/compiler/command_line_interface_tester.h" +#include "google/protobuf/compiler/php/php_generator.h" + +namespace google { +namespace protobuf { +namespace compiler { +namespace php { +namespace { + +class PhpGeneratorTest : public CommandLineInterfaceTester { + protected: + PhpGeneratorTest() { + RegisterGenerator("--php_out", "--php_opt", std::make_unique(), + "PHP test generator"); + + // Generate built-in protos. + CreateTempFile( + "google/protobuf/descriptor.proto", + google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); + } +}; + +TEST_F(PhpGeneratorTest, Basic) { + CreateTempFile("foo.proto", + R"schema( + syntax = "proto3"; + message Foo { + optional int32 bar = 1; + int32 baz = 2; + })schema"); + + RunProtoc( + "protocol_compiler --proto_path=$tmpdir --php_out=$tmpdir foo.proto"); + + ExpectNoErrors(); +} + +TEST_F(PhpGeneratorTest, Proto2File) { + CreateTempFile("foo.proto", + R"schema( + syntax = "proto2"; + message Foo { + optional int32 bar = 1; + })schema"); + + RunProtoc( + "protocol_compiler --proto_path=$tmpdir --php_out=$tmpdir foo.proto"); + + ExpectNoErrors(); +} + +TEST_F(PhpGeneratorTest, RequiredFieldError) { + CreateTempFile("foo.proto", + R"schema( + syntax = "proto2"; + message FooBar { + required int32 foo_message = 1; + })schema"); + + RunProtoc( + "protocol_compiler --proto_path=$tmpdir --php_out=$tmpdir foo.proto"); + + ExpectErrorSubstring( + "Can't generate PHP code for required field FooBar.foo_message"); +} + +TEST_F(PhpGeneratorTest, GroupFieldError) { + CreateTempFile("foo.proto", + R"schema( + syntax = "proto2"; + message Foo { + optional group Bar = 1 { + optional int32 baz = 1; + }; + })schema"); + + RunProtoc( + "protocol_compiler --proto_path=$tmpdir --php_out=$tmpdir foo.proto"); + + ExpectErrorSubstring("Can't generate PHP code for group field Foo.bar"); +} + +TEST_F(PhpGeneratorTest, ClosedEnumError) { + CreateTempFile("foo.proto", + R"schema( + syntax = "proto2"; + enum Foo { + BAR = 0; + })schema"); + + RunProtoc( + "protocol_compiler --proto_path=$tmpdir --php_out=$tmpdir foo.proto"); + + ExpectErrorSubstring("Can't generate PHP code for closed enum Foo"); +} + +} // namespace +} // namespace php +} // namespace compiler +} // namespace protobuf +} // namespace google diff --git a/src/google/protobuf/compiler/php/php_generator.cc b/src/google/protobuf/compiler/php/php_generator.cc index 433474c83e184..62498abc6f1dd 100644 --- a/src/google/protobuf/compiler/php/php_generator.cc +++ b/src/google/protobuf/compiler/php/php_generator.cc @@ -12,7 +12,6 @@ #include #include -#include "google/protobuf/compiler/code_generator.h" #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" #include "absl/log/absl_log.h" @@ -22,10 +21,11 @@ #include "absl/strings/str_replace.h" #include "absl/strings/str_split.h" #include "absl/strings/string_view.h" +#include "google/protobuf/compiler/code_generator.h" +#include "google/protobuf/compiler/php/names.h" #include "google/protobuf/compiler/retention.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" -#include "google/protobuf/descriptor_legacy.h" #include "google/protobuf/io/printer.h" #include "google/protobuf/io/zero_copy_stream.h" @@ -537,8 +537,20 @@ void Outdent(io::Printer* printer) { printer->Outdent(); } -void GenerateField(const FieldDescriptor* field, io::Printer* printer, - const Options& options) { +bool GenerateField(const FieldDescriptor* field, io::Printer* printer, + const Options& options, std::string* error) { + if (field->is_required()) { + *error = absl::StrCat("Can't generate PHP code for required field ", + field->full_name(), ".\n"); + return false; + } + if (field->type() == FieldDescriptor::TYPE_GROUP) { + *error = absl::StrCat("Can't generate PHP code for group field ", + field->full_name(), + ". Use regular message encoding instead.\n"); + return false; + } + if (field->is_repeated()) { GenerateFieldDocComment(printer, field, options, kFieldProperty); printer->Print( @@ -546,7 +558,7 @@ void GenerateField(const FieldDescriptor* field, io::Printer* printer, "name", field->name()); } else if (field->real_containing_oneof()) { // Oneof fields are handled by GenerateOneofField. - return; + return true; } else { std::string initial_value = field->has_presence() ? "null" : DefaultForField(field); @@ -556,6 +568,7 @@ void GenerateField(const FieldDescriptor* field, io::Printer* printer, "name", field->name(), "initial_value", initial_value); } + return true; } void GenerateOneofField(const OneofDescriptor* oneof, io::Printer* printer) { @@ -1270,9 +1283,17 @@ void LegacyReadOnlyGenerateClassFile(const FileDescriptor* file, "fullname", classname); } -void GenerateEnumFile(const FileDescriptor* file, const EnumDescriptor* en, +bool GenerateEnumFile(const FileDescriptor* file, const EnumDescriptor* en, const Options& options, - GeneratorContext* generator_context) { + GeneratorContext* generator_context, std::string* error) { + if (en->is_closed()) { + *error = absl::StrCat("Can't generate PHP code for closed enum ", + en->full_name(), + ". Please use either proto3 or editions without " + "`enum_type = CLOSED`.\n"); + return false; + } + std::string filename = GeneratedClassFileName(en, options); std::unique_ptr output( generator_context->Open(filename)); @@ -1403,15 +1424,18 @@ void GenerateEnumFile(const FileDescriptor* file, const EnumDescriptor* en, "old", en->name()); LegacyReadOnlyGenerateClassFile(file, en, options, generator_context); } + + return true; } -void GenerateMessageFile(const FileDescriptor* file, const Descriptor* message, +bool GenerateMessageFile(const FileDescriptor* file, const Descriptor* message, const Options& options, - GeneratorContext* generator_context) { + GeneratorContext* generator_context, + std::string* error) { // Don't generate MapEntry messages -- we use the PHP extension's native // support for map fields instead. if (message->options().map_entry()) { - return; + return true; } std::string filename = GeneratedClassFileName(message, options); @@ -1461,7 +1485,9 @@ void GenerateMessageFile(const FileDescriptor* file, const Descriptor* message, // Field and oneof definitions. for (int i = 0; i < message->field_count(); i++) { const FieldDescriptor* field = message->field(i); - GenerateField(field, &printer, options); + if (!GenerateField(field, &printer, options, error)) { + return false; + } } for (int i = 0; i < message->real_oneof_decl_count(); i++) { const OneofDescriptor* oneof = message->oneof_decl(i); @@ -1533,12 +1559,18 @@ void GenerateMessageFile(const FileDescriptor* file, const Descriptor* message, // Nested messages and enums. for (int i = 0; i < message->nested_type_count(); i++) { - GenerateMessageFile(file, message->nested_type(i), options, - generator_context); + if (!GenerateMessageFile(file, message->nested_type(i), options, + generator_context, error)) { + return false; + } } for (int i = 0; i < message->enum_type_count(); i++) { - GenerateEnumFile(file, message->enum_type(i), options, generator_context); + if (!GenerateEnumFile(file, message->enum_type(i), options, + generator_context, error)) { + return false; + } } + return true; } void GenerateServiceFile( @@ -1588,22 +1620,29 @@ void GenerateServiceFile( printer.Print("}\n\n"); } -void GenerateFile(const FileDescriptor* file, const Options& options, - GeneratorContext* generator_context) { +bool GenerateFile(const FileDescriptor* file, const Options& options, + GeneratorContext* generator_context, std::string* error) { GenerateMetadataFile(file, options, generator_context); for (int i = 0; i < file->message_type_count(); i++) { - GenerateMessageFile(file, file->message_type(i), options, - generator_context); + if (!GenerateMessageFile(file, file->message_type(i), options, + generator_context, error)) { + return false; + } } for (int i = 0; i < file->enum_type_count(); i++) { - GenerateEnumFile(file, file->enum_type(i), options, generator_context); + if (!GenerateEnumFile(file, file->enum_type(i), options, generator_context, + error)) { + return false; + } } if (file->options().php_generic_services()) { for (int i = 0; i < file->service_count(); i++) { GenerateServiceFile(file, file->service(i), options, generator_context); } } + + return true; } static std::string EscapePhpdoc(absl::string_view input) { @@ -2283,18 +2322,7 @@ bool Generator::Generate(const FileDescriptor* file, const Options& options, return false; } - if (!options.is_descriptor && - FileDescriptorLegacy(file).syntax() != - FileDescriptorLegacy::Syntax::SYNTAX_PROTO3) { - *error = - "Can only generate PHP code for proto3 .proto files.\n" - "Please add 'syntax = \"proto3\";' to the top of your .proto file.\n"; - return false; - } - - GenerateFile(file, options, generator_context); - - return true; + return GenerateFile(file, options, generator_context, error); } bool Generator::GenerateAll(const std::vector& files, diff --git a/src/google/protobuf/compiler/php/php_generator.h b/src/google/protobuf/compiler/php/php_generator.h index 8a045249db3a9..96de3c3ee1a84 100644 --- a/src/google/protobuf/compiler/php/php_generator.h +++ b/src/google/protobuf/compiler/php/php_generator.h @@ -8,12 +8,11 @@ #ifndef GOOGLE_PROTOBUF_COMPILER_PHP_GENERATOR_H__ #define GOOGLE_PROTOBUF_COMPILER_PHP_GENERATOR_H__ -#include "google/protobuf/compiler/code_generator.h" -#include "google/protobuf/compiler/php/names.h" -#include "google/protobuf/descriptor.h" - +#include #include +#include "google/protobuf/compiler/code_generator.h" +#include "google/protobuf/descriptor.h" #include "google/protobuf/port_def.inc" namespace google {