Document and enforce parser invariants

After considering fragment parsing, it made sense to change the
invariant estabilished in the previous commit. Specifically, parser
functions now assume they are call on the first line of their object and
return with the parser on the first line after their object. This means
code can call parse function immediately after each other without
advancing the parser in between.

It's possible this will change back later on... there seem to be
annoying edge cases with either choice, but I think making the functions
consistent is important.
This commit is contained in:
Billy Keyes
2019-03-27 22:41:37 -07:00
parent e04aad2256
commit 9b0fc30459
5 changed files with 110 additions and 78 deletions
+8 -4
View File
@@ -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 {
-54
View File
@@ -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
+6
View File
@@ -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)
}
+36 -20
View File
@@ -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
}
+60
View File
@@ -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