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 gohugoio#5324

common/herrors/error_locator.go

@@ -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

common/herrors/error_locator_test.go

@@ -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)
+
 }