Drop unnecessary parentheses in AutoAnnotation equals and `hashCode…

…` methods. Fixes #849. Closes #850. RELNOTES=Drop unnecessary parentheses in AutoAnnotation `equals` and `hashCode` methods. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=312284275
google · May 20, 2020 · b9ba06a · b9ba06a
1 parent 2797d38
commit b9ba06a
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 16 deletions.
diff --git a/value/src/main/java/com/google/auto/value/processor/autoannotation.vm b/value/src/main/java/com/google/auto/value/processor/autoannotation.vm
@@ -187,21 +187,24 @@ final class $className implements $annotationName {
 
 ## equals
 
+## An expression that compares `this.something` against `that.something()`.
+## It can appear in a chain of && conditions, so if that would cause precedence
+## problems the expression needs to be parenthesized.
 #macro (memberEqualsThatExpression $m)
   #if ($m.kind == "FLOAT")
     Float.floatToIntBits($m) == Float.floatToIntBits(that.${m}()) ##
   #elseif ($m.kind == "DOUBLE")
     Double.doubleToLongBits($m) == Double.doubleToLongBits(that.${m}()) ##
   #elseif ($m.kind.primitive)
-    $m == that.${m}() ##
+    ($m == that.${m}()) ## parens not strictly needed but avoid confusion when comparing booleans
   #elseif ($m.kind == "ARRAY")
     #if ($params.containsKey($m.toString()))
     `java.util.Arrays`.equals($m,
         (that instanceof $className)
             ? (($className) that).$m
             : that.${m}()) ##
     #else ## default value, so if |that| is also a $className then it has the same constant value
-    that instanceof $className || `java.util.Arrays`.equals($m, that.${m}())
+    (that instanceof $className || `java.util.Arrays`.equals($m, that.${m}()))
     #end
   #else
     ${m}.equals(that.${m}()) ##
@@ -224,7 +227,7 @@ final class $className implements $annotationName {
       $annotationName that = ($annotationName) o;
       return ##
            #foreach ($m in $members)
-           (#memberEqualsThatExpression ($m))##
+           #memberEqualsThatExpression ($m)##
              #if ($foreach.hasNext)
 
            && ##
@@ -239,6 +242,9 @@ final class $className implements $annotationName {
 
 ## hashCode
 
+## An expression that returns the hashCode of `this.something`.
+## It appears on the right-hand side of an ^ operator, so if that would cause precedence
+## problems the expression needs to be parenthesized.
 #macro (memberHashCodeExpression $m)
   #if ($m.kind == "LONG")
     (int) (($m >>> 32) ^ $m) ##
@@ -247,7 +253,7 @@ final class $className implements $annotationName {
   #elseif ($m.kind == "DOUBLE")
     (int) ((Double.doubleToLongBits($m) >>> 32) ^ Double.doubleToLongBits($m)) ##
   #elseif ($m.kind == "BOOLEAN")
-    $m ? 1231 : 1237 ##
+    ($m ? 1231 : 1237) ##
   #elseif ($m.kind.primitive)
     $m ##
   #elseif ($m.kind == "ARRAY")
@@ -278,7 +284,7 @@ final class $className implements $annotationName {
     #foreach ($m in $members)
       #if (!$invariableHashes.contains($m.toString()))
 
-        + ($m.nameHash ^ (#memberHashCodeExpression($m)))
+        + ($m.nameHash ^ #memberHashCodeExpression($m))
             // $m.nameHash is 127 * "${m}".hashCode()
       #end
     #end

diff --git a/value/src/test/java/com/google/auto/value/processor/AutoAnnotationCompilationTest.java b/value/src/test/java/com/google/auto/value/processor/AutoAnnotationCompilationTest.java
@@ -113,7 +113,7 @@ public void testSimple() {
             "    }",
             "    if (o instanceof MyAnnotation) {",
             "      MyAnnotation that = (MyAnnotation) o;",
-            "      return (value.equals(that.value()))",
+            "      return value.equals(that.value())",
             "          && (defaultedValue == that.defaultedValue());",
             "    }",
             "    return false;",
@@ -122,7 +122,7 @@ public void testSimple() {
             "  @Override public int hashCode() {",
             "    return ",
             "        " + invariableHash,
-            "        + (" + 127 * "value".hashCode() + " ^ (value.hashCode()))",
+            "        + (" + 127 * "value".hashCode() + " ^ value.hashCode())",
             "    ;",
             "  }",
             "}");
@@ -272,17 +272,17 @@ public void testGwtSimple() {
             "    }",
             "    if (o instanceof MyAnnotation) {",
             "      MyAnnotation that = (MyAnnotation) o;",
-            "      return (Arrays.equals(value,",
+            "      return Arrays.equals(value,",
             "          (that instanceof AutoAnnotation_AnnotationFactory_newMyAnnotation)",
             "              ? ((AutoAnnotation_AnnotationFactory_newMyAnnotation) that).value",
-            "              : that.value()));",
+            "              : that.value());",
             "    }",
             "    return false;",
             "  }",
             "",
             "  @Override public int hashCode() {",
             "    return ",
-            "        + (" + 127 * "value".hashCode() + " ^ (Arrays.hashCode(value)));",
+            "        + (" + 127 * "value".hashCode() + " ^ Arrays.hashCode(value));",
             "  }",
             "}");
     Compilation compilation =
@@ -396,22 +396,22 @@ public void testCollectionsForArrays() {
             "    }",
             "    if (o instanceof MyAnnotation) {",
             "      MyAnnotation that = (MyAnnotation) o;",
-            "      return (Arrays.equals(value,",
+            "      return Arrays.equals(value,",
             "          (that instanceof AutoAnnotation_AnnotationFactory_newMyAnnotation)",
             "              ? ((AutoAnnotation_AnnotationFactory_newMyAnnotation) that).value",
-            "              : that.value()))",
-            "          && (Arrays.equals(enums,",
+            "              : that.value())",
+            "          && Arrays.equals(enums,",
             "          (that instanceof AutoAnnotation_AnnotationFactory_newMyAnnotation)",
             "              ? ((AutoAnnotation_AnnotationFactory_newMyAnnotation) that).enums",
-            "              : that.enums()))",
+            "              : that.enums())",
             "    }",
             "    return false;",
             "  }",
             "",
             "  @Override public int hashCode() {",
             "    return ",
-            "        + (" + 127 * "value".hashCode() + " ^ (Arrays.hashCode(value)))",
-            "        + (" + 127 * "enums".hashCode() + " ^ (Arrays.hashCode(enums)));",
+            "        + (" + 127 * "value".hashCode() + " ^ Arrays.hashCode(value))",
+            "        + (" + 127 * "enums".hashCode() + " ^ Arrays.hashCode(enums));",
             "  }",
             "",
             "  private static int[] intArrayFromCollection(Collection<Integer> c) {",