Merge pull request #1788 from google/rubypackagecap

Ruby: translate package names from snake_case -> PascalCase.

6 files changed, 58 insertions, 7 deletions

diff --git a/Makefile.am b/Makefile.am
index 9dd749c6..ccbfd065 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -705,6 +705,7 @@ ruby_EXTRA_DIST=                                                             \
   ruby/tests/repeated_field_test.rb                                          \
   ruby/tests/stress.rb                                                       \
   ruby/tests/generated_code.proto                                            \
+  ruby/tests/test_import.proto                                               \
   ruby/tests/generated_code_test.rb                                          \
   ruby/travis-test.sh
 
diff --git a/ruby/Rakefile b/ruby/Rakefile
index f6354e1e..ba1cf4cf 100644
--- a/ruby/Rakefile
+++ b/ruby/Rakefile
@@ -80,10 +80,15 @@ end
 
 # Proto for tests.
 genproto_output << "tests/generated_code.rb"
+genproto_output << "tests/test_import.rb"
 file "tests/generated_code.rb" => "tests/generated_code.proto" do |file_task|
   sh "../src/protoc --ruby_out=. tests/generated_code.proto"
 end
 
+file "tests/test_import.rb" => "tests/test_import.proto" do |file_task|
+  sh "../src/protoc --ruby_out=. tests/test_import.proto"
+end
+
 task :genproto => genproto_output
 
 task :clean do
diff --git a/ruby/tests/generated_code.proto b/ruby/tests/generated_code.proto
index 42d82a6b..62fd83ed 100644
--- a/ruby/tests/generated_code.proto
+++ b/ruby/tests/generated_code.proto
@@ -1,6 +1,6 @@
 syntax = "proto3";
 
-package A.B.C;
+package a.b.c;
 
 message TestMessage {
   int32 optional_int32 = 1;
diff --git a/ruby/tests/generated_code_test.rb b/ruby/tests/generated_code_test.rb
index 26bafdd6..b92b0462 100644
--- a/ruby/tests/generated_code_test.rb
+++ b/ruby/tests/generated_code_test.rb
@@ -4,6 +4,7 @@
 $LOAD_PATH.unshift(File.expand_path(File.dirname(__FILE__)))
 
 require 'generated_code_pb'
+require 'test_import_pb'
 require 'test/unit'
 
 class GeneratedCodeTest < Test::Unit::TestCase
@@ -13,5 +14,6 @@ class GeneratedCodeTest < Test::Unit::TestCase
     # successfully creates message definitions and classes, not to test every
     # aspect of the extension (basic.rb is for that).
     m = A::B::C::TestMessage.new()
+    m2 = FooBar::TestImportedMessage.new()
   end
 end
diff --git a/ruby/tests/test_import.proto b/ruby/tests/test_import.proto
new file mode 100644
index 00000000..230484ee
--- /dev/null
+++ b/ruby/tests/test_import.proto
@@ -0,0 +1,5 @@
+syntax = "proto3";
+
+package foo_bar;
+
+message TestImportedMessage {}
diff --git a/src/google/protobuf/compiler/ruby/ruby_generator.cc b/src/google/protobuf/compiler/ruby/ruby_generator.cc
index 8813aec7..fbe3b4cb 100644
--- a/src/google/protobuf/compiler/ruby/ruby_generator.cc
+++ b/src/google/protobuf/compiler/ruby/ruby_generator.cc
@@ -237,15 +237,52 @@ void GenerateEnum(const google::protobuf::EnumDescriptor* en,
     "end\n");
 }
 
-// Module names, class names, and enum value names need to be Ruby constants,
-// which must start with a capital letter.
+// Locale-agnostic utility functions.
+bool IsLower(char ch) { return ch >= 'a' && ch <= 'z'; }
+
+bool IsUpper(char ch) { return ch >= 'A' && ch <= 'Z'; }
+
+bool IsAlpha(char ch) { return IsLower(ch) || IsUpper(ch); }
+
+char ToUpper(char ch) { return IsLower(ch) ? (ch - 'a' + 'A') : ch; }
+
+
+// Package names in protobuf are snake_case by convention, but Ruby module
+// names must be PascalCased.
+//
+//   foo_bar_baz -> FooBarBaz
+std::string PackageToModule(const std::string& name) {
+  bool next_upper = true;
+  std::string result;
+  result.reserve(name.size());
+
+  for (int i = 0; i < name.size(); i++) {
+    if (name[i] == '_') {
+      next_upper = true;
+    } else {
+      if (next_upper) {
+        result.push_back(ToUpper(name[i]));
+      } else {
+        result.push_back(name[i]);
+      }
+      next_upper = false;
+    }
+  }
+
+  return result;
+}
+
+// Class and enum names in protobuf should be PascalCased by convention, but
+// since there is nothing enforcing this we need to ensure that they are valid
+// Ruby constants.  That mainly means making sure that the first character is
+// an upper-case letter.
 std::string RubifyConstant(const std::string& name) {
   std::string ret = name;
   if (!ret.empty()) {
-    if (ret[0] >= 'a' && ret[0] <= 'z') {
+    if (IsLower(ret[0])) {
       // If it starts with a lowercase letter, capitalize it.
-      ret[0] = ret[0] - 'a' + 'A';
-    } else if (ret[0] < 'A' || ret[0] > 'Z') {
+      ret[0] = ToUpper(ret[0]);
+    } else if (!IsAlpha(ret[0])) {
       // Otherwise (e.g. if it begins with an underscore), we need to come up
       // with some prefix that starts with a capital letter. We could be smarter
       // here, e.g. try to strip leading underscores, but this may cause other
@@ -254,6 +291,7 @@ std::string RubifyConstant(const std::string& name) {
       ret = "PB_" + ret;
     }
   }
+
   return ret;
 }
 
@@ -314,7 +352,7 @@ int GeneratePackageModules(
       component = package_name.substr(0, dot_index);
       package_name = package_name.substr(dot_index + 1);
     }
-    component = RubifyConstant(component);
+    component = PackageToModule(component);
     printer->Print(
       "module $name$\n",
       "name", component);