Merge pull request #5791 from protocolbuffers/revert-5735-ruby-pkg-namespace

Revert "Fix Ruby module name generation when the ruby_package option is used"

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 {}

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 {}

src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit.proto

-syntax = "proto3";
-
-package one.two.a_three;
-
-option ruby_package = "A::B::C";
-
-message Four {
-  string a_string = 1;
-}

src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_legacy.proto

-syntax = "proto3";
-
-package one.two.a_three.and;
-
-option ruby_package = "AA.BB.CC";
-
-message Four {
-  string another_string = 1;
-}

src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_legacy_pb.rb

-# 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

src/google/protobuf/compiler/ruby/ruby_generated_pkg_explicit_pb.rb

-# 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

src/google/protobuf/compiler/ruby/ruby_generated_pkg_implicit.proto

-syntax = "proto3";
-
-package one.two.a_three;
-
-message Four {
-  string a_string = 1;
-}

src/google/protobuf/compiler/ruby/ruby_generated_pkg_implicit_pb.rb

-# 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

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);

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 <memory>
-#include <list>
 
 #include <google/protobuf/compiler/ruby/ruby_generator.h>
 #include <google/protobuf/compiler/command_line_interface.h>
@@ -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, "");
+
+  // 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));
 
-TEST(RubyGeneratorTest, Proto3ExplictLegacyPackageTest) {
-    RubyTest("/ruby_generated_pkg_explicit_legacy");
+  // 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