diff --git a/cmake/tests.cmake b/cmake/tests.cmake index e2559e910e..ee74094d50 100644 --- a/cmake/tests.cmake +++ b/cmake/tests.cmake @@ -107,11 +107,13 @@ set(fake_plugin_files ${fake_plugin_files} ${common_test_hdrs} ${common_test_srcs} + ${tests_proto_files} ) set(test_plugin_files ${test_plugin_files} ${common_test_hdrs} ${common_test_srcs} + ${tests_proto_files} ) add_executable(fake_plugin ${fake_plugin_files}) diff --git a/src/google/protobuf/compiler/BUILD.bazel b/src/google/protobuf/compiler/BUILD.bazel index 9b4c243d18..b5ee0c1a66 100644 --- a/src/google/protobuf/compiler/BUILD.bazel +++ b/src/google/protobuf/compiler/BUILD.bazel @@ -221,6 +221,9 @@ cc_library( visibility = ["//pkg:__pkg__"], deps = [ ":code_generator", + "//src/google/protobuf:cc_test_protos", + "//src/google/protobuf:descriptor_legacy", + "//src/google/protobuf:descriptor_visitor", "//src/google/protobuf/io", "//src/google/protobuf/stubs", "//src/google/protobuf/testing", diff --git a/src/google/protobuf/compiler/code_generator.cc b/src/google/protobuf/compiler/code_generator.cc index 2389143a03..f56a65097e 100644 --- a/src/google/protobuf/compiler/code_generator.cc +++ b/src/google/protobuf/compiler/code_generator.cc @@ -37,12 +37,14 @@ #include #include "absl/log/absl_log.h" +#include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_split.h" #include "absl/strings/string_view.h" #include "absl/strings/strip.h" #include "google/protobuf/compiler/plugin.pb.h" #include "google/protobuf/descriptor.h" +#include "google/protobuf/feature_resolver.h" namespace google { namespace protobuf { @@ -76,6 +78,13 @@ bool CodeGenerator::GenerateAll(const std::vector& files, return succeeded; } +absl::StatusOr CodeGenerator::BuildFeatureSetDefaults() + const { + return FeatureResolver::CompileDefaults( + FeatureSet::descriptor(), GetFeatureExtensions(), GetMinimumEdition(), + GetMaximumEdition()); +} + GeneratorContext::~GeneratorContext() {} io::ZeroCopyOutputStream* GeneratorContext::OpenForAppend( diff --git a/src/google/protobuf/compiler/code_generator.h b/src/google/protobuf/compiler/code_generator.h index a343672744..2b8c2ff075 100644 --- a/src/google/protobuf/compiler/code_generator.h +++ b/src/google/protobuf/compiler/code_generator.h @@ -43,6 +43,7 @@ #include #include +#include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "google/protobuf/compiler/retention.h" #include "google/protobuf/descriptor.h" @@ -131,6 +132,34 @@ class PROTOC_EXPORT CodeGenerator { // method can be removed. virtual bool HasGenerateAll() const { return true; } + // Returns all the feature extensions used by this generator. This must be in + // the generated pool, meaning that the extensions should be linked into this + // binary. Any generator features not included here will not get properly + // resolved and GetResolvedSourceFeatures will not provide useful values. + virtual std::vector GetFeatureExtensions() const { + return {}; + } + + // Returns the minimum edition (inclusive) supported by this generator. Any + // proto files with an edition before this will result in an error. + virtual absl::string_view GetMinimumEdition() const { + return PROTOBUF_MINIMUM_EDITION; + } + + // Returns the maximum edition (inclusive) supported by this generator. Any + // proto files with an edition after this will result in an error. + virtual absl::string_view GetMaximumEdition() const { + return PROTOBUF_MAXIMUM_EDITION; + } + + // Builds a default feature set mapping for this generator. + // + // This will use the extensions specified by GetFeatureExtensions(), with the + // supported edition range [GetMinimumEdition(), GetMaximumEdition]. It has + // no side-effects, and code generators only need to call this if they want to + // embed the defaults into the generated code. + absl::StatusOr BuildFeatureSetDefaults() const; + protected: // Retrieves the resolved source features for a given descriptor. All the // features that are imported (from the proto file) and linked in (from the diff --git a/src/google/protobuf/compiler/code_generator_unittest.cc b/src/google/protobuf/compiler/code_generator_unittest.cc index eea9d48407..9091baee3c 100644 --- a/src/google/protobuf/compiler/code_generator_unittest.cc +++ b/src/google/protobuf/compiler/code_generator_unittest.cc @@ -31,11 +31,13 @@ #include "google/protobuf/compiler/code_generator.h" #include +#include #include "google/protobuf/descriptor.pb.h" #include #include #include "absl/log/absl_log.h" +#include "absl/status/status.h" #include "absl/strings/str_format.h" #include "absl/strings/str_replace.h" #include "absl/strings/string_view.h" @@ -45,11 +47,15 @@ #include "google/protobuf/test_textproto.h" #include "google/protobuf/unittest_features.pb.h" +// Must be included last. +#include "google/protobuf/port_def.inc" + namespace google { namespace protobuf { namespace compiler { namespace { +using ::testing::HasSubstr; using ::testing::NotNull; class TestGenerator : public CodeGenerator { @@ -60,9 +66,37 @@ class TestGenerator : public CodeGenerator { return true; } + std::vector GetFeatureExtensions() const override { + return feature_extensions_; + } + void set_feature_extensions(std::vector extensions) { + feature_extensions_ = extensions; + } + + absl::string_view GetMinimumEdition() const override { + return minimum_edition_; + } + void set_minimum_edition(absl::string_view minimum_edition) { + minimum_edition_ = minimum_edition; + } + + absl::string_view GetMaximumEdition() const override { + return maximum_edition_; + } + void set_maximum_edition(absl::string_view maximum_edition) { + maximum_edition_ = maximum_edition; + } + // Expose the protected methods for testing. using CodeGenerator::GetResolvedSourceFeatures; using CodeGenerator::GetUnresolvedSourceFeatures; + + private: + absl::string_view minimum_edition_ = PROTOBUF_MINIMUM_EDITION; + absl::string_view maximum_edition_ = PROTOBUF_MAXIMUM_EDITION; + std::vector feature_extensions_ = { + DescriptorPool::generated_pool()->FindExtensionByNumber( + FeatureSet::descriptor(), pb::test.number())}; }; class SimpleErrorCollector : public io::ErrorCollector { @@ -74,11 +108,6 @@ class SimpleErrorCollector : public io::ErrorCollector { class CodeGeneratorTest : public ::testing::Test { protected: - void SetUp() override { - ASSERT_THAT(BuildFile(DescriptorProto::descriptor()->file()), NotNull()); - ASSERT_THAT(BuildFile(pb::TestMessage::descriptor()->file()), NotNull()); - } - const FileDescriptor* BuildFile(absl::string_view schema) { io::ArrayInputStream input_stream(schema.data(), static_cast(schema.size())); @@ -102,6 +131,8 @@ class CodeGeneratorTest : public ::testing::Test { }; TEST_F(CodeGeneratorTest, GetUnresolvedSourceFeaturesRoot) { + ASSERT_THAT(BuildFile(DescriptorProto::descriptor()->file()), NotNull()); + ASSERT_THAT(BuildFile(pb::TestMessage::descriptor()->file()), NotNull()); auto file = BuildFile(R"schema( edition = "2023"; package protobuf_unittest; @@ -123,6 +154,8 @@ TEST_F(CodeGeneratorTest, GetUnresolvedSourceFeaturesRoot) { } TEST_F(CodeGeneratorTest, GetUnresolvedSourceFeaturesInherited) { + ASSERT_THAT(BuildFile(DescriptorProto::descriptor()->file()), NotNull()); + ASSERT_THAT(BuildFile(pb::TestMessage::descriptor()->file()), NotNull()); auto file = BuildFile(R"schema( edition = "2023"; package protobuf_unittest; @@ -156,6 +189,14 @@ TEST_F(CodeGeneratorTest, GetUnresolvedSourceFeaturesInherited) { } TEST_F(CodeGeneratorTest, GetResolvedSourceFeaturesRoot) { + TestGenerator generator; + generator.set_feature_extensions( + {DescriptorPool::generated_pool()->FindExtensionByNumber( + FeatureSet::descriptor(), pb::test.number())}); + pool_.SetFeatureSetDefaults(*generator.BuildFeatureSetDefaults()); + + ASSERT_THAT(BuildFile(DescriptorProto::descriptor()->file()), NotNull()); + ASSERT_THAT(BuildFile(pb::TestMessage::descriptor()->file()), NotNull()); auto file = BuildFile(R"schema( edition = "2023"; package protobuf_unittest; @@ -177,13 +218,20 @@ TEST_F(CodeGeneratorTest, GetResolvedSourceFeaturesRoot) { EXPECT_EQ(features.field_presence(), FeatureSet::EXPLICIT); EXPECT_EQ(features.enum_type(), FeatureSet::CLOSED); - // TODO(b/296638633) Flip this once generators can specify their feature sets. - EXPECT_FALSE(ext.has_int_message_feature()); + EXPECT_TRUE(ext.has_int_message_feature()); EXPECT_EQ(ext.int_file_feature(), 8); EXPECT_EQ(ext.string_source_feature(), "file"); } TEST_F(CodeGeneratorTest, GetResolvedSourceFeaturesInherited) { + TestGenerator generator; + generator.set_feature_extensions( + {DescriptorPool::generated_pool()->FindExtensionByNumber( + FeatureSet::descriptor(), pb::test.number())}); + pool_.SetFeatureSetDefaults(*generator.BuildFeatureSetDefaults()); + + ASSERT_THAT(BuildFile(DescriptorProto::descriptor()->file()), NotNull()); + ASSERT_THAT(BuildFile(pb::TestMessage::descriptor()->file()), NotNull()); auto file = BuildFile(R"schema( edition = "2023"; package protobuf_unittest; @@ -223,6 +271,47 @@ TEST_F(CodeGeneratorTest, GetResolvedSourceFeaturesInherited) { EXPECT_EQ(ext.string_source_feature(), "field"); } +// TODO(b/234474291): Use the gtest versions once that's available in OSS. +MATCHER_P(HasError, msg_matcher, "") { + return arg.status().code() == absl::StatusCode::kFailedPrecondition && + ExplainMatchResult(msg_matcher, arg.status().message(), + result_listener); +} +MATCHER_P(IsOkAndHolds, matcher, "") { + return arg.ok() && ExplainMatchResult(matcher, *arg, result_listener); +} + +TEST_F(CodeGeneratorTest, BuildFeatureSetDefaultsInvalidExtension) { + TestGenerator generator; + generator.set_feature_extensions({nullptr}); + EXPECT_THAT(generator.BuildFeatureSetDefaults(), + HasError(HasSubstr("Unknown extension"))); +} + +TEST_F(CodeGeneratorTest, BuildFeatureSetDefaults) { + TestGenerator generator; + generator.set_feature_extensions({}); + generator.set_minimum_edition("2020"); + generator.set_maximum_edition("2024"); + EXPECT_THAT(generator.BuildFeatureSetDefaults(), + IsOkAndHolds(EqualsProto(R"pb( + defaults { + edition: "2023" + features { + field_presence: EXPLICIT + enum_type: OPEN + repeated_field_encoding: PACKED + message_encoding: LENGTH_PREFIXED + json_format: ALLOW + } + } + minimum_edition: "2020" + maximum_edition: "2024" + )pb"))); +} + +#include "google/protobuf/port_undef.inc" + } // namespace } // namespace compiler } // namespace protobuf diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index 39c7cdc3c4..8ff924fe22 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -34,13 +34,17 @@ #include "google/protobuf/compiler/command_line_interface.h" +#include + #include "absl/algorithm/container.h" #include "absl/container/btree_set.h" #include "absl/container/flat_hash_map.h" +#include "absl/status/statusor.h" #include "absl/types/span.h" #include "google/protobuf/compiler/allowlists/allowlists.h" #include "google/protobuf/descriptor_legacy.h" #include "google/protobuf/descriptor_visitor.h" +#include "google/protobuf/feature_resolver.h" #include "google/protobuf/stubs/platform_macros.h" @@ -1270,8 +1274,13 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) { descriptor_pool->EnforceWeakDependencies(true); - // Enforce extension declarations only when compiling. We want to skip this - // enforcement when protoc is just being invoked to encode or decode protos. + if (!SetupFeatureResolution(*descriptor_pool)) { + return EXIT_FAILURE; + } + + // Enforce extension declarations only when compiling. We want to skip + // this enforcement when protoc is just being invoked to encode or decode + // protos. if (mode_ == MODE_COMPILE) { descriptor_pool->EnforceExtensionDeclarations(true); } @@ -1525,6 +1534,56 @@ bool CommandLineInterface::VerifyInputFilesInDescriptors( return true; } +bool CommandLineInterface::SetupFeatureResolution(DescriptorPool& pool) { + // Calculate the feature defaults for each built-in generator. All generators + // that support editions must agree on the supported edition range. + std::vector feature_extensions; + absl::string_view minimum_edition = PROTOBUF_MINIMUM_EDITION; + absl::string_view maximum_edition = PROTOBUF_MAXIMUM_EDITION; + for (const auto& output : output_directives_) { + if (output.generator == nullptr) continue; + if ((output.generator->GetSupportedFeatures() & + CodeGenerator::FEATURE_SUPPORTS_EDITIONS) == 0) { + continue; + } + if (output.generator->GetMinimumEdition() != PROTOBUF_MINIMUM_EDITION) { + ABSL_LOG(ERROR) << "Built-in generator " << output.name + << " specifies a minimum edition " + << output.generator->GetMinimumEdition() + << " which is not the protoc minimum " + << PROTOBUF_MINIMUM_EDITION << "."; + return false; + } + if (output.generator->GetMaximumEdition() != PROTOBUF_MAXIMUM_EDITION) { + ABSL_LOG(ERROR) << "Built-in generator " << output.name + << " specifies a maximum edition " + << output.generator->GetMaximumEdition() + << " which is not the protoc maximum " + << PROTOBUF_MINIMUM_EDITION << "."; + return false; + } + for (const FieldDescriptor* ext : + output.generator->GetFeatureExtensions()) { + if (ext == nullptr) { + ABSL_LOG(ERROR) << "Built-in generator " << output.name + << " specifies an unknown feature extension."; + return false; + } + feature_extensions.push_back(ext); + } + } + absl::StatusOr defaults = + FeatureResolver::CompileDefaults(FeatureSet::descriptor(), + feature_extensions, minimum_edition, + maximum_edition); + if (!defaults.ok()) { + ABSL_LOG(ERROR) << defaults.status(); + return false; + } + pool.SetFeatureSetDefaults(std::move(defaults).value()); + return true; +} + bool CommandLineInterface::ParseInputFiles( DescriptorPool* descriptor_pool, DiskSourceTree* source_tree, std::vector* parsed_files) { diff --git a/src/google/protobuf/compiler/command_line_interface.h b/src/google/protobuf/compiler/command_line_interface.h index dbfb22ef0f..65c3f58d9b 100644 --- a/src/google/protobuf/compiler/command_line_interface.h +++ b/src/google/protobuf/compiler/command_line_interface.h @@ -292,6 +292,8 @@ class PROTOC_EXPORT CommandLineInterface { DiskSourceTree* source_tree, std::vector* parsed_files); + bool SetupFeatureResolution(DescriptorPool& pool); + // Generate the given output file from the given input. struct OutputDirective; // see below bool GenerateOutput(const std::vector& parsed_files, diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index c13391636e..fe569f4c23 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -215,6 +215,16 @@ class CommandLineInterfaceTest : public CommandLineInterfaceTester { RegisterGenerator(name, std::move(generator), description); } + void SetMockGeneratorTestCase(absl::string_view name) { +#ifdef _WIN32 + ::_putenv(absl::StrCat("TEST_CASE", "=", name).c_str()); +#else + ::setenv("TEST_CASE", name.data(), 1); +#endif + } + + MockCodeGenerator* mock_generator_ = nullptr; + private: // Was DisallowPlugins() called? bool disallow_plugins_ = false; @@ -242,9 +252,13 @@ class CommandLineInterfaceTest::NullCodeGenerator : public CodeGenerator { // =================================================================== CommandLineInterfaceTest::CommandLineInterfaceTest() { + // Reset the mock generator's test case environment variable. + SetMockGeneratorTestCase(""); + // Register generators. - RegisterGenerator("--test_out", "--test_opt", - std::make_unique("test_generator"), + auto mock_generator = std::make_unique("test_generator"); + mock_generator_ = mock_generator.get(); + RegisterGenerator("--test_out", "--test_opt", std::move(mock_generator), "Test output."); RegisterGenerator("-t", std::make_unique("test_generator"), "Test output."); @@ -1457,36 +1471,106 @@ TEST_F(CommandLineInterfaceTest, FeatureExtensionError) { CreateTempFile("google/protobuf/descriptor.proto", google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); CreateTempFile("features.proto", - R"schema( - syntax = "proto2"; - package pb; - import "google/protobuf/descriptor.proto"; - extend google.protobuf.FeatureSet { - optional TestFeatures test = 9999; - } - message TestFeatures { - repeated int32 repeated_feature = 1 [ - retention = RETENTION_RUNTIME, - targets = TARGET_TYPE_FIELD, - edition_defaults = { edition: "2023", value: "3" } - ]; - })schema"); + pb::TestInvalidFeatures::descriptor()->file()->DebugString()); CreateTempFile("foo.proto", R"schema( edition = "2023"; import "features.proto"; message Foo { int32 bar = 1; - int32 baz = 2 [features.(pb.test).repeated_feature = 5]; + int32 baz = 2 [features.(pb.test_invalid).repeated_feature = 5]; })schema"); + mock_generator_->set_feature_extensions( + {DescriptorPool::generated_pool()->FindExtensionByNumber( + FeatureSet::descriptor(), pb::test_invalid.number())}); + + Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir " + "--experimental_editions foo.proto"); + ExpectErrorSubstring( + "Feature field pb.TestInvalidFeatures.repeated_feature is an unsupported " + "repeated field"); +} + +TEST_F(CommandLineInterfaceTest, InvalidMinimumEditionError) { + CreateTempFile("foo.proto", R"schema(edition = "2023";)schema"); + + mock_generator_->set_minimum_edition("2022"); + Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir " "--experimental_editions foo.proto"); - // TODO(b/296638633) Flip this once generators can specify their feature sets. + ExpectErrorSubstring( + "generator --test_out specifies a minimum edition 2022 which is not the " + "protoc minimum 2023"); +} + +TEST_F(CommandLineInterfaceTest, InvalidMaximumEditionError) { + CreateTempFile("foo.proto", R"schema(edition = "2023";)schema"); + + mock_generator_->set_maximum_edition("2123"); + + Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir " + "--experimental_editions foo.proto"); + ExpectErrorSubstring( + "generator --test_out specifies a maximum edition 2123 which is not " + "the protoc maximum 2023"); +} + +TEST_F(CommandLineInterfaceTest, InvalidFeatureExtensionError) { + CreateTempFile("foo.proto", R"schema(edition = "2023";)schema"); + + mock_generator_->set_feature_extensions({nullptr}); + + Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir " + "--experimental_editions foo.proto"); + ExpectErrorSubstring( + "generator --test_out specifies an unknown feature extension"); +} + +TEST_F(CommandLineInterfaceTest, Plugin_InvalidFeatureExtensionError) { + CreateTempFile("foo.proto", R"schema( + edition = "2023"; + message Foo { + int32 i = 1; + } + )schema"); + + SetMockGeneratorTestCase("invalid_features"); + Run("protocol_compiler --experimental_editions " + "--proto_path=$tmpdir foo.proto --plug_out=$tmpdir"); + + ExpectErrorSubstring( + "error generating feature defaults: Unknown extension of " + "google.protobuf.FeatureSet"); +} + +TEST_F(CommandLineInterfaceTest, Plugin_MissingFeatureExtensionError) { + CreateTempFile("foo.proto", R"schema( + edition = "2023"; + message Foo { + int32 i = 1; + } + )schema"); + + SetMockGeneratorTestCase("no_feature_defaults"); + Run("protocol_compiler --experimental_editions " + "--proto_path=$tmpdir foo.proto --plug_out=$tmpdir"); + + ExpectErrorSubstring("Test features were not resolved properly"); +} + +TEST_F(CommandLineInterfaceTest, Plugin_TestFeatures) { + CreateTempFile("foo.proto", R"schema( + edition = "2023"; + message Foo { + int32 i = 1; + } + )schema"); + + Run("protocol_compiler --experimental_editions " + "--proto_path=$tmpdir foo.proto --plug_out=$tmpdir"); + ExpectNoErrors(); - // ExpectErrorSubstring( - // "Feature field pb.TestFeatures.repeated_feature is an unsupported " - // "repeated field"); } TEST_F(CommandLineInterfaceTest, Plugin_LegacyFeatures) { @@ -1657,8 +1741,9 @@ TEST_F(CommandLineInterfaceTest, PluginNoEditionsSupport) { } )schema"); + SetMockGeneratorTestCase("no_editions"); Run("protocol_compiler --experimental_editions " - "--proto_path=$tmpdir foo.proto --plug_out=no_editions:$tmpdir"); + "--proto_path=$tmpdir foo.proto --plug_out=$tmpdir"); ExpectErrorSubstring( "code generator prefix-gen-plug hasn't been updated to support editions"); diff --git a/src/google/protobuf/compiler/cpp/generator.h b/src/google/protobuf/compiler/cpp/generator.h index 94bd352346..83d40bacdd 100644 --- a/src/google/protobuf/compiler/cpp/generator.h +++ b/src/google/protobuf/compiler/cpp/generator.h @@ -37,11 +37,14 @@ #ifndef GOOGLE_PROTOBUF_COMPILER_CPP_GENERATOR_H__ #define GOOGLE_PROTOBUF_COMPILER_CPP_GENERATOR_H__ +#include #include #include +#include #include "google/protobuf/compiler/code_generator.h" #include "absl/status/status.h" +#include "google/protobuf/cpp_features.pb.h" // Must be included last. #include "google/protobuf/port_def.inc" @@ -92,6 +95,11 @@ class PROTOC_EXPORT CppGenerator : public CodeGenerator { return FEATURE_PROTO3_OPTIONAL | FEATURE_SUPPORTS_EDITIONS; } + std::vector GetFeatureExtensions() const override { + return {DescriptorPool::generated_pool()->FindExtensionByNumber( + FeatureSet::descriptor(), pb::cpp.number())}; + } + private: bool opensource_runtime_ = PROTO2_IS_OSS; std::string runtime_include_base_; diff --git a/src/google/protobuf/compiler/mock_code_generator.cc b/src/google/protobuf/compiler/mock_code_generator.cc index 8c5366a5a0..53b8b6d078 100644 --- a/src/google/protobuf/compiler/mock_code_generator.cc +++ b/src/google/protobuf/compiler/mock_code_generator.cc @@ -57,9 +57,12 @@ #include "absl/strings/substitute.h" #include "google/protobuf/compiler/plugin.pb.h" #include "google/protobuf/descriptor.h" +#include "google/protobuf/descriptor_legacy.h" +#include "google/protobuf/descriptor_visitor.h" #include "google/protobuf/io/printer.h" #include "google/protobuf/io/zero_copy_stream.h" #include "google/protobuf/text_format.h" +#include "google/protobuf/unittest_features.pb.h" #ifdef major #undef major @@ -92,7 +95,16 @@ static constexpr absl::string_view kFirstInsertionPoint = static constexpr absl::string_view kSecondInsertionPoint = " # @@protoc_insertion_point(second_mock_insertion_point) is here\n"; -MockCodeGenerator::MockCodeGenerator(absl::string_view name) : name_(name) {} +MockCodeGenerator::MockCodeGenerator(absl::string_view name) : name_(name) { + absl::string_view key = getenv("TEST_CASE"); + if (key == "no_editions") { + suppressed_features_ |= CodeGenerator::FEATURE_SUPPORTS_EDITIONS; + } else if (key == "invalid_features") { + feature_extensions_ = {nullptr}; + } else if (key == "no_feature_defaults") { + feature_extensions_ = {}; + } +} MockCodeGenerator::~MockCodeGenerator() = default; @@ -214,14 +226,17 @@ bool MockCodeGenerator::Generate(const FileDescriptor* file, const std::string& parameter, GeneratorContext* context, std::string* error) const { - std::vector> options; - ParseGeneratorParameter(parameter, &options); - for (const auto& option : options) { - const auto& key = option.first; - - if (key == "no_editions") { - suppressed_features_ |= CodeGenerator::FEATURE_SUPPORTS_EDITIONS; - } + if (FileDescriptorLegacy(file).syntax() == + FileDescriptorLegacy::SYNTAX_EDITIONS) { + internal::VisitDescriptors(*file, [&](const auto& descriptor) { + const FeatureSet& features = GetResolvedSourceFeatures(descriptor); + ABSL_CHECK(features.HasExtension(pb::test)) + << "Test features were not resolved properly"; + ABSL_CHECK(features.GetExtension(pb::test).has_int_file_feature()) + << "Test features were not resolved properly"; + ABSL_CHECK(features.GetExtension(pb::test).has_int_source_feature()) + << "Test features were not resolved properly"; + }); } bool annotate = false; diff --git a/src/google/protobuf/compiler/mock_code_generator.h b/src/google/protobuf/compiler/mock_code_generator.h index bdb30c91c3..1b68647be5 100644 --- a/src/google/protobuf/compiler/mock_code_generator.h +++ b/src/google/protobuf/compiler/mock_code_generator.h @@ -35,9 +35,15 @@ #include #include +#include #include "absl/strings/string_view.h" #include "google/protobuf/compiler/code_generator.h" +#include "google/protobuf/descriptor.h" +#include "google/protobuf/unittest_features.pb.h" + +// Must be included last. +#include "google/protobuf/port_def.inc" namespace google { namespace protobuf { @@ -116,12 +122,35 @@ class MockCodeGenerator : public CodeGenerator { uint64_t GetSupportedFeatures() const override; void SuppressFeatures(uint64_t features); + std::vector GetFeatureExtensions() const override { + return feature_extensions_; + } + void set_feature_extensions(std::vector extensions) { + feature_extensions_ = extensions; + } + + absl::string_view GetMinimumEdition() const override { + return minimum_edition_; + } + void set_minimum_edition(absl::string_view minimum_edition) { + minimum_edition_ = minimum_edition; + } + + absl::string_view GetMaximumEdition() const override { + return maximum_edition_; + } + void set_maximum_edition(absl::string_view maximum_edition) { + maximum_edition_ = maximum_edition; + } + private: std::string name_; - - // Mark this mutable so that our test plugin can modify it during the Generate - // call via generator flags. - mutable uint64_t suppressed_features_ = 0; + uint64_t suppressed_features_ = 0; + absl::string_view minimum_edition_ = PROTOBUF_MINIMUM_EDITION; + absl::string_view maximum_edition_ = PROTOBUF_MAXIMUM_EDITION; + std::vector feature_extensions_ = { + DescriptorPool::generated_pool()->FindExtensionByNumber( + FeatureSet::descriptor(), pb::test.number())}; static std::string GetOutputFileContent(absl::string_view generator_name, absl::string_view parameter, @@ -138,4 +167,6 @@ class MockCodeGenerator : public CodeGenerator { } // namespace protobuf } // namespace google +#include "google/protobuf/port_undef.inc" + #endif // GOOGLE_PROTOBUF_COMPILER_MOCK_CODE_GENERATOR_H__ diff --git a/src/google/protobuf/compiler/plugin.cc b/src/google/protobuf/compiler/plugin.cc index c077841987..5c03db54ac 100644 --- a/src/google/protobuf/compiler/plugin.cc +++ b/src/google/protobuf/compiler/plugin.cc @@ -33,12 +33,16 @@ #include "google/protobuf/compiler/plugin.h" #include +#include + #ifdef _WIN32 #include #else #include #endif +#include "absl/status/statusor.h" +#include "absl/strings/str_cat.h" #include "google/protobuf/compiler/code_generator.h" #include "google/protobuf/compiler/plugin.pb.h" #include "google/protobuf/descriptor.h" @@ -112,6 +116,17 @@ bool GenerateCode(const CodeGeneratorRequest& request, const CodeGenerator& generator, CodeGeneratorResponse* response, std::string* error_msg) { DescriptorPool pool; + + // Initialize feature set default mapping. + absl::StatusOr defaults = + generator.BuildFeatureSetDefaults(); + if (!defaults.ok()) { + *error_msg = absl::StrCat("error generating feature defaults: ", + defaults.status().message()); + return false; + } + pool.SetFeatureSetDefaults(*defaults); + for (int i = 0; i < request.proto_file_size(); i++) { const FileDescriptor* file = pool.BuildFile(request.proto_file(i)); if (file == nullptr) { diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 25c22a9c6a..6bd5aa41f0 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -1117,8 +1117,8 @@ const FeatureSetDefaults& GetCppFeatureSetDefaults() { static const FeatureSetDefaults* default_spec = [] { auto default_spec = FeatureResolver::CompileDefaults( FeatureSet::descriptor(), - // TODO(b/297261063) Move this range to a central location. - {pb::CppFeatures::descriptor()->file()->extension(0)}, "2023", "2023"); + {pb::CppFeatures::descriptor()->file()->extension(0)}, + PROTOBUF_MINIMUM_EDITION, PROTOBUF_MAXIMUM_EDITION); ABSL_CHECK(default_spec.ok()) << default_spec.status(); return new FeatureSetDefaults(std::move(default_spec).value()); }(); diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc index 3605e3ec2f..332752ed40 100644 --- a/src/google/protobuf/port_def.inc +++ b/src/google/protobuf/port_def.inc @@ -272,6 +272,16 @@ static_assert(PROTOBUF_ABSL_MIN(20230125, 3), #endif #define PROTOBUF_VERSION_SUFFIX "" +#ifdef PROTOBUF_MINIMUM_EDITION +#error PROTOBUF_MINIMUM_EDITION was previously defined +#endif +#define PROTOBUF_MINIMUM_EDITION "2023" + +#ifdef PROTOBUF_MAXIMUM_EDITION +#error PROTOBUF_MAXIMUM_EDITION was previously defined +#endif +#define PROTOBUF_MAXIMUM_EDITION "2023" + #ifdef PROTOBUF_ALWAYS_INLINE #error PROTOBUF_ALWAYS_INLINE was previously defined #endif diff --git a/src/google/protobuf/port_undef.inc b/src/google/protobuf/port_undef.inc index 6c8245086d..96131fceeb 100644 --- a/src/google/protobuf/port_undef.inc +++ b/src/google/protobuf/port_undef.inc @@ -64,6 +64,8 @@ #undef PROTOBUF_RTTI #undef PROTOBUF_VERSION #undef PROTOBUF_VERSION_SUFFIX +#undef PROTOBUF_MINIMUM_EDITION +#undef PROTOBUF_MAXIMUM_EDITION #undef PROTOBUF_FIELD_OFFSET #undef PROTOBUF_MIN_HEADER_VERSION_FOR_PROTOC #undef PROTOBUF_MIN_PROTOC_VERSION diff --git a/src/google/protobuf/unittest_features.proto b/src/google/protobuf/unittest_features.proto index 7ba44b19b2..6dd3dc8ac2 100644 --- a/src/google/protobuf/unittest_features.proto +++ b/src/google/protobuf/unittest_features.proto @@ -49,6 +49,10 @@ message TestMessage { } } +extend google.protobuf.FeatureSet { + optional TestInvalidFeatures test_invalid = 9996; +} + message TestFeatures { optional int32 int_file_feature = 1 [ retention = RETENTION_RUNTIME, @@ -193,3 +197,11 @@ message TestFeatures { edition_defaults = { edition: "2023", value: "'2023'" } ]; } + +message TestInvalidFeatures { + repeated int32 repeated_feature = 1 [ + retention = RETENTION_RUNTIME, + targets = TARGET_TYPE_FIELD, + edition_defaults = { edition: "2023", value: "3" } + ]; +}