From a59b3735891ea4e302e97d49cedb4df4768e42c0 Mon Sep 17 00:00:00 2001 From: Andrey Loskutov Date: Fri, 27 Nov 2015 22:56:24 +0100 Subject: Fixed violations of Ref.getObjectId() nullness contract Ref.getObjectId() is actually @Nullable (see ObjectIdRef() constructor javadoc), so clients must check the return value before dereferencing it. Fixed all potential compiler errors appearing in EGit after annotating Ref.getObjectId() with @Nullable. Change-Id: Idb9aac1a8d4039a664cf544604b90ec3fb083c22 Signed-off-by: Andrey Loskutov --- .../src/org/eclipse/egit/core/RepositoryUtil.java | 34 +++++++++++++++------- .../egit/ui/internal/clone/SourceBranchPage.java | 16 +++++++--- .../ui/internal/commit/command/RevertHandler.java | 8 +++-- .../egit/ui/internal/history/CommitGraphTable.java | 14 +++++++-- .../command/AbstractHistoryCommandHandler.java | 10 +++++-- .../egit/ui/internal/push/RefUpdateElement.java | 8 +++-- .../repository/RepositoriesViewLabelProvider.java | 13 +++++---- .../tree/RepositoriesViewPropertyTester.java | 13 +++++---- 8 files changed, 82 insertions(+), 34 deletions(-) diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryUtil.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryUtil.java index 7abe03af8f..a6c1dc762a 100644 --- a/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryUtil.java +++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryUtil.java @@ -191,15 +191,18 @@ public class RepositoryUtil { if (checkoutEntry != null) { Ref ref = repository.getRef(checkoutEntry.getToBranch()); if (ref != null) { - if (ref.getObjectId().getName() - .equals(commitId)) + ObjectId objectId = ref.getObjectId(); + if (objectId != null && objectId.getName() + .equals(commitId)) { return checkoutEntry.getToBranch(); + } ref = repository.peel(ref); } if (ref != null) { ObjectId id = ref.getPeeledObjectId(); - if (id != null && id.getName().equals(commitId)) + if (id != null && id.getName().equals(commitId)) { return checkoutEntry.getToBranch(); + } } } } @@ -289,7 +292,9 @@ public class RepositoryUtil { Map remoteBranches = repository .getRefDatabase().getRefs(Constants.R_HEADS); for (Ref branch : remoteBranches.values()) { - if (branch.getObjectId().name().equals(commitId)) { + ObjectId objectId = branch.getObjectId(); + if (objectId != null + && objectId.name().equals(commitId)) { branchNames.add(branch.getName()); } } @@ -311,7 +316,9 @@ public class RepositoryUtil { Map remoteBranches = repository .getRefDatabase().getRefs(Constants.R_REMOTES); for (Ref branch : remoteBranches.values()) { - if (branch.getObjectId().name().equals(commitId)) { + ObjectId objectId = branch.getObjectId(); + if (objectId != null + && objectId.name().equals(commitId)) { branchNames.add(branch.getName()); } } @@ -489,18 +496,25 @@ public class RepositoryUtil { */ public String getShortBranch(Repository repository) throws IOException { Ref head = repository.getRef(Constants.HEAD); - if (head == null || head.getObjectId() == null) + if (head == null) { return CoreText.RepositoryUtil_noHead; + } + ObjectId objectId = head.getObjectId(); + if (objectId == null) { + return CoreText.RepositoryUtil_noHead; + } - if (head.isSymbolic()) + if (head.isSymbolic()) { return repository.getBranch(); + } - String id = head.getObjectId().name(); + String id = objectId.name(); String ref = mapCommitToRef(repository, id, false); - if (ref != null) + if (ref != null) { return Repository.shortenRefName(ref) + ' ' + id.substring(0, 7); - else + } else { return id.substring(0, 7); + } } /** diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/SourceBranchPage.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/SourceBranchPage.java index 143623f992..1484b5f9e0 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/SourceBranchPage.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/SourceBranchPage.java @@ -47,6 +47,7 @@ import org.eclipse.jface.viewers.Viewer; import org.eclipse.jface.wizard.WizardPage; import org.eclipse.jgit.api.errors.TransportException; import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.storage.file.FileRepositoryBuilder; @@ -359,15 +360,22 @@ class SourceBranchPage extends WizardPage { final String masterBranchRef = Constants.R_HEADS + Constants.MASTER; for (final Ref r : listRemoteOp.getRemoteRefs()) { final String n = r.getName(); - if (!n.startsWith(Constants.R_HEADS)) + if (!n.startsWith(Constants.R_HEADS)) { continue; + } availableRefs.add(r); - if (idHEAD == null || headIsMaster) + if (idHEAD == null || headIsMaster) { + continue; + } + ObjectId objectId = r.getObjectId(); + if (objectId == null) { continue; - if (r.getObjectId().equals(idHEAD.getObjectId())) { + } + if (objectId.equals(idHEAD.getObjectId())) { headIsMaster = masterBranchRef.equals(r.getName()); - if (head == null || headIsMaster) + if (head == null || headIsMaster) { head = r; + } } } Collections.sort(availableRefs, new Comparator() { diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/command/RevertHandler.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/command/RevertHandler.java index ec0e792019..c1de1a9dd9 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/command/RevertHandler.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/command/RevertHandler.java @@ -30,6 +30,7 @@ import org.eclipse.egit.ui.internal.commit.RepositoryCommit; import org.eclipse.egit.ui.internal.dialogs.RevertFailureDialog; import org.eclipse.jface.dialogs.MessageDialog; import org.eclipse.jgit.api.MergeResult; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; @@ -101,9 +102,12 @@ public class RevertHandler extends CommitCommandHandler { } private boolean contains(List refs, RevCommit commit) { - for (Ref ref : refs) - if (ref.getObjectId().equals(commit.getId())) + for (Ref ref : refs) { + ObjectId objectId = ref.getObjectId(); + if (objectId != null && objectId.equals(commit.getId())) { return true; + } + } return false; } diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitGraphTable.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitGraphTable.java index 5841502114..75c7de7a87 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitGraphTable.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitGraphTable.java @@ -63,6 +63,7 @@ import org.eclipse.jface.viewers.StructuredSelection; import org.eclipse.jface.viewers.TableLayout; import org.eclipse.jface.viewers.TableViewer; import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revplot.PlotCommit; @@ -860,11 +861,18 @@ class CommitGraphTable { Map branches = lastInput.getRepository() .getRefDatabase().getRefs(Constants.R_HEADS); int count = 0; + IStructuredSelection selection = (IStructuredSelection) selectionProvider + .getSelection(); + if (selection.isEmpty()) { + return false; + } + ObjectId selectedId = ((RevCommit) selection.getFirstElement()) + .getId(); for (Ref branch : branches.values()) { - if (branch.getLeaf().getObjectId() - .equals(((RevCommit) ((IStructuredSelection) selectionProvider - .getSelection()).getFirstElement()).getId())) + ObjectId objectId = branch.getLeaf().getObjectId(); + if (objectId != null && objectId.equals(selectedId)) { count++; + } } return (count > 1); diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/command/AbstractHistoryCommandHandler.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/command/AbstractHistoryCommandHandler.java index 415ef14d24..6390f66579 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/command/AbstractHistoryCommandHandler.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/command/AbstractHistoryCommandHandler.java @@ -270,15 +270,19 @@ abstract class AbstractHistoryCommandHandler extends AbstractHandler { List nodes = new ArrayList(); try { Map branches = new HashMap(); - for (String refPrefix : refPrefixes) + for (String refPrefix : refPrefixes) { branches.putAll(repo.getRefDatabase().getRefs(refPrefix)); + } for (Ref branch : branches.values()) { - if (branch.getLeaf().getObjectId().equals(commit)) + ObjectId objectId = branch.getLeaf().getObjectId(); + if (objectId != null && objectId.equals(commit)) { availableBranches.add(branch); + } } RepositoryNode repoNode = new RepositoryNode(null, repo); - for (Ref ref : availableBranches) + for (Ref ref : availableBranches) { nodes.add(new RefNode(repoNode, repo, ref)); + } } catch (IOException e) { // ignore here diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/RefUpdateElement.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/RefUpdateElement.java index 130f7f0eb4..6288906f05 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/RefUpdateElement.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/RefUpdateElement.java @@ -23,6 +23,7 @@ import org.eclipse.jface.resource.ImageDescriptor; import org.eclipse.jface.viewers.IDecoration; import org.eclipse.jface.viewers.StyledString; import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; @@ -255,11 +256,14 @@ class RefUpdateElement extends WorkbenchAdapter { StyledString.DECORATIONS_STYLER); } else { String separator = update.isFastForward() ? ".." : "..."; //$NON-NLS-1$ //$NON-NLS-2$ + ObjectId objectId = oldRef.getObjectId(); + Object oldName = objectId != null + ? objectId.abbreviate(7).name() : "?"; //$NON-NLS-1$ styled.append(MessageFormat.format( UIText.RefUpdateElement_CommitRangeDecoration, update.getNewObjectId().abbreviate(7).name(), - separator, oldRef.getObjectId().abbreviate(7) - .name()), StyledString.DECORATIONS_STYLER); + separator, oldName), + StyledString.DECORATIONS_STYLER); styled.append(' '); styled.append(MessageFormat.format( UIText.RefUpdateElement_CommitCountDecoration, diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesViewLabelProvider.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesViewLabelProvider.java index f5d79f821f..fe1c2248e2 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesViewLabelProvider.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesViewLabelProvider.java @@ -183,17 +183,20 @@ public class RepositoriesViewLabelProvider extends ColumnLabelProvider RevCommit commit = rw.parseCommit(id); compareString = commit.getId().name(); } - } else if (refName.equals(Constants.HEAD)) + } else if (refName.equals(Constants.HEAD)) { return getDecoratedImage(image); - else { + } else { String leafname = leaf.getName(); if (leafname.startsWith(Constants.R_REFS) && leafname.equals(node.getRepository() - .getFullBranch())) + .getFullBranch())) { return getDecoratedImage(image); - else if (leaf.getObjectId().equals( - node.getRepository().resolve(Constants.HEAD))) + } + ObjectId objectId = leaf.getObjectId(); + if (objectId != null && objectId.equals( + node.getRepository().resolve(Constants.HEAD))) { return getDecoratedImage(image); + } // some other symbolic reference return image; } diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoriesViewPropertyTester.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoriesViewPropertyTester.java index 5504822e7b..d4cc1352b1 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoriesViewPropertyTester.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoriesViewPropertyTester.java @@ -17,6 +17,7 @@ import org.eclipse.core.expressions.PropertyTester; import org.eclipse.egit.ui.internal.ResourcePropertyTester; import org.eclipse.egit.ui.internal.trace.GitTraceLocation; import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.RepositoryState; @@ -64,17 +65,19 @@ public class RepositoriesViewPropertyTester extends PropertyTester { if (ref.getName().startsWith(Constants.R_REFS)) { return ref.getName().equals( node.getRepository().getFullBranch()); - } else if (ref.getName().equals(Constants.HEAD)) + } else if (ref.getName().equals(Constants.HEAD)) { return true; - else { + } else { String leafname = ref.getLeaf().getName(); if (leafname.startsWith(Constants.R_REFS) && leafname.equals(node.getRepository() - .getFullBranch())) + .getFullBranch())) { return true; - else - ref.getLeaf().getObjectId().equals( + } else { + ObjectId objectId = ref.getLeaf().getObjectId(); + return objectId != null && objectId.equals( node.getRepository().resolve(Constants.HEAD)); + } } } catch (IOException e) { return false; -- cgit v1.2.3