this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

Check for expected input in parsing functions

Instead of checking that a line has a certain prefix before calling a
header parsing function, the functions now check this and return nil
objects when called on the wrong line type. If the line passes this
basic check but is still invalid, and error is returned as before.

+163 -116
+52 -20
gitdiff/file_header.go
··· 9 9 "time" 10 10 ) 11 11 12 - func (p *parser) ParseGitFileHeader(f *File, header string) error { 13 - header = strings.TrimPrefix(header, fileHeaderPrefix) 12 + const ( 13 + devNull = "/dev/null" 14 + ) 15 + 16 + func (p *parser) ParseGitFileHeader() (*File, error) { 17 + const prefix = "diff --git " 18 + 19 + if !strings.HasPrefix(p.Line(0), prefix) { 20 + return nil, nil 21 + } 22 + header := p.Line(0)[len(prefix):] 23 + 14 24 defaultName, err := parseGitHeaderName(header) 15 25 if err != nil { 16 - return p.Errorf(0, "git file header: %v", err) 26 + return nil, p.Errorf(0, "git file header: %v", err) 17 27 } 18 28 19 - for err = p.Next(); err == nil; err = p.Next() { 29 + f := &File{} 30 + for { 31 + if err := p.Next(); err != nil { 32 + if err == io.EOF { 33 + break 34 + } 35 + return nil, err 36 + } 37 + 20 38 end, err := parseGitHeaderData(f, p.Line(0), defaultName) 21 39 if err != nil { 22 - return p.Errorf(1, "git file header: %v", err) 40 + return nil, p.Errorf(1, "git file header: %v", err) 23 41 } 24 42 if end { 25 43 break 26 44 } 27 45 } 28 - if err != nil && err != io.EOF { 29 - return err 30 - } 31 46 32 47 if f.OldName == "" && f.NewName == "" { 33 48 if defaultName == "" { 34 - return p.Errorf(0, "git file header: missing filename information") 49 + return nil, p.Errorf(0, "git file header: missing filename information") 35 50 } 36 51 f.OldName = defaultName 37 52 f.NewName = defaultName 38 53 } 39 54 40 55 if (f.NewName == "" && !f.IsDelete) || (f.OldName == "" && !f.IsNew) { 41 - return p.Errorf(0, "git file header: missing filename information") 56 + return nil, p.Errorf(0, "git file header: missing filename information") 42 57 } 43 58 44 - return nil 59 + return f, nil 45 60 } 46 61 47 - func (p *parser) ParseTraditionalFileHeader(f *File, oldLine, newLine string) error { 48 - oldName, _, err := parseName(strings.TrimPrefix(oldLine, oldFilePrefix), '\t', 0) 62 + func (p *parser) ParseTraditionalFileHeader() (*File, error) { 63 + const shortestValidFragHeader = "@@ -0,0 +1 @@\n" 64 + const ( 65 + oldPrefix = "--- " 66 + newPrefix = "+++ " 67 + ) 68 + 69 + oldLine, newLine := p.Line(0), p.Line(1) 70 + 71 + if !strings.HasPrefix(oldLine, oldPrefix) || !strings.HasPrefix(newLine, newPrefix) { 72 + return nil, nil 73 + } 74 + // heuristic: only a file header if followed by a (probable) fragment header 75 + if len(p.Line(2)) < len(shortestValidFragHeader) || !strings.HasPrefix(p.Line(2), "@@ -") { 76 + return nil, nil 77 + } 78 + 79 + oldName, _, err := parseName(oldLine[len(oldPrefix):], '\t', 0) 49 80 if err != nil { 50 - return p.Errorf(-1, "file header: %v", err) 81 + return nil, p.Errorf(0, "file header: %v", err) 51 82 } 52 83 53 - newName, _, err := parseName(strings.TrimPrefix(newLine, newFilePrefix), '\t', 0) 84 + newName, _, err := parseName(newLine[len(newPrefix):], '\t', 0) 54 85 if err != nil { 55 - return p.Errorf(0, "file header: %v", err) 86 + return nil, p.Errorf(1, "file header: %v", err) 56 87 } 57 88 89 + f := &File{} 58 90 switch { 59 91 case oldName == devNull || hasEpochTimestamp(oldLine): 60 92 f.IsNew = true ··· 73 105 f.NewName = newName 74 106 } 75 107 } 76 - return nil 108 + return f, nil 77 109 } 78 110 79 111 // parseGitHeaderName extracts a default file name from the Git file header ··· 115 147 end bool 116 148 parse func(*File, string, string) error 117 149 }{ 118 - {fragmentHeaderPrefix, true, nil}, 119 - {oldFilePrefix, false, parseGitHeaderOldName}, 120 - {newFilePrefix, false, parseGitHeaderNewName}, 150 + {"@@ -", true, nil}, 151 + {"--- ", false, parseGitHeaderOldName}, 152 + {"+++ ", false, parseGitHeaderNewName}, 121 153 {"old mode ", false, parseGitHeaderOldMode}, 122 154 {"new mode ", false, parseGitHeaderNewMode}, 123 155 {"deleted file mode ", false, parseGitHeaderDeletedMode},
+57 -25
gitdiff/file_header_test.go
··· 139 139 `, 140 140 Err: true, 141 141 }, 142 + "notGitHeader": { 143 + Input: `--- file.txt 144 + +++ file.txt 145 + @@ -0,0 +1 @@ 146 + `, 147 + Output: nil, 148 + }, 142 149 } 143 150 144 151 for name, test := range tests { ··· 146 153 p := &parser{r: bufio.NewReader(strings.NewReader(test.Input))} 147 154 p.Next() 148 155 149 - var f File 150 - err := p.ParseGitFileHeader(&f, p.Line(0)) 156 + f, err := p.ParseGitFileHeader() 151 157 if test.Err { 152 158 if err == nil { 153 159 t.Fatalf("expected error parsing git file header, got nil") ··· 158 164 t.Fatalf("unexpected error parsing git file header: %v", err) 159 165 } 160 166 161 - if test.Output != nil && !reflect.DeepEqual(f, *test.Output) { 162 - t.Errorf("incorrect file\nexpected: %+v\n actual: %+v", *test.Output, f) 167 + if !reflect.DeepEqual(test.Output, f) { 168 + t.Errorf("incorrect file\nexpected: %+v\n actual: %+v", test.Output, f) 163 169 } 164 170 }) 165 171 } ··· 167 173 168 174 func TestParseTraditionalFileHeader(t *testing.T) { 169 175 tests := map[string]struct { 170 - OldLine string 171 - NewLine string 172 - Output *File 173 - Err bool 176 + Input string 177 + Output *File 178 + Err bool 174 179 }{ 175 180 "fileContentChange": { 176 - OldLine: "--- dir/file_old.txt\t2019-03-21 23:00:00.0 -0700\n", 177 - NewLine: "+++ dir/file_new.txt\t2019-03-21 23:30:00.0 -0700\n", 181 + Input: `--- dir/file_old.txt 2019-03-21 23:00:00.0 -0700 182 + +++ dir/file_new.txt 2019-03-21 23:30:00.0 -0700 183 + @@ -0,0 +1 @@ 184 + `, 178 185 Output: &File{ 179 186 OldName: "dir/file_new.txt", 180 187 NewName: "dir/file_new.txt", 181 188 }, 182 189 }, 183 190 "newFile": { 184 - OldLine: "--- /dev/null\t1969-12-31 17:00:00.0 -0700\n", 185 - NewLine: "+++ dir/file.txt\t2019-03-21 23:30:00.0 -0700\n", 191 + Input: `--- /dev/null 1969-12-31 17:00:00.0 -0700 192 + +++ dir/file.txt 2019-03-21 23:30:00.0 -0700 193 + @@ -0,0 +1 @@ 194 + `, 186 195 Output: &File{ 187 196 NewName: "dir/file.txt", 188 197 IsNew: true, 189 198 }, 190 199 }, 191 200 "newFileTimestamp": { 192 - OldLine: "--- dir/file.txt\t1969-12-31 17:00:00.0 -0700\n", 193 - NewLine: "+++ dir/file.txt\t2019-03-21 23:30:00.0 -0700\n", 201 + Input: `--- dir/file.txt 1969-12-31 17:00:00.0 -0700 202 + +++ dir/file.txt 2019-03-21 23:30:00.0 -0700 203 + @@ -0,0 +1 @@ 204 + `, 194 205 Output: &File{ 195 206 NewName: "dir/file.txt", 196 207 IsNew: true, 197 208 }, 198 209 }, 199 210 "deleteFile": { 200 - OldLine: "--- dir/file.txt\t2019-03-21 23:30:00.0 -0700\n", 201 - NewLine: "+++ /dev/null\t1969-12-31 17:00:00.0 -0700\n", 211 + Input: `--- dir/file.txt 2019-03-21 23:30:00.0 -0700 212 + +++ /dev/null 1969-12-31 17:00:00.0 -0700 213 + @@ -0,0 +1 @@ 214 + `, 202 215 Output: &File{ 203 216 OldName: "dir/file.txt", 204 217 IsDelete: true, 205 218 }, 206 219 }, 207 220 "deleteFileTimestamp": { 208 - OldLine: "--- dir/file.txt\t2019-03-21 23:30:00.0 -0700\n", 209 - NewLine: "+++ dir/file.txt\t1969-12-31 17:00:00.0 -0700\n", 221 + Input: `--- dir/file.txt 2019-03-21 23:30:00.0 -0700 222 + +++ dir/file.txt 1969-12-31 17:00:00.0 -0700 223 + @@ -0,0 +1 @@ 224 + `, 210 225 Output: &File{ 211 226 OldName: "dir/file.txt", 212 227 IsDelete: true, 213 228 }, 214 229 }, 215 230 "useShortestPrefixName": { 216 - OldLine: "--- dir/file.txt\t2019-03-21 23:00:00.0 -0700\n", 217 - NewLine: "+++ dir/file.txt~\t2019-03-21 23:30:00.0 -0700\n", 231 + Input: `--- dir/file.txt 2019-03-21 23:00:00.0 -0700 232 + +++ dir/file.txt~ 2019-03-21 23:30:00.0 -0700 233 + @@ -0,0 +1 @@ 234 + `, 218 235 Output: &File{ 219 236 OldName: "dir/file.txt", 220 237 NewName: "dir/file.txt", 221 238 }, 222 239 }, 240 + "notTraditionalHeader": { 241 + Input: `diff --git a/dir/file.txt b/dir/file.txt 242 + --- a/dir/file.txt 243 + +++ b/dir/file.txt 244 + `, 245 + Output: nil, 246 + }, 247 + "noUnifiedFragment": { 248 + Input: `--- dir/file_old.txt 2019-03-21 23:00:00.0 -0700 249 + +++ dir/file_new.txt 2019-03-21 23:30:00.0 -0700 250 + context line 251 + +added line 252 + `, 253 + Output: nil, 254 + }, 223 255 } 224 256 225 257 for name, test := range tests { 226 258 t.Run(name, func(t *testing.T) { 227 - p := &parser{r: bufio.NewReader(strings.NewReader(""))} 259 + p := &parser{r: bufio.NewReader(strings.NewReader(test.Input))} 260 + p.Next() 228 261 229 - var f File 230 - err := p.ParseTraditionalFileHeader(&f, test.OldLine, test.NewLine) 262 + f, err := p.ParseTraditionalFileHeader() 231 263 if test.Err { 232 264 if err == nil { 233 265 t.Fatalf("expected error parsing traditional file header, got nil") ··· 238 270 t.Fatalf("unexpected error parsing traditional file header: %v", err) 239 271 } 240 272 241 - if test.Output != nil && !reflect.DeepEqual(f, *test.Output) { 242 - t.Errorf("incorrect file\nexpected: %+v\n actual: %+v", *test.Output, f) 273 + if !reflect.DeepEqual(test.Output, f) { 274 + t.Errorf("incorrect file\nexpected: %+v\n actual: %+v", test.Output, f) 243 275 } 244 276 }) 245 277 }
+48 -67
gitdiff/parser.go
··· 47 47 lines [3]string 48 48 } 49 49 50 - const ( 51 - fragmentHeaderPrefix = "@@ -" 52 - 53 - fileHeaderPrefix = "diff --git " 54 - oldFilePrefix = "--- " 55 - newFilePrefix = "+++ " 56 - 57 - devNull = "/dev/null" 58 - ) 59 - 60 50 // ParseNextFileHeader finds and parses the next file header in the stream. It 61 51 // returns nil if no headers are found before the end of the stream. 62 - func (p *parser) ParseNextFileHeader() (file *File, err error) { 63 - // based on find_header() in git/apply.c 64 - 65 - for err = p.Next(); err == nil; err = p.Next() { 66 - line := p.Line(0) 52 + func (p *parser) ParseNextFileHeader() (*File, error) { 53 + for { 54 + if err := p.Next(); err != nil { 55 + if err == io.EOF { 56 + break 57 + } 58 + return nil, err 59 + } 67 60 68 61 // check for disconnected fragment headers (corrupt patch) 69 - if isMaybeFragmentHeader(line) { 70 - var frag Fragment 71 - if err := parseFragmentHeader(&frag, line); err != nil { 72 - // not a valid header, nothing to worry about 73 - continue 74 - } 75 - return nil, p.Errorf(0, "patch fragment without header: %s", line) 62 + frag, err := p.ParseFragmentHeader() 63 + if err != nil { 64 + // not a valid header, nothing to worry about 65 + continue 66 + } 67 + if frag != nil { 68 + return nil, p.Errorf(0, "patch fragment without header: %s", p.Line(0)) 76 69 } 77 70 78 71 // check for a git-generated patch 79 - if strings.HasPrefix(line, fileHeaderPrefix) { 80 - file = new(File) 81 - if err := p.ParseGitFileHeader(file, line); err != nil { 82 - return nil, err 83 - } 72 + file, err := p.ParseGitFileHeader() 73 + if err != nil { 74 + return nil, err 75 + } 76 + if file != nil { 84 77 return file, nil 85 78 } 86 79 87 80 // check for a "traditional" patch 88 - if strings.HasPrefix(line, oldFilePrefix) && strings.HasPrefix(p.Line(1), newFilePrefix) { 89 - oldFileLine := line 90 - newFileLine := p.Line(1) 91 - 92 - // only a file header if followed by a (probable) unified fragment header 93 - if !isMaybeFragmentHeader(p.Line(2)) { 94 - continue 95 - } 96 - 97 - file = new(File) 98 - if err := p.ParseTraditionalFileHeader(file, oldFileLine, newFileLine); err != nil { 99 - return nil, err 100 - } 81 + file, err = p.ParseTraditionalFileHeader() 82 + if err != nil { 83 + return nil, err 84 + } 85 + if file != nil { 101 86 return file, nil 102 87 } 103 88 } 104 - 105 - if err != nil && err != io.EOF { 106 - return nil, err 107 - } 108 - return file, nil 89 + return nil, nil 109 90 } 110 91 111 92 // ParseFileChanges parses file changes until the next file header or the end ··· 164 145 return fmt.Errorf("gitdiff: line %d: %s", p.lineno+delta, fmt.Sprintf(msg, args...)) 165 146 } 166 147 167 - func isMaybeFragmentHeader(line string) bool { 168 - const shortestValidHeader = "@@ -0,0 +1 @@\n" 169 - return len(line) >= len(shortestValidHeader) && strings.HasPrefix(line, fragmentHeaderPrefix) 170 - } 148 + func (p *parser) ParseFragmentHeader() (*Fragment, error) { 149 + const ( 150 + startMark = "@@ -" 151 + endMark = " @@" 152 + ) 171 153 172 - // TODO(bkeyes): fix duplication with isMaybeFragmentHeader 173 - func parseFragmentHeader(f *Fragment, header string) (err error) { 174 - const startMark = "@@ " 175 - const endMark = " @@" 154 + if !strings.HasPrefix(p.Line(0), startMark) { 155 + return nil, nil 156 + } 176 157 177 - parts := strings.SplitAfterN(header, endMark, 2) 178 - if len(parts) < 2 || !strings.HasPrefix(parts[0], startMark) || !strings.HasSuffix(parts[0], endMark) { 179 - return fmt.Errorf("invalid fragment header") 158 + parts := strings.SplitAfterN(p.Line(0), endMark, 2) 159 + if len(parts) < 2 { 160 + return nil, fmt.Errorf("invalid fragment header") 180 161 } 181 162 182 - header = parts[0][len(startMark) : len(parts[0])-len(endMark)] 163 + f := &Fragment{} 183 164 f.Comment = strings.TrimSpace(parts[1]) 184 165 185 - ranges := strings.Split(header, " ") 166 + header := parts[0][len(startMark) : len(parts[0])-len(endMark)] 167 + ranges := strings.Split(header, " +") 186 168 if len(ranges) != 2 { 187 - return fmt.Errorf("invalid fragment header") 169 + return nil, fmt.Errorf("invalid fragment header") 188 170 } 189 171 190 - if !strings.HasPrefix(ranges[0], "-") || !strings.HasPrefix(ranges[1], "+") { 191 - return fmt.Errorf("invalid fragment header: bad range marker") 192 - } 193 - if f.OldPosition, f.OldLines, err = parseRange(ranges[0][1:]); err != nil { 194 - return fmt.Errorf("invalid fragment header: %v", err) 172 + var err error 173 + if f.OldPosition, f.OldLines, err = parseRange(ranges[0]); err != nil { 174 + return nil, fmt.Errorf("invalid fragment header: %v", err) 195 175 } 196 - if f.NewPosition, f.NewLines, err = parseRange(ranges[1][1:]); err != nil { 197 - return fmt.Errorf("invalid fragment header: %v", err) 176 + if f.NewPosition, f.NewLines, err = parseRange(ranges[1]); err != nil { 177 + return nil, fmt.Errorf("invalid fragment header: %v", err) 198 178 } 199 - return nil 179 + 180 + return f, nil 200 181 } 201 182 202 183 func parseRange(s string) (start int64, end int64, err error) {
+6 -4
gitdiff/parser_test.go
··· 119 119 120 120 for name, test := range tests { 121 121 t.Run(name, func(t *testing.T) { 122 - var frag Fragment 123 - err := parseFragmentHeader(&frag, test.Input) 122 + p := &parser{r: bufio.NewReader(strings.NewReader(test.Input))} 123 + p.Next() 124 + 125 + frag, err := p.ParseFragmentHeader() 124 126 if test.Err { 125 127 if err == nil { 126 128 t.Fatalf("expected error parsing header, but got nil") ··· 131 133 t.Fatalf("error parsing header: %v", err) 132 134 } 133 135 134 - if !reflect.DeepEqual(*test.Output, frag) { 135 - t.Fatalf("incorrect fragment\nexpected: %+v\nactual: %+v", *test.Output, frag) 136 + if !reflect.DeepEqual(test.Output, frag) { 137 + t.Fatalf("incorrect fragment\nexpected: %+v\nactual: %+v", test.Output, frag) 136 138 } 137 139 }) 138 140 }