PatchApplier: fix handling of last newline in text patch

If the last line came from the patch, use the patch to determine whether
or not there should be a trailing newline. Otherwise use the old text.

Add test cases for
- no newline at end, last line not in patch hunk
- no newline at end, last line in patch hunk
- patch removing the last newline
- patch adding a newline at the end of file not having one

all for core.autocrlf false, true, and input.

Add a test case where the "no newline" indicator line is not the last
line of the last hunk. This can happen if the patch ends with removals
at the file end.

Bug: 581234
Change-Id: I09d079b51479b89400ad300d0662c1dcb50deab6
Also-by: Yuriy Mitrofanov <a2terminator@mail.ru>
Signed-off-by: Thomas Wolf <twolf@apache.org>

org.eclipse.jgit.test/tst/org/eclipse/jgit/patch/PatchApplierTest.java

@@ -354,6 +354,7 @@ public void testShiftDown2() throws Exception {
 			Result result = applyPatch();
 			verifyChange(result, "ShiftDown2");
 		}
+
 	}
 
 	public static class InCore extends Base {
@@ -361,10 +362,44 @@ public static class InCore extends Base {
 		public InCore() {
 			super(true);
 		}
+
+		@Test
+		public void testNoNewlineAtEnd() throws Exception {
+			init("x_d");
+
+			Result result = applyPatch();
+			verifyChange(result, "x_d");
+		}
+
+		@Test
+		public void testNoNewlineAtEndInHunk() throws Exception {
+			init("x_e");
+
+			Result result = applyPatch();
+			verifyChange(result, "x_e");
+		}
+
+		@Test
+		public void testAddNewlineAtEnd() throws Exception {
+			init("x_add_nl");
+
+			Result result = applyPatch();
+			verifyChange(result, "x_add_nl");
+		}
+
+		@Test
+		public void testRemoveNewlineAtEnd() throws Exception {
+			init("x_last_rm_nl");
+
+			Result result = applyPatch();
+			verifyChange(result, "x_last_rm_nl");
+		}
 	}
 
 	public static class WithWorktree extends Base {
-		public WithWorktree() { super(false); }
+		public WithWorktree() {
+			super(false);
+		}
 
 		@Test
 		public void testModifyNL1() throws Exception {
@@ -473,6 +508,230 @@ public void testPatchWithCrLf2() throws Exception {
 			}
 		}
 
+		@Test
+		public void testNoNewlineAtEndAutoCRLF_true() throws Exception {
+			try {
+				db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION,
+						null, ConfigConstants.CONFIG_KEY_AUTOCRLF, true);
+
+				init("x_d_crlf", true, true);
+
+				Result result = applyPatch();
+				verifyChange(result, "x_d_crlf");
+			} finally {
+				db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_AUTOCRLF);
+			}
+		}
+
+		@Test
+		public void testNoNewlineAtEndAutoCRLF_false() throws Exception {
+			try {
+				db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION,
+						null, ConfigConstants.CONFIG_KEY_AUTOCRLF, false);
+
+				init("x_d", true, true);
+
+				Result result = applyPatch();
+				verifyChange(result, "x_d");
+			} finally {
+				db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_AUTOCRLF);
+			}
+		}
+
+		@Test
+		public void testNoNewlineAtEndAutoCRLF_input() throws Exception {
+			try {
+				db.getConfig().setString(ConfigConstants.CONFIG_CORE_SECTION,
+						null, ConfigConstants.CONFIG_KEY_AUTOCRLF, "input");
+
+				init("x_d", true, true);
+
+				Result result = applyPatch();
+				verifyChange(result, "x_d");
+			} finally {
+				db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_AUTOCRLF);
+			}
+		}
+
+		@Test
+		public void testNoNewlineAtEndInHunkAutoCRLF_true() throws Exception {
+			try {
+				db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION,
+						null, ConfigConstants.CONFIG_KEY_AUTOCRLF, true);
+
+				init("x_e_crlf", true, true);
+
+				Result result = applyPatch();
+				verifyChange(result, "x_e_crlf");
+			} finally {
+				db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_AUTOCRLF);
+			}
+		}
+
+		@Test
+		public void testNoNewlineAtEndInHunkAutoCRLF_false() throws Exception {
+			try {
+				db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION,
+						null, ConfigConstants.CONFIG_KEY_AUTOCRLF, false);
+
+				init("x_e", true, true);
+
+				Result result = applyPatch();
+				verifyChange(result, "x_e");
+			} finally {
+				db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_AUTOCRLF);
+			}
+		}
+
+		@Test
+		public void testNoNewlineAtEndInHunkAutoCRLF_input() throws Exception {
+			try {
+				db.getConfig().setString(ConfigConstants.CONFIG_CORE_SECTION,
+						null, ConfigConstants.CONFIG_KEY_AUTOCRLF, "input");
+
+				init("x_e", true, true);
+
+				Result result = applyPatch();
+				verifyChange(result, "x_e");
+			} finally {
+				db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_AUTOCRLF);
+			}
+		}
+
+		@Test
+		public void testAddNewlineAtEndAutoCRLF_true() throws Exception {
+			try {
+				db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION,
+						null, ConfigConstants.CONFIG_KEY_AUTOCRLF, true);
+
+				init("x_add_nl_crlf", true, true);
+
+				Result result = applyPatch();
+				verifyChange(result, "x_add_nl_crlf");
+			} finally {
+				db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_AUTOCRLF);
+			}
+		}
+
+		@Test
+		public void testAddNewlineAtEndAutoCRLF_false() throws Exception {
+			try {
+				db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION,
+						null, ConfigConstants.CONFIG_KEY_AUTOCRLF, false);
+
+				init("x_add_nl", true, true);
+
+				Result result = applyPatch();
+				verifyChange(result, "x_add_nl");
+			} finally {
+				db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_AUTOCRLF);
+			}
+		}
+
+		@Test
+		public void testAddNewlineAtEndAutoCRLF_input() throws Exception {
+			try {
+				db.getConfig().setString(ConfigConstants.CONFIG_CORE_SECTION,
+						null, ConfigConstants.CONFIG_KEY_AUTOCRLF, "input");
+
+				init("x_add_nl", true, true);
+
+				Result result = applyPatch();
+				verifyChange(result, "x_add_nl");
+			} finally {
+				db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_AUTOCRLF);
+			}
+		}
+
+		@Test
+		public void testRemoveNewlineAtEndAutoCRLF_true() throws Exception {
+			try {
+				db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION,
+						null, ConfigConstants.CONFIG_KEY_AUTOCRLF, true);
+
+				init("x_last_rm_nl_crlf", true, true);
+
+				Result result = applyPatch();
+				verifyChange(result, "x_last_rm_nl_crlf");
+			} finally {
+				db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_AUTOCRLF);
+			}
+		}
+
+		@Test
+		public void testRemoveNewlineAtEndAutoCRLF_false() throws Exception {
+			try {
+				db.getConfig().setBoolean(ConfigConstants.CONFIG_CORE_SECTION,
+						null, ConfigConstants.CONFIG_KEY_AUTOCRLF, false);
+
+				init("x_last_rm_nl", true, true);
+
+				Result result = applyPatch();
+				verifyChange(result, "x_last_rm_nl");
+			} finally {
+				db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_AUTOCRLF);
+			}
+		}
+
+		@Test
+		public void testRemoveNewlineAtEndAutoCRLF_input() throws Exception {
+			try {
+				db.getConfig().setString(ConfigConstants.CONFIG_CORE_SECTION,
+						null, ConfigConstants.CONFIG_KEY_AUTOCRLF, "input");
+
+				init("x_last_rm_nl", true, true);
+
+				Result result = applyPatch();
+				verifyChange(result, "x_last_rm_nl");
+			} finally {
+				db.getConfig().unset(ConfigConstants.CONFIG_CORE_SECTION, null,
+						ConfigConstants.CONFIG_KEY_AUTOCRLF);
+			}
+		}
+
+		@Test
+		public void testEditExample() throws Exception {
+			init("z_e", true, true);
+
+			Result result = applyPatch();
+			verifyChange(result, "z_e");
+		}
+
+		@Test
+		public void testEditNoNewline() throws Exception {
+			init("z_e_no_nl", true, true);
+
+			Result result = applyPatch();
+			verifyChange(result, "z_e_no_nl");
+		}
+
+		@Test
+		public void testEditAddNewline() throws Exception {
+			init("z_e_add_nl", true, true);
+
+			Result result = applyPatch();
+			verifyChange(result, "z_e_add_nl");
+		}
+
+		@Test
+		public void testEditRemoveNewline() throws Exception {
+			init("z_e_rm_nl", true, true);
+
+			Result result = applyPatch();
+			verifyChange(result, "z_e_rm_nl");
+		}
+
 		// Clean/smudge filter for testFiltering. The smudgetest test resources
 		// were created with C git using a clean filter sed -e "s/A/E/g" and the
 		// smudge filter sed -e "s/E/A/g". To keep the test independent of the

org.eclipse.jgit/src/org/eclipse/jgit/patch/PatchApplier.java

@@ -24,6 +24,7 @@
 import java.text.MessageFormat;
 import java.time.Instant;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
@@ -93,6 +94,9 @@
  */
 public class PatchApplier {
 
+	private static final byte[] NO_EOL = "\\ No newline at end of file" //$NON-NLS-1$
+			.getBytes(StandardCharsets.US_ASCII);
+
 	/** The tree before applying the patch. Only non-null for inCore operation. */
 	@Nullable
 	private final RevTree beforeTree;
@@ -762,6 +766,8 @@ private ContentStreamLoader applyText(RawText rt, FileHeader fh)
 		int afterLastHunk = 0;
 		int lineNumberShift = 0;
 		int lastHunkNewLine = -1;
+		boolean lastWasRemoval = false;
+		boolean noNewLineAtEndOfNew = false;
 		for (HunkHeader hh : fh.getHunks()) {
 			// We assume hunks to be ordered
 			if (hh.getNewStartLine() <= lastHunkNewLine) {
@@ -850,17 +856,26 @@ && canApplyAt(hunkLines, newLines, 0)) {
 				if (!hunkLine.hasRemaining()) {
 					// Completely empty line; accept as empty context line
 					applyAt++;
+					lastWasRemoval = false;
 					continue;
 				}
 				switch (hunkLine.array()[hunkLine.position()]) {
 				case ' ':
 					applyAt++;
+					lastWasRemoval = false;
 					break;
 				case '-':
 					newLines.remove(applyAt);
+					lastWasRemoval = true;
 					break;
 				case '+':
 					newLines.add(applyAt++, slice(hunkLine, 1));
+					lastWasRemoval = false;
+					break;
+				case '\\':
+					if (!lastWasRemoval && isNoNewlineAtEnd(hunkLine)) {
+						noNewLineAtEndOfNew = true;
+					}
 					break;
 				default:
 					break;
@@ -868,12 +883,15 @@ && canApplyAt(hunkLines, newLines, 0)) {
 			}
 			afterLastHunk = applyAt;
 		}
-		if (!isNoNewlineAtEndOfFile(fh)) {
+		// If the last line should have a newline, add a null sentinel
+		if (lastHunkNewLine >= 0 && afterLastHunk == newLines.size()) {
+			// Last line came from the patch
+			if (!noNewLineAtEndOfNew) {
+				newLines.add(null);
+			}
+		} else if (!rt.isMissingNewlineAtEnd()) {
 			newLines.add(null);
 		}
-		if (!rt.isMissingNewlineAtEnd()) {
-			oldLines.add(null);
-		}
 
 		// We could check if old == new, but the short-circuiting complicates
 		// logic for inCore patching, so just write the new thing regardless.
@@ -931,19 +949,9 @@ private ByteBuffer slice(ByteBuffer b, int off) {
 		return ByteBuffer.wrap(b.array(), newOffset, b.limit() - newOffset);
 	}
 
-	private boolean isNoNewlineAtEndOfFile(FileHeader fh) {
-		List<? extends HunkHeader> hunks = fh.getHunks();
-		if (hunks == null || hunks.isEmpty()) {
-			return false;
-		}
-		HunkHeader lastHunk = hunks.get(hunks.size() - 1);
-		byte[] buf = new byte[lastHunk.getEndOffset()
-				- lastHunk.getStartOffset()];
-		System.arraycopy(lastHunk.getBuffer(), lastHunk.getStartOffset(), buf,
-				0, buf.length);
-		RawText lhrt = new RawText(buf);
-		return lhrt.getString(lhrt.size() - 1)
-				.equals("\\ No newline at end of file"); //$NON-NLS-1$
+	private boolean isNoNewlineAtEnd(ByteBuffer hunkLine) {
+		return Arrays.equals(NO_EOL, 0, NO_EOL.length, hunkLine.array(),
+				hunkLine.position(), hunkLine.limit());
 	}
 
 	/**