Fix review comments.

Change-Id: Iafd4a3aa9d55e31478bf070311eeadfe95ece4d7
zphilip · Feb 20, 2015 · a6fc5da · a6fc5da
1 parent f2cf6cd
commit a6fc5da
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 11 deletions.
diff --git a/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java b/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java
@@ -3741,7 +3741,7 @@ public void testOneofClear() throws Exception {
     checkOneofCase(m, 0);
   }
 
-  public void testOneofMarshling() throws Exception {
+  public void testOneofMarshaling() throws Exception {
     TestAllTypesNano m = new TestAllTypesNano();
     TestAllTypesNano parsed = new TestAllTypesNano();
     {

diff --git a/src/google/protobuf/compiler/javanano/javanano_field.cc b/src/google/protobuf/compiler/javanano/javanano_field.cc
@@ -174,26 +174,17 @@ void GenerateOneofFieldEquals(const FieldDescriptor* descriptor,
   if (GetJavaType(descriptor) == JAVATYPE_BYTES) {
     printer->Print(variables,
       "if (this.has$capitalized_name$()) {\n"
-      "  if (!other.has$capitalized_name$() ||\n"
-      "      !java.util.Arrays.equals((byte[]) this.$oneof_name$_,\n"
+      "  if (!java.util.Arrays.equals((byte[]) this.$oneof_name$_,\n"
       "                               (byte[]) other.$oneof_name$_)) {\n"
       "    return false;\n"
       "  }\n"
-      "} else {\n"
-      "  if (other.has$capitalized_name$()) {\n"
-      "    return false;\n"
-      "  }\n"
       "}\n");
   } else {
     printer->Print(variables,
       "if (this.has$capitalized_name$()) {\n"
       "  if (!this.$oneof_name$_.equals(other.$oneof_name$_)) {\n"
       "    return false;\n"
       "  }\n"
-      "} else {\n"
-      "  if (other.has$capitalized_name$()) {\n"
-      "    return false;\n"
-      "  }\n"
       "}\n");
   }
 }

diff --git a/src/google/protobuf/compiler/javanano/javanano_message.cc b/src/google/protobuf/compiler/javanano/javanano_message.cc
@@ -551,6 +551,16 @@ void MessageGenerator::GenerateEquals(io::Printer* printer) {
     "$classname$ other = ($classname$) o;\n",
     "classname", descriptor_->name());
 
+  // Checking oneof case before checking each oneof field.
+  for (int i = 0; i < descriptor_->oneof_decl_count(); i++) {
+    const OneofDescriptor* oneof_desc = descriptor_->oneof_decl(i);
+    printer->Print(
+      "if (this.$oneof_name$Case_ != other.$oneof_name$Case_) {\n"
+      "  return false;\n"
+      "}\n",
+      "oneof_name", UnderscoresToCamelCase(oneof_desc));
+  }
+
   for (int i = 0; i < descriptor_->field_count(); i++) {
     const FieldDescriptor* field = descriptor_->field(i);
     field_generators_.get(field).GenerateEqualsCode(printer);