Implement java.nest_in_file_class feature for Edition 2024.

PiperOrigin-RevId: 688236375
protocolbuffers · Nov 14, 2024 · b2fbbf3 · b2fbbf3
1 parent f50d630
commit b2fbbf3
Show file tree

Hide file tree

Showing 22 changed files with 820 additions and 206 deletions.
diff --git a/cmake/tests.cmake b/cmake/tests.cmake
@@ -38,6 +38,7 @@ foreach(proto_file ${tests_protos})
     LANGUAGE cpp
     OUT_VAR pb_generated_files
     IMPORT_DIRS ${protobuf_SOURCE_DIR}/src
+    IMPORT_DIRS ${protobuf_SOURCE_DIR}/java/core/src/main/resources
   )
   set(tests_proto_files ${tests_proto_files} ${pb_generated_files})
 endforeach(proto_file)

diff --git a/java/core/BUILD.bazel b/java/core/BUILD.bazel
@@ -445,6 +445,7 @@ junit_tests(
         ":java_test_protos_java_proto",
         ":lite_test_protos_java_proto",
         ":test_util",
+        "//src/google/protobuf/compiler/java:test_nested_in_file_class_2024_java_proto",
         "@protobuf_maven//:com_google_guava_guava",
         "@protobuf_maven//:com_google_truth_truth",
         "@protobuf_maven//:junit_junit",
@@ -638,6 +639,7 @@ junit_tests(
         ":core",
         ":v25_test_util_srcjar",
         "//compatibility:v25_test_protos_srcjar",
+        "//src/google/protobuf/compiler/java:test_nested_in_file_class_2024_java_proto",
         "@protobuf_maven//:com_google_guava_guava",
         "@protobuf_maven//:com_google_truth_truth",
         "@protobuf_maven//:junit_junit",
@@ -688,6 +690,7 @@ junit_tests(
         ":core",
         ":v25_test_util_jar",
         "//compatibility:v25_test_protos_jar",
+        "//src/google/protobuf/compiler/java:test_nested_in_file_class_2024_java_proto",
         "@protobuf_maven//:com_google_guava_guava",
         "@protobuf_maven//:com_google_truth_truth",
         "@protobuf_maven//:junit_junit",

diff --git a/java/core/src/main/resources/google/protobuf/java_features.proto b/java/core/src/main/resources/google/protobuf/java_features.proto
@@ -83,4 +83,32 @@ message JavaFeatures {
     edition_defaults = { edition: EDITION_LEGACY, value: "true" },
     edition_defaults = { edition: EDITION_2024, value: "false" }
   ];
+
+  enum NestInFileClass {
+    // Invalid default, which should never be used.
+    NEST_IN_FILE_CLASS_UNKNOWN = 0;
+    // Do not nest the generated class in the file class.
+    NO = 1;
+    // Nest the generated class in the file class.
+    YES = 2;
+    // Fall back to the `java_multiple_files` option. Users won't be able to
+    // set this option.
+    LEGACY = 3 [feature_support = {
+      edition_introduced: EDITION_2024
+      edition_removed: EDITION_2024
+    }];
+  }
+
+  optional NestInFileClass nest_in_file_class = 5 [
+    retention = RETENTION_RUNTIME,
+    targets = TARGET_TYPE_FILE,
+    targets = TARGET_TYPE_MESSAGE,
+    targets = TARGET_TYPE_ENUM,
+    targets = TARGET_TYPE_SERVICE,
+    feature_support = {
+      edition_introduced: EDITION_2024,
+    },
+    edition_defaults = { edition: EDITION_LEGACY, value: "LEGACY" },
+    edition_defaults = { edition: EDITION_2024, value: "NO" }
+  ];
 }
diff --git a/java/core/src/test/java/com/google/protobuf/GeneratedMessageTest.java b/java/core/src/test/java/com/google/protobuf/GeneratedMessageTest.java
@@ -25,6 +25,8 @@
 import protobuf_unittest.OuterClassNameTest3OuterClass;
 import protobuf_unittest.OuterClassNameTestOuterClass;
 import protobuf_unittest.ServiceWithNoOuter;
+import protobuf_unittest.TestNestedInFileClass2024Proto.NestedInFileClassEnum;
+import protobuf_unittest.TestNestedInFileClass2024Proto.NestedInFileClassMessage;
 import protobuf_unittest.UnittestOptimizeFor.TestOptimizedForSize;
 import protobuf_unittest.UnittestOptimizeFor.TestOptionalOptimizedForSize;
 import protobuf_unittest.UnittestOptimizeFor.TestRequiredOptimizedForSize;
@@ -41,6 +43,8 @@
 import protobuf_unittest.UnittestProto.TestOneof2;
 import protobuf_unittest.UnittestProto.TestPackedTypes;
 import protobuf_unittest.UnittestProto.TestUnpackedTypes;
+import protobuf_unittest.UnnestedEnum;
+import protobuf_unittest.UnnestedMessage;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -1973,4 +1977,25 @@ public void getAllFields_repeatedFieldsAreNotMutable() {
     builder.clearField(repeatedMsgField);
     assertThat(list).hasSize(1);
   }
+
+  @Test
+  public void generateMultipleFilesEdition2024() throws Exception {
+    // Ensures that the generated code compiles.
+    UnnestedMessage unnested1 =
+        UnnestedMessage.newBuilder()
+            .setNestedInFileClassEnum(NestedInFileClassEnum.FOO_VALUE)
+            .build();
+    UnnestedMessage unnested2 =
+        UnnestedMessage.parseFrom(unnested1.toByteString(), ExtensionRegistry.getEmptyRegistry());
+    NestedInFileClassMessage nested1 =
+        NestedInFileClassMessage.newBuilder().setUnnestedEnum(UnnestedEnum.BAR_VALUE).build();
+    NestedInFileClassMessage nested2 =
+        NestedInFileClassMessage.parseFrom(
+            nested1.toByteString(), ExtensionRegistry.getEmptyRegistry());
+
+    assertThat(unnested1.getNestedInFileClassEnumValue()).isEqualTo(1);
+    assertThat(unnested2.getNestedInFileClassEnumValue()).isEqualTo(1);
+    assertThat(nested1.getUnnestedEnumValue()).isEqualTo(1);
+    assertThat(nested2.getUnnestedEnumValue()).isEqualTo(1);
+  }
 }
diff --git a/src/google/protobuf/compiler/command_line_interface_tester.cc b/src/google/protobuf/compiler/command_line_interface_tester.cc
@@ -175,6 +175,21 @@ void CommandLineInterfaceTester::ExpectFileContent(absl::string_view filename,
             file_contents);
 }
 
+std::string CommandLineInterfaceTester::ReadFile(absl::string_view filename) {
+  std::string path = absl::StrCat(temp_directory_, "/", filename);
+  std::string file_contents;
+  ABSL_CHECK_OK(File::GetContents(path, &file_contents, true));
+  return file_contents;
+}
+
+void CommandLineInterfaceTester::ReadDescriptorSet(
+    absl::string_view filename, FileDescriptorSet* descriptor_set) {
+  std::string file_contents = ReadFile(filename);
+  if (!descriptor_set->ParseFromString(file_contents)) {
+    FAIL() << "Could not parse file contents: " << filename;
+  }
+}
+
 }  // namespace compiler
 }  // namespace protobuf
 }  // namespace google
diff --git a/src/google/protobuf/compiler/command_line_interface_tester.h b/src/google/protobuf/compiler/command_line_interface_tester.h
@@ -123,6 +123,10 @@ class CommandLineInterfaceTester : public testing::Test {
 
   void ExpectFileContent(absl::string_view filename, absl::string_view content);
 
+  std::string ReadFile(absl::string_view filename);
+  void ReadDescriptorSet(absl::string_view filename,
+                         FileDescriptorSet* descriptor_set);
+
  private:
   // The object we are testing.
   CommandLineInterface cli_;

diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc
@@ -186,10 +186,6 @@ class CommandLineInterfaceTest : public CommandLineInterfaceTester {
 #endif  // _WIN32
 
 
-  std::string ReadFile(absl::string_view filename);
-  void ReadDescriptorSet(absl::string_view filename,
-                         FileDescriptorSet* descriptor_set);
-
   void WriteDescriptorSet(absl::string_view filename,
                           const FileDescriptorSet* descriptor_set);
 
@@ -336,21 +332,6 @@ void CommandLineInterfaceTest::ExpectNullCodeGeneratorCalled(
 #endif  // _WIN32
 
 
-std::string CommandLineInterfaceTest::ReadFile(absl::string_view filename) {
-  std::string path = absl::StrCat(temp_directory(), "/", filename);
-  std::string file_contents;
-  ABSL_CHECK_OK(File::GetContents(path, &file_contents, true));
-  return file_contents;
-}
-
-void CommandLineInterfaceTest::ReadDescriptorSet(
-    absl::string_view filename, FileDescriptorSet* descriptor_set) {
-  std::string file_contents = ReadFile(filename);
-  if (!descriptor_set->ParseFromString(file_contents)) {
-    FAIL() << "Could not parse file contents: " << filename;
-  }
-}
-
 FeatureSetDefaults CommandLineInterfaceTest::ReadEditionDefaults(
     absl::string_view filename) {
   FeatureSetDefaults defaults;
@@ -2310,6 +2291,19 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsInvalidMaximumUnknown) {
   ExpectErrorSubstring("unknown edition \"2022\"");
 }
 
+TEST_F(CommandLineInterfaceTest, JavaMultipleFilesEdition2024Invalid) {
+  CreateTempFile("foo.proto",
+                 R"schema(
+                 edition = "2024";
+                 option java_multiple_files = true;
+                 message Bar {}
+                 )schema");
+  Run("protocol_compiler --proto_path=$tmpdir "
+      "foo.proto --test_out=$tmpdir --experimental_editions");
+  ExpectErrorSubstring(
+      "`java_multiple_files` is not supported in editions 2024 and above");
+}
+
 
 TEST_F(CommandLineInterfaceTest, DirectDependencies_Missing_EmptyList) {
   CreateTempFile("foo.proto",

diff --git a/src/google/protobuf/compiler/java/BUILD.bazel b/src/google/protobuf/compiler/java/BUILD.bazel
@@ -256,7 +256,11 @@ cc_test(
         ":test_file_name_cc_proto",
         ":test_multiple_file_no_cc_proto",
         ":test_multiple_file_yes_cc_proto",
+        "//:protobuf",
+        "//src/google/protobuf",
+        "//src/google/protobuf:port",
         "//src/google/protobuf/compiler:command_line_interface",
+        "//src/google/protobuf/compiler:command_line_interface_tester",
         "//src/google/protobuf/stubs:lite",
         "//src/google/protobuf/testing",
         "//src/google/protobuf/testing:file",
@@ -315,6 +319,21 @@ cc_proto_library(
     deps = [":test_multiple_file_yes_proto"],
 )
 
+proto_library(
+    name = "test_nested_in_file_class_2024_proto",
+    testonly = 1,
+    srcs = ["test_nested_in_file_class_2024.proto"],
+    strip_import_prefix = "/src",
+    deps = ["//:java_features_proto"],
+)
+
+java_proto_library(
+    name = "test_nested_in_file_class_2024_java_proto",
+    testonly = 1,
+    visibility = ["//java/core:__subpackages__"],
+    deps = [":test_nested_in_file_class_2024_proto"],
+)
+
 ################################################################################
 # Distribution packaging
 ################################################################################

diff --git a/src/google/protobuf/compiler/java/file.cc b/src/google/protobuf/compiler/java/file.cc
@@ -349,17 +349,21 @@ void FileGenerator::Generate(io::Printer* printer) {
 
   // -----------------------------------------------------------------
 
-  if (!MultipleJavaFiles(file_, immutable_api_)) {
-    for (int i = 0; i < file_->enum_type_count(); i++) {
+  for (int i = 0; i < file_->enum_type_count(); i++) {
+    if (NestedInFileClass(*file_->enum_type(i), immutable_api_)) {
       generator_factory_->NewEnumGenerator(file_->enum_type(i))
           ->Generate(printer);
     }
-    for (int i = 0; i < file_->message_type_count(); i++) {
+  }
+  for (int i = 0; i < file_->message_type_count(); i++) {
+    if (NestedInFileClass(*file_->message_type(i), immutable_api_)) {
       message_generators_[i]->GenerateInterface(printer);
       message_generators_[i]->Generate(printer);
     }
-    if (HasGenericServices(file_, context_->EnforceLite())) {
-      for (int i = 0; i < file_->service_count(); i++) {
+  }
+  if (HasGenericServices(file_, context_->EnforceLite())) {
+    for (int i = 0; i < file_->service_count(); i++) {
+      if (NestedInFileClass(*file_->service(i), immutable_api_)) {
         std::unique_ptr<ServiceGenerator> generator(
             generator_factory_->NewServiceGenerator(file_->service(i)));
         generator->Generate(printer);
@@ -575,41 +579,51 @@ static void GenerateSibling(
 }
 
 void FileGenerator::GenerateSiblings(
-    const std::string& package_dir, GeneratorContext* context,
-    std::vector<std::string>* file_list,
+    GeneratorContext* context, std::vector<std::string>* file_list,
     std::vector<std::string>* annotation_list) {
-  if (MultipleJavaFiles(file_, immutable_api_)) {
-    for (int i = 0; i < file_->enum_type_count(); i++) {
-      std::unique_ptr<EnumGenerator> generator(
-          generator_factory_->NewEnumGenerator(file_->enum_type(i)));
-      GenerateSibling<EnumGenerator>(
-          package_dir, java_package_, file_->enum_type(i), context, file_list,
-          options_.annotate_code, annotation_list, "", generator.get(),
-          options_.opensource_runtime, &EnumGenerator::Generate);
-    }
-    for (int i = 0; i < file_->message_type_count(); i++) {
-      if (immutable_api_) {
-        GenerateSibling<MessageGenerator>(
-            package_dir, java_package_, file_->message_type(i), context,
-            file_list, options_.annotate_code, annotation_list, "OrBuilder",
-            message_generators_[i].get(), options_.opensource_runtime,
-            &MessageGenerator::GenerateInterface);
-      }
+  for (int i = 0; i < file_->enum_type_count(); i++) {
+    auto* enum_type = file_->enum_type(i);
+    if (NestedInFileClass(*file_->enum_type(i), immutable_api_)) continue;
+    std::unique_ptr<EnumGenerator> generator(
+        generator_factory_->NewEnumGenerator(file_->enum_type(i)));
+    std::string java_package =
+        JavaPackageForType(*enum_type, immutable_api_, context_->options());
+    GenerateSibling<EnumGenerator>(
+        JavaPackageToDir(java_package), java_package, file_->enum_type(i),
+        context, file_list, options_.annotate_code, annotation_list, "",
+        generator.get(), options_.opensource_runtime, &EnumGenerator::Generate);
+  }
+  for (int i = 0; i < file_->message_type_count(); i++) {
+    auto* message_type = file_->message_type(i);
+    if (NestedInFileClass(*message_type, immutable_api_)) continue;
+    std::string java_package =
+        JavaPackageForType(*message_type, immutable_api_, context_->options());
+    if (immutable_api_) {
       GenerateSibling<MessageGenerator>(
-          package_dir, java_package_, file_->message_type(i), context,
-          file_list, options_.annotate_code, annotation_list, "",
+          JavaPackageToDir(java_package), java_package, message_type, context,
+          file_list, options_.annotate_code, annotation_list, "OrBuilder",
           message_generators_[i].get(), options_.opensource_runtime,
-          &MessageGenerator::Generate);
+          &MessageGenerator::GenerateInterface);
     }
-    if (HasGenericServices(file_, context_->EnforceLite())) {
-      for (int i = 0; i < file_->service_count(); i++) {
-        std::unique_ptr<ServiceGenerator> generator(
-            generator_factory_->NewServiceGenerator(file_->service(i)));
-        GenerateSibling<ServiceGenerator>(
-            package_dir, java_package_, file_->service(i), context, file_list,
-            options_.annotate_code, annotation_list, "", generator.get(),
-            options_.opensource_runtime, &ServiceGenerator::Generate);
-      }
+    GenerateSibling<MessageGenerator>(
+        JavaPackageToDir(java_package), java_package, message_type, context,
+        file_list, options_.annotate_code, annotation_list, "",
+        message_generators_[i].get(), options_.opensource_runtime,
+        &MessageGenerator::Generate);
+  }
+  if (HasGenericServices(file_, context_->EnforceLite())) {
+    for (int i = 0; i < file_->service_count(); i++) {
+      auto* service = file_->service(i);
+      if (NestedInFileClass(*service, immutable_api_)) continue;
+      std::unique_ptr<ServiceGenerator> generator(
+          generator_factory_->NewServiceGenerator(service));
+      std::string java_package =
+          JavaPackageForType(*service, immutable_api_, context_->options());
+      GenerateSibling<ServiceGenerator>(
+          JavaPackageToDir(java_package), java_package, service, context,
+          file_list, options_.annotate_code, annotation_list, "",
+          generator.get(), options_.opensource_runtime,
+          &ServiceGenerator::Generate);
     }
   }
 }

diff --git a/src/google/protobuf/compiler/java/file.h b/src/google/protobuf/compiler/java/file.h
@@ -61,8 +61,7 @@ class FileGenerator {
   // If we aren't putting everything into one file, this will write all the
   // files other than the outer file (i.e. one for each message, enum, and
   // service type).
-  void GenerateSiblings(const std::string& package_dir,
-                        GeneratorContext* generator_context,
+  void GenerateSiblings(GeneratorContext* generator_context,
                         std::vector<std::string>* file_list,
                         std::vector<std::string>* annotation_list);
 

diff --git a/src/google/protobuf/compiler/java/full/message.cc b/src/google/protobuf/compiler/java/full/message.cc
@@ -103,12 +103,12 @@ void ImmutableMessageGenerator::GenerateStaticVariables(
   if (descriptor_->containing_type() != nullptr) {
     vars["parent"] = UniqueFileScopeIdentifier(descriptor_->containing_type());
   }
-  if (MultipleJavaFiles(descriptor_->file(), /* immutable = */ true)) {
+  if (NestedInFileClass(*descriptor_, /* immutable = */ true)) {
     // We can only make these package-private since the classes that use them
     // are in separate files.
-    vars["private"] = "";
-  } else {
     vars["private"] = "private ";
+  } else {
+    vars["private"] = "";
   }
   if (*bytecode_estimate <= kMaxStaticSize) {
     vars["final"] = "final ";
@@ -179,12 +179,12 @@ void ImmutableMessageGenerator::GenerateFieldAccessorTable(
     io::Printer* printer, int* bytecode_estimate) {
   absl::flat_hash_map<absl::string_view, std::string> vars;
   vars["identifier"] = UniqueFileScopeIdentifier(descriptor_);
-  if (MultipleJavaFiles(descriptor_->file(), /* immutable = */ true)) {
+  if (NestedInFileClass(*descriptor_, /* immutable = */ true)) {
     // We can only make these package-private since the classes that use them
     // are in separate files.
-    vars["private"] = "";
-  } else {
     vars["private"] = "private ";
+  } else {
+    vars["private"] = "";
   }
   if (*bytecode_estimate <= kMaxStaticSize) {
     vars["final"] = "final ";

diff --git a/src/google/protobuf/compiler/java/generator.cc b/src/google/protobuf/compiler/java/generator.cc
@@ -142,8 +142,7 @@ bool JavaGenerator::Generate(const FileDescriptor* file,
     file_generator->Generate(&printer);
 
     // Generate sibling files.
-    file_generator->GenerateSiblings(package_dir, context, &all_files,
-                                     &all_annotations);
+    file_generator->GenerateSiblings(context, &all_files, &all_annotations);
 
     if (file_options.annotate_code) {
       std::unique_ptr<io::ZeroCopyOutputStream> info_output(