From bb211e851ec2158e479784bd68784784bf6594dc Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Wed, 27 Feb 2019 20:56:03 -0800 Subject: [PATCH] Revert "Fix Ruby module name generation when the ruby_package option is used (#5735)" This reverts commit 9638dcc340a83996e88b4e6f4f372572bb33d19c. --- ruby/tests/test_ruby_package.proto | 2 +- ruby/tests/test_ruby_package_proto2.proto | 2 +- .../ruby/ruby_generated_pkg_explicit.proto | 9 --- .../ruby_generated_pkg_explicit_legacy.proto | 9 --- .../ruby_generated_pkg_explicit_legacy_pb.rb | 20 ------ .../ruby/ruby_generated_pkg_explicit_pb.rb | 20 ------ .../ruby/ruby_generated_pkg_implicit.proto | 7 -- .../ruby/ruby_generated_pkg_implicit_pb.rb | 20 ------ .../protobuf/compiler/ruby/ruby_generator.cc | 27 ++------ .../compiler/ruby/ruby_generator_unittest.cc | 68 +++++++++++++------ 10 files changed, 53 insertions(+), 131 deletions(-) delete mode 100644 src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit.proto delete mode 100644 src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_legacy.proto delete mode 100644 src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_legacy_pb.rb delete mode 100644 src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_pb.rb delete mode 100644 src/google/protobuf/compiler/ruby/ruby_generated_pkg_implicit.proto delete mode 100644 src/google/protobuf/compiler/ruby/ruby_generated_pkg_implicit_pb.rb diff --git a/ruby/tests/test_ruby_package.proto b/ruby/tests/test_ruby_package.proto index 54b7aca50..b87256202 100644 --- a/ruby/tests/test_ruby_package.proto +++ b/ruby/tests/test_ruby_package.proto @@ -2,6 +2,6 @@ syntax = "proto3"; package foo_bar; -option ruby_package = "A::B"; +option ruby_package = "A.B"; message TestRubyPackageMessage {} diff --git a/ruby/tests/test_ruby_package_proto2.proto b/ruby/tests/test_ruby_package_proto2.proto index c55bde435..4309044e5 100644 --- a/ruby/tests/test_ruby_package_proto2.proto +++ b/ruby/tests/test_ruby_package_proto2.proto @@ -2,6 +2,6 @@ syntax = "proto2"; package foo_bar_proto2; -option ruby_package = "A::B::Proto2"; +option ruby_package = "A.B.Proto2"; message TestRubyPackageMessage {} diff --git a/src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit.proto b/src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit.proto deleted file mode 100644 index 8d7c948a1..000000000 --- a/src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit.proto +++ /dev/null @@ -1,9 +0,0 @@ -syntax = "proto3"; - -package one.two.a_three; - -option ruby_package = "A::B::C"; - -message Four { - string a_string = 1; -} diff --git a/src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_legacy.proto b/src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_legacy.proto deleted file mode 100644 index 7a0d26086..000000000 --- a/src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_legacy.proto +++ /dev/null @@ -1,9 +0,0 @@ -syntax = "proto3"; - -package one.two.a_three.and; - -option ruby_package = "AA.BB.CC"; - -message Four { - string another_string = 1; -} diff --git a/src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_legacy_pb.rb b/src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_legacy_pb.rb deleted file mode 100644 index 74f3bf3d7..000000000 --- a/src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_legacy_pb.rb +++ /dev/null @@ -1,20 +0,0 @@ -# Generated by the protocol buffer compiler. DO NOT EDIT! -# source: ruby_generated_pkg_explicit_legacy.proto - -require 'google/protobuf' - -Google::Protobuf::DescriptorPool.generated_pool.build do - add_file("ruby_generated_pkg_explicit_legacy.proto", :syntax => :proto3) do - add_message "one.two.a_three.and.Four" do - optional :another_string, :string, 1 - end - end -end - -module AA - module BB - module CC - Four = Google::Protobuf::DescriptorPool.generated_pool.lookup("one.two.a_three.and.Four").msgclass - end - end -end diff --git a/src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_pb.rb b/src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_pb.rb deleted file mode 100644 index 24ff21e17..000000000 --- a/src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_pb.rb +++ /dev/null @@ -1,20 +0,0 @@ -# Generated by the protocol buffer compiler. DO NOT EDIT! -# source: ruby_generated_pkg_explicit.proto - -require 'google/protobuf' - -Google::Protobuf::DescriptorPool.generated_pool.build do - add_file("ruby_generated_pkg_explicit.proto", :syntax => :proto3) do - add_message "one.two.a_three.Four" do - optional :a_string, :string, 1 - end - end -end - -module A - module B - module C - Four = Google::Protobuf::DescriptorPool.generated_pool.lookup("one.two.a_three.Four").msgclass - end - end -end diff --git a/src/google/protobuf/compiler/ruby/ruby_generated_pkg_implicit.proto b/src/google/protobuf/compiler/ruby/ruby_generated_pkg_implicit.proto deleted file mode 100644 index 544db64d9..000000000 --- a/src/google/protobuf/compiler/ruby/ruby_generated_pkg_implicit.proto +++ /dev/null @@ -1,7 +0,0 @@ -syntax = "proto3"; - -package one.two.a_three; - -message Four { - string a_string = 1; -} diff --git a/src/google/protobuf/compiler/ruby/ruby_generated_pkg_implicit_pb.rb b/src/google/protobuf/compiler/ruby/ruby_generated_pkg_implicit_pb.rb deleted file mode 100644 index 3a1dd67d2..000000000 --- a/src/google/protobuf/compiler/ruby/ruby_generated_pkg_implicit_pb.rb +++ /dev/null @@ -1,20 +0,0 @@ -# Generated by the protocol buffer compiler. DO NOT EDIT! -# source: ruby_generated_pkg_implicit.proto - -require 'google/protobuf' - -Google::Protobuf::DescriptorPool.generated_pool.build do - add_file("ruby_generated_pkg_implicit.proto", :syntax => :proto3) do - add_message "one.two.a_three.Four" do - optional :a_string, :string, 1 - end - end -end - -module One - module Two - module AThree - Four = Google::Protobuf::DescriptorPool.generated_pool.lookup("one.two.a_three.Four").msgclass - end - end -end diff --git a/src/google/protobuf/compiler/ruby/ruby_generator.cc b/src/google/protobuf/compiler/ruby/ruby_generator.cc index 1079dc319..c5d9809d4 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generator.cc +++ b/src/google/protobuf/compiler/ruby/ruby_generator.cc @@ -416,43 +416,26 @@ int GeneratePackageModules( const FileDescriptor* file, google::protobuf::io::Printer* printer) { int levels = 0; - bool need_change_to_module = true; + bool need_change_to_module; std::string package_name; - // Determine the name to use in either format: - // proto package: one.two.three - // option ruby_package: One::Two::Three if (file->options().has_ruby_package()) { package_name = file->options().ruby_package(); - - // If :: is in the package use the Ruby formated name as-is - // -> A::B::C - // otherwise, use the dot seperator - // -> A.B.C - if (package_name.find("::") != std::string::npos) { - need_change_to_module = false; - } else { - GOOGLE_LOG(WARNING) << "ruby_package option should be in the form of:" - << " 'A::B::C' and not 'A.B.C'"; - } + need_change_to_module = false; } else { package_name = file->package(); + need_change_to_module = true; } - // Use the appropriate delimter - string delimiter = need_change_to_module ? "." : "::"; - int delimiter_size = need_change_to_module ? 1 : 2; - - // Extract each module name and indent while (!package_name.empty()) { - size_t dot_index = package_name.find(delimiter); + size_t dot_index = package_name.find("."); string component; if (dot_index == string::npos) { component = package_name; package_name = ""; } else { component = package_name.substr(0, dot_index); - package_name = package_name.substr(dot_index + delimiter_size); + package_name = package_name.substr(dot_index + 1); } if (need_change_to_module) { component = PackageToModule(component); diff --git a/src/google/protobuf/compiler/ruby/ruby_generator_unittest.cc b/src/google/protobuf/compiler/ruby/ruby_generator_unittest.cc index d93a68d9a..2e9b2e129 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generator_unittest.cc +++ b/src/google/protobuf/compiler/ruby/ruby_generator_unittest.cc @@ -29,7 +29,6 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include -#include #include #include @@ -57,7 +56,7 @@ string FindRubyTestDir() { // Some day, we may integrate build systems between protoc and the language // extensions to the point where we can do this test in a more automated way. -void RubyTest(string proto_file) { +TEST(RubyGeneratorTest, Proto3GeneratorTest) { string ruby_tests = FindRubyTestDir(); google::protobuf::compiler::CommandLineInterface cli; @@ -69,23 +68,22 @@ void RubyTest(string proto_file) { // Copy generated_code.proto to the temporary test directory. string test_input; GOOGLE_CHECK_OK(File::GetContents( - ruby_tests + proto_file + ".proto", + ruby_tests + "/ruby_generated_code.proto", &test_input, true)); GOOGLE_CHECK_OK(File::SetContents( - TestTempDir() + proto_file + ".proto", + TestTempDir() + "/ruby_generated_code.proto", test_input, true)); // Invoke the proto compiler (we will be inside TestTempDir() at this point). string ruby_out = "--ruby_out=" + TestTempDir(); string proto_path = "--proto_path=" + TestTempDir(); - string proto_target = TestTempDir() + proto_file + ".proto"; const char* argv[] = { "protoc", ruby_out.c_str(), proto_path.c_str(), - proto_target.c_str(), + "ruby_generated_code.proto", }; EXPECT_EQ(0, cli.Run(4, argv)); @@ -93,35 +91,61 @@ void RubyTest(string proto_file) { // Load the generated output and compare to the expected result. string output; GOOGLE_CHECK_OK(File::GetContentsAsText( - TestTempDir() + proto_file + "_pb.rb", + TestTempDir() + "/ruby_generated_code_pb.rb", &output, true)); string expected_output; GOOGLE_CHECK_OK(File::GetContentsAsText( - ruby_tests + proto_file + "_pb.rb", + ruby_tests + "/ruby_generated_code_pb.rb", &expected_output, true)); EXPECT_EQ(expected_output, output); } -TEST(RubyGeneratorTest, Proto3GeneratorTest) { - RubyTest("/ruby_generated_code"); -} - TEST(RubyGeneratorTest, Proto2GeneratorTest) { - RubyTest("/ruby_generated_code_proto2"); -} + string ruby_tests = FindRubyTestDir(); -TEST(RubyGeneratorTest, Proto3ImplicitPackageTest) { - RubyTest("/ruby_generated_pkg_implicit"); -} + google::protobuf::compiler::CommandLineInterface cli; + cli.SetInputsAreProtoPathRelative(true); -TEST(RubyGeneratorTest, Proto3ExplictPackageTest) { - RubyTest("/ruby_generated_pkg_explicit"); -} + ruby::Generator ruby_generator; + cli.RegisterGenerator("--ruby_out", &ruby_generator, ""); -TEST(RubyGeneratorTest, Proto3ExplictLegacyPackageTest) { - RubyTest("/ruby_generated_pkg_explicit_legacy"); + // Copy generated_code.proto to the temporary test directory. + string test_input; + GOOGLE_CHECK_OK(File::GetContents( + ruby_tests + "/ruby_generated_code_proto2.proto", + &test_input, + true)); + GOOGLE_CHECK_OK(File::SetContents( + TestTempDir() + "/ruby_generated_code_proto2.proto", + test_input, + true)); + + // Invoke the proto compiler (we will be inside TestTempDir() at this point). + string ruby_out = "--ruby_out=" + TestTempDir(); + string proto_path = "--proto_path=" + TestTempDir(); + const char* argv[] = { + "protoc", + ruby_out.c_str(), + proto_path.c_str(), + "ruby_generated_code_proto2.proto", + }; + + EXPECT_EQ(0, cli.Run(4, argv)); + + // Load the generated output and compare to the expected result. + string output; + GOOGLE_CHECK_OK(File::GetContents( + TestTempDir() + "/ruby_generated_code_proto2_pb.rb", + &output, + true)); + string expected_output; + GOOGLE_CHECK_OK(File::GetContents( + ruby_tests + "/ruby_generated_code_proto2_pb.rb", + &expected_output, + true)); + EXPECT_EQ(expected_output, output); } } // namespace