Null-annotated Repository class and fixed related compiler errors

org.eclipse.jgit.lib.Repository class is an example of the API which
should be written with Java 8 java.util.Optional<T> type. Unfortunately
this API is already released and widely used. The good clients are
currently doing their best with checking return values for null and bad
clients do not know how bad their code is.

I've tried not to change any logic and to be as less intrusive as
possible. Most of the JGit code was well prepared to this, only few
classes needed some smaller fixes.

This change fixes all compiler errors in JGit and replaces possible
NPE's with either appropriate exceptions, avoiding multiple "Nullable
return" method calls or early returning from the method.

Because annotating getDirectory() and getFS() as Nullable would cause
lot of additional changes in JGit and EGit they are postponed.

Change-Id: Ie8369d2c9c5fac5ce83b3b1b9bc217d7b55502a3
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>

8 files changed, 33 insertions, 11 deletions

diff --git a/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties b/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties
index aa32478ff0..8aeb7e8753 100644
--- a/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties
+++ b/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties
@@ -143,6 +143,7 @@ notAJgitCommand={0} is not a jgit command
 notARevision=Not a revision: {0}
 notATree={0} is not a tree
 notAValidRefName={0} is not a valid ref name
+notAValidCommitName={0} is not a valid commit name
 notAnIndexFile={0} is not an index file
 notAnObject={0} is not an object
 notFound=!! NOT FOUND !!
diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Branch.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Branch.java
index 83a1ca7e25..65aa24f356 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Branch.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Branch.java
@@ -154,10 +154,14 @@ class Branch extends TextBuiltin {
 					startBranch = Constants.HEAD;
 				Ref startRef = db.getRef(startBranch);
 				ObjectId startAt = db.resolve(startBranch + "^0"); //$NON-NLS-1$
-				if (startRef != null)
+				if (startRef != null) {
 					startBranch = startRef.getName();
-				else
+				} else if (startAt != null) {
 					startBranch = startAt.name();
+				} else {
+					throw die(MessageFormat.format(
+							CLIText.get().notAValidCommitName, startBranch));
+				}
 				startBranch = Repository.shortenRefName(startBranch);
 				String newRefName = newHead;
 				if (!newRefName.startsWith(Constants.R_HEADS))
@@ -249,7 +253,7 @@ class Branch extends TextBuiltin {
 		String current = db.getBranch();
 		ObjectId head = db.resolve(Constants.HEAD);
 		for (String branch : branches) {
-			if (current.equals(branch)) {
+			if (branch.equals(current)) {
 				throw die(MessageFormat.format(CLIText.get().cannotDeleteTheBranchWhichYouAreCurrentlyOn, branch));
 			}
 			RefUpdate update = db.updateRef((remote ? Constants.R_REMOTES
diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Commit.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Commit.java
index f18242d684..38d8d70cef 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Commit.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Commit.java
@@ -96,6 +96,9 @@ class Commit extends TextBuiltin {
 			commitCmd.setAmend(amend);
 			commitCmd.setAll(all);
 			Ref head = db.getRef(Constants.HEAD);
+			if (head == null) {
+				throw die(CLIText.get().onBranchToBeBorn);
+			}
 			RevCommit commit;
 			try {
 				commit = commitCmd.call();
diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Merge.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Merge.java
index e0ff0583cb..cd65af9549 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Merge.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Merge.java
@@ -120,7 +120,7 @@ class Merge extends TextBuiltin {
 			throw die(MessageFormat.format(
 					CLIText.get().refDoesNotExistOrNoCommit, ref));
 
-		Ref oldHead = db.getRef(Constants.HEAD);
+		Ref oldHead = getOldHead();
 		MergeResult result;
 		try (Git git = new Git(db)) {
 			MergeCommand mergeCmd = git.merge().setStrategy(mergeStrategy)
@@ -205,6 +205,14 @@ class Merge extends TextBuiltin {
 		}
 	}
 
+	private Ref getOldHead() throws IOException {
+		Ref oldHead = db.getRef(Constants.HEAD);
+		if (oldHead == null) {
+			throw die(CLIText.get().onBranchToBeBorn);
+		}
+		return oldHead;
+	}
+
 	private boolean isMergedInto(Ref oldHead, AnyObjectId src)
 			throws IOException {
 		try (RevWalk revWalk = new RevWalk(db)) {
diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/DiffAlgorithms.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/DiffAlgorithms.java
index df7ebb78b8..d856989011 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/DiffAlgorithms.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/DiffAlgorithms.java
@@ -242,9 +242,10 @@ class DiffAlgorithms extends TextBuiltin {
 			}
 		});
 
-		if (db.getDirectory() != null) {
-			String name = db.getDirectory().getName();
-			File parent = db.getDirectory().getParentFile();
+		File directory = db.getDirectory();
+		if (directory != null) {
+			String name = directory.getName();
+			File parent = directory.getParentFile();
 			if (name.equals(Constants.DOT_GIT) && parent != null)
 				name = parent.getName();
 			outw.println(name + ": start at " + startId.name());
diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/RebuildCommitGraph.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/RebuildCommitGraph.java
index 494055a265..6260cd99fa 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/RebuildCommitGraph.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/RebuildCommitGraph.java
@@ -117,9 +117,12 @@ class RebuildCommitGraph extends TextBuiltin {
 	@Override
 	protected void run() throws Exception {
 		if (!really && !db.getRefDatabase().getRefs(ALL).isEmpty()) {
+			File directory = db.getDirectory();
+			String absolutePath = directory == null ? "null" //$NON-NLS-1$
+					: directory.getAbsolutePath();
 			errw.println(
 				MessageFormat.format(CLIText.get().fatalThisProgramWillDestroyTheRepository
-					, db.getDirectory().getAbsolutePath(), REALLY));
+					, absolutePath, REALLY));
 			throw die(CLIText.get().needApprovalToDestroyCurrentRepository);
 		}
 		if (!refList.isFile())
diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/TextHashFunctions.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/TextHashFunctions.java
index dcbc37bed6..887ad08af7 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/TextHashFunctions.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/TextHashFunctions.java
@@ -341,9 +341,10 @@ class TextHashFunctions extends TextBuiltin {
 			}
 		}
 
-		if (db.getDirectory() != null) {
-			String name = db.getDirectory().getName();
-			File parent = db.getDirectory().getParentFile();
+		File directory = db.getDirectory();
+		if (directory != null) {
+			String name = directory.getName();
+			File parent = directory.getParentFile();
 			if (name.equals(Constants.DOT_GIT) && parent != null)
 				name = parent.getName();
 			outw.println(name + ":"); //$NON-NLS-1$
diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/internal/CLIText.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/internal/CLIText.java
index 29d48ebd46..ce2b10c98e 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/internal/CLIText.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/internal/CLIText.java
@@ -202,6 +202,7 @@ public class CLIText extends TranslationBundle {
 	/***/ public String notARevision;
 	/***/ public String notATree;
 	/***/ public String notAValidRefName;
+	/***/ public String notAValidCommitName;
 	/***/ public String notAnIndexFile;
 	/***/ public String notAnObject;
 	/***/ public String notFound;