Skip to content

Commit 851d5af

Browse files
zhangskzcopybara-github
zhangskz
authored and
copybara-github
committed
Internal change
PiperOrigin-RevId: 758810204
1 parent a014dbb commit 851d5af

File tree

3 files changed

+162
-1
lines changed

3 files changed

+162
-1
lines changed

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

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,9 @@ using google::protobuf::io::win32::write;
124124
static const char* kDefaultDirectDependenciesViolationMsg =
125125
"File is imported but not declared in --direct_dependencies: %s";
126126

127+
static const char* kDefaultOptionDependenciesViolationMsg =
128+
"File is option imported but not declared in --option_dependencies: %s";
129+
127130
// Returns true if the text begins with a Windows-style absolute path, starting
128131
// with a drive letter. Example: "C:\foo".
129132
static bool StartsWithWindowsAbsolutePath(absl::string_view text) {
@@ -958,7 +961,9 @@ const char* const CommandLineInterface::kPathSeparator = ":";
958961

959962
CommandLineInterface::CommandLineInterface()
960963
: direct_dependencies_violation_msg_(
961-
kDefaultDirectDependenciesViolationMsg) {}
964+
kDefaultDirectDependenciesViolationMsg),
965+
option_dependencies_violation_msg_(
966+
kDefaultOptionDependenciesViolationMsg) {}
962967

963968
CommandLineInterface::~CommandLineInterface() = default;
964969

@@ -1645,6 +1650,26 @@ bool CommandLineInterface::ParseInputFiles(
16451650
break;
16461651
}
16471652
}
1653+
1654+
// Enforce --option_dependencies.
1655+
if (option_dependencies_explicitly_set_) {
1656+
bool indirect_option_imports = false;
1657+
for (int i = 0; i < parsed_file->option_dependency_count(); ++i) {
1658+
if (option_dependencies_.find(parsed_file->option_dependency_name(i)) ==
1659+
option_dependencies_.end()) {
1660+
indirect_option_imports = true;
1661+
std::cerr << parsed_file->name() << ": "
1662+
<< absl::StrReplaceAll(
1663+
option_dependencies_violation_msg_,
1664+
{{"%s", parsed_file->option_dependency_name(i)}})
1665+
<< std::endl;
1666+
}
1667+
if (indirect_option_imports) {
1668+
result = false;
1669+
break;
1670+
}
1671+
}
1672+
}
16481673
}
16491674
descriptor_pool->ClearDirectInputFiles();
16501675
return result;
@@ -1658,6 +1683,8 @@ void CommandLineInterface::Clear() {
16581683
input_files_.clear();
16591684
direct_dependencies_.clear();
16601685
direct_dependencies_violation_msg_ = kDefaultDirectDependenciesViolationMsg;
1686+
option_dependencies_.clear();
1687+
option_dependencies_violation_msg_ = kDefaultOptionDependenciesViolationMsg;
16611688
output_directives_.clear();
16621689
codec_type_.clear();
16631690
descriptor_set_in_names_.clear();
@@ -2139,7 +2166,23 @@ CommandLineInterface::InterpretArgument(const std::string& name,
21392166

21402167
} else if (name == "--direct_dependencies_violation_msg") {
21412168
direct_dependencies_violation_msg_ = value;
2169+
} else if (name == "--option_dependencies") {
2170+
if (option_dependencies_explicitly_set_) {
2171+
std::cerr << name
2172+
<< " may only be passed once. To specify multiple "
2173+
"option dependencies, pass them all as a single "
2174+
"parameter separated by ':'."
2175+
<< std::endl;
2176+
return PARSE_ARGUMENT_FAIL;
2177+
}
21422178

2179+
option_dependencies_explicitly_set_ = true;
2180+
std::vector<std::string> direct =
2181+
absl::StrSplit(value, ':', absl::SkipEmpty());
2182+
ABSL_DCHECK(option_dependencies_.empty());
2183+
option_dependencies_.insert(direct.begin(), direct.end());
2184+
} else if (name == "--option_dependencies_violation_msg") {
2185+
option_dependencies_violation_msg_ = value;
21432186
} else if (name == "--descriptor_set_in") {
21442187
if (!descriptor_set_in_names_.empty()) {
21452188
std::cerr << name

‎src/google/protobuf/compiler/command_line_interface.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,16 @@ class PROTOC_EXPORT CommandLineInterface {
423423
// presented to the user. "%s" will be replaced with the violating import.
424424
std::string direct_dependencies_violation_msg_;
425425

426+
// Names of proto files which are allowed to be option imported. Used by build
427+
// systems to enforce option-depend-on-what-you-option-import.
428+
absl::flat_hash_set<std::string> option_dependencies_;
429+
bool option_dependencies_explicitly_set_ = false;
430+
431+
// If there's a violation of option-depend-on-what-you-option-import, this
432+
// string will be presented to the user. "%s" will be replaced with the
433+
// violating import.
434+
std::string option_dependencies_violation_msg_;
435+
426436
// output_directives_ lists all the files we are supposed to output and what
427437
// generator to use for each.
428438
struct OutputDirective {

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2481,6 +2481,27 @@ TEST_F(CommandLineInterfaceTest, DirectDependencies_Missing) {
24812481
"bar.proto\n");
24822482
}
24832483

2484+
TEST_F(CommandLineInterfaceTest,
2485+
DirectDependencies_Missing_WithOptionDependencies) {
2486+
CreateTempFile("foo.proto", R"schema(
2487+
syntax = "proto2";
2488+
import "bar.proto";
2489+
message Foo { optional Bar bar = 1; }
2490+
)schema");
2491+
CreateTempFile("bar.proto", R"schema(
2492+
syntax = "proto2";
2493+
message Bar { optional string text = 1; }
2494+
)schema");
2495+
2496+
Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir "
2497+
"--direct_dependencies= "
2498+
"--option_dependencies=bar.proto foo.proto");
2499+
2500+
ExpectErrorText(
2501+
"foo.proto: File is imported but not declared in --direct_dependencies: "
2502+
"bar.proto\n");
2503+
}
2504+
24842505
TEST_F(CommandLineInterfaceTest, DirectDependencies_NoViolation) {
24852506
CreateTempFile("foo.proto",
24862507
"syntax = \"proto2\";\n"
@@ -2549,6 +2570,93 @@ TEST_F(CommandLineInterfaceTest, DirectDependencies_CustomErrorMessage) {
25492570
ExpectErrorText("foo.proto: Bla \"bar.proto\" Bla\n");
25502571
}
25512572

2573+
TEST_F(CommandLineInterfaceTest, OptionDependencies_Missing_EmptyList) {
2574+
CreateTempFile("foo.proto", R"schema(
2575+
edition = "2024";
2576+
import option "bar.proto";
2577+
2578+
option (bar_opt) = 1;
2579+
)schema");
2580+
CreateTempFile("bar.proto", R"schema(
2581+
syntax = "proto2";
2582+
import "google/protobuf/descriptor.proto";
2583+
extend google.protobuf.FileOptions {
2584+
optional int32 bar_opt = 99990;
2585+
}
2586+
)schema");
2587+
CreateTempFile("google/protobuf/descriptor.proto",
2588+
google::protobuf::DescriptorProto::descriptor()->file()->DebugString());
2589+
2590+
Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir "
2591+
"--option_dependencies= foo.proto "
2592+
"--experimental_editions");
2593+
ExpectErrorText(
2594+
"foo.proto: File is option imported but not declared in "
2595+
"--option_dependencies: "
2596+
"bar.proto\n");
2597+
}
2598+
2599+
TEST_F(CommandLineInterfaceTest, OptionDependencies_Missing) {
2600+
CreateTempFile("foo.proto", R"schema(
2601+
edition = "2024";
2602+
import option "bar.proto";
2603+
import option "bla.proto";
2604+
2605+
option (bar_opt) = 1;
2606+
option (bla_opt) = 2;
2607+
)schema");
2608+
CreateTempFile("bar.proto", R"schema(
2609+
syntax = "proto2";
2610+
import "google/protobuf/descriptor.proto";
2611+
extend google.protobuf.FileOptions {
2612+
optional int32 bar_opt = 99990;
2613+
}
2614+
)schema");
2615+
CreateTempFile("bla.proto", R"schema(
2616+
syntax = "proto2";
2617+
import "google/protobuf/descriptor.proto";
2618+
extend google.protobuf.FileOptions {
2619+
optional int32 bla_opt = 99991;
2620+
}
2621+
)schema");
2622+
CreateTempFile("google/protobuf/descriptor.proto",
2623+
google::protobuf::DescriptorProto::descriptor()->file()->DebugString());
2624+
2625+
Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir "
2626+
"--option_dependencies=bla.proto foo.proto");
2627+
ExpectErrorText(
2628+
"foo.proto: File is option imported but not declared in "
2629+
"--option_dependencies: bar.proto\n");
2630+
}
2631+
2632+
TEST_F(CommandLineInterfaceTest,
2633+
OptionDependencies_Missing_WithDirectDependencies) {
2634+
CreateTempFile("foo.proto", R"schema(
2635+
edition = "2024";
2636+
import option "bar.proto";
2637+
2638+
option (bar_opt) = 1;
2639+
)schema");
2640+
CreateTempFile("bar.proto", R"schema(
2641+
syntax = "proto2";
2642+
import "google/protobuf/descriptor.proto";
2643+
extend google.protobuf.FileOptions {
2644+
optional int32 bar_opt = 99990;
2645+
}
2646+
)schema");
2647+
CreateTempFile("google/protobuf/descriptor.proto",
2648+
google::protobuf::DescriptorProto::descriptor()->file()->DebugString());
2649+
2650+
Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir "
2651+
"--direct_dependencies=bar.proto "
2652+
"--option_dependencies= foo.proto");
2653+
2654+
ExpectErrorText(
2655+
"foo.proto: File is option imported but not declared in "
2656+
"--option_dependencies: "
2657+
"bar.proto\n");
2658+
}
2659+
25522660
TEST_F(CommandLineInterfaceTest, CwdRelativeInputs) {
25532661
// Test that we can accept working-directory-relative input files.
25542662

0 commit comments

Comments
 (0)