Nano: don't generate accessor methods for nested methods

For nested message objects, don't generate accessor methods because they have
a default value that is not a valid value (null), so there is no reason to have
get/set/has/clear methods for them. Clients and protos (while serializing) can
check against the invalid value to see if it's been set.

Change-Id: Ic63400889581271b8cbcd9c45c84519d4921fd4b

5 files changed, 53 insertions, 138 deletions

diff --git a/java/README.txt b/java/README.txt
index 976ec84..ced78d2 100644
--- a/java/README.txt
+++ b/java/README.txt
@@ -507,9 +507,9 @@ optional_field_style={default,accessors,reftypes} (default: default)
   useful when you need to serialize a field with the default value, or check
   if a field has been explicitly set to its default value from the wire.
 
-  In the 'accessors' style, required fields are still translated to one
-  public mutable Java field each, and repeated fields are still translated
-  to arrays. No accessors are generated for them.
+  In the 'accessors' style, required and nested message fields are still
+  translated to one public mutable Java field each, repeated fields are still
+  translated to arrays. No accessors are generated for them.
 
   IMPORTANT: When using the 'accessors' style, ProGuard should always
   be enabled with optimization (don't use -dontoptimize) and allowing
diff --git a/java/src/test/java/com/google/protobuf/NanoTest.java b/java/src/test/java/com/google/protobuf/NanoTest.java
index 0e1c9bc..4b8d63d 100644
--- a/java/src/test/java/com/google/protobuf/NanoTest.java
+++ b/java/src/test/java/com/google/protobuf/NanoTest.java
@@ -2271,9 +2271,10 @@ public class NanoTest extends TestCase {
   public void testNanoWithAccessorsBasic() throws Exception {
     TestNanoAccessors msg = new TestNanoAccessors();
 
-    // Makes sure required and repeated fields are still public fields
+    // Makes sure required, repeated, and message fields are still public
     msg.id = 3;
     msg.repeatedBytes = new byte[2][3];
+    msg.optionalNestedMessage = null;
 
     // Test accessors
     assertEquals(0, msg.getOptionalInt32());
@@ -2295,10 +2296,6 @@ public class NanoTest extends TestCase {
       msg.setOptionalString(null);
       fail();
     } catch (NullPointerException expected) {}
-    try {
-      msg.setOptionalNestedMessage(null);
-      fail();
-    } catch (NullPointerException expected) {}
 
     // Test has bit on bytes field with defaults and clear() re-clones the default array
     assertFalse(msg.hasDefaultBytes());
@@ -2352,8 +2349,8 @@ public class NanoTest extends TestCase {
       assertFalse(msg.hasDefaultBytes());
       assertFalse(msg.hasDefaultFloatNan());
       assertFalse(msg.hasDefaultNestedEnum());
-      msg.setOptionalNestedMessage(new TestNanoAccessors.NestedMessage());
-      msg.getOptionalNestedMessage().setBb(2);
+      msg.optionalNestedMessage = new TestNanoAccessors.NestedMessage();
+      msg.optionalNestedMessage.setBb(2);
       msg.setOptionalNestedEnum(TestNanoAccessors.BAZ);
       msg.setDefaultInt32(msg.getDefaultInt32());
     }
@@ -2366,8 +2363,8 @@ public class NanoTest extends TestCase {
 
     // Has fields true upon parse.
     TestNanoAccessors newMsg = TestNanoAccessors.parseFrom(result);
-    assertEquals(2, newMsg.getOptionalNestedMessage().getBb());
-    assertTrue(newMsg.getOptionalNestedMessage().hasBb());
+    assertEquals(2, newMsg.optionalNestedMessage.getBb());
+    assertTrue(newMsg.optionalNestedMessage.hasBb());
     assertEquals(TestNanoAccessors.BAZ, newMsg.getOptionalNestedEnum());
     assertTrue(newMsg.hasOptionalNestedEnum());
 
@@ -2376,6 +2373,38 @@ public class NanoTest extends TestCase {
     assertEquals(41, newMsg.getDefaultInt32());
   }
 
+  public void testNanoWithAccessorsPublicFieldTypes() throws Exception {
+    TestNanoAccessors msg = new TestNanoAccessors();
+    assertNull(msg.optionalNestedMessage);
+    assertEquals(0, msg.id);
+    assertEquals(0, msg.repeatedNestedEnum.length);
+
+    TestNanoAccessors newMsg = TestNanoAccessors.parseFrom(MessageNano.toByteArray(msg));
+    assertNull(newMsg.optionalNestedMessage);
+    assertEquals(0, newMsg.id);
+    assertEquals(0, newMsg.repeatedNestedEnum.length);
+
+    TestNanoAccessors.NestedMessage nestedMessage = new TestNanoAccessors.NestedMessage();
+    nestedMessage.setBb(5);
+    newMsg.optionalNestedMessage = nestedMessage;
+    newMsg.id = -1;
+    newMsg.repeatedNestedEnum = new int[] { TestAllTypesNano.FOO };
+
+    TestNanoAccessors newMsg2 = TestNanoAccessors.parseFrom(MessageNano.toByteArray(newMsg));
+    assertEquals(nestedMessage.getBb(), newMsg2.optionalNestedMessage.getBb());
+    assertEquals(-1, newMsg2.id);
+    assertEquals(TestAllTypesNano.FOO, newMsg2.repeatedNestedEnum[0]);
+
+    newMsg2.optionalNestedMessage = null;
+    newMsg2.id = 0;
+    newMsg2.repeatedNestedEnum = null;
+
+    TestNanoAccessors newMsg3 = TestNanoAccessors.parseFrom(MessageNano.toByteArray(newMsg2));
+    assertNull(newMsg3.optionalNestedMessage);
+    assertEquals(0, newMsg3.id);
+    assertEquals(0, newMsg3.repeatedNestedEnum.length);
+  }
+
   public void testNanoWithAccessorsSerialize() throws Exception {
     TestNanoAccessors msg = new TestNanoAccessors();
     msg.setOptionalInt32(msg.getOptionalInt32());
@@ -2383,7 +2412,7 @@ public class NanoTest extends TestCase {
     msg.setOptionalBytes(msg.getOptionalBytes());
     TestNanoAccessors.NestedMessage nestedMessage = new TestNanoAccessors.NestedMessage();
     nestedMessage.setBb(nestedMessage.getBb());
-    msg.setOptionalNestedMessage(nestedMessage);
+    msg.optionalNestedMessage = nestedMessage;
     msg.setOptionalNestedEnum(msg.getOptionalNestedEnum());
     msg.setDefaultInt32(msg.getDefaultInt32());
     msg.setDefaultString(msg.getDefaultString());
@@ -2400,8 +2429,7 @@ public class NanoTest extends TestCase {
     assertTrue(newMsg.hasOptionalInt32());
     assertTrue(newMsg.hasOptionalString());
     assertTrue(newMsg.hasOptionalBytes());
-    assertTrue(newMsg.hasOptionalNestedMessage());
-    assertTrue(newMsg.getOptionalNestedMessage().hasBb());
+    assertTrue(newMsg.optionalNestedMessage.hasBb());
     assertTrue(newMsg.hasOptionalNestedEnum());
     assertTrue(newMsg.hasDefaultInt32());
     assertTrue(newMsg.hasDefaultString());
@@ -2411,7 +2439,7 @@ public class NanoTest extends TestCase {
     assertEquals(0, newMsg.getOptionalInt32());
     assertEquals(0, newMsg.getOptionalString().length());
     assertEquals(0, newMsg.getOptionalBytes().length);
-    assertEquals(0, newMsg.getOptionalNestedMessage().getBb());
+    assertEquals(0, newMsg.optionalNestedMessage.getBb());
     assertEquals(TestNanoAccessors.FOO, newMsg.getOptionalNestedEnum());
     assertEquals(41, newMsg.getDefaultInt32());
     assertEquals("hello", newMsg.getDefaultString());
@@ -2856,15 +2884,15 @@ public class NanoTest extends TestCase {
         .setOptionalInt32(5)
         .setOptionalString("Hello")
         .setOptionalBytes(new byte[] {1, 2, 3})
-        .setOptionalNestedMessage(new TestNanoAccessors.NestedMessage().setBb(27))
         .setOptionalNestedEnum(TestNanoAccessors.BAR)
         .setDefaultFloatNan(1.0f);
+    message.optionalNestedMessage = new TestNanoAccessors.NestedMessage().setBb(27);
     message.repeatedInt32 = new int[] { 5, 6, 7, 8 };
     message.repeatedString = new String[] { "One", "Two" };
     message.repeatedBytes = new byte[][] { { 2, 7 }, { 2, 7 } };
     message.repeatedNestedMessage = new TestNanoAccessors.NestedMessage[] {
-      message.getOptionalNestedMessage(),
-      message.getOptionalNestedMessage()
+      message.optionalNestedMessage,
+      message.optionalNestedMessage
     };
     message.repeatedNestedEnum = new int[] {
       TestAllTypesNano.BAR,
diff --git a/src/google/protobuf/compiler/javanano/javanano_field.cc b/src/google/protobuf/compiler/javanano/javanano_field.cc
index 62e133e..2581669 100644
--- a/src/google/protobuf/compiler/javanano/javanano_field.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_field.cc
@@ -78,8 +78,9 @@ FieldGeneratorMap::FieldGeneratorMap(const Descriptor* descriptor, const Params
 
 FieldGenerator* FieldGeneratorMap::MakeGenerator(const FieldDescriptor* field,
     const Params &params, int* next_has_bit_index) {
+  JavaType java_type = GetJavaType(field);
   if (field->is_repeated()) {
-    switch (GetJavaType(field)) {
+    switch (java_type) {
       case JAVATYPE_MESSAGE:
         return new RepeatedMessageFieldGenerator(field, params);
       case JAVATYPE_ENUM:
@@ -87,14 +88,13 @@ FieldGenerator* FieldGeneratorMap::MakeGenerator(const FieldDescriptor* field,
       default:
         return new RepeatedPrimitiveFieldGenerator(field, params);
     }
-  } else if (params.optional_field_accessors() && field->is_optional()) {
+  } else if (params.optional_field_accessors() && field->is_optional()
+      && java_type != JAVATYPE_MESSAGE) {
     // We need a has-bit for each primitive/enum field because their default
     // values could be same as explicitly set values. But we don't need it
     // for a message field because they have no defaults and Nano uses 'null'
     // for unset messages, which cannot be set explicitly.
-    switch (GetJavaType(field)) {
-      case JAVATYPE_MESSAGE:
-        return new AccessorMessageFieldGenerator(field, params);
+    switch (java_type) {
       case JAVATYPE_ENUM:
         return new AccessorEnumFieldGenerator(
             field, params, (*next_has_bit_index)++);
@@ -103,7 +103,7 @@ FieldGenerator* FieldGeneratorMap::MakeGenerator(const FieldDescriptor* field,
             field, params, (*next_has_bit_index)++);
     }
   } else {
-    switch (GetJavaType(field)) {
+    switch (java_type) {
       case JAVATYPE_MESSAGE:
         return new MessageFieldGenerator(field, params);
       case JAVATYPE_ENUM:
diff --git a/src/google/protobuf/compiler/javanano/javanano_message_field.cc b/src/google/protobuf/compiler/javanano/javanano_message_field.cc
index d9abea3..74d3f85 100644
--- a/src/google/protobuf/compiler/javanano/javanano_message_field.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_message_field.cc
@@ -149,97 +149,6 @@ GenerateHashCodeCode(io::Printer* printer) const {
 
 // ===================================================================
 
-AccessorMessageFieldGenerator::
-AccessorMessageFieldGenerator(
-    const FieldDescriptor* descriptor, const Params& params)
-  : FieldGenerator(params), descriptor_(descriptor) {
-  SetMessageVariables(params, descriptor, &variables_);
-}
-
-AccessorMessageFieldGenerator::~AccessorMessageFieldGenerator() {}
-
-void AccessorMessageFieldGenerator::
-GenerateMembers(io::Printer* printer) const {
-  printer->Print(variables_,
-    "private $type$ $name$_;\n"
-    "public $type$ get$capitalized_name$() {\n"
-    "  return $name$_;\n"
-    "}\n"
-    "public $message_name$ set$capitalized_name$($type$ value) {\n"
-    "  if (value == null) {\n"
-    "    throw new java.lang.NullPointerException();\n"
-    "  }\n"
-    "  $name$_ = value;\n"
-    "  return this;\n"
-    "}\n"
-    "public boolean has$capitalized_name$() {\n"
-    "  return $name$_ != null;\n"
-    "}\n"
-    "public $message_name$ clear$capitalized_name$() {\n"
-    "  $name$_ = null;\n"
-    "  return this;"
-    "}\n");
-}
-
-void AccessorMessageFieldGenerator::
-GenerateClearCode(io::Printer* printer) const {
-  printer->Print(variables_,
-    "$name$_ = null;\n");
-}
-
-void AccessorMessageFieldGenerator::
-GenerateMergingCode(io::Printer* printer) const {
-  printer->Print(variables_,
-    "if ($name$_ == null) {\n"
-    "  $name$_ = new $type$();\n"
-    "}\n");
-
-  if (descriptor_->type() == FieldDescriptor::TYPE_GROUP) {
-    printer->Print(variables_,
-      "input.readGroup($name$_, $number$);\n");
-  } else {
-    printer->Print(variables_,
-      "input.readMessage($name$_);\n");
-  }
-}
-
-void AccessorMessageFieldGenerator::
-GenerateSerializationCode(io::Printer* printer) const {
-  printer->Print(variables_,
-    "if ($name$_ != null) {\n"
-    "  output.write$group_or_message$($number$, $name$_);\n"
-    "}\n");
-}
-
-void AccessorMessageFieldGenerator::
-GenerateSerializedSizeCode(io::Printer* printer) const {
-  printer->Print(variables_,
-    "if ($name$_ != null) {\n"
-    "  size += com.google.protobuf.nano.CodedOutputByteBufferNano\n"
-    "    .compute$group_or_message$Size($number$, $name$_);\n"
-    "}\n");
-}
-
-void AccessorMessageFieldGenerator::
-GenerateEqualsCode(io::Printer* printer) const {
-  printer->Print(variables_,
-    "if ($name$_ == null) {\n"
-    "  if (other.$name$_ != null) {\n"
-    "    return false;\n"
-    "  }\n"
-    "} else if (!$name$_.equals(other.$name$_)) {\n"
-    "  return false;\n"
-    "}\n");
-}
-
-void AccessorMessageFieldGenerator::
-GenerateHashCodeCode(io::Printer* printer) const {
-  printer->Print(variables_,
-    "result = 31 * result + ($name$_ == null ? 0 : $name$_.hashCode());\n");
-}
-
-// ===================================================================
-
 RepeatedMessageFieldGenerator::
 RepeatedMessageFieldGenerator(const FieldDescriptor* descriptor, const Params& params)
   : FieldGenerator(params), descriptor_(descriptor) {
diff --git a/src/google/protobuf/compiler/javanano/javanano_message_field.h b/src/google/protobuf/compiler/javanano/javanano_message_field.h
index b724351..55e2610 100644
--- a/src/google/protobuf/compiler/javanano/javanano_message_field.h
+++ b/src/google/protobuf/compiler/javanano/javanano_message_field.h
@@ -65,28 +65,6 @@ class MessageFieldGenerator : public FieldGenerator {
   GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(MessageFieldGenerator);
 };
 
-class AccessorMessageFieldGenerator : public FieldGenerator {
- public:
-  explicit AccessorMessageFieldGenerator(
-      const FieldDescriptor* descriptor, const Params& params);
-  ~AccessorMessageFieldGenerator();
-
-  // implements FieldGenerator ---------------------------------------
-  void GenerateMembers(io::Printer* printer) const;
-  void GenerateClearCode(io::Printer* printer) const;
-  void GenerateMergingCode(io::Printer* printer) const;
-  void GenerateSerializationCode(io::Printer* printer) const;
-  void GenerateSerializedSizeCode(io::Printer* printer) const;
-  void GenerateEqualsCode(io::Printer* printer) const;
-  void GenerateHashCodeCode(io::Printer* printer) const;
-
- private:
-  const FieldDescriptor* descriptor_;
-  map<string, string> variables_;
-
-  GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(AccessorMessageFieldGenerator);
-};
-
 class RepeatedMessageFieldGenerator : public FieldGenerator {
  public:
   explicit RepeatedMessageFieldGenerator(const FieldDescriptor* descriptor,