[error prone] suppress AmbiguousMethodReference in AnyObjectId

Move the implementation of the static equals() method to a new method
and suppress the error. Deprecate the old method to signal that we
intend to remove it in the next major release.

See https://errorprone.info/bugpattern/AmbiguousMethodReference

Change-Id: I5e29c97f4db3e11770be589a6ccd785e2c9ac7f2
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java

@@ -728,7 +728,7 @@ public class TestRepository<R extends Repository> implements AutoCloseable {
 		ThreeWayMerger merger = MergeStrategy.RECURSIVE.newMerger(db, true);
 		merger.setBase(parent.getTree());
 		if (merger.merge(head, commit)) {
-			if (AnyObjectId.equals(head.getTree(), merger.getResultTreeId()))
+			if (AnyObjectId.isEqual(head.getTree(), merger.getResultTreeId()))
 				return null;
 			tick(1);
 			org.eclipse.jgit.lib.CommitBuilder b =

org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/VerifyReftable.java

@@ -189,7 +189,7 @@ class VerifyReftable extends TextBuiltin {
 			return;
 		}
 
-		if (!AnyObjectId.equals(exp.getObjectId(), act.getObjectId())) {
+		if (!AnyObjectId.isEqual(exp.getObjectId(), act.getObjectId())) {
 			throw die(String.format("expected %s to be %s, found %s",
 					exp.getName(),
 					id(exp.getObjectId()),
@@ -197,7 +197,8 @@ class VerifyReftable extends TextBuiltin {
 		}
 
 		if (exp.getPeeledObjectId() != null
-				&& !AnyObjectId.equals(exp.getPeeledObjectId(), act.getPeeledObjectId())) {
+				&& !AnyObjectId.isEqual(exp.getPeeledObjectId(),
+						act.getPeeledObjectId())) {
 			throw die(String.format("expected %s to be %s, found %s",
 					exp.getName(),
 					id(exp.getPeeledObjectId()),

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java

@@ -976,7 +976,7 @@ public class DfsGarbageCollectorTest {
 				rw.markStart(rw.parseCommit(ref.getObjectId()));
 			}
 			for (RevCommit next; (next = rw.next()) != null;) {
-				if (AnyObjectId.equals(next, id)) {
+				if (AnyObjectId.isEqual(next, id)) {
 					return true;
 				}
 			}

org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java

@@ -89,8 +89,8 @@ public class RevObjectTest extends RevWalkTestCase {
 		assertEquals(a1.hashCode(), a2.hashCode());
 		assertEquals(b1.hashCode(), b2.hashCode());
 
-		assertTrue(AnyObjectId.equals(a1, a2));
+		assertTrue(AnyObjectId.isEqual(a1, a2));
-		assertTrue(AnyObjectId.equals(b1, b2));
+		assertTrue(AnyObjectId.isEqual(b1, b2));
 	}
 
 	@Test

org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java

@@ -157,8 +157,8 @@ public class CherryPickCommand extends GitCommand<CherryPickResult> {
 				merger.setCommitNames(new String[] { "BASE", ourName, //$NON-NLS-1$
 						cherryPickName });
 				if (merger.merge(newHead, srcCommit)) {
-					if (AnyObjectId.equals(newHead.getTree().getId(), merger
+					if (AnyObjectId.isEqual(newHead.getTree().getId(),
-							.getResultTreeId()))
+							merger.getResultTreeId()))
 						continue;
 					DirCacheCheckout dco = new DirCacheCheckout(repo,
 							newHead.getTree(), repo.lockDirCache(),

org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java

@@ -575,7 +575,7 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 			ObjectId headId = getHead().getObjectId();
 			// getHead() checks for null
 			assert headId != null;
-			if (!AnyObjectId.equals(headId, newParents.get(0)))
+			if (!AnyObjectId.isEqual(headId, newParents.get(0)))
 				checkoutCommit(headId.getName(), newParents.get(0));
 
 			// Use the cherry-pick strategy if all non-first parents did not

org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java

@@ -175,8 +175,8 @@ public class RevertCommand extends GitCommand<RevCommit> {
 						+ "This reverts commit " + srcCommit.getId().getName() //$NON-NLS-1$
 						+ ".\n"; //$NON-NLS-1$
 				if (merger.merge(headCommit, srcParent)) {
-					if (AnyObjectId.equals(headCommit.getTree().getId(), merger
+					if (AnyObjectId.isEqual(headCommit.getTree().getId(),
-							.getResultTreeId()))
+							merger.getResultTreeId()))
 						continue;
 					DirCacheCheckout dco = new DirCacheCheckout(repo,
 							headCommit.getTree(), repo.lockDirCache(),

org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/KetchReplica.java

@@ -345,7 +345,7 @@ public abstract class KetchReplica {
 	}
 
 	private static boolean equals(@Nullable ObjectId a, LogIndex b) {
-		return a != null && b != null && AnyObjectId.equals(a, b);
+		return a != null && b != null && AnyObjectId.isEqual(a, b);
 	}
 
 	/**
@@ -749,7 +749,7 @@ public abstract class KetchReplica {
 				Ref oldRef = remote.remove(name);
 				ObjectId oldId = getId(oldRef);
 				ObjectId newId = tw.getObjectId(0);
-				if (!AnyObjectId.equals(oldId, newId)) {
+				if (!AnyObjectId.isEqual(oldId, newId)) {
 					delta.add(new ReceiveCommand(oldId, newId, name));
 				}
 			}

org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/LagCheck.java

@@ -106,7 +106,7 @@ class LagCheck implements AutoCloseable {
 			return UNKNOWN;
 		}
 
-		if (AnyObjectId.equals(remoteId, ObjectId.zeroId())) {
+		if (AnyObjectId.isEqual(remoteId, ObjectId.zeroId())) {
 			// Replica does not have the txnAccepted reference.
 			return LAGGING;
 		}

org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/RemoteGitReplica.java

@@ -200,7 +200,7 @@ public class RemoteGitReplica extends KetchReplica {
 	private static boolean isExpectedValue(Map<String, Ref> adv,
 			RemoteRefUpdate u) {
 		Ref r = adv.get(u.getRemoteName());
-		if (!AnyObjectId.equals(getId(r), u.getExpectedOldObjectId())) {
+		if (!AnyObjectId.isEqual(getId(r), u.getExpectedOldObjectId())) {
 			((RemoteCommand) u).cmd.setResult(LOCK_FAILURE);
 			return false;
 		}

org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/StageBuilder.java

@@ -138,7 +138,7 @@ public class StageBuilder {
 		try (RevWalk rw = new RevWalk(git);
 				TreeWalk tw = new TreeWalk(rw.getObjectReader());
 				ObjectInserter ins = git.newObjectInserter()) {
-			if (AnyObjectId.equals(oldTree, ObjectId.zeroId())) {
+			if (AnyObjectId.isEqual(oldTree, ObjectId.zeroId())) {
 				tw.addTree(new EmptyTreeIterator());
 			} else {
 				tw.addTree(rw.parseTree(oldTree));

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/ReftableBatchRefUpdate.java

@@ -252,7 +252,7 @@ public class ReftableBatchRefUpdate extends BatchRefUpdate {
 
 	private static boolean matchOld(ReceiveCommand cmd, @Nullable Ref ref) {
 		if (ref == null) {
-			return AnyObjectId.equals(ObjectId.zeroId(), cmd.getOldId())
+			return AnyObjectId.isEqual(ObjectId.zeroId(), cmd.getOldId())
 					&& cmd.getOldSymref() == null;
 		} else if (ref.isSymbolic()) {
 			return ref.getTarget().getName().equals(cmd.getOldSymref());
@@ -368,7 +368,7 @@ public class ReftableBatchRefUpdate extends BatchRefUpdate {
 			String name = cmd.getRefName();
 			ObjectId newId = cmd.getNewId();
 			String newSymref = cmd.getNewSymref();
-			if (AnyObjectId.equals(ObjectId.zeroId(), newId)
+			if (AnyObjectId.isEqual(ObjectId.zeroId(), newId)
 					&& newSymref == null) {
 				refs.add(new ObjectIdRef.Unpeeled(NEW, name, null));
 				continue;

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/UnpackedObjectCache.java

@@ -112,7 +112,7 @@ class UnpackedObjectCache {
 				if (obj == null)
 					break;
 
-				if (AnyObjectId.equals(obj, toFind))
+				if (AnyObjectId.isEqual(obj, toFind))
 					return true;
 
 				if (++i == ids.length())
@@ -132,7 +132,7 @@ class UnpackedObjectCache {
 						continue;
 				}
 
-				if (AnyObjectId.equals(obj, toAdd))
+				if (AnyObjectId.isEqual(obj, toAdd))
 					return true;
 
 				if (++i == ids.length())

org.eclipse.jgit/src/org/eclipse/jgit/lib/AnyObjectId.java

@@ -60,19 +60,37 @@ import org.eclipse.jgit.util.NB;
 public abstract class AnyObjectId implements Comparable<AnyObjectId> {
 
 	/**
-	 * Compare to object identifier byte sequences for equality.
+	 * Compare two object identifier byte sequences for equality.
 	 *
 	 * @param firstObjectId
 	 *            the first identifier to compare. Must not be null.
 	 * @param secondObjectId
 	 *            the second identifier to compare. Must not be null.
 	 * @return true if the two identifiers are the same.
+	 * @deprecated use {@link #isEqual(AnyObjectId, AnyObjectId)} instead
 	 */
+	@Deprecated
+	@SuppressWarnings("AmbiguousMethodReference")
 	public static boolean equals(final AnyObjectId firstObjectId,
 			final AnyObjectId secondObjectId) {
-		if (firstObjectId == secondObjectId)
+		return isEqual(firstObjectId, secondObjectId);
-			return true;
+	}
 
+	/**
+	 * Compare two object identifier byte sequences for equality.
+	 *
+	 * @param firstObjectId
+	 *            the first identifier to compare. Must not be null.
+	 * @param secondObjectId
+	 *            the second identifier to compare. Must not be null.
+	 * @return true if the two identifiers are the same.
+	 * @since 5.4
+	 */
+	public static boolean isEqual(final AnyObjectId firstObjectId,
+			final AnyObjectId secondObjectId) {
+		if (firstObjectId == secondObjectId) {
+			return true;
+		}
 		// We test word 3 first since the git file-based ODB
 		// uses the first byte of w1, and we use w2 as the
 		// hash code, one of those probably came up with these
@@ -80,7 +98,6 @@ public abstract class AnyObjectId implements Comparable<AnyObjectId> {
 		// Therefore the first two words are very likely to be
 		// identical. We want to break away from collisions as
 		// quickly as possible.
-		//
 		return firstObjectId.w3 == secondObjectId.w3
 				&& firstObjectId.w4 == secondObjectId.w4
 				&& firstObjectId.w5 == secondObjectId.w5
@@ -276,9 +293,9 @@ public abstract class AnyObjectId implements Comparable<AnyObjectId> {
 	 *            the other id to compare to. May be null.
 	 * @return true only if both ObjectIds have identical bits.
 	 */
-	@SuppressWarnings("NonOverridingEquals")
+	@SuppressWarnings({ "NonOverridingEquals", "AmbiguousMethodReference" })
 	public final boolean equals(AnyObjectId other) {
-		return other != null ? equals(this, other) : false;
+		return other != null ? isEqual(this, other) : false;
 	}
 
 	/** {@inheritDoc} */

org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdSubclassMap.java

@@ -103,8 +103,9 @@ public class ObjectIdSubclassMap<V extends ObjectId>
 		V obj;
 
 		while ((obj = tbl[i]) != null) {
-			if (AnyObjectId.equals(obj, toFind))
+			if (AnyObjectId.isEqual(obj, toFind)) {
 				return obj;
+			}
 			i = (i + 1) & msk;
 		}
 		return null;
@@ -162,7 +163,7 @@ public class ObjectIdSubclassMap<V extends ObjectId>
 		V obj;
 
 		while ((obj = tbl[i]) != null) {
-			if (AnyObjectId.equals(obj, newValue))
+			if (AnyObjectId.isEqual(obj, newValue))
 				return obj;
 			i = (i + 1) & msk;
 		}

org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java

@@ -753,7 +753,7 @@ public abstract class RefUpdate {
 			if (expValue != null) {
 				final ObjectId o;
 				o = oldValue != null ? oldValue : ObjectId.zeroId();
-				if (!AnyObjectId.equals(expValue, o)) {
+				if (!AnyObjectId.isEqual(expValue, o)) {
 					return Result.LOCK_FAILURE;
 				}
 			}

org.eclipse.jgit/src/org/eclipse/jgit/notes/NoteMapMerger.java

@@ -295,14 +295,14 @@ public class NoteMapMerger {
 	private static boolean sameNote(Note a, Note b) {
 		if (a == null && b == null)
 			return true;
-		return a != null && b != null && AnyObjectId.equals(a, b);
+		return a != null && b != null && AnyObjectId.isEqual(a, b);
 	}
 
 	private static boolean sameContent(Note a, Note b) {
 		if (a == null && b == null)
 			return true;
 		return a != null && b != null
-				&& AnyObjectId.equals(a.getData(), b.getData());
+				&& AnyObjectId.isEqual(a.getData(), b.getData());
 	}
 
 	private static InMemoryNoteBucket addIfNotNull(InMemoryNoteBucket result,

org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java

@@ -429,7 +429,7 @@ public class ReceiveCommand {
 		this.newId = ObjectId.zeroId();
 		this.newSymref = newSymref;
 		this.name = name;
-		if (AnyObjectId.equals(ObjectId.zeroId(), oldId)) {
+		if (AnyObjectId.isEqual(ObjectId.zeroId(), oldId)) {
 			type = Type.CREATE;
 		} else if (newSymref != null) {
 			type = Type.UPDATE;
@@ -468,7 +468,7 @@ public class ReceiveCommand {
 		this.name = name;
 		if (oldSymref == null) {
 			type = Type.CREATE;
-		} else if (!AnyObjectId.equals(ObjectId.zeroId(), newId)) {
+		} else if (!AnyObjectId.isEqual(ObjectId.zeroId(), newId)) {
 			type = Type.UPDATE;
 		} else {
 			type = Type.DELETE;
@@ -750,7 +750,7 @@ public class ReceiveCommand {
 	public void updateType(RevWalk walk) throws IOException {
 		if (typeIsCorrect)
 			return;
-		if (type == Type.UPDATE && !AnyObjectId.equals(oldId, newId)) {
+		if (type == Type.UPDATE && !AnyObjectId.isEqual(oldId, newId)) {
 			RevObject o = walk.parseAny(oldId);
 			RevObject n = walk.parseAny(newId);
 			if (!(o instanceof RevCommit)

org.eclipse.jgit/src/org/eclipse/jgit/transport/TrackingRefUpdate.java

@@ -175,7 +175,7 @@ public class TrackingRefUpdate {
 		private RefUpdate.Result decode(ReceiveCommand.Result status) {
 			switch (status) {
 			case OK:
-				if (AnyObjectId.equals(oldObjectId, newObjectId))
+				if (AnyObjectId.isEqual(oldObjectId, newObjectId))
 					return RefUpdate.Result.NO_CHANGE;
 				switch (getType()) {
 				case CREATE:

org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkFetchConnection.java

@@ -657,7 +657,7 @@ class WalkFetchConnection extends BaseFetchConnection {
 		}
 
 		ObjectId act = inserter.insert(type, raw);
-		if (!AnyObjectId.equals(id, act)) {
+		if (!AnyObjectId.isEqual(id, act)) {
 			throw new TransportException(MessageFormat.format(
 					JGitText.get().incorrectHashFor, id.name(), act.name(),
 					Constants.typeString(type),

org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkPushConnection.java

@@ -165,7 +165,7 @@ class WalkPushConnection extends BaseConnection implements PushConnection {
 				continue;
 			}
 
-			if (AnyObjectId.equals(ObjectId.zeroId(), u.getNewObjectId()))
+			if (AnyObjectId.isEqual(ObjectId.zeroId(), u.getNewObjectId()))
 				deleteCommand(u);
 			else
 				updates.add(u);

org.eclipse.jgit/src/org/eclipse/jgit/util/RefMap.java

@@ -443,11 +443,13 @@ public class RefMap extends AbstractMap<String, Ref> {
 					if (r.getName().equals(ref.getName())) {
 						final ObjectId a = r.getObjectId();
 						final ObjectId b = ref.getObjectId();
-						if (a != null && b != null && AnyObjectId.equals(a, b))
+						if (a != null && b != null
+								&& AnyObjectId.isEqual(a, b)) {
 							return true;
 						}
 					}
 				}
+			}
 			return false;
 		}