Skip to content

Commit a2a0511

Browse files
zhangskzcopybara-github
authored andcommitted
Ban import weak and weak field option in edition 2024 in protoc (parser and c++ runtime).
Also expands edition-gating of `import option` to descriptor.cc. PiperOrigin-RevId: 761993162
1 parent 8543a71 commit a2a0511

File tree

9 files changed

+254
-172
lines changed

9 files changed

+254
-172
lines changed

src/google/protobuf/compiler/command_line_interface_unittest.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,7 @@ TEST_F(CommandLineInterfaceTest, MultipleInputs_UnusedImport_DescriptorSetIn) {
675675

676676
file_descriptor_proto = file_descriptor_set.add_file();
677677
file_descriptor_proto->set_name("import_custom_unknown_options.proto");
678+
file_descriptor_proto->set_edition(google::protobuf::Edition::EDITION_2024);
678679
file_descriptor_proto->add_option_dependency("custom_options.proto");
679680
// Add custom message option to unknown field. This custom option is
680681
// not known in generated pool, thus option will be in unknown fields.
@@ -688,7 +689,8 @@ TEST_F(CommandLineInterfaceTest, MultipleInputs_UnusedImport_DescriptorSetIn) {
688689

689690
Run("protocol_compiler --test_out=$tmpdir --plug_out=$tmpdir "
690691
"--descriptor_set_in=$tmpdir/foo.bin "
691-
"import_custom_unknown_options.proto");
692+
"import_custom_unknown_options.proto "
693+
"--experimental_editions");
692694

693695
// TODO: Fix this test. This test case only happens when
694696
// CommandLineInterface::Run() is used instead of invoke protoc combined

src/google/protobuf/compiler/parser.cc

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2508,13 +2508,13 @@ bool Parser::ParseImport(RepeatedPtrField<std::string>* dependency,
25082508
DO(Consume("import"));
25092509
std::string import_file;
25102510
if (LookingAt("option")) {
2511+
if (edition_ < Edition::EDITION_2024) {
2512+
RecordError("option import is not supported before edition 2024.");
2513+
}
25112514
LocationRecorder option_import_location(
25122515
root_location, FileDescriptorProto::kOptionDependencyFieldNumber,
25132516
option_dependency->size());
25142517
option_import_location.StartAt(import_start);
2515-
if (edition_ < Edition::EDITION_2024) {
2516-
RecordError("option import is not supported before edition 2024.");
2517-
}
25182518
DO(Consume("option"));
25192519
DO(ConsumeString(&import_file,
25202520
"Expected a string naming the file to import."));
@@ -2542,6 +2542,11 @@ bool Parser::ParseImport(RepeatedPtrField<std::string>* dependency,
25422542
DO(Consume("public"));
25432543
*public_dependency->Add() = dependency->size();
25442544
} else if (LookingAt("weak")) {
2545+
if (edition_ >= Edition::EDITION_2024) {
2546+
RecordError(
2547+
"weak import is not supported in edition 2024 and above. Consider "
2548+
"using option import instead.");
2549+
}
25452550
LocationRecorder weak_location(
25462551
root_location, FileDescriptorProto::kWeakDependencyFieldNumber,
25472552
weak_dependency->size());

src/google/protobuf/compiler/parser_unittest.cc

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2203,6 +2203,16 @@ TEST_F(ParseErrorTest, OptionImportBefore2024) {
22032203
"2:15: option import is not supported before edition 2024.\n");
22042204
}
22052205

2206+
TEST_F(ParseErrorTest, WeakImportAfter2024) {
2207+
ExpectHasErrors(
2208+
R"schema(
2209+
edition = "2024";
2210+
import weak "foo.proto";
2211+
)schema",
2212+
"2:15: weak import is not supported in edition 2024 and above. Consider "
2213+
"using option import instead.\n");
2214+
}
2215+
22062216
// ===================================================================
22072217
// Test that errors detected by DescriptorPool correctly report line and
22082218
// column numbers. We have one test for every call to RecordLocation() in
@@ -3553,25 +3563,44 @@ class SourceInfoTest : public ParserTest {
35533563

35543564
TEST_F(SourceInfoTest, BasicFileDecls) {
35553565
EXPECT_TRUE(
3556-
Parse("$a$edition = \"2024\";$i$\n"
3566+
Parse("$a$edition = \"2023\";$i$\n"
35573567
"$b$package foo.bar;$c$\n"
35583568
"$d$import \"baz.proto\";$e$\n"
35593569
"$f$import\"qux.proto\";$h$\n"
35603570
"$j$import $k$public$l$ \"bar.proto\";$m$\n"
35613571
"$n$import $o$weak$p$ \"bar.proto\";$q$\n"
3562-
"$r$import option \"bar.proto\";$s$\n"
35633572
"\n"
35643573
"// comment ignored\n"));
35653574

3566-
EXPECT_TRUE(HasSpan('a', 's', file_));
3575+
EXPECT_TRUE(HasSpan('a', 'q', file_));
35673576
EXPECT_TRUE(HasSpan('b', 'c', file_, "package"));
35683577
EXPECT_TRUE(HasSpan('d', 'e', file_, "dependency", 0));
35693578
EXPECT_TRUE(HasSpan('f', 'h', file_, "dependency", 1));
35703579
EXPECT_TRUE(HasSpan('j', 'm', file_, "dependency", 2));
35713580
EXPECT_TRUE(HasSpan('k', 'l', file_, "public_dependency", 0));
35723581
EXPECT_TRUE(HasSpan('n', 'q', file_, "dependency", 3));
35733582
EXPECT_TRUE(HasSpan('o', 'p', file_, "weak_dependency", 0));
3574-
EXPECT_TRUE(HasSpan('r', 's', file_, "option_dependency", 0));
3583+
EXPECT_TRUE(HasSpan('a', 'i', file_, "syntax"));
3584+
}
3585+
3586+
TEST_F(SourceInfoTest, BasicFileDecls_Edition2024) {
3587+
EXPECT_TRUE(
3588+
Parse("$a$edition = \"2024\";$i$\n"
3589+
"$b$package foo.bar;$c$\n"
3590+
"$d$import \"baz.proto\";$e$\n"
3591+
"$f$import\"qux.proto\";$h$\n"
3592+
"$j$import $k$public$l$ \"bar.proto\";$m$\n"
3593+
"$n$import option \"bar.proto\";$o$\n"
3594+
"\n"
3595+
"// comment ignored\n"));
3596+
3597+
EXPECT_TRUE(HasSpan('a', 'o', file_));
3598+
EXPECT_TRUE(HasSpan('b', 'c', file_, "package"));
3599+
EXPECT_TRUE(HasSpan('d', 'e', file_, "dependency", 0));
3600+
EXPECT_TRUE(HasSpan('f', 'h', file_, "dependency", 1));
3601+
EXPECT_TRUE(HasSpan('j', 'm', file_, "dependency", 2));
3602+
EXPECT_TRUE(HasSpan('k', 'l', file_, "public_dependency", 0));
3603+
EXPECT_TRUE(HasSpan('n', 'o', file_, "option_dependency", 0));
35753604
EXPECT_TRUE(HasSpan('a', 'i', file_, "syntax"));
35763605
}
35773606

src/google/protobuf/descriptor.cc

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5389,7 +5389,7 @@ Symbol DescriptorBuilder::FindSymbol(const absl::string_view name,
53895389
if (dep != nullptr && IsInPackage(dep, name)) return result;
53905390
}
53915391
for (const auto* dep : option_dependencies_) {
5392-
// Note: An dependency may be nullptr if it was not found or had errors.
5392+
// Note: A dependency may be nullptr if it was not found or had errors.
53935393
if (dep != nullptr && IsInPackage(dep, name)) return result;
53945394
}
53955395
}
@@ -8489,6 +8489,12 @@ void DescriptorBuilder::ValidateOptions(const FileDescriptor* file,
84898489
ValidateProto3(file, proto);
84908490
}
84918491

8492+
if (file->edition() < Edition::EDITION_2024 &&
8493+
file->option_dependency_count() > 0) {
8494+
AddError("option", proto, DescriptorPool::ErrorCollector::IMPORT,
8495+
"option imports are not supported before edition 2024.");
8496+
}
8497+
84928498
if (file->edition() >= Edition::EDITION_2024) {
84938499
if (file->options().has_java_multiple_files()) {
84948500
AddError(file->name(), proto, DescriptorPool::ErrorCollector::OPTION_NAME,
@@ -8497,6 +8503,10 @@ void DescriptorBuilder::ValidateOptions(const FileDescriptor* file,
84978503
" `nest_in_file_class = NO` (equivalent to "
84988504
"`java_multiple_files = true`).");
84998505
}
8506+
if (file->weak_dependency_count() > 0) {
8507+
AddError("weak", proto, DescriptorPool::ErrorCollector::IMPORT,
8508+
"weak imports are not allowed under edition 2024 and beyond.");
8509+
}
85008510
}
85018511
}
85028512

0 commit comments

Comments
 (0)