hugolib: Continue the file context/line number errors work

See gohugoio#5324
bep · Oct 22, 2018 · d1661b8 · d1661b8
1 parent 7930d21
commit d1661b8
Show file tree

Hide file tree

Showing 23 changed files with 444 additions and 236 deletions.
diff --git a/commands/server_errors.go b/commands/server_errors.go
@@ -72,7 +72,7 @@ var buildErrorTemplate = `<!doctype html>
 		<main>
 			{{ highlight .Error "apl" "noclasses=true,style=monokai" }}
 			{{ with .File }}
-			{{ $params := printf "noclasses=true,style=monokai,linenos=table,hl_lines=%d,linenostart=%d" (add .Pos 1) .LineNumber }}
+			{{ $params := printf "noclasses=true,style=monokai,linenos=table,hl_lines=%d,linenostart=%d" (add .Pos 1) (sub .LineNumber .Pos) }}
 			{{ $lexer := .ChromaLexer | default "go-html-template" }}
 			{{  highlight (delimit .Lines "\n") $lexer $params }}
 			{{ end }}

diff --git a/common/herrors/error_locator.go b/common/herrors/error_locator.go
@@ -16,12 +16,17 @@ package herrors
 
 import (
 	"bufio"
+	"fmt"
 	"io"
 	"strings"
 
+	"github.com/gohugoio/hugo/helpers"
+
 	"github.com/spf13/afero"
 )
 
+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
 
@@ -34,6 +39,8 @@ var SimpleLineMatcher = func(le FileError, lineNumber int, line string) bool {
 // ErrorContext contains contextual information about an error. This will
 // typically be the lines surrounding some problem in a file.
 type ErrorContext struct {
+	// The source filename.
+	Filename string
 
 	// If a match will contain the matched line and up to 2 lines before and after.
 	// Will be empty if no match.
@@ -45,6 +52,9 @@ type ErrorContext struct {
 	// The linenumber in the source file from where the Lines start. Starting at 1.
 	LineNumber int
 
+	// The column number in the source file. Starting at 1.
+	ColumnNumber int
+
 	// The lexer to use for syntax highlighting.
 	// https://gohugo.io/content-management/syntax-highlighting/#list-of-chroma-highlighting-languages
 	ChromaLexer string
@@ -60,7 +70,7 @@ type ErrorWithFileContext struct {
 }
 
 func (e *ErrorWithFileContext) Error() string {
-	return e.cause.Error()
+	return fmt.Sprintf(fileErrorFormat, e.Filename, e.LineNumber, e.ColumnNumber, e.cause.Error())
 }
 
 func (e *ErrorWithFileContext) Cause() error {
@@ -69,39 +79,40 @@ 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, filename string, fs afero.Fs, chromaLexer string, matcher LineMatcher) (error, bool) {
+func WithFileContextForFile(e error, realFilename, filename string, fs afero.Fs, matcher LineMatcher) (error, bool) {
 	f, err := fs.Open(filename)
 	if err != nil {
 		return e, false
 	}
 	defer f.Close()
-	return WithFileContext(e, f, chromaLexer, matcher)
+	return WithFileContext(e, realFilename, f, matcher)
 }
 
 // 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, r io.Reader, chromaLexer string, matcher LineMatcher) (error, bool) {
+func WithFileContext(e error, realFilename string, r io.Reader, matcher LineMatcher) (error, bool) {
 	if e == nil {
 		panic("error missing")
 	}
 	le := UnwrapFileError(e)
 	if le == nil {
 		var ok bool
-		if le, ok = ToFileError("bash", e).(FileError); !ok {
+		if le, ok = ToFileError("", e).(FileError); !ok {
 			return e, false
 		}
 	}
 
 	errCtx := locateError(r, le, matcher)
+	errCtx.Filename = realFilename
 
 	if errCtx.LineNumber == -1 {
 		return e, false
 	}
 
-	if chromaLexer != "" {
-		errCtx.ChromaLexer = chromaLexer
-	} else {
+	if le.Type() != "" {
 		errCtx.ChromaLexer = chromaLexerFromType(le.Type())
+	} else {
+		errCtx.ChromaLexer = chromaLexerFromFilename(realFilename)
 	}
 
 	return &ErrorWithFileContext{cause: e, ErrorContext: errCtx}, true
@@ -124,9 +135,22 @@ func UnwrapErrorWithFileContext(err error) *ErrorWithFileContext {
 }
 
 func chromaLexerFromType(fileType string) string {
+	switch fileType {
+	case "html", "htm":
+		return "go-html-template"
+	}
 	return fileType
 }
 
+func chromaLexerFromFilename(filename string) string {
+	if strings.Contains(filename, "layouts") {
+		return "go-html-template"
+	}
+
+	ext := helpers.ExtNoDelimiter(filename)
+	return chromaLexerFromType(ext)
+}
+
 func locateErrorInString(le FileError, src string, matcher LineMatcher) ErrorContext {
 	return locateError(strings.NewReader(src), nil, matcher)
 }
@@ -135,6 +159,11 @@ 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()
+	}
+
 	lineNo := 0
 
 	var buff [6]string
@@ -152,7 +181,7 @@ func locateError(r io.Reader, le FileError, matches LineMatcher) ErrorContext {
 
 		if errCtx.Pos == -1 && matches(le, lineNo, txt) {
 			errCtx.Pos = i
-			errCtx.LineNumber = lineNo - i
+			errCtx.LineNumber = lineNo
 		}
 
 		if errCtx.Pos == -1 && i == 2 {
@@ -171,7 +200,7 @@ func locateError(r io.Reader, le FileError, matches LineMatcher) ErrorContext {
 		if matches(le, lineNo, "") {
 			buff[i] = ""
 			errCtx.Pos = i
-			errCtx.LineNumber = lineNo - 1
+			errCtx.LineNumber = lineNo
 
 			i++
 		}

diff --git a/common/herrors/error_locator_test.go b/common/herrors/error_locator_test.go
@@ -41,7 +41,7 @@ LINE 8
 	location := locateErrorInString(nil, lines, lineMatcher)
 	assert.Equal([]string{"LINE 3", "LINE 4", "This is THEONE", "LINE 6", "LINE 7"}, location.Lines)
 
-	assert.Equal(3, location.LineNumber)
+	assert.Equal(5, location.LineNumber)
 	assert.Equal(2, location.Pos)
 
 	assert.Equal([]string{"This is THEONE"}, locateErrorInString(nil, `This is THEONE`, lineMatcher).Lines)
@@ -92,7 +92,7 @@ I
 J`, lineMatcher)
 
 	assert.Equal([]string{"D", "E", "F", "G", "H"}, location.Lines)
-	assert.Equal(4, location.LineNumber)
+	assert.Equal(6, location.LineNumber)
 	assert.Equal(2, location.Pos)
 
 	// Test match EOF
@@ -106,7 +106,7 @@ C
 `, lineMatcher)
 
 	assert.Equal([]string{"B", "C", ""}, location.Lines)
-	assert.Equal(3, location.LineNumber)
+	assert.Equal(4, location.LineNumber)
 	assert.Equal(2, location.Pos)
 
 }
diff --git a/common/herrors/file_error.go b/common/herrors/file_error.go
@@ -13,10 +13,6 @@
 
 package herrors
 
-import (
-	"fmt"
-)
-
 var _ causer = (*fileError)(nil)
 
 // FileError represents an error when handling a file: Parsing a config file,
@@ -27,16 +23,18 @@ type FileError interface {
 	// LineNumber gets the error location, starting at line 1.
 	LineNumber() int
 
+	ColumnNumber() int
+
 	// A string identifying the type of file, e.g. JSON, TOML, markdown etc.
 	Type() string
 }
 
 var _ FileError = (*fileError)(nil)
 
 type fileError struct {
-	lineNumber int
-	fileType   string
-	msg        string
+	lineNumber   int
+	columnNumber int
+	fileType     string
 
 	cause error
 }
@@ -45,32 +43,28 @@ func (e *fileError) LineNumber() int {
 	return e.lineNumber
 }
 
+func (e *fileError) ColumnNumber() int {
+	return e.columnNumber
+}
+
 func (e *fileError) Type() string {
 	return e.fileType
 }
 
 func (e *fileError) Error() string {
-	return e.msg
+	if e.cause == nil {
+		return ""
+	}
+	return e.cause.Error()
 }
 
 func (f *fileError) Cause() error {
 	return f.cause
 }
 
-func (e *fileError) Format(s fmt.State, verb rune) {
-	switch verb {
-	case 'v':
-		fallthrough
-	case 's':
-		fmt.Fprintf(s, "%s:%d: %s:%s", e.fileType, e.lineNumber, e.msg, e.cause)
-	case 'q':
-		fmt.Fprintf(s, "%q:%d: %q:%q", e.fileType, e.lineNumber, e.msg, e.cause)
-	}
-}
-
 // NewFileError creates a new FileError.
-func NewFileError(fileType string, lineNumber int, msg string, err error) FileError {
-	return &fileError{cause: err, fileType: fileType, lineNumber: lineNumber, msg: msg}
+func NewFileError(fileType string, lineNumber, columnNumber int, err error) FileError {
+	return &fileError{cause: err, fileType: fileType, lineNumber: lineNumber, columnNumber: columnNumber}
 }
 
 // UnwrapFileError tries to unwrap a FileError from err.
@@ -101,9 +95,10 @@ func ToFileError(fileType string, err error) error {
 // If will fall back to returning the original error if a line number cannot be extracted.
 func ToFileErrorWithOffset(fileType string, err error, offset int) error {
 	for _, handle := range lineNumberExtractors {
-		lno, msg := handle(err, offset)
+
+		lno, col := handle(err)
 		if lno > 0 {
-			return NewFileError(fileType, lno, msg, err)
+			return NewFileError(fileType, lno+offset, col, err)
 		}
 	}
 	// Fall back to the original.

diff --git a/common/herrors/file_error_test.go b/common/herrors/file_error_test.go
@@ -28,16 +28,16 @@ func TestToLineNumberError(t *testing.T) {
 	assert := require.New(t)
 
 	for i, test := range []struct {
-		in         error
-		offset     int
-		lineNumber int
+		in           error
+		offset       int
+		lineNumber   int
+		columnNumber int
 	}{
-		{errors.New("no line number for you"), 0, -1},
-		{errors.New(`template: _default/single.html:2:15: executing "_default/single.html" at <.Titles>: can't evaluate field`), 0, 2},
-		{errors.New("parse failed: template: _default/bundle-resource-meta.html:11: unexpected in operand"), 0, 11},
-		{errors.New(`failed:: template: _default/bundle-resource-meta.html:2:7: executing "main" at <.Titles>`), 0, 2},
-		{errors.New("error in front matter: Near line 32 (last key parsed 'title')"), 0, 32},
-		{errors.New("error in front matter: Near line 32 (last key parsed 'title')"), 2, 34},
+		{errors.New("no line number for you"), 0, -1, 1},
+		{errors.New(`template: _default/single.html:4:15: executing "_default/single.html" at <.Titles>: can't evaluate field Titles in type *hugolib.PageOutput`), 0, 4, 15},
+		{errors.New("parse failed: template: _default/bundle-resource-meta.html:11: unexpected in operand"), 0, 11, 1},
+		{errors.New(`failed:: template: _default/bundle-resource-meta.html:2:7: executing "main" at <.Titles>`), 0, 2, 7},
+		{errors.New("error in front matter: Near line 32 (last key parsed 'title')"), 0, 32, 1},
 	} {
 
 		got := ToFileErrorWithOffset("template", test.in, test.offset)
@@ -48,6 +48,7 @@ func TestToLineNumberError(t *testing.T) {
 		if test.lineNumber > 0 {
 			assert.True(ok)
 			assert.Equal(test.lineNumber, le.LineNumber(), errMsg)
+			assert.Equal(test.columnNumber, le.ColumnNumber(), errMsg)
 			assert.Contains(got.Error(), strconv.Itoa(le.LineNumber()))
 		} else {
 			assert.False(ok)

diff --git a/common/herrors/line_number_extractors.go b/common/herrors/line_number_extractors.go
@@ -14,14 +14,13 @@
 package herrors
 
 import (
-	"fmt"
 	"regexp"
 	"strconv"
 )
 
 var lineNumberExtractors = []lineNumberExtractor{
 	// Template/shortcode parse errors
-	newLineNumberErrHandlerFromRegexp("(.*?:)(\\d+)(:.*)"),
+	newLineNumberErrHandlerFromRegexp("(.*?:)(\\d+)(:)(\\d+)?(.*)"),
 
 	// TOML parse errors
 	newLineNumberErrHandlerFromRegexp("(.*Near line )(\\d+)(\\s.*)"),
@@ -30,30 +29,34 @@ var lineNumberExtractors = []lineNumberExtractor{
 	newLineNumberErrHandlerFromRegexp("(line )(\\d+)(:)"),
 }
 
-type lineNumberExtractor func(e error, offset int) (int, string)
+type lineNumberExtractor func(e error) (int, int)
 
 func newLineNumberErrHandlerFromRegexp(expression string) lineNumberExtractor {
 	re := regexp.MustCompile(expression)
 	return extractLineNo(re)
 }
 
 func extractLineNo(re *regexp.Regexp) lineNumberExtractor {
-	return func(e error, offset int) (int, string) {
+	return func(e error) (int, int) {
 		if e == nil {
 			panic("no error")
 		}
+		col := 1
 		s := e.Error()
 		m := re.FindStringSubmatch(s)
-		if len(m) == 4 {
-			i, _ := strconv.Atoi(m[2])
-			msg := e.Error()
-			if offset != 0 {
-				i = i + offset
-				msg = re.ReplaceAllString(s, fmt.Sprintf("${1}%d${3}", i))
+		if len(m) >= 4 {
+			lno, _ := strconv.Atoi(m[2])
+			if len(m) > 4 {
+				col, _ = strconv.Atoi(m[4])
 			}
-			return i, msg
+
+			if col <= 0 {
+				col = 1
+			}
+
+			return lno, col
 		}
 
-		return -1, ""
+		return -1, col
 	}
 }