Fix EOF handling in TextFragment#ApplyStrict

io.EOF was not properly accounted for when dealing with patches that are
missing trailing newline characters.

Author: bluekeyes

gitdiff/apply.go

@@ -53,6 +53,9 @@ func applyError(err error, args ...interface{}) error {
 
 	e, ok := err.(*ApplyError)
 	if !ok {
+		if err == io.EOF {
+			err = io.ErrUnexpectedEOF
+		}
 		e = &ApplyError{err: err}
 	}
 	for _, arg := range args {
@@ -115,8 +118,8 @@ func (f *TextFragment) ApplyStrict(dst io.Writer, src LineReader) error {
 	// line numbers are zero-indexed, positions are one-indexed
 	limit := f.OldPosition - 1
 
-	// an EOF is allowed here: the fragment applies to the last line of the
-	// source but it does not have a newline character
+	// io.EOF is acceptable here: the first line of the patch is the last of
+	// the source and it has no newline character
 	nextLine, n, err := copyLines(dst, src, limit)
 	if err != nil && err != io.EOF {
 		return applyError(err, lineNum(n))
@@ -127,16 +130,16 @@ func (f *TextFragment) ApplyStrict(dst io.Writer, src LineReader) error {
 		if err := applyTextLine(dst, nextLine, line); err != nil {
 			return applyError(err, lineNum(n), fragLineNum(i))
 		}
-		if fromSrc(line) {
+		if line.Old() {
 			used++
 		}
 		// advance reader if the next fragment line appears in src and we're behind
-		if i < len(f.Lines)-1 && fromSrc(f.Lines[i+1]) && int64(n)-limit < used {
+		if i < len(f.Lines)-1 && f.Lines[i+1].Old() && int64(n)-limit < used {
 			nextLine, n, err = src.ReadLine()
-			if err != nil {
-				if err == io.EOF {
-					err = io.ErrUnexpectedEOF
-				}
+			switch {
+			case err == io.EOF && f.Lines[i+1].NoEOL():
+				continue
+			case err != nil:
 				return applyError(err, lineNum(n), fragLineNum(i+1)) // report for _next_ line in fragment
 			}
 		}
@@ -159,14 +162,10 @@ func applyTextLine(dst io.Writer, src string, line Line) (err error) {
 	return
 }
 
-func fromSrc(line Line) bool {
-	return line.Op != OpAdd
-}
-
 // copyLines copies from src to dst until the line at limit, exclusive. Returns
-// the line at limit and the line number. The line number may not equal the
-// limit if and only if a non-EOF error occurs. A negative limit means the
-// first read should return io.EOF and no data.
+// the line at limit and the line number. If the error is nil or io.EOF, the
+// line number equals limit. A negative limit checks that the source has no
+// more lines to read.
 func copyLines(dst io.Writer, src LineReader, limit int64) (string, int, error) {
 	// TODO(bkeyes): fix int vs int64 for limit and return value
 	for {

gitdiff/apply_test.go

@@ -12,14 +12,18 @@ func TestTextFragmentApplyStrict(t *testing.T) {
 		File string
 		Err  bool
 	}{
-		"createFile":   {File: "new"},
-		"deleteFile":   {File: "delete_all"},
-		"addStart":     {File: "add_start"},
-		"addMiddle":    {File: "add_middle"},
-		"addEnd":       {File: "add_end"},
-		"changeStart":  {File: "change_start"},
-		"changeMiddle": {File: "change_middle"},
-		"changeEnd":    {File: "change_end"},
+		"createFile": {File: "new"},
+		"deleteFile": {File: "delete_all"},
+
+		"addStart":    {File: "add_start"},
+		"addMiddle":   {File: "add_middle"},
+		"addEnd":      {File: "add_end"},
+		"addEndNoEOL": {File: "add_end_noeol"},
+
+		"changeStart":       {File: "change_start"},
+		"changeMiddle":      {File: "change_middle"},
+		"changeEnd":         {File: "change_end"},
+		"changeSingleNoEOL": {File: "change_single_noeol"},
 	}
 
 	for name, test := range tests {

gitdiff/gitdiff.go

@@ -137,6 +137,21 @@ func (fl Line) String() string {
 	return fl.Op.String() + fl.Line
 }
 
+// Old returns true if the line appears in the old content of the fragment.
+func (fl Line) Old() bool {
+	return fl.Op != OpAdd
+}
+
+// New returns true if the line appears in the new content of the fragment.
+func (fl Line) New() bool {
+	return fl.Op == OpAdd
+}
+
+// NoEOL returns true if the line is missing a trailing newline character.
+func (fl Line) NoEOL() bool {
+	return len(fl.Line) == 0 || fl.Line[len(fl.Line)-1] != '\n'
+}
+
 // LineOp describes the type of a text fragment line: context, added, or removed.
 type LineOp int
 

gitdiff/testdata/apply/text_fragment_add_end_noeol.dst

@@ -0,0 +1,5 @@
+line 1
+line 2
+line 3
+line 4
+line 5

gitdiff/testdata/apply/text_fragment_add_end_noeol.patch

@@ -0,0 +1,11 @@
+diff --git a/gitdiff/testdata/apply/text_fragment_add_end_noeol.src b/gitdiff/testdata/apply/text_fragment_add_end_noeol.src
+--- a/gitdiff/testdata/apply/text_fragment_add_end_noeol.src
++++ b/gitdiff/testdata/apply/text_fragment_add_end_noeol.src
+@@ -1,3 +1,5 @@
+ line 1
+ line 2
+-line 3
+\ No newline at end of file
++line 3
++line 4
++line 5

gitdiff/testdata/apply/text_fragment_add_end_noeol.src

@@ -0,0 +1,3 @@
+line 1
+line 2
+line 3

gitdiff/testdata/apply/text_fragment_change_single_noeol.dst

@@ -0,0 +1 @@
+new line a

gitdiff/testdata/apply/text_fragment_change_single_noeol.patch

@@ -0,0 +1,8 @@
+diff --git a/gitdiff/testdata/apply/text_fragment_change_single_noeol.src b/gitdiff/testdata/apply/text_fragment_change_single_noeol.src
+--- a/gitdiff/testdata/apply/text_fragment_change_single_noeol.src
++++ b/gitdiff/testdata/apply/text_fragment_change_single_noeol.src
+@@ -1 +1 @@
+-line 1
+\ No newline at end of file
++new line a
+\ No newline at end of file

gitdiff/testdata/apply/text_fragment_change_single_noeol.src

@@ -0,0 +1 @@
+line 1