Error Prone: Enable and fix ReferenceEquality
An unfortunate number of instances of this warning are actually
depending on this behavior, but in those cases it's probably better to
have an actual method making the intent explicit. The exception is
WrappableSearcherManager, which is intended to have minimal diff from
the implementation we copied from Lucene.
In the cases in PatchScriptBuilder, it was using "x == y || x.equals(y)"
as a sort of null-safe comparison, which we now have Objects#equals for.
Change-Id: I8781765813459c7fd03a45d4ef0fb355b8c5f486
diff --git a/java/com/google/gerrit/lucene/WrappableSearcherManager.java b/java/com/google/gerrit/lucene/WrappableSearcherManager.java
index ba8d7da..4044b90 100644
--- a/java/com/google/gerrit/lucene/WrappableSearcherManager.java
+++ b/java/com/google/gerrit/lucene/WrappableSearcherManager.java
@@ -177,7 +177,7 @@
* Expert: creates a searcher from the provided {@link IndexReader} using the provided {@link
* SearcherFactory}. NOTE: this decRefs incoming reader on throwing an exception.
*/
- @SuppressWarnings("resource")
+ @SuppressWarnings({"resource", "ReferenceEquality"})
public static IndexSearcher getSearcher(SearcherFactory searcherFactory, IndexReader reader)
throws IOException {
boolean success = false;
diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java b/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java
index 976813f..e92a0f6 100644
--- a/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java
+++ b/java/com/google/gerrit/server/index/change/ChangeIndexRewriter.java
@@ -155,7 +155,7 @@
MutableInteger leafTerms = new MutableInteger();
Predicate<ChangeData> out = rewriteImpl(in, index, opts, leafTerms);
- if (in == out || out instanceof IndexPredicate) {
+ if (isSameInstance(in, out) || out instanceof IndexPredicate) {
return new IndexedChangeQuery(index, out, opts);
} else if (out == null /* cannot rewrite */) {
return in;
@@ -207,7 +207,7 @@
for (int i = 0; i < n; i++) {
Predicate<ChangeData> c = in.getChild(i);
Predicate<ChangeData> nc = rewriteImpl(c, index, opts, leafTerms);
- if (nc == c) {
+ if (isSameInstance(nc, c)) {
isIndexed.set(i);
newChildren.add(c);
} else if (nc == null /* cannot rewrite c */) {
@@ -291,4 +291,9 @@
return p.getChildCount() > 0
&& (p instanceof AndPredicate || p instanceof OrPredicate || p instanceof NotPredicate);
}
+
+ @SuppressWarnings("ReferenceEquality")
+ private static <T> boolean isSameInstance(T a, T b) {
+ return a == b;
+ }
}
diff --git a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
index 61f0180..acf88e1 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
@@ -37,6 +37,7 @@
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
+import java.util.Objects;
import java.util.Optional;
import org.eclipse.jgit.diff.Edit;
import org.eclipse.jgit.errors.CorruptObjectException;
@@ -500,8 +501,7 @@
try {
final boolean reuse;
if (Patch.COMMIT_MSG.equals(path)) {
- if (comparisonType.isAgainstParentOrAutoMerge()
- && (aId == within || within.equals(aId))) {
+ if (comparisonType.isAgainstParentOrAutoMerge() && Objects.equals(aId, within)) {
id = ObjectId.zeroId();
src = Text.EMPTY;
srcContent = Text.NO_BYTES;
@@ -520,8 +520,7 @@
}
reuse = false;
} else if (Patch.MERGE_LIST.equals(path)) {
- if (comparisonType.isAgainstParentOrAutoMerge()
- && (aId == within || within.equals(aId))) {
+ if (comparisonType.isAgainstParentOrAutoMerge() && Objects.equals(aId, within)) {
id = ObjectId.zeroId();
src = Text.EMPTY;
srcContent = Text.NO_BYTES;
diff --git a/java/com/google/gerrit/sshd/commands/UploadArchive.java b/java/com/google/gerrit/sshd/commands/UploadArchive.java
index c49622d..a58e472 100644
--- a/java/com/google/gerrit/sshd/commands/UploadArchive.java
+++ b/java/com/google/gerrit/sshd/commands/UploadArchive.java
@@ -163,7 +163,8 @@
}
}
- @SuppressWarnings("StringEquality") // JGit API depends on reference equality with sentinel.
+ // JGit API depends on reference equality with sentinel.
+ @SuppressWarnings({"ReferenceEquality", "StringEquality"})
private static boolean isPacketLineEnd(String s) {
return s == PacketLineIn.END;
}
diff --git a/tools/BUILD b/tools/BUILD
index 65ac23d..c3cc95b 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -85,7 +85,7 @@
"-Xep:PreconditionsInvalidPlaceholder:ERROR",
"-Xep:ProtoFieldPreconditionsCheckNotNull:ERROR",
"-Xep:ProtocolBufferOrdinal:ERROR",
- #"-Xep:ReferenceEquality:ERROR",
+ "-Xep:ReferenceEquality:ERROR",
"-Xep:RequiredModifiers:ERROR",
"-Xep:ShortCircuitBoolean:ERROR",
"-Xep:SimpleDateFormatConstant:ERROR",
