Revision: Add helper method for comparison to Revision.NULL When Error Prone checks are enabled, comparing to Revision.NULL with the == or != operator results in a warning: [ReferenceEquality] Comparison using reference equality instead of value equality The NULL instance is used as a sentinel, so comparing with reference equality is correct. Rather than suppressing the warning at every place in the code, add a helper method that can be used, and suppress the warning only there. Note that the "ReferenceEquality" warning is Error Prone specific and suppressing it results in an "Unsupported @SuppressWarnings" warning. Change-Id: Ia50404f76f88c405b9298a645760c019384c92ff
diff --git a/java/com/google/gitiles/DiffServlet.java b/java/com/google/gitiles/DiffServlet.java index 5b1801d..5a9f07b 100644 --- a/java/com/google/gitiles/DiffServlet.java +++ b/java/com/google/gitiles/DiffServlet.java
@@ -152,7 +152,7 @@ if (newCommit.getParentCount() > 0) { return Arrays.asList(newCommit.getParents()).contains(oldRevision.getId()); } - return oldRevision == Revision.NULL; + return Revision.isNull(oldRevision); } private static boolean isFile(TreeWalk tw) {
diff --git a/java/com/google/gitiles/GitilesView.java b/java/com/google/gitiles/GitilesView.java index 02020ee..e721c26 100644 --- a/java/com/google/gitiles/GitilesView.java +++ b/java/com/google/gitiles/GitilesView.java
@@ -228,7 +228,7 @@ public Builder setOldRevision(Revision revision) { if (type != Type.DIFF && type != Type.LOG) { revision = firstNonNull(revision, Revision.NULL); - checkState(revision == Revision.NULL, "cannot set old revision on %s view", type); + checkState(Revision.isNull(revision), "cannot set old revision on %s view", type); } this.oldRevision = revision; return this; @@ -399,7 +399,7 @@ } private void checkRevision() { - checkView(revision != Revision.NULL, "missing revision on %s view", type); + checkView(!Revision.isNull(revision), "missing revision on %s view", type); checkRepositoryIndex(); } @@ -427,7 +427,7 @@ private void checkRootedDoc() { checkView(hostName != null, "missing hostName on %s view", type); checkView(servletPath != null, "missing hostName on %s view", type); - checkView(revision != Revision.NULL, "missing revision on %s view", type); + checkView(!Revision.isNull(revision), "missing revision on %s view", type); checkView(path != null, "missing path on %s view", type); } } @@ -561,7 +561,7 @@ } public String getRevisionRange() { - if (oldRevision == Revision.NULL) { + if (Revision.isNull(oldRevision)) { if (type == Type.LOG || type == Type.DIFF) { // For types that require two revisions, NULL indicates the empty // tree/commit. @@ -676,9 +676,9 @@ break; case LOG: url.append(repositoryName).append("/+log"); - if (revision != Revision.NULL) { + if (!Revision.isNull(revision)) { url.append('/'); - if (oldRevision != Revision.NULL) { + if (!Revision.isNull(oldRevision)) { url.append(oldRevision.getName()).append(".."); } url.append(revision.getName()); @@ -764,10 +764,10 @@ // separate links in "old..new". breadcrumbs.add(breadcrumb(getRevisionRange(), diff().copyFrom(this).setPathPart(""))); } else if (type == Type.LOG) { - if (revision != Revision.NULL) { + if (!Revision.isNull(revision)) { // TODO(dborowitz): Add something in the navigation area (probably not // a breadcrumb) to allow switching between /+log/ and /+/. - if (oldRevision == Revision.NULL) { + if (Revision.isNull(oldRevision)) { breadcrumbs.add(breadcrumb(revision.getName(), log().copyFrom(this).setPathPart(null))); } else { breadcrumbs.add(breadcrumb(getRevisionRange(), log().copyFrom(this).setPathPart(null))); @@ -776,7 +776,7 @@ breadcrumbs.add(breadcrumb(Constants.HEAD, log().copyFrom(this))); } path = Strings.emptyToNull(path); - } else if (revision != Revision.NULL) { + } else if (!Revision.isNull(revision)) { breadcrumbs.add(breadcrumb(revision.getName(), revision().copyFrom(this))); } if (path != null) { @@ -850,7 +850,7 @@ } private static boolean isFirstParent(Revision rev1, Revision rev2) { - return rev2 == Revision.NULL + return Revision.isNull(rev2) || rev2.getName().equals(rev1.getName() + "^") || rev2.getName().equals(rev1.getName() + "~1"); }
diff --git a/java/com/google/gitiles/LogServlet.java b/java/com/google/gitiles/LogServlet.java index a91aeab..4d038ae 100644 --- a/java/com/google/gitiles/LogServlet.java +++ b/java/com/google/gitiles/LogServlet.java
@@ -113,7 +113,7 @@ } String title = "Log - "; - if (view.getOldRevision() != Revision.NULL) { + if (!Revision.isNull(view.getOldRevision())) { title += view.getRevisionRange(); } else { title += view.getRevision().getName(); @@ -175,7 +175,7 @@ private static GitilesView getView(HttpServletRequest req, Repository repo) throws IOException { GitilesView view = ViewFilter.getView(req); - if (view.getRevision() != Revision.NULL) { + if (!Revision.isNull(view.getRevision())) { return view; } Ref headRef = repo.exactRef(Constants.HEAD); @@ -225,7 +225,7 @@ RevWalk walk = new RevWalk(repo); try { walk.markStart(walk.parseCommit(view.getRevision().getId())); - if (view.getOldRevision() != Revision.NULL) { + if (!Revision.isNull(view.getOldRevision())) { walk.markUninteresting(walk.parseCommit(view.getOldRevision().getId())); } } catch (IncorrectObjectTypeException iote) {
diff --git a/java/com/google/gitiles/LogSoyData.java b/java/com/google/gitiles/LogSoyData.java index 8bc243f..96ef6ae 100644 --- a/java/com/google/gitiles/LogSoyData.java +++ b/java/com/google/gitiles/LogSoyData.java
@@ -176,14 +176,14 @@ private GitilesView.Builder copyAndCanonicalizeView(String revision) { // Canonicalize the view by using full SHAs. GitilesView.Builder copy = GitilesView.log().copyFrom(view); - if (view.getRevision() != Revision.NULL) { + if (!Revision.isNull(view.getRevision())) { copy.setRevision(view.getRevision()); } else if (revision != null) { copy.setRevision(Revision.named(revision)); } else { copy.setRevision(Revision.NULL); } - if (view.getOldRevision() != Revision.NULL) { + if (!Revision.isNull(view.getOldRevision())) { copy.setOldRevision(view.getOldRevision()); } return copy;
diff --git a/java/com/google/gitiles/Revision.java b/java/com/google/gitiles/Revision.java index 3fc8d37..02a04f8 100644 --- a/java/com/google/gitiles/Revision.java +++ b/java/com/google/gitiles/Revision.java
@@ -88,6 +88,11 @@ this.peeledType = peeledType; } + @SuppressWarnings("ReferenceEquality") + public static boolean isNull(Revision r) { + return r == NULL; + } + public String getName() { return name; }
diff --git a/java/com/google/gitiles/RevisionParser.java b/java/com/google/gitiles/RevisionParser.java index 89311a3..d530cdd 100644 --- a/java/com/google/gitiles/RevisionParser.java +++ b/java/com/google/gitiles/RevisionParser.java
@@ -225,7 +225,7 @@ if (!cache.isVisible(repo, walk, access, id)) { return false; } - if (result.getOldRevision() != null && result.getOldRevision() != Revision.NULL) { + if (result.getOldRevision() != null && !Revision.isNull(result.getOldRevision())) { return cache.isVisible(repo, walk, access, result.getOldRevision().getId(), id); } return true;
diff --git a/java/com/google/gitiles/ViewFilter.java b/java/com/google/gitiles/ViewFilter.java index f400ff0..2e517a1 100644 --- a/java/com/google/gitiles/ViewFilter.java +++ b/java/com/google/gitiles/ViewFilter.java
@@ -119,7 +119,7 @@ } private boolean normalize(GitilesView.Builder view, HttpServletResponse res) throws IOException { - if (view.getOldRevision() != Revision.NULL) { + if (!Revision.isNull(view.getOldRevision())) { return false; } Revision r = view.getRevision();