From 7bd4e0bb0e868592a89659173e287c1656c0c6f1 Mon Sep 17 00:00:00 2001 From: Billy Keyes Date: Thu, 9 Jan 2020 22:05:24 -0800 Subject: [PATCH] Fix EOF handling in TextFragment#ApplyStrict io.EOF was not properly accounted for when dealing with patches that are missing trailing newline characters. --- gitdiff/apply.go | 29 +++++++++---------- gitdiff/apply_test.go | 20 ++++++++----- gitdiff/gitdiff.go | 15 ++++++++++ .../apply/text_fragment_add_end_noeol.dst | 5 ++++ .../apply/text_fragment_add_end_noeol.patch | 11 +++++++ .../apply/text_fragment_add_end_noeol.src | 3 ++ .../text_fragment_change_single_noeol.dst | 1 + .../text_fragment_change_single_noeol.patch | 8 +++++ .../text_fragment_change_single_noeol.src | 1 + 9 files changed, 70 insertions(+), 23 deletions(-) create mode 100644 gitdiff/testdata/apply/text_fragment_add_end_noeol.dst create mode 100644 gitdiff/testdata/apply/text_fragment_add_end_noeol.patch create mode 100644 gitdiff/testdata/apply/text_fragment_add_end_noeol.src create mode 100644 gitdiff/testdata/apply/text_fragment_change_single_noeol.dst create mode 100644 gitdiff/testdata/apply/text_fragment_change_single_noeol.patch create mode 100644 gitdiff/testdata/apply/text_fragment_change_single_noeol.src diff --git a/gitdiff/apply.go b/gitdiff/apply.go index f4e2b71..df03695 100644 --- a/gitdiff/apply.go +++ b/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 { diff --git a/gitdiff/apply_test.go b/gitdiff/apply_test.go index a7a027b..a3d866a 100644 --- a/gitdiff/apply_test.go +++ b/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 { diff --git a/gitdiff/gitdiff.go b/gitdiff/gitdiff.go index a728276..589f5a7 100644 --- a/gitdiff/gitdiff.go +++ b/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 diff --git a/gitdiff/testdata/apply/text_fragment_add_end_noeol.dst b/gitdiff/testdata/apply/text_fragment_add_end_noeol.dst new file mode 100644 index 0000000..94c99a3 --- /dev/null +++ b/gitdiff/testdata/apply/text_fragment_add_end_noeol.dst @@ -0,0 +1,5 @@ +line 1 +line 2 +line 3 +line 4 +line 5 diff --git a/gitdiff/testdata/apply/text_fragment_add_end_noeol.patch b/gitdiff/testdata/apply/text_fragment_add_end_noeol.patch new file mode 100644 index 0000000..ec3cea4 --- /dev/null +++ b/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 diff --git a/gitdiff/testdata/apply/text_fragment_add_end_noeol.src b/gitdiff/testdata/apply/text_fragment_add_end_noeol.src new file mode 100644 index 0000000..8cf2f17 --- /dev/null +++ b/gitdiff/testdata/apply/text_fragment_add_end_noeol.src @@ -0,0 +1,3 @@ +line 1 +line 2 +line 3 \ No newline at end of file diff --git a/gitdiff/testdata/apply/text_fragment_change_single_noeol.dst b/gitdiff/testdata/apply/text_fragment_change_single_noeol.dst new file mode 100644 index 0000000..ed59e08 --- /dev/null +++ b/gitdiff/testdata/apply/text_fragment_change_single_noeol.dst @@ -0,0 +1 @@ +new line a \ No newline at end of file diff --git a/gitdiff/testdata/apply/text_fragment_change_single_noeol.patch b/gitdiff/testdata/apply/text_fragment_change_single_noeol.patch new file mode 100644 index 0000000..f945234 --- /dev/null +++ b/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 diff --git a/gitdiff/testdata/apply/text_fragment_change_single_noeol.src b/gitdiff/testdata/apply/text_fragment_change_single_noeol.src new file mode 100644 index 0000000..dcf168c --- /dev/null +++ b/gitdiff/testdata/apply/text_fragment_change_single_noeol.src @@ -0,0 +1 @@ +line 1 \ No newline at end of file