Allow Java reserved keywords to be used in extensions (#5709)

* add check for reserved keywords in extensions

* add all reserved keywords

* use string without std::

* add test and update name in lite version

* test updates

java/core/src/test/java/com/google/protobuf/ExtensionRegistryFactoryTest.java

@@ -30,6 +30,8 @@
 
 package com.google.protobuf;
 
+import protobuf_unittest.NestedExtension;
+import protobuf_unittest.NestedExtensionLite;
 import protobuf_unittest.NonNestedExtension;
 import protobuf_unittest.NonNestedExtensionLite;
 import java.lang.reflect.Method;
@@ -69,6 +71,8 @@ public class ExtensionRegistryFactoryTest extends TestCase {
     void testAdd();
 
     void testAdd_immutable();
+
+    void testExtensionRenamesKeywords();
   }
 
   /** Test implementations for the non-Lite usage of ExtensionRegistryFactory. */
@@ -156,6 +160,23 @@ public class ExtensionRegistryFactoryTest extends TestCase {
       } catch (IllegalArgumentException expected) {
       }
     }
+
+    @Override
+    public void testExtensionRenamesKeywords() {
+      assertTrue(NonNestedExtension.if_ instanceof GeneratedMessage.GeneratedExtension);
+      assertTrue(NestedExtension.MyNestedExtension.default_ instanceof GeneratedMessage.GeneratedExtension);
+
+      NonNestedExtension.MessageToBeExtended msg =
+              NonNestedExtension.MessageToBeExtended.newBuilder()
+                      .setExtension(NonNestedExtension.if_, "!fi")
+                      .build();
+      assertEquals("!fi", msg.getExtension(NonNestedExtension.if_));
+
+      msg = NonNestedExtension.MessageToBeExtended.newBuilder()
+              .setExtension(NestedExtension.MyNestedExtension.default_, 8)
+              .build();
+      assertEquals(8, msg.getExtension(NestedExtension.MyNestedExtension.default_).intValue());
+    }
   }
 
   /** Test implementations for the Lite usage of ExtensionRegistryFactory. */
@@ -202,6 +223,23 @@ public class ExtensionRegistryFactoryTest extends TestCase {
       } catch (UnsupportedOperationException expected) {
       }
     }
+
+    @Override
+    public void testExtensionRenamesKeywords() {
+      assertTrue(NonNestedExtensionLite.package_ instanceof GeneratedMessageLite.GeneratedExtension);
+      assertTrue(NestedExtensionLite.MyNestedExtensionLite.private_ instanceof GeneratedMessageLite.GeneratedExtension);
+
+      NonNestedExtensionLite.MessageLiteToBeExtended msg =
+              NonNestedExtensionLite.MessageLiteToBeExtended.newBuilder()
+              .setExtension(NonNestedExtensionLite.package_, true)
+              .build();
+      assertTrue(msg.getExtension(NonNestedExtensionLite.package_));
+
+      msg = NonNestedExtensionLite.MessageLiteToBeExtended.newBuilder()
+              .setExtension(NestedExtensionLite.MyNestedExtensionLite.private_, 2.4)
+              .build();
+      assertEquals(2.4, msg.getExtension(NestedExtensionLite.MyNestedExtensionLite.private_), 0.001);
+    }
   }
 
   /** Defines a suite of tests which the JUnit3 runner retrieves by reflection. */

java/core/src/test/proto/com/google/protobuf/nested_extension.proto

@@ -43,5 +43,6 @@ package protobuf_unittest;
 message MyNestedExtension {
   extend MessageToBeExtended {
     optional MessageToBeExtended recursiveExtension = 2;
+    optional int32 default = 2002;
   }
 }

java/core/src/test/proto/com/google/protobuf/nested_extension_lite.proto

@@ -45,5 +45,6 @@ import "com/google/protobuf/non_nested_extension_lite.proto";
 message MyNestedExtensionLite {
   extend MessageLiteToBeExtended {
     optional MessageLiteToBeExtended recursiveExtensionLite = 3;
+    optional double private = 2004;
   }
 }

java/core/src/test/proto/com/google/protobuf/non_nested_extension.proto

@@ -45,4 +45,5 @@ message MyNonNestedExtension {}
 
 extend MessageToBeExtended {
   optional MyNonNestedExtension nonNestedExtension = 1;
+  optional string if = 2000;
 }

java/core/src/test/proto/com/google/protobuf/non_nested_extension_lite.proto

@@ -46,4 +46,5 @@ message MyNonNestedExtensionLite {}
 
 extend MessageLiteToBeExtended {
   optional MyNonNestedExtensionLite nonNestedExtensionLite = 1;
+  optional bool package = 2006;
 }

src/google/protobuf/compiler/java/java_extension.cc

@@ -68,7 +68,7 @@ void ExtensionGenerator::InitTemplateVars(
     std::map<std::string, std::string>* vars_pointer) {
   std::map<std::string, std::string>& vars = *vars_pointer;
   vars["scope"] = scope;
-  vars["name"] = UnderscoresToCamelCase(descriptor);
+  vars["name"] = UnderscoresToCamelCaseCheckReserved(descriptor);
   vars["containing_type"] =
       name_resolver->GetClassName(descriptor->containing_type(), immutable);
   vars["number"] = StrCat(descriptor->number());
@@ -153,7 +153,7 @@ int ImmutableExtensionGenerator::GenerateNonNestedInitializationCode(
     // Only applies to non-nested extensions.
     printer->Print(
         "$name$.internalInit(descriptor.getExtensions().get($index$));\n",
-        "name", UnderscoresToCamelCase(descriptor_), "index",
+        "name", UnderscoresToCamelCaseCheckReserved(descriptor_), "index",
         StrCat(descriptor_->index()));
     bytecode_estimate += 21;
   }
@@ -165,7 +165,7 @@ int ImmutableExtensionGenerator::GenerateRegistrationCode(
   printer->Print(
     "registry.add($scope$.$name$);\n",
     "scope", scope_,
-    "name", UnderscoresToCamelCase(descriptor_));
+    "name", UnderscoresToCamelCaseCheckReserved(descriptor_));
   return 7;
 }
 

src/google/protobuf/compiler/java/java_extension_lite.cc

@@ -109,7 +109,7 @@ int ImmutableExtensionLiteGenerator::GenerateRegistrationCode(
   printer->Print(
     "registry.add($scope$.$name$);\n",
     "scope", scope_,
-    "name", UnderscoresToCamelCase(descriptor_));
+    "name", UnderscoresToCamelCaseCheckReserved(descriptor_));
   return 7;
 }
 

src/google/protobuf/compiler/java/java_helpers.cc

@@ -76,6 +76,16 @@ const char* kForbiddenWordList[] = {
   "class",
 };
 
+const std::unordered_set<string> kReservedNames = {
+  "abstract", "assert", "boolean", "break", "byte", "case", "catch", "char",
+  "class", "const", "continue", "default", "do", "double", "else", "enum",
+  "extends", "final", "finally", "float", "for", "goto", "if", "implements",
+  "import", "instanceof", "int", "interface", "long", "native", "new", "package",
+  "private", "protected", "public", "return", "short", "static", "strictfp", "super",
+  "switch", "synchronized", "this", "throw", "throws", "transient", "try", "void", 
+  "volatile", "while",
+};
+
 const int kDefaultLookUpStartFieldNumber = 40;
 
 bool IsForbidden(const std::string& field_name) {
@@ -195,6 +205,14 @@ std::string UnderscoresToCamelCase(const MethodDescriptor* method) {
   return UnderscoresToCamelCase(method->name(), false);
 }
 
+std::string UnderscoresToCamelCaseCheckReserved(const FieldDescriptor* field) {
+  std::string name = UnderscoresToCamelCase(field);
+  if (kReservedNames.find(name) != kReservedNames.end()) {
+    return name + "_";
+  }
+  return name;
+}
+
 std::string UniqueFileScopeIdentifier(const Descriptor* descriptor) {
   return "static_" + StringReplace(descriptor->full_name(), ".", "_", true);
 }

src/google/protobuf/compiler/java/java_helpers.h

@@ -83,6 +83,9 @@ std::string UnderscoresToCapitalizedCamelCase(const FieldDescriptor* field);
 // of lower-casing the first letter of the name.)
 std::string UnderscoresToCamelCase(const MethodDescriptor* method);
 
+// Same as UnderscoresToCamelCase, but checks for reserved keywords
+std::string UnderscoresToCamelCaseCheckReserved(const FieldDescriptor* field);
+
 // Similar to UnderscoresToCamelCase, but guarentees that the result is a
 // complete Java identifier by adding a _ if needed.
 std::string CamelCaseFieldName(const FieldDescriptor* field);