Error Prone: Enable and fix ImmutableEnumChecker

* Mark some enums using @Immutable from Error Prone (and also import
  that library).
* Suppress one usage of an effectively-immutable JGit class which we
  don't control.
* Fixing ReviewerStateInternal to store a String internally rather than
  a FooterKey. FooterKey is actually not immutable: it has a package-
  private byte array field. Keep returning FooterKey from the interface
  for type safety; given that one method was already constructing a new
  FooterKey dynamically on every invocation, and that this class is
  primarily used for constructing ChangeUpdates, this shouldn't be a
  performance issue.

Change-Id: Ieb7ee10e9be39184cff7e0d1d18ff6b433016b31

Documentation/licenses.txt

@@ -52,6 +52,7 @@ Apache2.0
 * commons:pool
 * commons:validator
 * dropwizard:dropwizard-core
+* errorprone:annotations
 * flogger:api
 * fonts:robotofonts
 * guice:guice

WORKSPACE

@@ -186,6 +186,12 @@ maven_jar(
     sha1 = "94ad16d728b374d65bd897625f3fbb3da223a2b6",
 )
 
+maven_jar(
+    name = "error-prone-annotations",
+    artifact = "com.google.errorprone:error_prone_annotations:2.3.3",
+    sha1 = "42aa5155a54a87d70af32d4b0d06bf43779de0e2",
+)
+
 FLOGGER_VERS = "0.4"
 
 maven_jar(

java/com/google/gerrit/reviewdb/BUILD

@@ -13,6 +13,7 @@ java_library(
         "//lib:protobuf",
         "//lib/auto:auto-value",
         "//lib/auto:auto-value-annotations",
+        "//lib/errorprone:annotations",
         "//lib/jgit/org.eclipse.jgit:jgit",
         "//proto:entities_java_proto",
     ],

java/com/google/gerrit/reviewdb/converter/AccountIdProtoConverter.java

@@ -14,10 +14,12 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.protobuf.Parser;
 
+@Immutable
 public enum AccountIdProtoConverter implements ProtoConverter<Entities.Account_Id, Account.Id> {
   INSTANCE;
 

java/com/google/gerrit/reviewdb/converter/BranchNameKeyProtoConverter.java

@@ -14,11 +14,13 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.gerrit.reviewdb.client.BranchNameKey;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.protobuf.Parser;
 
+@Immutable
 public enum BranchNameKeyProtoConverter
     implements ProtoConverter<Entities.Branch_NameKey, BranchNameKey> {
   INSTANCE;

java/com/google/gerrit/reviewdb/converter/ChangeIdProtoConverter.java

@@ -14,10 +14,12 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.protobuf.Parser;
 
+@Immutable
 public enum ChangeIdProtoConverter implements ProtoConverter<Entities.Change_Id, Change.Id> {
   INSTANCE;
 

java/com/google/gerrit/reviewdb/converter/ChangeKeyProtoConverter.java

@@ -14,10 +14,12 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.protobuf.Parser;
 
+@Immutable
 public enum ChangeKeyProtoConverter implements ProtoConverter<Entities.Change_Key, Change.Key> {
   INSTANCE;
 

java/com/google/gerrit/reviewdb/converter/ChangeMessageKeyProtoConverter.java

@@ -14,11 +14,13 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.protobuf.Parser;
 
+@Immutable
 public enum ChangeMessageKeyProtoConverter
     implements ProtoConverter<Entities.ChangeMessage_Key, ChangeMessage.Key> {
   INSTANCE;

java/com/google/gerrit/reviewdb/converter/ChangeMessageProtoConverter.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
@@ -22,6 +23,7 @@ import com.google.protobuf.Parser;
 import java.sql.Timestamp;
 import java.util.Objects;
 
+@Immutable
 public enum ChangeMessageProtoConverter
     implements ProtoConverter<Entities.ChangeMessage, ChangeMessage> {
   INSTANCE;

java/com/google/gerrit/reviewdb/converter/ChangeProtoConverter.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.BranchNameKey;
@@ -22,6 +23,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.protobuf.Parser;
 import java.sql.Timestamp;
 
+@Immutable
 public enum ChangeProtoConverter implements ProtoConverter<Entities.Change, Change> {
   INSTANCE;
 

java/com/google/gerrit/reviewdb/converter/LabelIdProtoConverter.java

@@ -14,10 +14,12 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.gerrit.reviewdb.client.LabelId;
 import com.google.protobuf.Parser;
 
+@Immutable
 public enum LabelIdProtoConverter implements ProtoConverter<Entities.LabelId, LabelId> {
   INSTANCE;
 

java/com/google/gerrit/reviewdb/converter/ObjectIdProtoConverter.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.protobuf.Parser;
 import org.eclipse.jgit.lib.ObjectId;
@@ -30,6 +31,7 @@ import org.eclipse.jgit.lib.ObjectId;
  *   <li>This maintains backwards wire compatibility with a pre-NoteDb implementation.
  * </ul>
  */
+@Immutable
 public enum ObjectIdProtoConverter implements ProtoConverter<Entities.ObjectId, ObjectId> {
   INSTANCE;
 

java/com/google/gerrit/reviewdb/converter/PatchSetApprovalKeyProtoConverter.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.LabelId;
@@ -21,6 +22,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.protobuf.Parser;
 
+@Immutable
 public enum PatchSetApprovalKeyProtoConverter
     implements ProtoConverter<Entities.PatchSetApproval_Key, PatchSetApproval.Key> {
   INSTANCE;

java/com/google/gerrit/reviewdb/converter/PatchSetApprovalProtoConverter.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
@@ -21,6 +22,7 @@ import com.google.protobuf.Parser;
 import java.sql.Timestamp;
 import java.util.Objects;
 
+@Immutable
 public enum PatchSetApprovalProtoConverter
     implements ProtoConverter<Entities.PatchSetApproval, PatchSetApproval> {
   INSTANCE;

java/com/google/gerrit/reviewdb/converter/PatchSetIdProtoConverter.java

@@ -14,11 +14,13 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.protobuf.Parser;
 
+@Immutable
 public enum PatchSetIdProtoConverter implements ProtoConverter<Entities.PatchSet_Id, PatchSet.Id> {
   INSTANCE;
 

java/com/google/gerrit/reviewdb/converter/PatchSetProtoConverter.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.reviewdb.converter;
 
 import com.google.common.collect.ImmutableList;
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.PatchSet;
@@ -23,6 +24,7 @@ import java.sql.Timestamp;
 import java.util.List;
 import org.eclipse.jgit.lib.ObjectId;
 
+@Immutable
 public enum PatchSetProtoConverter implements ProtoConverter<Entities.PatchSet, PatchSet> {
   INSTANCE;
 

java/com/google/gerrit/reviewdb/converter/ProjectNameKeyProtoConverter.java

@@ -14,10 +14,12 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.gerrit.proto.Entities;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.protobuf.Parser;
 
+@Immutable
 public enum ProjectNameKeyProtoConverter
     implements ProtoConverter<Entities.Project_NameKey, Project.NameKey> {
   INSTANCE;

java/com/google/gerrit/reviewdb/converter/ProtoConverter.java

@@ -14,9 +14,11 @@
 
 package com.google.gerrit.reviewdb.converter;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.protobuf.MessageLite;
 import com.google.protobuf.Parser;
 
+@Immutable
 public interface ProtoConverter<P extends MessageLite, C> {
 
   P toProto(C valueClass);

java/com/google/gerrit/server/change/ArchiveFormat.java

@@ -35,7 +35,9 @@ public enum ArchiveFormat {
   TXZ("application/x-xz", new TxzFormat()),
   ZIP("application/x-zip", new ZipFormat());
 
+  @SuppressWarnings("ImmutableEnumChecker") // ArchiveCommand.Format is effectively immutable.
   private final ArchiveCommand.Format<?> format;
+
   private final String mimeType;
 
   ArchiveFormat(String mimeType, ArchiveCommand.Format<?> format) {

java/com/google/gerrit/server/notedb/ReviewerStateInternal.java

@@ -21,13 +21,13 @@ import org.eclipse.jgit.revwalk.FooterKey;
 /** State of a reviewer on a change. */
 public enum ReviewerStateInternal {
   /** The user has contributed at least one nonzero vote on the change. */
-  REVIEWER(new FooterKey("Reviewer"), ReviewerState.REVIEWER),
+  REVIEWER("Reviewer", ReviewerState.REVIEWER),
 
   /** The reviewer was added to the change, but has not voted. */
-  CC(new FooterKey("CC"), ReviewerState.CC),
+  CC("CC", ReviewerState.CC),
 
   /** The user was previously a reviewer on the change, but was removed. */
-  REMOVED(new FooterKey("Removed"), ReviewerState.REMOVED);
+  REMOVED("Removed", ReviewerState.REMOVED);
 
   public static ReviewerStateInternal fromReviewerState(ReviewerState state) {
     return ReviewerStateInternal.values()[state.ordinal()];
@@ -50,20 +50,20 @@ public enum ReviewerStateInternal {
     }
   }
 
-  private final FooterKey footerKey;
+  private final String footer;
   private final ReviewerState state;
 
-  ReviewerStateInternal(FooterKey footerKey, ReviewerState state) {
+  ReviewerStateInternal(String footer, ReviewerState state) {
-    this.footerKey = footerKey;
+    this.footer = footer;
     this.state = state;
   }
 
   FooterKey getFooterKey() {
-    return footerKey;
+    return new FooterKey(footer);
   }
 
   FooterKey getByEmailFooterKey() {
-    return new FooterKey(footerKey.getName() + "-email");
+    return new FooterKey(footer + "-email");
   }
 
   public ReviewerState asReviewerState() {

lib/errorprone/BUILD

@@ -0,0 +1,6 @@
+java_library(
+    name = "annotations",
+    data = ["//lib:LICENSE-Apache2.0"],
+    visibility = ["//visibility:public"],
+    exports = ["@error-prone-annotations//jar"],
+)

tools/BUILD

@@ -62,7 +62,7 @@ java_package_configuration(
         "-Xep:FutureReturnValueIgnored:ERROR",
         "-Xep:GetClassOnEnum:ERROR",
         "-Xep:ImmutableAnnotationChecker:ERROR",
-        #"-Xep:ImmutableEnumChecker:ERROR",
+        "-Xep:ImmutableEnumChecker:ERROR",
         "-Xep:IncompatibleModifiers:ERROR",
         "-Xep:InjectOnConstructorOfAbstractClass:ERROR",
         "-Xep:InputStreamSlowMultibyteRead:ERROR",