Permalink
Browse files

herrors: Improve handling of JSON errors

`*json.UnmarshalTypeError` and `*json.SyntaxError` has a byte `Offset`, so use that.

This commit also reworks/simplifies the errror line matching logic. This also makes the file reading unbuffered, but that should be fine in this error case.

See #5324
  • Loading branch information...
bep committed Oct 23, 2018
1 parent ed7b3e2 commit f669ef6bec25155d015b6ab231c53caef4fa5cdc
@@ -15,9 +15,9 @@
package herrors
import (
"bufio"
"fmt"
"io"
"io/ioutil"
"strings"
"github.com/gohugoio/hugo/helpers"
@@ -27,13 +27,20 @@ import (
var fileErrorFormat = "\"%s:%d:%d\": %s"
// LineMatcher is used to match a line with an error.
type LineMatcher func(le FileError, lineNumber int, line string) bool
// LineMatcher contains the elements used to match an error to a line
type LineMatcher struct {
FileError FileError
LineNumber int
Offset int
Line string
}
// LineMatcherFn is used to match a line with an error.
type LineMatcherFn func(m LineMatcher) bool
// SimpleLineMatcher matches if the current line number matches the line number
// in the error.
var SimpleLineMatcher = func(le FileError, lineNumber int, line string) bool {
return le.LineNumber() == lineNumber
// SimpleLineMatcher simply matches by line number.
var SimpleLineMatcher = func(m LineMatcher) bool {
return m.FileError.LineNumber() == m.LineNumber
}
// ErrorContext contains contextual information about an error. This will
@@ -79,7 +86,7 @@ func (e *ErrorWithFileContext) Cause() error {
// WithFileContextForFile will try to add a file context with lines matching the given matcher.
// If no match could be found, the original error is returned with false as the second return value.
func WithFileContextForFile(e error, realFilename, filename string, fs afero.Fs, matcher LineMatcher) (error, bool) {
func WithFileContextForFile(e error, realFilename, filename string, fs afero.Fs, matcher LineMatcherFn) (error, bool) {
f, err := fs.Open(filename)
if err != nil {
return e, false
@@ -90,25 +97,40 @@ func WithFileContextForFile(e error, realFilename, filename string, fs afero.Fs,
// WithFileContextForFile will try to add a file context with lines matching the given matcher.
// If no match could be found, the original error is returned with false as the second return value.
func WithFileContext(e error, realFilename string, r io.Reader, matcher LineMatcher) (error, bool) {
func WithFileContext(e error, realFilename string, r io.Reader, matcher LineMatcherFn) (error, bool) {
if e == nil {
panic("error missing")
}
le := UnwrapFileError(e)
if le == nil {
var ok bool
if le, ok = ToFileError("", e).(FileError); !ok {
return e, false
}
}
errCtx := locateError(r, le, matcher)
errCtx.Filename = realFilename
var errCtx ErrorContext
if le.Offset() != -1 {
errCtx = locateError(r, le, func(m LineMatcher) bool {
if le.Offset() >= m.Offset && le.Offset() < m.Offset+len(m.Line) {
fe := m.FileError
m.FileError = ToFileErrorWithOffset(fe, -fe.LineNumber()+m.LineNumber)
}
return matcher(m)
})
} else {
errCtx = locateError(r, le, matcher)
}
if errCtx.LineNumber == -1 {
return e, false
}
errCtx.Filename = realFilename
if le.Type() != "" {
errCtx.ChromaLexer = chromaLexerFromType(le.Type())
} else {
@@ -151,72 +173,66 @@ func chromaLexerFromFilename(filename string) string {
return chromaLexerFromType(ext)
}
func locateErrorInString(le FileError, src string, matcher LineMatcher) ErrorContext {
return locateError(strings.NewReader(src), nil, matcher)
func locateErrorInString(src string, matcher LineMatcherFn) ErrorContext {
return locateError(strings.NewReader(src), &fileError{}, matcher)
}
func locateError(r io.Reader, le FileError, matches LineMatcher) ErrorContext {
var errCtx ErrorContext
s := bufio.NewScanner(r)
errCtx.ColumnNumber = 1
if le != nil {
errCtx.ColumnNumber = le.ColumnNumber()
func locateError(r io.Reader, le FileError, matches LineMatcherFn) ErrorContext {
if le == nil {
panic("must provide an error")
}
lineNo := 0
errCtx := ErrorContext{LineNumber: -1, ColumnNumber: 1, Pos: -1}
b, err := ioutil.ReadAll(r)
if err != nil {
return errCtx
}
var buff [6]string
i := 0
errCtx.Pos = -1
lines := strings.Split(string(b), "\n")
for s.Scan() {
lineNo++
txt := s.Text()
buff[i] = txt
if le != nil && le.ColumnNumber() >= 0 {
errCtx.ColumnNumber = le.ColumnNumber()
}
if errCtx.Pos != -1 && i >= 5 {
break
lineNo := 0
posBytes := 0
for li, line := range lines {
lineNo = li + 1
m := LineMatcher{
FileError: le,
LineNumber: lineNo,
Offset: posBytes,
Line: line,
}
if errCtx.Pos == -1 && matches(le, lineNo, txt) {
errCtx.Pos = i
if errCtx.Pos == -1 && matches(m) {
errCtx.LineNumber = lineNo
break
}
if errCtx.Pos == -1 && i == 2 {
// Shift left
buff[0], buff[1] = buff[i-1], buff[i]
} else {
i++
}
posBytes += len(line)
}
// Go's template parser will typically report "unexpected EOF" errors on the
// empty last line that is supressed by the scanner.
// Do an explicit check for that.
if errCtx.Pos == -1 {
lineNo++
if matches(le, lineNo, "") {
buff[i] = ""
errCtx.Pos = i
errCtx.LineNumber = lineNo
if errCtx.LineNumber != -1 {
low := errCtx.LineNumber - 3
if low < 0 {
low = 0
}
i++
if errCtx.LineNumber > 2 {
errCtx.Pos = 2
} else {
errCtx.Pos = errCtx.LineNumber - 1
}
}
if errCtx.Pos != -1 {
low := errCtx.Pos - 2
if low < 0 {
low = 0
high := errCtx.LineNumber + 2
if high > len(lines) {
high = len(lines)
}
high := i
errCtx.Lines = buff[low:high]
} else {
errCtx.Pos = -1
errCtx.LineNumber = -1
errCtx.Lines = lines[low:high]
}
return errCtx
@@ -24,8 +24,8 @@ import (
func TestErrorLocator(t *testing.T) {
assert := require.New(t)
lineMatcher := func(le FileError, lineno int, line string) bool {
return strings.Contains(line, "THEONE")
lineMatcher := func(m LineMatcher) bool {
return strings.Contains(m.Line, "THEONE")
}
lines := `LINE 1
@@ -38,49 +38,51 @@ LINE 7
LINE 8
`
location := locateErrorInString(nil, lines, lineMatcher)
location := locateErrorInString(lines, lineMatcher)
assert.Equal([]string{"LINE 3", "LINE 4", "This is THEONE", "LINE 6", "LINE 7"}, location.Lines)
assert.Equal(5, location.LineNumber)
assert.Equal(2, location.Pos)
assert.Equal([]string{"This is THEONE"}, locateErrorInString(nil, `This is THEONE`, lineMatcher).Lines)
assert.Equal([]string{"This is THEONE"}, locateErrorInString(`This is THEONE`, lineMatcher).Lines)
location = locateErrorInString(nil, `L1
location = locateErrorInString(`L1
This is THEONE
L2
`, lineMatcher)
assert.Equal(2, location.LineNumber)
assert.Equal(1, location.Pos)
assert.Equal([]string{"L1", "This is THEONE", "L2"}, location.Lines)
assert.Equal([]string{"L1", "This is THEONE", "L2", ""}, location.Lines)
location = locateErrorInString(nil, `This is THEONE
location = locateErrorInString(`This is THEONE
L2
`, lineMatcher)
assert.Equal(0, location.Pos)
assert.Equal([]string{"This is THEONE", "L2"}, location.Lines)
assert.Equal([]string{"This is THEONE", "L2", ""}, location.Lines)
location = locateErrorInString(nil, `L1
location = locateErrorInString(`L1
This THEONE
`, lineMatcher)
assert.Equal([]string{"L1", "This THEONE"}, location.Lines)
assert.Equal([]string{"L1", "This THEONE", ""}, location.Lines)
assert.Equal(1, location.Pos)
location = locateErrorInString(nil, `L1
location = locateErrorInString(`L1
L2
This THEONE
`, lineMatcher)
assert.Equal([]string{"L1", "L2", "This THEONE"}, location.Lines)
assert.Equal([]string{"L1", "L2", "This THEONE", ""}, location.Lines)
assert.Equal(2, location.Pos)
location = locateErrorInString(nil, "NO MATCH", lineMatcher)
location = locateErrorInString("NO MATCH", lineMatcher)
assert.Equal(-1, location.LineNumber)
assert.Equal(-1, location.Pos)
assert.Equal(0, len(location.Lines))
lineMatcher = func(le FileError, lineno int, line string) bool {
return lineno == 6
lineMatcher = func(m LineMatcher) bool {
return m.LineNumber == 6
}
location = locateErrorInString(nil, `A
location = locateErrorInString(`A
B
C
D
@@ -96,11 +98,11 @@ J`, lineMatcher)
assert.Equal(2, location.Pos)
// Test match EOF
lineMatcher = func(le FileError, lineno int, line string) bool {
return lineno == 4
lineMatcher = func(m LineMatcher) bool {
return m.LineNumber == 4
}
location = locateErrorInString(nil, `A
location = locateErrorInString(`A
B
C
`, lineMatcher)
@@ -109,4 +111,18 @@ C
assert.Equal(4, location.LineNumber)
assert.Equal(2, location.Pos)
offsetMatcher := func(m LineMatcher) bool {
return m.Offset == 1
}
location = locateErrorInString(`A
B
C
D
E`, offsetMatcher)
assert.Equal([]string{"A", "B", "C", "D"}, location.Lines)
assert.Equal(2, location.LineNumber)
assert.Equal(1, location.Pos)
}
Oops, something went wrong.

0 comments on commit f669ef6

Please sign in to comment.