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>

13 files changed, 142 insertions, 44 deletions

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 6e3b3047fb..5f80b8103b 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -45,6 +45,7 @@ cannotBeCombined=Cannot be combined.
 cannotBeRecursiveWhenTreesAreIncluded=TreeWalk shouldn't be recursive when tree objects are included.
 cannotChangeActionOnComment=Cannot change action on comment line in git-rebase-todo file, old action: {0}, new action: {1}.
 cannotChangeToComment=Cannot change a non-comment line to a comment line.
+cannotCheckoutFromUnbornBranch=Cannot checkout from unborn branch
 cannotCheckoutOursSwitchBranch=Checking out ours/theirs is only possible when checking out index, not when switching branches.
 cannotCombineSquashWithNoff=Cannot combine --squash with --no-ff.
 cannotCombineTreeFilterWithRevFilter=Cannot combine TreeFilter {0} with RevFilter {1}.
@@ -345,6 +346,7 @@ invalidPathReservedOnWindows=Invalid path (''{0}'' is reserved on Windows): {1}
 invalidReflogRevision=Invalid reflog revision: {0}
 invalidRefName=Invalid ref name: {0}
 invalidRemote=Invalid remote: {0}
+invalidRepositoryStateNoHead=Invalid repository --- cannot read HEAD
 invalidShallowObject=invalid shallow object {0}, expected commit
 invalidStageForPath=Invalid stage {0} for path {1}
 invalidTagOption=Invalid tag option: {0}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CheckoutCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CheckoutCommand.java
index 8d85bfcb15..8743ea9ac7 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CheckoutCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CheckoutCommand.java
@@ -222,6 +222,12 @@ public class CheckoutCommand extends GitCommand<Ref> {
 			}
 
 			Ref headRef = repo.getRef(Constants.HEAD);
+			if (headRef == null) {
+				// TODO Git CLI supports checkout from unborn branch, we should
+				// also allow this
+				throw new UnsupportedOperationException(
+						JGitText.get().cannotCheckoutFromUnbornBranch);
+			}
 			String shortHeadRef = getShortBranchName(headRef);
 			String refLogMessage = "checkout: moving from " + shortHeadRef; //$NON-NLS-1$
 			ObjectId branch;
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 753bc85bc6..8582bbb0dc 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
@@ -668,12 +668,13 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 	}
 
 	private void writeRewrittenHashes() throws RevisionSyntaxException,
-			IOException {
+			IOException, RefNotFoundException {
 		File currentCommitFile = rebaseState.getFile(CURRENT_COMMIT);
 		if (!currentCommitFile.exists())
 			return;
 
-		String head = repo.resolve(Constants.HEAD).getName();
+		ObjectId headId = getHead().getObjectId();
+		String head = headId.getName();
 		String currentCommits = rebaseState.readFile(CURRENT_COMMIT);
 		for (String current : currentCommits.split("\n")) //$NON-NLS-1$
 			RebaseState
@@ -743,8 +744,8 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 
 	private void resetSoftToParent() throws IOException,
 			GitAPIException, CheckoutConflictException {
-		Ref orig_head = repo.getRef(Constants.ORIG_HEAD);
-		ObjectId orig_headId = orig_head.getObjectId();
+		Ref ref = repo.getRef(Constants.ORIG_HEAD);
+		ObjectId orig_head = ref == null ? null : ref.getObjectId();
 		try {
 			// we have already commited the cherry-picked commit.
 			// what we need is to have changes introduced by this
@@ -755,7 +756,7 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 		} finally {
 			// set ORIG_HEAD back to where we started because soft
 			// reset moved it
-			repo.writeOrigHead(orig_headId);
+			repo.writeOrigHead(orig_head);
 		}
 	}
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RenameBranchCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RenameBranchCommand.java
index 607253b76d..0731dd45ec 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RenameBranchCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RenameBranchCommand.java
@@ -51,6 +51,7 @@ import org.eclipse.jgit.api.errors.DetachedHeadException;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.api.errors.InvalidRefNameException;
 import org.eclipse.jgit.api.errors.JGitInternalException;
+import org.eclipse.jgit.api.errors.NoHeadException;
 import org.eclipse.jgit.api.errors.RefAlreadyExistsException;
 import org.eclipse.jgit.api.errors.RefNotFoundException;
 import org.eclipse.jgit.internal.JGitText;
@@ -121,6 +122,10 @@ public class RenameBranchCommand extends GitCommand<Ref> {
 				fullOldName = ref.getName();
 			} else {
 				fullOldName = repo.getFullBranch();
+				if (fullOldName == null) {
+					throw new NoHeadException(
+							JGitText.get().invalidRepositoryStateNoHead);
+				}
 				if (ObjectId.isId(fullOldName))
 					throw new DetachedHeadException();
 			}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index b03038b062..6680564ade 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -104,6 +104,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String cannotBeRecursiveWhenTreesAreIncluded;
 	/***/ public String cannotChangeActionOnComment;
 	/***/ public String cannotChangeToComment;
+	/***/ public String cannotCheckoutFromUnbornBranch;
 	/***/ public String cannotCheckoutOursSwitchBranch;
 	/***/ public String cannotCombineSquashWithNoff;
 	/***/ public String cannotCombineTreeFilterWithRevFilter;
@@ -411,6 +412,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String invalidURL;
 	/***/ public String invalidWildcards;
 	/***/ public String invalidRefSpec;
+	/***/ public String invalidRepositoryStateNoHead;
 	/***/ public String invalidWindowSize;
 	/***/ public String isAStaticFlagAndHasNorevWalkInstance;
 	/***/ public String JRELacksMD5Implementation;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
index e7005c247e..4c40538b6a 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
@@ -90,6 +90,7 @@ import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Ref.Storage;
 import org.eclipse.jgit.lib.RefDatabase;
 import org.eclipse.jgit.lib.ReflogEntry;
+import org.eclipse.jgit.lib.ReflogReader;
 import org.eclipse.jgit.revwalk.ObjectWalk;
 import org.eclipse.jgit.revwalk.RevObject;
 import org.eclipse.jgit.revwalk.RevWalk;
@@ -592,7 +593,11 @@ public class GC {
 	 * @throws IOException
 	 */
 	private Set<ObjectId> listRefLogObjects(Ref ref, long minTime) throws IOException {
-		List<ReflogEntry> rlEntries = repo.getReflogReader(ref.getName())
+		ReflogReader reflogReader = repo.getReflogReader(ref.getName());
+		if (reflogReader == null) {
+			return Collections.emptySet();
+		}
+		List<ReflogEntry> rlEntries = reflogReader
 				.getReverseEntries();
 		if (rlEntries == null || rlEntries.isEmpty())
 			return Collections.<ObjectId> emptySet();
@@ -635,10 +640,7 @@ public class GC {
 	 */
 	private Set<ObjectId> listNonHEADIndexObjects()
 			throws CorruptObjectException, IOException {
-		try {
-			if (repo.getIndexFile() == null)
-				return Collections.emptySet();
-		} catch (NoWorkTreeException e) {
+		if (repo.isBare()) {
 			return Collections.emptySet();
 		}
 		try (TreeWalk treeWalk = new TreeWalk(repo)) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefRename.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefRename.java
index 05eb31183d..59f852b8c7 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefRename.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefRename.java
@@ -170,7 +170,7 @@ public abstract class RefRename {
 	 */
 	protected boolean needToUpdateHEAD() throws IOException {
 		Ref head = source.getRefDatabase().getRef(Constants.HEAD);
-		if (head.isSymbolic()) {
+		if (head != null && head.isSymbolic()) {
 			head = head.getTarget();
 			return head.getName().equals(source.getName());
 		}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
index 8d512ccb24..c91f2dab72 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
@@ -64,6 +64,8 @@ import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import org.eclipse.jgit.annotations.NonNull;
+import org.eclipse.jgit.annotations.Nullable;
 import org.eclipse.jgit.attributes.AttributesNodeProvider;
 import org.eclipse.jgit.dircache.DirCache;
 import org.eclipse.jgit.errors.AmbiguousObjectException;
@@ -138,6 +140,7 @@ public abstract class Repository implements AutoCloseable {
 	}
 
 	/** @return listeners observing only events on this repository. */
+	@NonNull
 	public ListenerList getListenerList() {
 		return myListeners;
 	}
@@ -182,7 +185,16 @@ public abstract class Repository implements AutoCloseable {
 	 */
 	public abstract void create(boolean bare) throws IOException;
 
-	/** @return local metadata directory; null if repository isn't local. */
+	/**
+	 * @return local metadata directory; {@code null} if repository isn't local.
+	 */
+	/*
+	 * TODO This method should be annotated as Nullable, because in some
+	 * specific configurations metadata is not located in the local file system
+	 * (for example in memory databases). In "usual" repositories this
+	 * annotation would only cause compiler errors at places where the actual
+	 * directory can never be null.
+	 */
 	public File getDirectory() {
 		return gitDir;
 	}
@@ -190,24 +202,29 @@ public abstract class Repository implements AutoCloseable {
 	/**
 	 * @return the object database which stores this repository's data.
 	 */
+	@NonNull
 	public abstract ObjectDatabase getObjectDatabase();
 
 	/** @return a new inserter to create objects in {@link #getObjectDatabase()} */
+	@NonNull
 	public ObjectInserter newObjectInserter() {
 		return getObjectDatabase().newInserter();
 	}
 
 	/** @return a new reader to read objects from {@link #getObjectDatabase()} */
+	@NonNull
 	public ObjectReader newObjectReader() {
 		return getObjectDatabase().newReader();
 	}
 
 	/** @return the reference database which stores the reference namespace. */
+	@NonNull
 	public abstract RefDatabase getRefDatabase();
 
 	/**
 	 * @return the configuration of this repository
 	 */
+	@NonNull
 	public abstract StoredConfig getConfig();
 
 	/**
@@ -217,11 +234,20 @@ public abstract class Repository implements AutoCloseable {
 	 *         instance for each use.
 	 * @since 4.2
 	 */
+	@NonNull
 	public abstract AttributesNodeProvider createAttributesNodeProvider();
 
 
 	/**
-	 * @return the used file system abstraction
+	 * @return the used file system abstraction, or or {@code null} if
+	 *         repository isn't local.
+	 */
+	/*
+	 * TODO This method should be annotated as Nullable, because in some
+	 * specific configurations metadata is not located in the local file system
+	 * (for example in memory databases). In "usual" repositories this
+	 * annotation would only cause compiler errors at places where the actual
+	 * directory can never be null.
 	 */
 	public FS getFS() {
 		return fs;
@@ -255,6 +281,7 @@ public abstract class Repository implements AutoCloseable {
 	 * @throws IOException
 	 *             the object store cannot be accessed.
 	 */
+	@NonNull
 	public ObjectLoader open(final AnyObjectId objectId)
 			throws MissingObjectException, IOException {
 		return getObjectDatabase().open(objectId);
@@ -282,6 +309,7 @@ public abstract class Repository implements AutoCloseable {
 	 * @throws IOException
 	 *             the object store cannot be accessed.
 	 */
+	@NonNull
 	public ObjectLoader open(AnyObjectId objectId, int typeHint)
 			throws MissingObjectException, IncorrectObjectTypeException,
 			IOException {
@@ -300,6 +328,7 @@ public abstract class Repository implements AutoCloseable {
 	 *             a symbolic ref was passed in and could not be resolved back
 	 *             to the base ref, as the symbolic ref could not be read.
 	 */
+	@NonNull
 	public RefUpdate updateRef(final String ref) throws IOException {
 		return updateRef(ref, false);
 	}
@@ -318,6 +347,7 @@ public abstract class Repository implements AutoCloseable {
 	 *             a symbolic ref was passed in and could not be resolved back
 	 *             to the base ref, as the symbolic ref could not be read.
 	 */
+	@NonNull
 	public RefUpdate updateRef(final String ref, final boolean detach) throws IOException {
 		return getRefDatabase().newUpdate(ref, detach);
 	}
@@ -334,6 +364,7 @@ public abstract class Repository implements AutoCloseable {
 	 *             the rename could not be performed.
 	 *
 	 */
+	@NonNull
 	public RefRename renameRef(final String fromRef, final String toRef) throws IOException {
 		return getRefDatabase().newRename(fromRef, toRef);
 	}
@@ -373,7 +404,8 @@ public abstract class Repository implements AutoCloseable {
 	 *
 	 * @param revstr
 	 *            A git object references expression
-	 * @return an ObjectId or null if revstr can't be resolved to any ObjectId
+	 * @return an ObjectId or {@code null} if revstr can't be resolved to any
+	 *         ObjectId
 	 * @throws AmbiguousObjectException
 	 *             {@code revstr} contains an abbreviated ObjectId and this
 	 *             repository contains more than one object which match to the
@@ -387,6 +419,7 @@ public abstract class Repository implements AutoCloseable {
 	 * @throws IOException
 	 *             on serious errors
 	 */
+	@Nullable
 	public ObjectId resolve(final String revstr)
 			throws AmbiguousObjectException, IncorrectObjectTypeException,
 			RevisionSyntaxException, IOException {
@@ -408,10 +441,12 @@ public abstract class Repository implements AutoCloseable {
 	 * expects a branch or revision id.
 	 *
 	 * @param revstr
-	 * @return object id or ref name from resolved expression
+	 * @return object id or ref name from resolved expression or {@code null} if
+	 *         given expression cannot be resolved
 	 * @throws AmbiguousObjectException
 	 * @throws IOException
 	 */
+	@Nullable
 	public String simplify(final String revstr)
 			throws AmbiguousObjectException, IOException {
 		try (RevWalk rw = new RevWalk(this)) {
@@ -425,6 +460,7 @@ public abstract class Repository implements AutoCloseable {
 		}
 	}
 
+	@Nullable
 	private Object resolve(final RevWalk rw, final String revstr)
 			throws IOException {
 		char[] revChars = revstr.toCharArray();
@@ -728,11 +764,13 @@ public abstract class Repository implements AutoCloseable {
 		return true;
 	}
 
+	@Nullable
 	private RevObject parseSimple(RevWalk rw, String revstr) throws IOException {
 		ObjectId id = resolveSimple(revstr);
 		return id != null ? rw.parseAny(id) : null;
 	}
 
+	@Nullable
 	private ObjectId resolveSimple(final String revstr) throws IOException {
 		if (ObjectId.isId(revstr))
 			return ObjectId.fromString(revstr);
@@ -760,6 +798,7 @@ public abstract class Repository implements AutoCloseable {
 		return null;
 	}
 
+	@Nullable
 	private String resolveReflogCheckout(int checkoutNo)
 			throws IOException {
 		ReflogReader reader = getReflogReader(Constants.HEAD);
@@ -801,6 +840,7 @@ public abstract class Repository implements AutoCloseable {
 		return rw.parseCommit(entry.getNewId());
 	}
 
+	@Nullable
 	private ObjectId resolveAbbreviation(final String revstr) throws IOException,
 			AmbiguousObjectException {
 		AbbreviatedObjectId id = AbbreviatedObjectId.fromString(revstr);
@@ -837,11 +877,13 @@ public abstract class Repository implements AutoCloseable {
 		getRefDatabase().close();
 	}
 
+	@NonNull
 	@SuppressWarnings("nls")
 	public String toString() {
 		String desc;
-		if (getDirectory() != null)
-			desc = getDirectory().getPath();
+		File directory = getDirectory();
+		if (directory != null)
+			desc = directory.getPath();
 		else
 			desc = getClass().getSimpleName() + "-" //$NON-NLS-1$
 					+ System.identityHashCode(this);
@@ -861,10 +903,12 @@ public abstract class Repository implements AutoCloseable {
 	 * current ObjectId in hexadecimal string format.
 	 *
 	 * @return name of current branch (for example {@code refs/heads/master}),
-	 *         an ObjectId in hex format if the current branch is detached,
-	 *         or null if the repository is corrupt and has no HEAD reference.
+	 *         an ObjectId in hex format if the current branch is detached, or
+	 *         {@code null} if the repository is corrupt and has no HEAD
+	 *         reference.
 	 * @throws IOException
 	 */
+	@Nullable
 	public String getFullBranch() throws IOException {
 		Ref head = getRef(Constants.HEAD);
 		if (head == null)
@@ -883,16 +927,17 @@ public abstract class Repository implements AutoCloseable {
 	 * leading prefix {@code refs/heads/} is removed from the reference before
 	 * it is returned to the caller.
 	 *
-	 * @return name of current branch (for example {@code master}), an
-	 *         ObjectId in hex format if the current branch is detached,
-	 *         or null if the repository is corrupt and has no HEAD reference.
+	 * @return name of current branch (for example {@code master}), an ObjectId
+	 *         in hex format if the current branch is detached, or {@code null}
+	 *         if the repository is corrupt and has no HEAD reference.
 	 * @throws IOException
 	 */
+	@Nullable
 	public String getBranch() throws IOException {
 		String name = getFullBranch();
 		if (name != null)
 			return shortenRefName(name);
-		return name;
+		return null;
 	}
 
 	/**
@@ -905,6 +950,7 @@ public abstract class Repository implements AutoCloseable {
 	 *
 	 * @return unmodifiable collection of other known objects.
 	 */
+	@NonNull
 	public Set<ObjectId> getAdditionalHaves() {
 		return Collections.emptySet();
 	}
@@ -916,9 +962,10 @@ public abstract class Repository implements AutoCloseable {
 	 *            the name of the ref to lookup. May be a short-hand form, e.g.
 	 *            "master" which is is automatically expanded to
 	 *            "refs/heads/master" if "refs/heads/master" already exists.
-	 * @return the Ref with the given name, or null if it does not exist
+	 * @return the Ref with the given name, or {@code null} if it does not exist
 	 * @throws IOException
 	 */
+	@Nullable
 	public Ref getRef(final String name) throws IOException {
 		return getRefDatabase().getRef(name);
 	}
@@ -926,6 +973,7 @@ public abstract class Repository implements AutoCloseable {
 	/**
 	 * @return mutable map of all known refs (heads, tags, remotes).
 	 */
+	@NonNull
 	public Map<String, Ref> getAllRefs() {
 		try {
 			return getRefDatabase().getRefs(RefDatabase.ALL);
@@ -939,6 +987,7 @@ public abstract class Repository implements AutoCloseable {
 	 *         of the entry contains the ref with the full tag name
 	 *         ("refs/tags/v1.0").
 	 */
+	@NonNull
 	public Map<String, Ref> getTags() {
 		try {
 			return getRefDatabase().getRefs(Constants.R_TAGS);
@@ -960,6 +1009,7 @@ public abstract class Repository implements AutoCloseable {
 	 *         will be true and getPeeledObjectId will contain the peeled object
 	 *         (or null).
 	 */
+	@NonNull
 	public Ref peel(final Ref ref) {
 		try {
 			return getRefDatabase().peel(ref);
@@ -974,6 +1024,7 @@ public abstract class Repository implements AutoCloseable {
 	/**
 	 * @return a map with all objects referenced by a peeled ref.
 	 */
+	@NonNull
 	public Map<AnyObjectId, Set<Ref>> getAllRefsByPeeledObjectId() {
 		Map<String, Ref> allRefs = getAllRefs();
 		Map<AnyObjectId, Set<Ref>> ret = new HashMap<AnyObjectId, Set<Ref>>(allRefs.size());
@@ -998,11 +1049,13 @@ public abstract class Repository implements AutoCloseable {
 	}
 
 	/**
-	 * @return the index file location
+	 * @return the index file location or {@code null} if repository isn't
+	 *         local.
 	 * @throws NoWorkTreeException
 	 *             if this is bare, which implies it has no working directory.
 	 *             See {@link #isBare()}.
 	 */
+	@NonNull
 	public File getIndexFile() throws NoWorkTreeException {
 		if (isBare())
 			throw new NoWorkTreeException();
@@ -1027,6 +1080,7 @@ public abstract class Repository implements AutoCloseable {
 	 *             the index file is using a format or extension that this
 	 *             library does not support.
 	 */
+	@NonNull
 	public DirCache readDirCache() throws NoWorkTreeException,
 			CorruptObjectException, IOException {
 		return DirCache.read(this);
@@ -1051,6 +1105,7 @@ public abstract class Repository implements AutoCloseable {
 	 *             the index file is using a format or extension that this
 	 *             library does not support.
 	 */
+	@NonNull
 	public DirCache lockDirCache() throws NoWorkTreeException,
 			CorruptObjectException, IOException {
 		// we want DirCache to inform us so that we can inform registered
@@ -1076,6 +1131,7 @@ public abstract class Repository implements AutoCloseable {
 	/**
 	 * @return an important state
 	 */
+	@NonNull
 	public RepositoryState getRepositoryState() {
 		if (isBare() || getDirectory() == null)
 			return RepositoryState.BARE;
@@ -1218,6 +1274,7 @@ public abstract class Repository implements AutoCloseable {
 	 * @return normalized repository relative path or the empty
 	 *         string if the file is not relative to the work directory.
 	 */
+	@NonNull
 	public static String stripWorkDir(File workDir, File file) {
 		final String filePath = file.getPath();
 		final String workDirPath = workDir.getPath();
@@ -1252,6 +1309,7 @@ public abstract class Repository implements AutoCloseable {
 	 *             if this is bare, which implies it has no working directory.
 	 *             See {@link #isBare()}.
 	 */
+	@NonNull
 	public File getWorkTree() throws NoWorkTreeException {
 		if (isBare())
 			throw new NoWorkTreeException();
@@ -1275,6 +1333,7 @@ public abstract class Repository implements AutoCloseable {
 	 *
 	 * @return a more user friendly ref name
 	 */
+	@NonNull
 	public static String shortenRefName(String refName) {
 		if (refName.startsWith(Constants.R_HEADS))
 			return refName.substring(Constants.R_HEADS.length());
@@ -1290,9 +1349,10 @@ public abstract class Repository implements AutoCloseable {
 	 * @return the remote branch name part of <code>refName</code>, i.e. without
 	 *         the <code>refs/remotes/&lt;remote&gt;</code> prefix, if
 	 *         <code>refName</code> represents a remote tracking branch;
-	 *         otherwise null.
+	 *         otherwise {@code null}.
 	 * @since 3.4
 	 */
+	@Nullable
 	public String shortenRemoteBranchName(String refName) {
 		for (String remote : getRemoteNames()) {
 			String remotePrefix = Constants.R_REMOTES + remote + "/"; //$NON-NLS-1$
@@ -1307,9 +1367,10 @@ public abstract class Repository implements AutoCloseable {
 	 * @return the remote name part of <code>refName</code>, i.e. without the
 	 *         <code>refs/remotes/&lt;remote&gt;</code> prefix, if
 	 *         <code>refName</code> represents a remote tracking branch;
-	 *         otherwise null.
+	 *         otherwise {@code null}.
 	 * @since 3.4
 	 */
+	@Nullable
 	public String getRemoteName(String refName) {
 		for (String remote : getRemoteNames()) {
 			String remotePrefix = Constants.R_REMOTES + remote + "/"; //$NON-NLS-1$
@@ -1321,12 +1382,13 @@ public abstract class Repository implements AutoCloseable {
 
 	/**
 	 * @param refName
-	 * @return a {@link ReflogReader} for the supplied refname, or null if the
-	 *         named ref does not exist.
+	 * @return a {@link ReflogReader} for the supplied refname, or {@code null}
+	 *         if the named ref does not exist.
 	 * @throws IOException
 	 *             the ref could not be accessed.
 	 * @since 3.0
 	 */
+	@Nullable
 	public abstract ReflogReader getReflogReader(String refName)
 			throws IOException;
 
@@ -1342,6 +1404,7 @@ public abstract class Repository implements AutoCloseable {
 	 *             if this is bare, which implies it has no working directory.
 	 *             See {@link #isBare()}.
 	 */
+	@Nullable
 	public String readMergeCommitMsg() throws IOException, NoWorkTreeException {
 		return readCommitMsgFile(Constants.MERGE_MSG);
 	}
@@ -1376,6 +1439,7 @@ public abstract class Repository implements AutoCloseable {
 	 *             See {@link #isBare()}.
 	 * @since 4.0
 	 */
+	@Nullable
 	public String readCommitEditMsg() throws IOException, NoWorkTreeException {
 		return readCommitMsgFile(Constants.COMMIT_EDITMSG);
 	}
@@ -1410,6 +1474,7 @@ public abstract class Repository implements AutoCloseable {
 	 *             if this is bare, which implies it has no working directory.
 	 *             See {@link #isBare()}.
 	 */
+	@Nullable
 	public List<ObjectId> readMergeHeads() throws IOException, NoWorkTreeException {
 		if (isBare() || getDirectory() == null)
 			throw new NoWorkTreeException();
@@ -1453,6 +1518,7 @@ public abstract class Repository implements AutoCloseable {
 	 *             if this is bare, which implies it has no working directory.
 	 *             See {@link #isBare()}.
 	 */
+	@Nullable
 	public ObjectId readCherryPickHead() throws IOException,
 			NoWorkTreeException {
 		if (isBare() || getDirectory() == null)
@@ -1476,6 +1542,7 @@ public abstract class Repository implements AutoCloseable {
 	 *             if this is bare, which implies it has no working directory.
 	 *             See {@link #isBare()}.
 	 */
+	@Nullable
 	public ObjectId readRevertHead() throws IOException, NoWorkTreeException {
 		if (isBare() || getDirectory() == null)
 			throw new NoWorkTreeException();
@@ -1541,6 +1608,7 @@ public abstract class Repository implements AutoCloseable {
 	 *             if this is bare, which implies it has no working directory.
 	 *             See {@link #isBare()}.
 	 */
+	@Nullable
 	public ObjectId readOrigHead() throws IOException, NoWorkTreeException {
 		if (isBare() || getDirectory() == null)
 			throw new NoWorkTreeException();
@@ -1561,6 +1629,7 @@ public abstract class Repository implements AutoCloseable {
 	 *             if this is bare, which implies it has no working directory.
 	 *             See {@link #isBare()}.
 	 */
+	@Nullable
 	public String readSquashCommitMsg() throws IOException {
 		return readCommitMsgFile(Constants.SQUASH_MSG);
 	}
@@ -1582,6 +1651,7 @@ public abstract class Repository implements AutoCloseable {
 		writeCommitMsg(squashMsgFile, msg);
 	}
 
+	@Nullable
 	private String readCommitMsgFile(String msgFilename) throws IOException {
 		if (isBare() || getDirectory() == null)
 			throw new NoWorkTreeException();
@@ -1615,9 +1685,11 @@ public abstract class Repository implements AutoCloseable {
 	 * Read a file from the git directory.
 	 *
 	 * @param filename
-	 * @return the raw contents or null if the file doesn't exist or is empty
+	 * @return the raw contents or {@code null} if the file doesn't exist or is
+	 *         empty
 	 * @throws IOException
 	 */
+	@Nullable
 	private byte[] readGitDirectoryFile(String filename) throws IOException {
 		File file = new File(getDirectory(), filename);
 		try {
@@ -1674,6 +1746,7 @@ public abstract class Repository implements AutoCloseable {
 	 * @throws IOException
 	 * @since 3.2
 	 */
+	@NonNull
 	public List<RebaseTodoLine> readRebaseTodo(String path,
 			boolean includeComments)
 			throws IOException {
@@ -1703,6 +1776,7 @@ public abstract class Repository implements AutoCloseable {
 	 * @return the names of all known remotes
 	 * @since 3.4
 	 */
+	@NonNull
 	public Set<String> getRemoteNames() {
 		return getConfig()
 				.getSubsections(ConfigConstants.CONFIG_REMOTE_SECTION);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
index 8a6343c3cc..de08e4b6a0 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java
@@ -786,11 +786,6 @@ public class ResolveMerger extends ThreeWayMerger {
 	private File writeMergedFile(MergeResult<RawText> result)
 			throws FileNotFoundException, IOException {
 		File workTree = db.getWorkTree();
-		if (workTree == null)
-			// TODO: This should be handled by WorkingTreeIterators which
-			// support write operations
-			throw new UnsupportedOperationException();
-
 		FS fs = db.getFS();
 		File of = new File(workTree, tw.getPathString());
 		File parentFolder = of.getParentFile();
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/HMACSHA1NonceGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/HMACSHA1NonceGenerator.java
index 7e9434a0f0..622680a27f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/HMACSHA1NonceGenerator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/HMACSHA1NonceGenerator.java
@@ -42,6 +42,7 @@
  */
 package org.eclipse.jgit.transport;
 
+import java.io.File;
 import java.io.UnsupportedEncodingException;
 import java.security.InvalidKeyException;
 import java.security.NoSuchAlgorithmException;
@@ -85,12 +86,16 @@ public class HMACSHA1NonceGenerator implements NonceGenerator {
 	public synchronized String createNonce(Repository repo, long timestamp)
 			throws IllegalStateException {
 		String path;
-		if (repo instanceof DfsRepository)
+		if (repo instanceof DfsRepository) {
 			path = ((DfsRepository) repo).getDescription().getRepositoryName();
-		else if (repo.getDirectory() != null)
-			path = repo.getDirectory().getPath();
-		else
-			throw new IllegalStateException();
+		} else {
+			File directory = repo.getDirectory();
+			if (directory != null) {
+				path = directory.getPath();
+			} else {
+				throw new IllegalStateException();
+			}
+		}
 
 		String input = path + ":" + String.valueOf(timestamp); //$NON-NLS-1$
 		byte[] rawHmac;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportAmazonS3.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportAmazonS3.java
index 7729c11ff9..23c506b128 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportAmazonS3.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportAmazonS3.java
@@ -148,8 +148,9 @@ public class TransportAmazonS3 extends HttpTransport implements WalkTransport {
 		super(local, uri);
 
 		Properties props = loadProperties();
-		if (!props.containsKey("tmpdir") && local.getDirectory() != null) //$NON-NLS-1$
-			props.put("tmpdir", local.getDirectory().getPath()); //$NON-NLS-1$
+		File directory = local.getDirectory();
+		if (!props.containsKey("tmpdir") && directory != null) //$NON-NLS-1$
+			props.put("tmpdir", directory.getPath()); //$NON-NLS-1$
 
 		s3 = new AmazonS3(props);
 		bucket = uri.getHost();
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportGitSsh.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportGitSsh.java
index b27fa0d6b2..52f0f04562 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportGitSsh.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportGitSsh.java
@@ -46,6 +46,7 @@
 
 package org.eclipse.jgit.transport;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.text.MessageFormat;
@@ -235,9 +236,10 @@ public class TransportGitSsh extends SshTransport implements PackTransport {
 			ProcessBuilder pb = new ProcessBuilder();
 			pb.command(args);
 
-			if (local.getDirectory() != null)
+			File directory = local.getDirectory();
+			if (directory != null)
 				pb.environment().put(Constants.GIT_DIR_KEY,
-						local.getDirectory().getPath());
+						directory.getPath());
 
 			try {
 				return pb.start();
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java
index 28d628b345..ec581b397a 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32_Cygwin.java
@@ -168,6 +168,9 @@ public class FS_Win32_Cygwin extends FS_Win32 {
 	@Override
 	public File findHook(Repository repository, String hookName) {
 		final File gitdir = repository.getDirectory();
+		if (gitdir == null) {
+			return null;
+		}
 		final Path hookPath = gitdir.toPath().resolve(Constants.HOOKS)
 				.resolve(hookName);
 		if (Files.isExecutable(hookPath))