diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index 0e34c948b9fe6..b2be67ca1ec4d 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -121,9 +121,12 @@ using google::protobuf::io::win32::setmode; using google::protobuf::io::win32::write; #endif -static const char* kDefaultDirectDependenciesViolationMsg = +constexpr absl::string_view kDefaultDirectDependenciesViolationMsg = "File is imported but not declared in --direct_dependencies: %s"; +constexpr absl::string_view kDefaultOptionDependenciesViolationMsg = + "File is option imported but not declared in --option_dependencies: %s"; + // Returns true if the text begins with a Windows-style absolute path, starting // with a drive letter. Example: "C:\foo". static bool StartsWithWindowsAbsolutePath(absl::string_view text) { @@ -958,7 +961,9 @@ const char* const CommandLineInterface::kPathSeparator = ":"; CommandLineInterface::CommandLineInterface() : direct_dependencies_violation_msg_( - kDefaultDirectDependenciesViolationMsg) {} + kDefaultDirectDependenciesViolationMsg), + option_dependencies_violation_msg_( + kDefaultOptionDependenciesViolationMsg) {} CommandLineInterface::~CommandLineInterface() = default; @@ -1645,6 +1650,26 @@ bool CommandLineInterface::ParseInputFiles( break; } } + + // Enforce --option_dependencies. + if (option_dependencies_explicitly_set_) { + bool indirect_option_imports = false; + for (int i = 0; i < parsed_file->option_dependency_count(); ++i) { + if (option_dependencies_.find(parsed_file->option_dependency_name(i)) == + option_dependencies_.end()) { + indirect_option_imports = true; + std::cerr << parsed_file->name() << ": " + << absl::StrReplaceAll( + option_dependencies_violation_msg_, + {{"%s", parsed_file->option_dependency_name(i)}}) + << std::endl; + } + } + if (indirect_option_imports) { + result = false; + break; + } + } } descriptor_pool->ClearDirectInputFiles(); return result; @@ -1657,7 +1682,11 @@ void CommandLineInterface::Clear() { proto_path_.clear(); input_files_.clear(); direct_dependencies_.clear(); - direct_dependencies_violation_msg_ = kDefaultDirectDependenciesViolationMsg; + direct_dependencies_violation_msg_ = + std::string(kDefaultDirectDependenciesViolationMsg); + option_dependencies_.clear(); + option_dependencies_violation_msg_ = + std::string(kDefaultOptionDependenciesViolationMsg); output_directives_.clear(); codec_type_.clear(); descriptor_set_in_names_.clear(); @@ -2139,7 +2168,23 @@ CommandLineInterface::InterpretArgument(const std::string& name, } else if (name == "--direct_dependencies_violation_msg") { direct_dependencies_violation_msg_ = value; + } else if (name == "--option_dependencies") { + if (option_dependencies_explicitly_set_) { + std::cerr << name + << " may only be passed once. To specify multiple " + "option dependencies, pass them all as a single " + "parameter separated by ':'." + << std::endl; + return PARSE_ARGUMENT_FAIL; + } + option_dependencies_explicitly_set_ = true; + std::vector direct = + absl::StrSplit(value, ':', absl::SkipEmpty()); + ABSL_DCHECK(option_dependencies_.empty()); + option_dependencies_.insert(direct.begin(), direct.end()); + } else if (name == "--option_dependencies_violation_msg") { + option_dependencies_violation_msg_ = value; } else if (name == "--descriptor_set_in") { if (!descriptor_set_in_names_.empty()) { std::cerr << name @@ -2520,7 +2565,13 @@ Parse PROTO_FILES and generate output based on the options given: are counted as occupied fields numbers. --enable_codegen_trace Enables tracing which parts of protoc are responsible for what codegen output. Not supported - by all backends or on all platforms.)"; + by all backends or on all platforms. + --direct_dependencies A colon delimited list of imports that are + allowed to be used in "import" + declarations, when explictily provided. + --option_dependencies A colon delimited list of imports that are + allowed to be used in "import option" + declarations, when explicitly provided.)"; std::cout << R"( --notices Show notice file and exit.)"; if (!plugin_prefix_.empty()) { diff --git a/src/google/protobuf/compiler/command_line_interface.h b/src/google/protobuf/compiler/command_line_interface.h index 86fff7639635f..c9243f8490484 100644 --- a/src/google/protobuf/compiler/command_line_interface.h +++ b/src/google/protobuf/compiler/command_line_interface.h @@ -423,6 +423,16 @@ class PROTOC_EXPORT CommandLineInterface { // presented to the user. "%s" will be replaced with the violating import. std::string direct_dependencies_violation_msg_; + // Names of proto files which are allowed to be option imported. Used by build + // systems to enforce option-depend-on-what-you-option-import. + absl::flat_hash_set option_dependencies_; + bool option_dependencies_explicitly_set_ = false; + + // If there's a violation of option-depend-on-what-you-option-import, this + // string will be presented to the user. "%s" will be replaced with the + // violating import. + std::string option_dependencies_violation_msg_; + // output_directives_ lists all the files we are supposed to output and what // generator to use for each. struct OutputDirective { diff --git a/src/google/protobuf/compiler/command_line_interface_tester.cc b/src/google/protobuf/compiler/command_line_interface_tester.cc index 290227864ed68..e115d2ffa9e24 100644 --- a/src/google/protobuf/compiler/command_line_interface_tester.cc +++ b/src/google/protobuf/compiler/command_line_interface_tester.cc @@ -130,11 +130,9 @@ void CommandLineInterfaceTester::ExpectNoErrors() { void CommandLineInterfaceTester::ExpectErrorText( absl::string_view expected_text) { EXPECT_NE(0, return_code_); - EXPECT_THAT(captured_stderr_, - HasSubstr(absl::StrReplaceAll(expected_text, - {{"$tmpdir", temp_directory_}}))); + EXPECT_EQ(captured_stderr_, + absl::StrReplaceAll(expected_text, {{"$tmpdir", temp_directory_}})); } - void CommandLineInterfaceTester::ExpectErrorSubstring( absl::string_view expected_substring) { EXPECT_NE(0, return_code_); diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index 33e02ba11feb1..65080cfb82a0d 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -2481,6 +2481,50 @@ TEST_F(CommandLineInterfaceTest, DirectDependencies_Missing) { "bar.proto\n"); } +TEST_F(CommandLineInterfaceTest, DirectDependencies_Missing_Multiple) { + CreateTempFile("foo.proto", + "syntax = \"proto2\";\n" + "import \"bar.proto\";\n" + "import \"bla.proto\";\n" + "message Foo { optional Bar bar = 1; optional Bla bla = 2; }"); + CreateTempFile("bar.proto", + "syntax = \"proto2\";\n" + "message Bar { optional string text = 1; }"); + CreateTempFile("bla.proto", + "syntax = \"proto2\";\n" + "message Bla { optional int64 number = 1; }"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir " + "--direct_dependencies= foo.proto"); + + ExpectErrorText( + "foo.proto: File is imported but not declared in --direct_dependencies: " + "bar.proto\n" + "foo.proto: File is imported but not declared in --direct_dependencies: " + "bla.proto\n"); +} + +TEST_F(CommandLineInterfaceTest, + DirectDependencies_Missing_WithOptionDependencies) { + CreateTempFile("foo.proto", R"schema( + syntax = "proto2"; + import "bar.proto"; + message Foo { optional Bar bar = 1; } + )schema"); + CreateTempFile("bar.proto", R"schema( + syntax = "proto2"; + message Bar { optional string text = 1; } + )schema"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir " + "--direct_dependencies= " + "--option_dependencies=bar.proto foo.proto"); + + ExpectErrorText( + "foo.proto: File is imported but not declared in --direct_dependencies: " + "bar.proto\n"); +} + TEST_F(CommandLineInterfaceTest, DirectDependencies_NoViolation) { CreateTempFile("foo.proto", "syntax = \"proto2\";\n" @@ -2549,6 +2593,229 @@ TEST_F(CommandLineInterfaceTest, DirectDependencies_CustomErrorMessage) { ExpectErrorText("foo.proto: Bla \"bar.proto\" Bla\n"); } +TEST_F(CommandLineInterfaceTest, OptionDependencies_Missing_EmptyList) { + CreateTempFile("foo.proto", R"schema( + edition = "2024"; + import option "bar.proto"; + + option (bar_opt) = 1; + )schema"); + CreateTempFile("bar.proto", R"schema( + syntax = "proto2"; + import "google/protobuf/descriptor.proto"; + extend google.protobuf.FileOptions { + optional int32 bar_opt = 99990; + } + )schema"); + CreateTempFile("google/protobuf/descriptor.proto", + google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir " + "--option_dependencies= foo.proto " + "--experimental_editions"); + ExpectErrorText( + "foo.proto: File is option imported but not declared in " + "--option_dependencies: " + "bar.proto\n"); +} + +TEST_F(CommandLineInterfaceTest, OptionDependencies_Missing) { + CreateTempFile("foo.proto", R"schema( + edition = "2024"; + import option "bar.proto"; + import option "bla.proto"; + + option (bar_opt) = 1; + option (bla_opt) = 2; + )schema"); + CreateTempFile("bar.proto", R"schema( + syntax = "proto2"; + import "google/protobuf/descriptor.proto"; + extend google.protobuf.FileOptions { + optional int32 bar_opt = 99990; + } + )schema"); + CreateTempFile("bla.proto", R"schema( + syntax = "proto2"; + import "google/protobuf/descriptor.proto"; + extend google.protobuf.FileOptions { + optional int32 bla_opt = 99991; + } + )schema"); + CreateTempFile("google/protobuf/descriptor.proto", + google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir " + "--option_dependencies=bla.proto foo.proto"); + ExpectErrorText( + "foo.proto: File is option imported but not declared in " + "--option_dependencies: bar.proto\n"); +} + +TEST_F(CommandLineInterfaceTest, OptionDependencies_Missing_Multiple) { + CreateTempFile("foo.proto", R"schema( + edition = "2024"; + import option "bar.proto"; + import option "bla.proto"; + + option (bar_opt) = 1; + option (bla_opt) = 2; + )schema"); + CreateTempFile("bar.proto", R"schema( + syntax = "proto2"; + import "google/protobuf/descriptor.proto"; + extend google.protobuf.FileOptions { + optional int32 bar_opt = 99990; + } + )schema"); + CreateTempFile("bla.proto", R"schema( + syntax = "proto2"; + import "google/protobuf/descriptor.proto"; + extend google.protobuf.FileOptions { + optional int32 bla_opt = 99991; + } + )schema"); + CreateTempFile("google/protobuf/descriptor.proto", + google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir " + "--option_dependencies= foo.proto"); + ExpectErrorText( + "foo.proto: File is option imported but not declared in " + "--option_dependencies: bar.proto\n" + "foo.proto: File is option imported but not declared in " + "--option_dependencies: bla.proto\n"); +} + +TEST_F(CommandLineInterfaceTest, + OptionDependencies_Missing_WithDirectDependencies) { + CreateTempFile("foo.proto", R"schema( + edition = "2024"; + import option "bar.proto"; + + option (bar_opt) = 1; + )schema"); + CreateTempFile("bar.proto", R"schema( + syntax = "proto2"; + import "google/protobuf/descriptor.proto"; + extend google.protobuf.FileOptions { + optional int32 bar_opt = 99990; + } + )schema"); + CreateTempFile("google/protobuf/descriptor.proto", + google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir " + "--direct_dependencies=bar.proto " + "--option_dependencies= foo.proto"); + + ExpectErrorText( + "foo.proto: File is option imported but not declared in " + "--option_dependencies: " + "bar.proto\n"); +} + +TEST_F(CommandLineInterfaceTest, OptionDependencies_NoViolation) { + CreateTempFile("foo.proto", R"schema( + edition = "2024"; + import option "bar.proto"; + + option (bar_opt) = 1; + )schema"); + CreateTempFile("bar.proto", R"schema( + syntax = "proto2"; + import "google/protobuf/descriptor.proto"; + extend google.protobuf.FileOptions { + optional int32 bar_opt = 99990; + } + )schema"); + CreateTempFile("google/protobuf/descriptor.proto", + google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir " + "--option_dependencies=bar.proto foo.proto " + "--experimental_editions"); + + ExpectNoErrors(); +} + +TEST_F(CommandLineInterfaceTest, OptionDependencies_NoViolation_MultiImports) { + CreateTempFile("foo.proto", R"schema( + edition = "2024"; + import option "bar.proto"; + import option "bla.proto"; + + option (bar_opt) = 1; + option (bla_opt) = 2; + )schema"); + CreateTempFile("bar.proto", R"schema( + syntax = "proto2"; + import "google/protobuf/descriptor.proto"; + extend google.protobuf.FileOptions { + optional int32 bar_opt = 99990; + } + )schema"); + CreateTempFile("bla.proto", R"schema( + syntax = "proto2"; + import "google/protobuf/descriptor.proto"; + extend google.protobuf.FileOptions { + optional int32 bla_opt = 99991; + } + )schema"); + CreateTempFile("google/protobuf/descriptor.proto", + google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir " + "--option_dependencies=bar.proto:bla.proto foo.proto " + "--experimental_editions"); + + ExpectNoErrors(); +} + +TEST_F(CommandLineInterfaceTest, OptionDependencies_ProvidedMultipleTimes) { + CreateTempFile("foo.proto", "edition = \"2024\";\n"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir " + "--option_dependencies=bar.proto --option_dependencies=bla.proto " + "foo.proto " + "--experimental_editions"); + + ExpectErrorText( + "--option_dependencies may only be passed once. To specify multiple " + "option dependencies, pass them all as a single parameter separated by " + "':'.\n"); +} + +TEST_F(CommandLineInterfaceTest, OptionDependencies_CustomErrorMessage) { + CreateTempFile("foo.proto", R"schema( + edition = "2024"; + import option "bar.proto"; + + option (bar_opt) = 1; + )schema"); + CreateTempFile("bar.proto", R"schema( + syntax = "proto2"; + import "google/protobuf/descriptor.proto"; + extend google.protobuf.FileOptions { + optional int32 bar_opt = 99990; + } + )schema"); + CreateTempFile("google/protobuf/descriptor.proto", + google::protobuf::DescriptorProto::descriptor()->file()->DebugString()); + + std::vector commands; + commands.push_back("protocol_compiler"); + commands.push_back("--test_out=$tmpdir"); + commands.push_back("--proto_path=$tmpdir"); + commands.push_back("--option_dependencies="); + commands.push_back("--option_dependencies_violation_msg=Bla \"%s\" Bla"); + commands.push_back("foo.proto"); + commands.push_back("--experimental_editions"); + RunWithArgs(commands); + + ExpectErrorText("foo.proto: Bla \"bar.proto\" Bla\n"); +} + TEST_F(CommandLineInterfaceTest, CwdRelativeInputs) { // Test that we can accept working-directory-relative input files. @@ -2973,8 +3240,9 @@ TEST_F(CommandLineInterfaceTest, ParseErrors) { Run("protocol_compiler --test_out=$tmpdir " "--proto_path=$tmpdir foo.proto"); - ExpectErrorText( - "foo.proto:2:1: Expected top-level statement (e.g. \"message\").\n"); + ExpectErrorSubstring( + "foo.proto:2:1: Expected top-level statement (e.g. " + "\"message\").\n"); } TEST_F(CommandLineInterfaceTest, ParseErrors_DescriptorSetIn) { @@ -3007,14 +3275,18 @@ TEST_F(CommandLineInterfaceTest, ParseErrorsMultipleFiles) { Run("protocol_compiler --test_out=$tmpdir " "--proto_path=$tmpdir foo.proto"); - ExpectErrorText( - "bar.proto:2:1: Expected top-level statement (e.g. \"message\").\n"); - ExpectErrorText( - "baz.proto:2:1: Import \"bar.proto\" was not found or had errors.\n"); - ExpectErrorText( - "foo.proto:2:1: Import \"bar.proto\" was not found or had errors.\n"); - ExpectErrorText( - "foo.proto:3:1: Import \"baz.proto\" was not found or had errors.\n"); + ExpectErrorSubstring( + "bar.proto:2:1: Expected top-level statement (e.g. " + "\"message\").\n"); + ExpectErrorSubstring( + "baz.proto:2:1: Import \"bar.proto\" was not found or had " + "errors.\n"); + ExpectErrorSubstring( + "foo.proto:2:1: Import \"bar.proto\" was not found or had " + "errors.\n"); + ExpectErrorSubstring( + "foo.proto:3:1: Import \"baz.proto\" was not found or had " + "errors.\n"); } TEST_F(CommandLineInterfaceTest, RecursiveImportFails) { @@ -3477,8 +3749,9 @@ TEST_F(CommandLineInterfaceTest, GccFormatErrors) { Run("protocol_compiler --test_out=$tmpdir " "--proto_path=$tmpdir --error_format=gcc foo.proto"); - ExpectErrorText( - "foo.proto:2:1: Expected top-level statement (e.g. \"message\").\n"); + ExpectErrorSubstring( + "foo.proto:2:1: Expected top-level statement (e.g. " + "\"message\").\n"); } TEST_F(CommandLineInterfaceTest, MsvsFormatErrors) {