diff --git a/gitdiff/file_header.go b/gitdiff/file_header.go index eb8f65a..4ff5ff5 100644 --- a/gitdiff/file_header.go +++ b/gitdiff/file_header.go @@ -32,9 +32,6 @@ func (p *parser) ParseGitFileHeader() (*File, error) { if err != nil { return nil, p.Errorf(1, "git file header: %v", err) } - if end { - break - } if err := p.Next(); err != nil { if err == io.EOF { @@ -42,6 +39,10 @@ func (p *parser) ParseGitFileHeader() (*File, error) { } return nil, err } + + if end { + break + } } if f.OldName == "" && f.NewName == "" { @@ -76,11 +77,14 @@ func (p *parser) ParseTraditionalFileHeader() (*File, error) { return nil, nil } - // advance past the first line so parser is at end of header + // advance past the first two lines so parser is after the header // no EOF check needed because we know there are >=3 valid lines if err := p.Next(); err != nil { return nil, err } + if err := p.Next(); err != nil { + return nil, err + } oldName, _, err := parseName(oldLine[len(oldPrefix):], '\t', 0) if err != nil { diff --git a/gitdiff/file_header_test.go b/gitdiff/file_header_test.go index 62f2cde..46581c3 100644 --- a/gitdiff/file_header_test.go +++ b/gitdiff/file_header_test.go @@ -277,60 +277,6 @@ context line } } -func TestParserAdvancment(t *testing.T) { - tests := map[string]struct { - Input string - Parse func(p *parser) error - NextLine string - }{ - "ParseGitFileHeader": { - Input: `diff --git a/dir/file.txt b/dir/file.txt -index 9540595..30e6333 100644 ---- a/dir/file.txt -+++ b/dir/file.txt -@@ -1,2 +1,3 @@ -context line -`, - Parse: func(p *parser) error { - _, err := p.ParseGitFileHeader() - return err - }, - NextLine: "@@ -1,2 +1,3 @@\n", - }, - "ParseTraditionalFileHeader": { - Input: `--- dir/file.txt -+++ dir/file.txt -@@ -1,2 +1,3 @@ -context line -`, - Parse: func(p *parser) error { - _, err := p.ParseTraditionalFileHeader() - return err - }, - NextLine: "@@ -1,2 +1,3 @@\n", - }, - } - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - p := &parser{r: bufio.NewReader(strings.NewReader(test.Input))} - p.Next() - - if err := test.Parse(p); err != nil { - t.Fatalf("unexpected error while parsing: %v", err) - } - - if err := p.Next(); err != nil { - t.Fatalf("advancing the parser after parsing returned an error: %v", err) - } - - if test.NextLine != p.Line(0) { - t.Errorf("incorrect next line after parsing\nexpected: %q\nactual: %q", test.NextLine, p.Line(0)) - } - }) - } -} - func TestCleanName(t *testing.T) { tests := map[string]struct { Input string diff --git a/gitdiff/gitdiff.go b/gitdiff/gitdiff.go index 8c749a9..3327194 100644 --- a/gitdiff/gitdiff.go +++ b/gitdiff/gitdiff.go @@ -1,6 +1,7 @@ package gitdiff import ( + "fmt" "os" ) @@ -35,3 +36,8 @@ type Fragment struct { NewPosition int64 NewLines int64 } + +// Header returns the cannonical header of this fragment. +func (f *Fragment) Header() string { + return fmt.Sprintf("@@ -%d,%d +%d,%d @@ %s", f.OldPosition, f.OldLines, f.NewPosition, f.NewLines, f.Comment) +} diff --git a/gitdiff/parser.go b/gitdiff/parser.go index 6eaaf9e..bc0751c 100644 --- a/gitdiff/parser.go +++ b/gitdiff/parser.go @@ -11,22 +11,27 @@ import ( // Parse parses a patch with changes for one or more files. Any content // preceding the first file header is ignored. If an error occurs while // parsing, files will contain all files parsed before the error. -func Parse(r io.Reader) (files []*File, err error) { +func Parse(r io.Reader) ([]*File, error) { p := &parser{r: bufio.NewReader(r)} + if err := p.Next(); err != nil { + if err == io.EOF { + return nil, nil + } + return nil, err + } - var file *File + var files []*File for { - file, err = p.ParseNextFileHeader() + file, err := p.ParseNextFileHeader() if err != nil { - return + return files, err } if file == nil { break } - err = p.ParseFileChanges(file) - if err != nil { - return + if err = p.ParseFragments(file); err != nil { + return files, err } files = append(files, file) @@ -39,6 +44,12 @@ func Parse(r io.Reader) (files []*File, err error) { // this would enable OID validation, p-value guessing, and prefix stripping // by allowing users to set or override defaults +// parser invariants: +// - methods that parse objects start on the first line of the first object +// - methods that parse objects return on the first line after the last object +// - if a parse method returns a nil object, the parser was not advanced +// - any exported parsing methods must initialize the parser by calling Next() + type parser struct { r *bufio.Reader @@ -50,26 +61,20 @@ type parser struct { // ParseNextFileHeader finds and parses the next file header in the stream. It // returns nil if no headers are found before the end of the stream. func (p *parser) ParseNextFileHeader() (*File, error) { + var file *File for { - if err := p.Next(); err != nil { - if err == io.EOF { - break - } - return nil, err - } - // check for disconnected fragment headers (corrupt patch) frag, err := p.ParseFragmentHeader() if err != nil { // not a valid header, nothing to worry about - continue + goto NextLine } if frag != nil { - return nil, p.Errorf(0, "patch fragment without header: %s", p.Line(0)) + return nil, p.Errorf(-1, "patch fragment without file header: %s", frag.Header()) } // check for a git-generated patch - file, err := p.ParseGitFileHeader() + file, err = p.ParseGitFileHeader() if err != nil { return nil, err } @@ -85,13 +90,21 @@ func (p *parser) ParseNextFileHeader() (*File, error) { if file != nil { return file, nil } + + NextLine: + if err := p.Next(); err != nil { + if err == io.EOF { + break + } + return nil, err + } } return nil, nil } -// ParseFileChanges parses file changes until the next file header or the end -// of the stream and attaches them to the given file. -func (p *parser) ParseFileChanges(f *File) error { +// ParseFragments parses fragments until the next file header or the end of the +// stream and attaches them to the given file. +func (p *parser) ParseFragments(f *File) error { panic("TODO(bkeyes): unimplemented") } @@ -177,6 +190,9 @@ func (p *parser) ParseFragmentHeader() (*Fragment, error) { return nil, fmt.Errorf("invalid fragment header: %v", err) } + if err := p.Next(); err != nil && err != io.EOF { + return nil, err + } return f, nil } diff --git a/gitdiff/parser_test.go b/gitdiff/parser_test.go index 49f9327..d665c85 100644 --- a/gitdiff/parser_test.go +++ b/gitdiff/parser_test.go @@ -73,6 +73,66 @@ func TestLineOperations(t *testing.T) { }) } +func TestParserAdvancment(t *testing.T) { + tests := map[string]struct { + Input string + Parse func(p *parser) error + EndLine string + }{ + "ParseGitFileHeader": { + Input: `diff --git a/dir/file.txt b/dir/file.txt +index 9540595..30e6333 100644 +--- a/dir/file.txt ++++ b/dir/file.txt +@@ -1,2 +1,3 @@ +context line +`, + Parse: func(p *parser) error { + _, err := p.ParseGitFileHeader() + return err + }, + EndLine: "@@ -1,2 +1,3 @@\n", + }, + "ParseTraditionalFileHeader": { + Input: `--- dir/file.txt ++++ dir/file.txt +@@ -1,2 +1,3 @@ +context line +`, + Parse: func(p *parser) error { + _, err := p.ParseTraditionalFileHeader() + return err + }, + EndLine: "@@ -1,2 +1,3 @@\n", + }, + "ParseFragmentHeader": { + Input: `@@ -1,2 +1,3 @@ +context line +`, + Parse: func(p *parser) error { + _, err := p.ParseFragmentHeader() + return err + }, + EndLine: "context line\n", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + p := &parser{r: bufio.NewReader(strings.NewReader(test.Input))} + p.Next() + + if err := test.Parse(p); err != nil { + t.Fatalf("unexpected error while parsing: %v", err) + } + + if test.EndLine != p.Line(0) { + t.Errorf("incorrect position after parsing\nexpected: %q\nactual: %q", test.EndLine, p.Line(0)) + } + }) + } +} + func TestParseFragmentHeader(t *testing.T) { tests := map[string]struct { Input string