Allow code generators to specify whether or not they support editions.

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
protocolbuffers · Jul 15, 2023 · e2cec78 · e2cec78
1 parent f1de28a
commit e2cec78
Show file tree

Hide file tree

Showing 13 changed files with 122 additions and 51 deletions.
diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel
@@ -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 = [

diff --git a/src/google/protobuf/compiler/code_generator.h b/src/google/protobuf/compiler/code_generator.h
@@ -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.

diff --git a/src/google/protobuf/compiler/code_generator_unittest.cc b/src/google/protobuf/compiler/code_generator_unittest.cc
@@ -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";

diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc
@@ -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(
@@ -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.
@@ -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;

diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc
@@ -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
 
 

diff --git a/src/google/protobuf/compiler/cpp/generator.h b/src/google/protobuf/compiler/cpp/generator.h
@@ -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:

diff --git a/src/google/protobuf/compiler/fake_plugin.cc b/src/google/protobuf/compiler/fake_plugin.cc
@@ -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
@@ -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(

diff --git a/src/google/protobuf/compiler/mock_code_generator.cc b/src/google/protobuf/compiler/mock_code_generator.cc
@@ -39,6 +39,7 @@
 #include <memory>
 #include <ostream>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "google/protobuf/testing/file.h"
@@ -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_;
 }
 
@@ -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_")) {

diff --git a/src/google/protobuf/compiler/mock_code_generator.h b/src/google/protobuf/compiler/mock_code_generator.h
@@ -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,

diff --git a/src/google/protobuf/compiler/plugin.pb.cc b/src/google/protobuf/compiler/plugin.pb.cc
diff --git a/src/google/protobuf/compiler/plugin.pb.h b/src/google/protobuf/compiler/plugin.pb.h
diff --git a/src/google/protobuf/compiler/plugin.proto b/src/google/protobuf/compiler/plugin.proto
@@ -122,6 +122,7 @@ message CodeGeneratorResponse {
   enum Feature {
     FEATURE_NONE = 0;
     FEATURE_PROTO3_OPTIONAL = 1;
+    FEATURE_SUPPORTS_EDITIONS = 2;
   }
 
   // Represents a single generated file.

diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc
@@ -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);
         }