diff --git a/common/herrors/error_locator.go b/common/herrors/error_locator.go index 306f8f46b17..ba895e8929c 100644 --- a/common/herrors/error_locator.go +++ b/common/herrors/error_locator.go @@ -15,11 +15,14 @@ package herrors import ( - "bufio" + "encoding/json" "fmt" "io" + "io/ioutil" "strings" + "github.com/pkg/errors" + "github.com/gohugoio/hugo/helpers" "github.com/spf13/afero" @@ -27,13 +30,21 @@ 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 { + Err error + ErrLineNUmber int + 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.ErrLineNUmber == m.LineNumber } // ErrorContext contains contextual information about an error. This will @@ -79,7 +90,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,34 +101,73 @@ 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) + + var errOffset int + var typ string + + if le == nil { + errOffset, typ = extractOffsetAndType(e) + if errOffset == -1 { + var ok bool + if le, ok = ToFileError("", e).(FileError); !ok { + return e, false + } + } + } + + var errCtx ErrorContext + if le == nil { - var ok bool - if le, ok = ToFileError("", e).(FileError); !ok { + if errOffset == -1 { return e, false } + + errCtx = locateError(r, nil, func(m LineMatcher) bool { + if errOffset >= m.Offset && errOffset < m.Offset+len(m.Line) { + m.ErrLineNUmber = m.LineNumber + } + return matcher(m) + }) + errCtx.ChromaLexer = chromaLexerFromType(typ) + + } else { + errCtx = locateError(r, le, matcher) } - errCtx := locateError(r, le, matcher) errCtx.Filename = realFilename if errCtx.LineNumber == -1 { return e, false } - if le.Type() != "" { - errCtx.ChromaLexer = chromaLexerFromType(le.Type()) - } else { - errCtx.ChromaLexer = chromaLexerFromFilename(realFilename) + if errCtx.ChromaLexer == "" { + if le.Type() != "" { + errCtx.ChromaLexer = chromaLexerFromType(le.Type()) + } else { + errCtx.ChromaLexer = chromaLexerFromFilename(realFilename) + } } return &ErrorWithFileContext{cause: e, ErrorContext: errCtx}, true } +func extractOffsetAndType(e error) (int, string) { + e = errors.Cause(e) + switch v := e.(type) { + case *json.UnmarshalTypeError: + return int(v.Offset), "json" + case *json.SyntaxError: + return int(v.Offset), "json" + default: + return -1, "" + } +} + // UnwrapErrorWithFileContext tries to unwrap an ErrorWithFileContext from err. // It returns nil if this is not possible. func UnwrapErrorWithFileContext(err error) *ErrorWithFileContext { @@ -151,72 +201,67 @@ func chromaLexerFromFilename(filename string) string { return chromaLexerFromType(ext) } -func locateErrorInString(le FileError, src string, matcher LineMatcher) ErrorContext { +func locateErrorInString(src string, matcher LineMatcherFn) ErrorContext { return locateError(strings.NewReader(src), nil, matcher) } -func locateError(r io.Reader, le FileError, matches LineMatcher) ErrorContext { - var errCtx ErrorContext - s := bufio.NewScanner(r) +func locateError(r io.Reader, le FileError, matches LineMatcherFn) ErrorContext { + errCtx := ErrorContext{LineNumber: -1, ColumnNumber: 1, Pos: -1} - errCtx.ColumnNumber = 1 - if le != nil { - errCtx.ColumnNumber = le.ColumnNumber() + b, err := ioutil.ReadAll(r) + if err != nil { + return errCtx } - lineNo := 0 + lines := strings.Split(string(b), "\n") - var buff [6]string - i := 0 - errCtx.Pos = -1 + if le != nil && le.ColumnNumber() >= 0 { + errCtx.ColumnNumber = le.ColumnNumber() + } - for s.Scan() { - lineNo++ - txt := s.Text() - buff[i] = txt + lineNo := 0 + posBytes := 0 + errLineNumber := -1 + if le != nil { + errLineNumber = le.LineNumber() + } - if errCtx.Pos != -1 && i >= 5 { - break + for li, line := range lines { + lineNo = li + 1 + m := LineMatcher{ + Err: le, + ErrLineNUmber: errLineNumber, + 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 diff --git a/common/herrors/error_locator_test.go b/common/herrors/error_locator_test.go index caa6e638541..e7bc3cb190e 100644 --- a/common/herrors/error_locator_test.go +++ b/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) + } diff --git a/go.sum b/go.sum index 23e14989e7d..7ec692a602b 100644 --- a/go.sum +++ b/go.sum @@ -65,6 +65,7 @@ github.com/magefile/mage v1.4.0 h1:RI7B1CgnPAuu2O9lWszwya61RLmfL0KCdo+QyyI/Bhk= github.com/magefile/mage v1.4.0/go.mod h1:IUDi13rsHje59lecXokTfGX0QIzO45uVPlXnJYsXepA= github.com/magiconair/properties v1.8.0 h1:LLgXmsheXeRoUOBOjtwPQCWIYqM/LU1ayDtDePerRcY= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= +github.com/markbates/inflect v0.0.0-20171215194931-a12c3aec81a6 h1:LZhVjIISSbj8qLf2qDPP0D8z0uvOWAW5C85ly5mJW6c= github.com/markbates/inflect v0.0.0-20171215194931-a12c3aec81a6/go.mod h1:oTeZL2KHA7CUX6X+fovmK9OvIOFuqu0TwdQrZjLTh88= github.com/mattn/go-isatty v0.0.4 h1:bnP0vzxcAdeI1zdubAl5PjU6zsERjGZb7raWodagDYs= github.com/mattn/go-isatty v0.0.4/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4= diff --git a/hugolib/hugo_sites.go b/hugolib/hugo_sites.go index a184e887709..65e3260f6b8 100644 --- a/hugolib/hugo_sites.go +++ b/hugolib/hugo_sites.go @@ -1,4 +1,4 @@ -// Copyright 2016-present The Hugo Authors. All rights reserved. +// Copyright 2018 The Hugo Authors. All rights reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/hugolib/hugo_sites_build_errors_test.go b/hugolib/hugo_sites_build_errors_test.go index f290022e041..5e47c4b165a 100644 --- a/hugolib/hugo_sites_build_errors_test.go +++ b/hugolib/hugo_sites_build_errors_test.go @@ -170,6 +170,20 @@ func TestSiteBuildErrors(t *testing.T) { }, }, + { + name: "Invalid JSON front matter", + fileType: tomlcontent, + fileFixer: func(content string) string { + return strings.Replace(content, "\"description\":", "\"description\"", 1) + }, + assertBuildError: func(a testSiteBuildErrorAsserter, err error) { + fe := a.getFileError(err) + + assert.Equal(3, fe.LineNumber) + assert.Equal("json", fe.ErrorContext.ChromaLexer) + + }, + }, { name: "Panic in template Execute", fileType: single, @@ -246,6 +260,16 @@ description = "Descriptioon" Some content. +`)) + + b.WithContent("myjson.md", f(tomlcontent, `{ + "title": "This is a title", + "description": "This is a description." +} + +Some content. + + `)) createErr := b.CreateSitesE() diff --git a/tpl/template.go b/tpl/template.go index 09710206e1f..69652c7d05f 100644 --- a/tpl/template.go +++ b/tpl/template.go @@ -162,18 +162,22 @@ func (t *TemplateAdapter) addFileContext(name string, inerr error) error { // Since this can be a composite of multiple template files (single.html + baseof.html etc.) // we potentially need to look in both -- and cannot rely on line number alone. - lineMatcher := func(le herrors.FileError, lineNumber int, line string) bool { - if le.LineNumber() != lineNumber { + lineMatcher := func(m herrors.LineMatcher) bool { + if m.ErrLineNUmber != m.LineNumber { return false } if !hasMaster { return true } - identifiers := t.extractIdentifiers(le.Error()) + if m.Err == nil { + return false + } + + identifiers := t.extractIdentifiers(m.Err.Error()) for _, id := range identifiers { - if strings.Contains(line, id) { + if strings.Contains(m.Line, id) { return true } }