Accessor style for optional fields.

This CL implements the 'optional_field_style=accessors' option.
All optional fields will now be 1 Java field and 1 bit in a shared
bitfield behind get/set/has/clear accessor methods. The setter
performs null check for reference types (Strings and byte[]s).

Also decentralized the clear code generation.

Change-Id: I60ac78329e352e76c2f8139fba1f292383080ad3

3 files changed, 194 insertions, 7 deletions

diff --git a/java/README.txt b/java/README.txt
index 8bfaab0..9728f48 100644
--- a/java/README.txt
+++ b/java/README.txt
@@ -411,9 +411,10 @@ Nano version
 Nano is even smaller than micro, especially in the number of generated
 functions. It is like micro except:
 
-- No setter/getter/hazzer functions.
-- Has state is not available. Outputs all fields not equal to their
-  default. (See important implications below.)
+- Setter/getter/hazzer/clearer functions are opt-in.
+- If not opted in, has state is not available. Serialization outputs
+  all fields not  equal to their default. (See important implications
+  below.)
 - CodedInputStreamMicro is renamed to CodedInputByteBufferNano and can
   only take byte[] (not InputStream).
 - Similar rename from CodedOutputStreamMicro to
@@ -426,7 +427,7 @@ functions. It is like micro except:
   MessageNano.
 - "bytes" are of java type byte[].
 
-IMPORTANT: If you have fields with defaults
+IMPORTANT: If you have fields with defaults and opt out of accessors
 
 How fields with defaults are serialized has changed. Because we don't
 keep "has" state, any field equal to its default is assumed to be not
@@ -435,7 +436,8 @@ change the default value of a field. Senders compiled against an older
 version of the proto continue to match against the old default, and
 don't send values to the receiver even though the receiver assumes the
 new default value. Therefore, think carefully about the implications
-of changing the default value.
+of changing the default value. Alternatively, turn on accessors and
+enjoy the benefit of the explicit has() checks.
 
 IMPORTANT: If you have "bytes" fields with non-empty defaults
 
@@ -451,7 +453,8 @@ Nano Generator options
 java_package           -> <file-name>|<package-name>
 java_outer_classname   -> <file-name>|<package-name>
 java_multiple_files    -> true or false
-java_nano_generate_has -> true or false
+java_nano_generate_has -> true or false [DEPRECATED]
+optional_field_style   -> default or accessors
 
 java_package:
 java_outer_classname:
@@ -459,6 +462,8 @@ java_multiple_files:
   Same as Micro version.
 
 java_nano_generate_has={true,false} (default: false)
+  DEPRECATED. Use optional_field_style=accessors.
+
   If true, generates a public boolean variable has<fieldname>
   accompanying each optional or required field (not present for
   repeated fields, groups or messages). It is set to false initially
@@ -473,6 +478,34 @@ java_nano_generate_has={true,false} (default: false)
   many cases reading the default works and determining whether the
   field was received over the wire is irrelevant.
 
+optional_field_style={default,accessors} (default: default)
+  Defines the style of the generated code for _optional_ fields only.
+  In the default style, optional fields translate into public mutable
+  Java fields, and the serialization process is as discussed in the
+  "IMPORTANT" section above. When set to 'accessors', each optional
+  field is encapsulated behind 4 accessors, namely get<fieldname>(),
+  set<fieldname>(), has<fieldname>() and clear<fieldname>() methods,
+  with the standard semantics. The hazzer's return value determines
+  whether a field is serialized, so this style is 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.
+
+  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.
+
+  optional_field_style=accessors cannot be used together with
+  java_nano_generate_has=true. If you need the 'has' flag for any
+  required field (you have no reason to), you can only use
+  java_nano_generate_has=true.
+
+  IMPORTANT: When using the 'accessor' style, ProGuard should always
+  be enabled with optimization (don't use -dontoptimize) and allowing
+  access modification (use -allowaccessmodification). This removes the
+  unused accessors and maybe inline the rest at the call sites,
+  reducing the final code size.
+  TODO(maxtroy): find ProGuard config that would work the best.
+
 To use nano protobufs:
 
 - Link with the generated jar file
diff --git a/java/pom.xml b/java/pom.xml
index bf5f666..f36d65f 100644
--- a/java/pom.xml
+++ b/java/pom.xml
@@ -157,6 +157,12 @@
                   <arg value="--proto_path=src/test/java" />
                   <arg value="../src/google/protobuf/unittest_has_nano.proto" />
                 </exec>
+                <exec executable="../src/protoc">
+                  <arg value="--javanano_out=optional_field_style=accessors:target/generated-test-sources" />
+                  <arg value="--proto_path=../src" />
+                  <arg value="--proto_path=src/test/java" />
+                  <arg value="../src/google/protobuf/unittest_accessors_nano.proto" />
+                </exec>
               </tasks>
               <testSourceRoot>target/generated-test-sources</testSourceRoot>
               <!--testSourceRoot>target/generated-test-sources/opt-space</testSourceRoot-->
diff --git a/java/src/test/java/com/google/protobuf/NanoTest.java b/java/src/test/java/com/google/protobuf/NanoTest.java
index b00f289..fb6cccb 100644
--- a/java/src/test/java/com/google/protobuf/NanoTest.java
+++ b/java/src/test/java/com/google/protobuf/NanoTest.java
@@ -40,6 +40,7 @@ import com.google.protobuf.nano.MessageScopeEnumRefNano;
 import com.google.protobuf.nano.MultipleImportingNonMultipleNano1;
 import com.google.protobuf.nano.MultipleImportingNonMultipleNano2;
 import com.google.protobuf.nano.MultipleNameClashNano;
+import com.google.protobuf.nano.NanoAccessorsOuterClass.TestNanoAccessors;
 import com.google.protobuf.nano.NanoHasOuterClass.TestAllTypesNanoHas;
 import com.google.protobuf.nano.NanoOuterClass;
 import com.google.protobuf.nano.NanoOuterClass.TestAllTypesNano;
@@ -48,7 +49,6 @@ import com.google.protobuf.nano.UnittestMultipleNano;
 import com.google.protobuf.nano.UnittestRecursiveNano.RecursiveMessageNano;
 import com.google.protobuf.nano.UnittestSimpleNano.SimpleMessageNano;
 import com.google.protobuf.nano.UnittestSingleNano.SingleMessageNano;
-import com.google.protobuf.nano.UnittestStringutf8Nano.StringUtf8;
 
 import junit.framework.TestCase;
 
@@ -62,6 +62,7 @@ import java.util.List;
  * @author ulas@google.com Ulas Kirazci
  */
 public class NanoTest extends TestCase {
+  @Override
   public void setUp() throws Exception {
   }
 
@@ -2243,6 +2244,153 @@ public class NanoTest extends TestCase {
     assertEquals(0, newMsg.id);
   }
 
+  public void testNanoWithAccessorsBasic() throws Exception {
+    TestNanoAccessors msg = new TestNanoAccessors();
+
+    // Makes sure required and repeated fields are still public fields
+    msg.id = 3;
+    msg.repeatedBytes = new byte[2][3];
+
+    // Test accessors
+    assertEquals(0, msg.getOptionalInt32());
+    assertFalse(msg.hasOptionalInt32());
+    msg.setOptionalInt32(135);
+    assertEquals(135, msg.getOptionalInt32());
+    assertTrue(msg.hasOptionalInt32());
+    msg.clearOptionalInt32();
+    assertFalse(msg.hasOptionalInt32());
+    msg.setOptionalInt32(0); // default value
+    assertTrue(msg.hasOptionalInt32());
+
+    // Test NPE
+    try {
+      msg.setOptionalBytes(null);
+      fail();
+    } catch (NullPointerException expected) {}
+    try {
+      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());
+    byte[] defaultBytes = msg.getDefaultBytes();
+    msg.setDefaultBytes(defaultBytes);
+    assertTrue(msg.hasDefaultBytes());
+    msg.clearDefaultBytes();
+    assertFalse(msg.hasDefaultBytes());
+    defaultBytes[0]++; // modify original array
+    assertFalse(Arrays.equals(defaultBytes, msg.getDefaultBytes()));
+
+    // Test has bits that require additional bit fields
+    assertFalse(msg.hasBitFieldCheck());
+    msg.setBitFieldCheck(0);
+    assertTrue(msg.hasBitFieldCheck());
+    assertFalse(msg.hasBeforeBitFieldCheck()); // checks bit field does not leak
+    assertFalse(msg.hasAfterBitFieldCheck());
+
+    // Test clear() clears has bits
+    msg.setOptionalString("hi");
+    msg.setDefaultString("there");
+    msg.clear();
+    assertFalse(msg.hasOptionalString());
+    assertFalse(msg.hasDefaultString());
+    assertFalse(msg.hasBitFieldCheck());
+  }
+
+  public void testNanoWithAccessorsParseFrom() throws Exception {
+    TestNanoAccessors msg = null;
+    // Test false on creation, after clear and upon empty parse.
+    for (int i = 0; i < 3; i++) {
+      if (i == 0) {
+        msg = new TestNanoAccessors();
+      } else if (i == 1) {
+        msg.clear();
+      } else if (i == 2) {
+        msg = TestNanoAccessors.parseFrom(new byte[0]);
+      }
+      assertFalse(msg.hasOptionalInt32());
+      assertFalse(msg.hasOptionalString());
+      assertFalse(msg.hasOptionalBytes());
+      assertFalse(msg.hasOptionalNestedEnum());
+      assertFalse(msg.hasDefaultInt32());
+      assertFalse(msg.hasDefaultString());
+      assertFalse(msg.hasDefaultBytes());
+      assertFalse(msg.hasDefaultFloatNan());
+      assertFalse(msg.hasDefaultNestedEnum());
+      msg.setOptionalNestedMessage(new TestNanoAccessors.NestedMessage());
+      msg.getOptionalNestedMessage().setBb(2);
+      msg.setOptionalNestedEnum(TestNanoAccessors.BAZ);
+      msg.setDefaultInt32(msg.getDefaultInt32());
+    }
+
+    byte [] result = MessageNano.toByteArray(msg);
+    int msgSerializedSize = msg.getSerializedSize();
+    //System.out.printf("mss=%d result.length=%d\n", msgSerializedSize, result.length);
+    assertTrue(msgSerializedSize == 14);
+    assertEquals(result.length, msgSerializedSize);
+
+    // Has fields true upon parse.
+    TestNanoAccessors newMsg = TestNanoAccessors.parseFrom(result);
+    assertEquals(2, newMsg.getOptionalNestedMessage().getBb());
+    assertTrue(newMsg.getOptionalNestedMessage().hasBb());
+    assertEquals(TestNanoAccessors.BAZ, newMsg.getOptionalNestedEnum());
+    assertTrue(newMsg.hasOptionalNestedEnum());
+
+    // Has field true on fields with explicit default values from wire.
+    assertTrue(newMsg.hasDefaultInt32());
+    assertEquals(41, newMsg.getDefaultInt32());
+  }
+
+  public void testNanoWithAccessorsSerialize() throws Exception {
+    TestNanoAccessors msg = new TestNanoAccessors();
+    msg.setOptionalInt32(msg.getOptionalInt32());
+    msg.setOptionalString(msg.getOptionalString());
+    msg.setOptionalBytes(msg.getOptionalBytes());
+    TestNanoAccessors.NestedMessage nestedMessage = new TestNanoAccessors.NestedMessage();
+    nestedMessage.setBb(nestedMessage.getBb());
+    msg.setOptionalNestedMessage(nestedMessage);
+    msg.setOptionalNestedEnum(msg.getOptionalNestedEnum());
+    msg.setDefaultInt32(msg.getDefaultInt32());
+    msg.setDefaultString(msg.getDefaultString());
+    msg.setDefaultBytes(msg.getDefaultBytes());
+    msg.setDefaultFloatNan(msg.getDefaultFloatNan());
+    msg.setDefaultNestedEnum(msg.getDefaultNestedEnum());
+
+    byte [] result = MessageNano.toByteArray(msg);
+    int msgSerializedSize = msg.getSerializedSize();
+    assertEquals(result.length, msgSerializedSize);
+
+    // Now deserialize and find that all fields are set and equal to their defaults.
+    TestNanoAccessors newMsg = TestNanoAccessors.parseFrom(result);
+    assertTrue(newMsg.hasOptionalInt32());
+    assertTrue(newMsg.hasOptionalString());
+    assertTrue(newMsg.hasOptionalBytes());
+    assertTrue(newMsg.hasOptionalNestedMessage());
+    assertTrue(newMsg.getOptionalNestedMessage().hasBb());
+    assertTrue(newMsg.hasOptionalNestedEnum());
+    assertTrue(newMsg.hasDefaultInt32());
+    assertTrue(newMsg.hasDefaultString());
+    assertTrue(newMsg.hasDefaultBytes());
+    assertTrue(newMsg.hasDefaultFloatNan());
+    assertTrue(newMsg.hasDefaultNestedEnum());
+    assertEquals(0, newMsg.getOptionalInt32());
+    assertEquals(0, newMsg.getOptionalString().length());
+    assertEquals(0, newMsg.getOptionalBytes().length);
+    assertEquals(0, newMsg.getOptionalNestedMessage().getBb());
+    assertEquals(TestNanoAccessors.FOO, newMsg.getOptionalNestedEnum());
+    assertEquals(41, newMsg.getDefaultInt32());
+    assertEquals("hello", newMsg.getDefaultString());
+    assertEquals("world", new String(newMsg.getDefaultBytes(), "UTF-8"));
+    assertEquals(TestNanoAccessors.BAR, newMsg.getDefaultNestedEnum());
+    assertEquals(Float.NaN, newMsg.getDefaultFloatNan());
+    assertEquals(0, newMsg.id);
+  }
+
   /**
    * Tests that fields with a default value of NaN are not serialized when
    * set to NaN. This is a special case as NaN != NaN, so normal equality