Skip to content

Commit 0416c3e

Browse files
protobuf-github-botcopybara-github
protobuf-github-bot
authored and
copybara-github
committed
Implement Protobuf Java Immutable API nest_in_file_class feature for Edition 2024.
PiperOrigin-RevId: 741582931
1 parent b95e4b0 commit 0416c3e

File tree

12 files changed

+528
-76
lines changed

12 files changed

+528
-76
lines changed

‎src/google/protobuf/compiler/command_line_interface_unittest.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2370,6 +2370,32 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsInvalidMaximumUnknown) {
23702370
ExpectErrorSubstring("unknown edition \"2022\"");
23712371
}
23722372

2373+
TEST_F(CommandLineInterfaceTest, JavaMultipleFilesEdition2024Invalid) {
2374+
CreateTempFile("foo.proto",
2375+
R"schema(
2376+
edition = "2024";
2377+
option java_multiple_files = true;
2378+
message Bar {}
2379+
)schema");
2380+
Run("protocol_compiler --proto_path=$tmpdir "
2381+
"foo.proto --test_out=$tmpdir --experimental_editions");
2382+
ExpectErrorSubstring(
2383+
"`java_multiple_files` is not supported in editions 2024 and above");
2384+
}
2385+
2386+
TEST_F(CommandLineInterfaceTest, JavaNestInFileClassFor) {
2387+
CreateTempFile("foo.proto",
2388+
R"schema(
2389+
edition = "2024";
2390+
option java_multiple_files = true;
2391+
message Bar {}
2392+
)schema");
2393+
Run("protocol_compiler --proto_path=$tmpdir "
2394+
"foo.proto --test_out=$tmpdir --experimental_editions");
2395+
ExpectErrorSubstring(
2396+
"`java_multiple_files` is not supported in editions 2024 and above");
2397+
}
2398+
23732399

23742400
TEST_F(CommandLineInterfaceTest, DirectDependencies_Missing_EmptyList) {
23752401
CreateTempFile("foo.proto",

‎src/google/protobuf/compiler/java/BUILD.bazel

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33
################################################################################
44

55
load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test")
6-
load("@rules_java//java:defs.bzl", "java_library")
76
load("@rules_pkg//pkg:mappings.bzl", "pkg_files", "strip_prefix")
8-
load("@rules_shell//shell:sh_test.bzl", "sh_test")
97
load("//build_defs:cpp_opts.bzl", "COPTS")
108

119
package(
@@ -74,6 +72,7 @@ cc_library(
7472
"@abseil-cpp//absl/container:flat_hash_set",
7573
"@abseil-cpp//absl/log:absl_check",
7674
"@abseil-cpp//absl/log:absl_log",
75+
"@abseil-cpp//absl/status",
7776
"@abseil-cpp//absl/strings",
7877
"@abseil-cpp//absl/strings:str_format",
7978
],
@@ -162,6 +161,7 @@ cc_library(
162161
"@abseil-cpp//absl/container:flat_hash_set",
163162
"@abseil-cpp//absl/log:absl_check",
164163
"@abseil-cpp//absl/log:absl_log",
164+
"@abseil-cpp//absl/status",
165165
"@abseil-cpp//absl/strings",
166166
"@abseil-cpp//absl/strings:str_format",
167167
],
@@ -218,6 +218,11 @@ cc_test(
218218
"//:protobuf",
219219
"//src/google/protobuf",
220220
"//src/google/protobuf/compiler:command_line_interface_tester",
221+
"//src/google/protobuf/testing",
222+
"//src/google/protobuf/testing:file",
223+
"@abseil-cpp//absl/log:absl_check",
224+
"@abseil-cpp//absl/strings",
225+
"@abseil-cpp//absl/strings:string_view",
221226
"@googletest//:gtest",
222227
"@googletest//:gtest_main",
223228
],

‎src/google/protobuf/compiler/java/file.cc

Lines changed: 53 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "absl/container/btree_set.h"
1919
#include "absl/log/absl_check.h"
2020
#include "absl/log/absl_log.h"
21+
#include "absl/status/status.h"
2122
#include "absl/strings/str_cat.h"
2223
#include "absl/strings/string_view.h"
2324
#include "google/protobuf/compiler/code_generator.h"
@@ -33,6 +34,7 @@
3334
#include "google/protobuf/compiler/java/shared_code_generator.h"
3435
#include "google/protobuf/compiler/retention.h"
3536
#include "google/protobuf/compiler/versions.h"
37+
#include "google/protobuf/descriptor.h"
3638
#include "google/protobuf/descriptor.pb.h"
3739
#include "google/protobuf/descriptor_visitor.h"
3840
#include "google/protobuf/dynamic_message.h"
@@ -252,6 +254,19 @@ bool FileGenerator::Validate(std::string* error) {
252254
"https://github.com/protocolbuffers/protobuf/blob/main/java/"
253255
"lite.md";
254256
}
257+
google::protobuf::internal::VisitDescriptors(*file_, [&](const EnumDescriptor& enm) {
258+
absl::Status status = ValidateNestInFileClassFeature(enm);
259+
if (!status.ok()) {
260+
absl::StrAppend(error, status.message());
261+
}
262+
});
263+
264+
google::protobuf::internal::VisitDescriptors(*file_, [&](const Descriptor& message) {
265+
absl::Status status = ValidateNestInFileClassFeature(message);
266+
if (!status.ok()) {
267+
absl::StrAppend(error, status.message());
268+
}
269+
});
255270

256271
return error->empty();
257272
}
@@ -338,17 +353,21 @@ void FileGenerator::Generate(io::Printer* printer) {
338353

339354
// -----------------------------------------------------------------
340355

341-
if (!MultipleJavaFiles(file_, immutable_api_)) {
342-
for (int i = 0; i < file_->enum_type_count(); i++) {
356+
for (int i = 0; i < file_->enum_type_count(); i++) {
357+
if (NestedInFileClass(*file_->enum_type(i), immutable_api_)) {
343358
generator_factory_->NewEnumGenerator(file_->enum_type(i))
344359
->Generate(printer);
345360
}
346-
for (int i = 0; i < file_->message_type_count(); i++) {
361+
}
362+
for (int i = 0; i < file_->message_type_count(); i++) {
363+
if (NestedInFileClass(*file_->message_type(i), immutable_api_)) {
347364
message_generators_[i]->GenerateInterface(printer);
348365
message_generators_[i]->Generate(printer);
349366
}
350-
if (HasGenericServices(file_, context_->EnforceLite())) {
351-
for (int i = 0; i < file_->service_count(); i++) {
367+
}
368+
if (HasGenericServices(file_, context_->EnforceLite())) {
369+
for (int i = 0; i < file_->service_count(); i++) {
370+
if (NestedInFileClass(*file_->service(i), immutable_api_)) {
352371
std::unique_ptr<ServiceGenerator> generator(
353372
generator_factory_->NewServiceGenerator(file_->service(i)));
354373
generator->Generate(printer);
@@ -564,38 +583,39 @@ void FileGenerator::GenerateSiblings(
564583
const std::string& package_dir, GeneratorContext* context,
565584
std::vector<std::string>* file_list,
566585
std::vector<std::string>* annotation_list) {
567-
if (MultipleJavaFiles(file_, immutable_api_)) {
568-
for (int i = 0; i < file_->enum_type_count(); i++) {
569-
std::unique_ptr<EnumGenerator> generator(
570-
generator_factory_->NewEnumGenerator(file_->enum_type(i)));
571-
GenerateSibling<EnumGenerator>(
572-
package_dir, java_package_, file_->enum_type(i), context, file_list,
573-
options_.annotate_code, annotation_list, "", generator.get(),
574-
options_.opensource_runtime, &EnumGenerator::Generate);
575-
}
576-
for (int i = 0; i < file_->message_type_count(); i++) {
577-
if (immutable_api_) {
578-
GenerateSibling<MessageGenerator>(
579-
package_dir, java_package_, file_->message_type(i), context,
580-
file_list, options_.annotate_code, annotation_list, "OrBuilder",
581-
message_generators_[i].get(), options_.opensource_runtime,
582-
&MessageGenerator::GenerateInterface);
583-
}
586+
for (int i = 0; i < file_->enum_type_count(); i++) {
587+
if (NestedInFileClass(*file_->enum_type(i), immutable_api_)) continue;
588+
std::unique_ptr<EnumGenerator> generator(
589+
generator_factory_->NewEnumGenerator(file_->enum_type(i)));
590+
GenerateSibling<EnumGenerator>(
591+
package_dir, java_package_, file_->enum_type(i), context, file_list,
592+
options_.annotate_code, annotation_list, "", generator.get(),
593+
options_.opensource_runtime, &EnumGenerator::Generate);
594+
}
595+
for (int i = 0; i < file_->message_type_count(); i++) {
596+
if (NestedInFileClass(*file_->message_type(i), immutable_api_)) continue;
597+
if (immutable_api_) {
584598
GenerateSibling<MessageGenerator>(
585599
package_dir, java_package_, file_->message_type(i), context,
586-
file_list, options_.annotate_code, annotation_list, "",
600+
file_list, options_.annotate_code, annotation_list, "OrBuilder",
587601
message_generators_[i].get(), options_.opensource_runtime,
588-
&MessageGenerator::Generate);
602+
&MessageGenerator::GenerateInterface);
589603
}
590-
if (HasGenericServices(file_, context_->EnforceLite())) {
591-
for (int i = 0; i < file_->service_count(); i++) {
592-
std::unique_ptr<ServiceGenerator> generator(
593-
generator_factory_->NewServiceGenerator(file_->service(i)));
594-
GenerateSibling<ServiceGenerator>(
595-
package_dir, java_package_, file_->service(i), context, file_list,
596-
options_.annotate_code, annotation_list, "", generator.get(),
597-
options_.opensource_runtime, &ServiceGenerator::Generate);
598-
}
604+
GenerateSibling<MessageGenerator>(
605+
package_dir, java_package_, file_->message_type(i), context, file_list,
606+
options_.annotate_code, annotation_list, "",
607+
message_generators_[i].get(), options_.opensource_runtime,
608+
&MessageGenerator::Generate);
609+
}
610+
if (HasGenericServices(file_, context_->EnforceLite())) {
611+
for (int i = 0; i < file_->service_count(); i++) {
612+
if (NestedInFileClass(*file_->service(i), immutable_api_)) continue;
613+
std::unique_ptr<ServiceGenerator> generator(
614+
generator_factory_->NewServiceGenerator(file_->service(i)));
615+
GenerateSibling<ServiceGenerator>(
616+
package_dir, java_package_, file_->service(i), context, file_list,
617+
options_.annotate_code, annotation_list, "", generator.get(),
618+
options_.opensource_runtime, &ServiceGenerator::Generate);
599619
}
600620
}
601621
}

‎src/google/protobuf/compiler/java/full/message.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,12 @@ void ImmutableMessageGenerator::GenerateStaticVariables(
103103
if (descriptor_->containing_type() != nullptr) {
104104
vars["parent"] = UniqueFileScopeIdentifier(descriptor_->containing_type());
105105
}
106-
if (MultipleJavaFiles(descriptor_->file(), /* immutable = */ true)) {
106+
if (NestedInFileClass(*descriptor_, /* immutable = */ true)) {
107+
vars["private"] = "private ";
108+
} else {
107109
// We can only make these package-private since the classes that use them
108110
// are in separate files.
109111
vars["private"] = "";
110-
} else {
111-
vars["private"] = "private ";
112112
}
113113
if (*bytecode_estimate <= kMaxStaticSize) {
114114
vars["final"] = "final ";
@@ -179,12 +179,12 @@ void ImmutableMessageGenerator::GenerateFieldAccessorTable(
179179
io::Printer* printer, int* bytecode_estimate) {
180180
absl::flat_hash_map<absl::string_view, std::string> vars;
181181
vars["identifier"] = UniqueFileScopeIdentifier(descriptor_);
182-
if (MultipleJavaFiles(descriptor_->file(), /* immutable = */ true)) {
182+
if (NestedInFileClass(*descriptor_, /* immutable = */ true)) {
183+
vars["private"] = "private ";
184+
} else {
183185
// We can only make these package-private since the classes that use them
184186
// are in separate files.
185187
vars["private"] = "";
186-
} else {
187-
vars["private"] = "private ";
188188
}
189189
if (*bytecode_estimate <= kMaxStaticSize) {
190190
vars["final"] = "final ";

‎src/google/protobuf/compiler/java/generator.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ class PROTOC_EXPORT JavaGenerator : public CodeGenerator {
6161

6262
using CodeGenerator::GetEdition;
6363
using CodeGenerator::GetResolvedSourceFeatures;
64+
using CodeGenerator::GetUnresolvedSourceFeatures;
6465

6566
private:
6667
bool opensource_runtime_ = google::protobuf::internal::IsOss();

0 commit comments

Comments
 (0)