From: Matthias Sohn Date: Thu, 8 Aug 2019 13:09:46 +0000 (+0200) Subject: [error prone] suppress AmbiguousMethodReference in AnyObjectId X-Git-Tag: v5.4.1.201908211225-r~15 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=8f7e8513463487cfbcd41e40a684abec9ec10778;p=jgit.git [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 --- diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java index af4cc1cb9b..e89cf0fb83 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java @@ -728,7 +728,7 @@ public class TestRepository 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 = diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/VerifyReftable.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/VerifyReftable.java index b38de14af9..15b6ff9a9a 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/VerifyReftable.java +++ 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()), diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java index bfa30d5b4b..5a5ae1d7a3 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java +++ b/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; } } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java index 4969305de0..e3972207f7 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java +++ b/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.equals(b1, b2)); + assertTrue(AnyObjectId.isEqual(a1, a2)); + assertTrue(AnyObjectId.isEqual(b1, b2)); } @Test diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java index 65b72f7d95..c9dd547b49 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CherryPickCommand.java @@ -157,8 +157,8 @@ public class CherryPickCommand extends GitCommand { merger.setCommitNames(new String[] { "BASE", ourName, //$NON-NLS-1$ cherryPickName }); if (merger.merge(newHead, srcCommit)) { - if (AnyObjectId.equals(newHead.getTree().getId(), merger - .getResultTreeId())) + if (AnyObjectId.isEqual(newHead.getTree().getId(), + merger.getResultTreeId())) continue; DirCacheCheckout dco = new DirCacheCheckout(repo, newHead.getTree(), repo.lockDirCache(), diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java index 593874c121..0dacd4dfbf 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java @@ -575,7 +575,7 @@ public class RebaseCommand extends GitCommand { 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 diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java index 46e0df7263..ddd60b6fa2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RevertCommand.java @@ -175,8 +175,8 @@ public class RevertCommand extends GitCommand { + "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 - .getResultTreeId())) + if (AnyObjectId.isEqual(headCommit.getTree().getId(), + merger.getResultTreeId())) continue; DirCacheCheckout dco = new DirCacheCheckout(repo, headCommit.getTree(), repo.lockDirCache(), diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/KetchReplica.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/KetchReplica.java index a0176d7d2e..0e8377dd02 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/KetchReplica.java +++ b/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)); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/LagCheck.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/LagCheck.java index c09d872f0a..53fd198006 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/LagCheck.java +++ b/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; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/RemoteGitReplica.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/RemoteGitReplica.java index 3c9b187c47..4bed575f26 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/RemoteGitReplica.java +++ b/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 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; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/StageBuilder.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/StageBuilder.java index ae82dced21..815984deb2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/ketch/StageBuilder.java +++ b/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)); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/ReftableBatchRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/ReftableBatchRefUpdate.java index 47ac4ec72e..07fd00f149 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/ReftableBatchRefUpdate.java +++ b/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; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/UnpackedObjectCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/UnpackedObjectCache.java index 967754a627..ea0d269053 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/UnpackedObjectCache.java +++ b/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()) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/AnyObjectId.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/AnyObjectId.java index 791829b429..4252e41384 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/AnyObjectId.java +++ b/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 { /** - * 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 true; + return isEqual(firstObjectId, secondObjectId); + } + /** + * 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 { // 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 { * 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} */ diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdSubclassMap.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdSubclassMap.java index cd57bda82e..470275beb1 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdSubclassMap.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectIdSubclassMap.java @@ -103,8 +103,9 @@ public class ObjectIdSubclassMap 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 obj; while ((obj = tbl[i]) != null) { - if (AnyObjectId.equals(obj, newValue)) + if (AnyObjectId.isEqual(obj, newValue)) return obj; i = (i + 1) & msk; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java index 7841627666..33c7070331 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java +++ b/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; } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/notes/NoteMapMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/notes/NoteMapMerger.java index 325ff4f268..ba7223b8f0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/notes/NoteMapMerger.java +++ b/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, diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java index d61aeb04d2..a9a995cd3f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java +++ b/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) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TrackingRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TrackingRefUpdate.java index ba2a673a18..550a9ef372 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TrackingRefUpdate.java +++ b/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: diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkFetchConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkFetchConnection.java index 2bb58144ba..b289e42177 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkFetchConnection.java +++ b/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), diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkPushConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkPushConnection.java index 4c754252a3..d9103f8b27 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkPushConnection.java +++ b/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); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/RefMap.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/RefMap.java index d7a4c2535a..9663e3cef5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/RefMap.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/RefMap.java @@ -443,8 +443,10 @@ public class RefMap extends AbstractMap { 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; + } } } }