Skip to content

Commit

Permalink
Allow code generators to specify whether or not they support editions.
Browse files Browse the repository at this point in the history
Editions are still flag-guarded by the `--experimental_editions` flag for now, but once that's removed in a later release generators will need to explicitly specify that their support.  This will avoid cases where generators may happen to work for editions but produce incorrect code.

PiperOrigin-RevId: 547959326
  • Loading branch information
mkruskal-google committed Jul 15, 2023
1 parent f1de28a commit e2cec78
Show file tree
Hide file tree
Showing 13 changed files with 122 additions and 51 deletions.
34 changes: 1 addition & 33 deletions src/google/protobuf/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -739,39 +739,7 @@ proto_library(

proto_library(
name = "generic_test_protos",
srcs = [
"map_proto2_unittest.proto",
"map_proto3_unittest.proto",
"map_unittest.proto",
"unittest.proto",
"unittest_arena.proto",
"unittest_custom_options.proto",
"unittest_drop_unknown_fields.proto",
"unittest_embed_optimize_for.proto",
"unittest_empty.proto",
"unittest_enormous_descriptor.proto",
"unittest_import.proto",
"unittest_import_public.proto",
"unittest_lazy_dependencies.proto",
"unittest_lazy_dependencies_custom_option.proto",
"unittest_lazy_dependencies_enum.proto",
"unittest_lite_imports_nonlite.proto",
"unittest_mset.proto",
"unittest_mset_wire_format.proto",
"unittest_no_field_presence.proto",
"unittest_no_generic_services.proto",
"unittest_optimize_for.proto",
"unittest_preserve_unknown_enum.proto",
"unittest_preserve_unknown_enum2.proto",
"unittest_proto3.proto",
"unittest_proto3_arena.proto",
"unittest_proto3_arena_lite.proto",
"unittest_proto3_bad_macros.proto",
"unittest_proto3_lite.proto",
"unittest_proto3_optional.proto",
"unittest_retention.proto",
"unittest_well_known_types.proto",
],
srcs = [":test_proto_srcs"],
strip_import_prefix = "/src",
visibility = ["//:__subpackages__"],
deps = [
Expand Down
2 changes: 2 additions & 0 deletions src/google/protobuf/compiler/code_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,10 @@ class PROTOC_EXPORT CodeGenerator {

// This must be kept in sync with plugin.proto. See that file for
// documentation on each value.
// TODO(b/291092901) Use CodeGeneratorResponse.Feature here.
enum Feature {
FEATURE_PROTO3_OPTIONAL = 1,
FEATURE_SUPPORTS_EDITIONS = 2,
};

// Implement this to indicate what features this code generator supports.
Expand Down
14 changes: 14 additions & 0 deletions src/google/protobuf/compiler/code_generator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,20 @@ TEST_F(CodeGeneratorTest, GetSourceFeaturesInherited) {
EXPECT_EQ(ext.string_source_feature(), "field");
}

TEST_F(CodeGeneratorTest, GetRuntimeProtoTrivial) {
auto file = BuildFile(R"schema(
edition = "2023";
package protobuf_unittest;
)schema");
ASSERT_THAT(file, NotNull());

FileDescriptorProto proto = TestGenerator::GetRuntimeProto(*file);
const FeatureSet& features = proto.options().features();

EXPECT_TRUE(features.has_raw_features());
EXPECT_THAT(features.raw_features(), EqualsProto(R"pb()pb"));
}

TEST_F(CodeGeneratorTest, GetRuntimeProtoRoot) {
auto file = BuildFile(R"schema(
edition = "2023";
Expand Down
29 changes: 29 additions & 0 deletions src/google/protobuf/compiler/command_line_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,26 @@ std::string PluginName(absl::string_view plugin_prefix,
}


bool EnforceEditionsSupport(
const std::string& codegen_name, uint64_t supported_features,
const std::vector<const FileDescriptor*>& parsed_files) {
if ((supported_features & CodeGenerator::FEATURE_SUPPORTS_EDITIONS) == 0) {
for (const auto fd : parsed_files) {
if (FileDescriptorLegacy(fd).syntax() ==
FileDescriptorLegacy::SYNTAX_EDITIONS) {
std::cerr << fd->name() << ": is an editions file, but code generator "
<< codegen_name
<< " hasn't been updated to support editions yet. Please ask "
"the owner of this code generator to add support or "
"switch back to proto2/proto3."
<< std::endl;
return false;
}
}
}
return true;
}

} // namespace

void CommandLineInterface::GetTransitiveDependencies(
Expand Down Expand Up @@ -2490,6 +2510,12 @@ bool CommandLineInterface::GenerateOutput(
return false;
}

if (!EnforceEditionsSupport(
output_directive.name,
output_directive.generator->GetSupportedFeatures(), parsed_files)) {
return false;
}

if (!output_directive.generator->GenerateAll(parsed_files, parameters,
generator_context, &error)) {
// Generator returned an error.
Expand Down Expand Up @@ -2686,6 +2712,9 @@ bool CommandLineInterface::GeneratePluginOutput(
} else if (!EnforceProto3OptionalSupport(
plugin_name, response.supported_features(), parsed_files)) {
return false;
} else if (!EnforceEditionsSupport(plugin_name, response.supported_features(),
parsed_files)) {
return false;
}

return true;
Expand Down
35 changes: 35 additions & 0 deletions src/google/protobuf/compiler/command_line_interface_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1643,6 +1643,41 @@ TEST_F(CommandLineInterfaceTest, Plugin_SourceFeatures) {
}
}

TEST_F(CommandLineInterfaceTest, GeneratorNoEditionsSupport) {
CreateTempFile("foo.proto", R"schema(
edition = "2023";
message Foo {
int32 i = 1;
}
)schema");

CreateGeneratorWithMissingFeatures("--no_editions_out",
"Doesn't support editions",
CodeGenerator::FEATURE_SUPPORTS_EDITIONS);

Run("protocol_compiler --experimental_editions "
"--proto_path=$tmpdir foo.proto --no_editions_out=$tmpdir");

ExpectErrorSubstring(
"code generator --no_editions_out hasn't been updated to support "
"editions");
}

TEST_F(CommandLineInterfaceTest, PluginNoEditionsSupport) {
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=no_editions:$tmpdir");

ExpectErrorSubstring(
"code generator prefix-gen-plug hasn't been updated to support editions");
}

#endif // PROTOBUF_FUTURE_EDITIONS


Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/compiler/cpp/generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class PROTOC_EXPORT CppGenerator : public CodeGenerator {
std::string* error) const override;

uint64_t GetSupportedFeatures() const override {
return FEATURE_PROTO3_OPTIONAL;
return FEATURE_PROTO3_OPTIONAL | FEATURE_SUPPORTS_EDITIONS;
}

private:
Expand Down
9 changes: 7 additions & 2 deletions src/google/protobuf/compiler/fake_plugin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@
#include "google/protobuf/compiler/plugin.pb.h"
#include "google/protobuf/io/io_win32.h"

using google::protobuf::compiler::CodeGeneratorRequest;
using google::protobuf::compiler::CodeGeneratorResponse;

// This fake protoc plugin does nothing but write out the CodeGeneratorRequest
// in base64. This is not very useful except that it gives us a way to make
// assertions in tests about the contents of requests that protoc sends to
Expand All @@ -50,10 +53,12 @@ int main(int argc, char* argv[]) {
google::protobuf::io::win32::setmode(STDOUT_FILENO, _O_BINARY);
#endif

google::protobuf::compiler::CodeGeneratorRequest request;
CodeGeneratorRequest request;
ABSL_CHECK(request.ParseFromFileDescriptor(STDIN_FILENO));
ABSL_CHECK(!request.file_to_generate().empty());
google::protobuf::compiler::CodeGeneratorResponse response;
CodeGeneratorResponse response;
response.set_supported_features(
CodeGeneratorResponse::FEATURE_SUPPORTS_EDITIONS);
response.add_file()->set_name(
absl::StrCat(request.file_to_generate(0), ".request"));
response.mutable_file(0)->set_content(
Expand Down
14 changes: 13 additions & 1 deletion src/google/protobuf/compiler/mock_code_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include <memory>
#include <ostream>
#include <string>
#include <utility>
#include <vector>

#include "google/protobuf/testing/file.h"
Expand Down Expand Up @@ -96,7 +97,8 @@ MockCodeGenerator::MockCodeGenerator(absl::string_view name) : name_(name) {}
MockCodeGenerator::~MockCodeGenerator() = default;

uint64_t MockCodeGenerator::GetSupportedFeatures() const {
uint64_t all_features = CodeGenerator::FEATURE_PROTO3_OPTIONAL;
uint64_t all_features = CodeGenerator::FEATURE_PROTO3_OPTIONAL |
CodeGenerator::FEATURE_SUPPORTS_EDITIONS;
return all_features & ~suppressed_features_;
}

Expand Down Expand Up @@ -212,6 +214,16 @@ bool MockCodeGenerator::Generate(const FileDescriptor* file,
const std::string& parameter,
GeneratorContext* context,
std::string* error) const {
std::vector<std::pair<std::string, std::string>> options;
ParseGeneratorParameter(parameter, &options);
for (const auto& option : options) {
const auto& key = option.first;

if (key == "no_editions") {
suppressed_features_ |= CodeGenerator::FEATURE_SUPPORTS_EDITIONS;
}
}

bool annotate = false;
for (int i = 0; i < file->message_type_count(); i++) {
if (absl::StartsWith(file->message_type(i)->name(), "MockCodeGenerator_")) {
Expand Down
5 changes: 4 additions & 1 deletion src/google/protobuf/compiler/mock_code_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,10 @@ class MockCodeGenerator : public CodeGenerator {

private:
std::string name_;
uint64_t suppressed_features_ = 0;

// Mark this mutable so that our test plugin can modify it during the Generate
// call via generator flags.
mutable uint64_t suppressed_features_ = 0;

static std::string GetOutputFileContent(absl::string_view generator_name,
absl::string_view parameter,
Expand Down
17 changes: 10 additions & 7 deletions src/google/protobuf/compiler/plugin.pb.cc

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions src/google/protobuf/compiler/plugin.pb.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions src/google/protobuf/compiler/plugin.proto
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ message CodeGeneratorResponse {
enum Feature {
FEATURE_NONE = 0;
FEATURE_PROTO3_OPTIONAL = 1;
FEATURE_SUPPORTS_EDITIONS = 2;
}

// Represents a single generated file.
Expand Down
3 changes: 0 additions & 3 deletions src/google/protobuf/descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2773,9 +2773,6 @@ FileDescriptorProto InternalFeatureHelper::GetGeneratorProto(
if (&features != &FeatureSet::default_instance() &&
!IsLegacyFeatureSet(features)) {
*proto.mutable_options()->mutable_features() = features;
}
const auto& raw_features = GetRawFeatures(desc);
if (&raw_features != &FeatureSet::default_instance()) {
*proto.mutable_options()->mutable_features()->mutable_raw_features() =
GetRawFeatures(desc);
}
Expand Down

0 comments on commit e2cec78

Please sign in to comment.