Rework errors in compiler/parser (#5128)

* Add ErrorList, a list of Errors, and SourceSet, a struct containing
  Zed program text and source file offsets.  In the future,
  compiler/semantic will use these types to return multiple errors that
  point into the program.

* Remove ImproveError and NewError.

* Change ParseZed to return an ErrorList when Parse fails.

* Change ParseZed and ConcatSource to return a SourceSet.

* In api.Error, replace the `Info interface{}` field with
  `ComplationErrors parser.ErrorList`.

Co-authored-by: Matthew Nibecker <[email protected]>

api/api.go

@@ -3,6 +3,7 @@ package api
 import (
 	"context"
 
+	"github.com/brimdata/zed/compiler/parser"
 	"github.com/brimdata/zed/lakeparse"
 	"github.com/brimdata/zed/order"
 	"github.com/brimdata/zed/pkg/nano"
@@ -20,10 +21,10 @@ func RequestIDFromContext(ctx context.Context) string {
 }
 
 type Error struct {
-	Type    string      `json:"type"`
-	Kind    string      `json:"kind"`
-	Message string      `json:"error"`
-	Info    interface{} `json:"info,omitempty"`
+	Type              string           `json:"type"`
+	Kind              string           `json:"kind"`
+	Message           string           `json:"error"`
+	CompilationErrors parser.ErrorList `json:"compilation_errors,omitempty"`
 }
 
 func (e Error) Error() string {

api/client/connection.go

@@ -297,23 +297,19 @@ func (c *Connection) Revert(ctx context.Context, poolID ksuid.KSUID, branchName
 // As for Connection.Do, if the returned error is nil, the user is expected to
 // call Response.Body.Close.
 func (c *Connection) Query(ctx context.Context, head *lakeparse.Commitish, src string, filenames ...string) (*Response, error) {
-	src, srcInfo, err := parser.ConcatSource(filenames, src)
+	sset, err := parser.ConcatSource(filenames, src)
 	if err != nil {
 		return nil, err
 	}
-	body := api.QueryRequest{Query: src}
+	body := api.QueryRequest{Query: string(sset.Text)}
 	if head != nil {
 		body.Head = *head
 	}
 	req := c.NewRequest(ctx, http.MethodPost, "/query?ctrl=T", body)
 	res, err := c.Do(req)
-	var ae *api.Error
-	if errors.As(err, &ae) {
-		if m, ok := ae.Info.(map[string]interface{}); ok {
-			if offset, ok := m["parse_error_offset"].(float64); ok {
-				return res, parser.NewError(src, srcInfo, int(offset))
-			}
-		}
+	if ae := (*api.Error)(nil); errors.As(err, &ae) && len(ae.CompilationErrors) > 0 {
+		ae.CompilationErrors.SetSourceSet(sset)
+		return nil, ae.CompilationErrors
 	}
 	return res, err
 }

cli/queryflags/flags.go

@@ -87,15 +87,10 @@ func singleArgError(src string, err error) error {
 		src = src[:20] + "..."
 	}
 	fmt.Fprintf(&b, "\n - a file could not be found with the name %q", src)
-	var perr *parser.Error
-	if errors.As(err, &perr) {
-		b.WriteString("\n - the argument could not be compiled as a valid Zed query due to parse error (")
-		if perr.LineNum > 0 {
-			fmt.Fprintf(&b, "line %d, ", perr.LineNum)
-		}
-		fmt.Fprintf(&b, "column %d):", perr.Column)
-		for _, l := range strings.Split(perr.ParseErrorContext(), "\n") {
-			fmt.Fprintf(&b, "\n   %s", l)
+	if list := (parser.ErrorList)(nil); errors.As(err, &list) {
+		b.WriteString("\n - the argument could not be compiled as a valid Zed query:")
+		for _, line := range strings.Split(list.Error(), "\n") {
+			fmt.Fprintf(&b, "\n   %s", line)
 		}
 	} else {
 		b.WriteString("\n - the argument did not parse as a valid Zed query")

cmd/zq/ztests/single-arg-error.yaml

@@ -6,6 +6,7 @@ outputs:
     data: |
       zq: could not invoke zq with a single argument because:
        - a file could not be found with the name "file sample.zson | c..."
-       - the argument could not be compiled as a valid Zed query due to parse error (column 25):
+       - the argument could not be compiled as a valid Zed query:
+         error parsing Zed at line 1, column 26:
          file sample.zson | count(
                               === ^ ===

compiler/job.go

@@ -89,7 +89,8 @@ func (j *Job) Parallelize(n int) error {
 }
 
 func Parse(src string, filenames ...string) (ast.Seq, error) {
-	return parser.ParseZed(filenames, src)
+	seq, _, err := parser.ParseZed(filenames, src)
+	return seq, err
 }
 
 // MustParse is like Parse but panics if an error is encountered.

compiler/parser/api.go

@@ -8,138 +8,133 @@ import (
 	"github.com/brimdata/zed/compiler/ast"
 )
 
-// ParseZed calls ConcatSource followed by Parse.  If Parse fails, it calls
-// ImproveError.
-func ParseZed(filenames []string, src string) (ast.Seq, error) {
-	src, srcInfo, err := ConcatSource(filenames, src)
+// ParseZed calls ConcatSource followed by Parse.  If Parse returns an error,
+// ConcatSource tries to convert it to an ErrorList.
+func ParseZed(filenames []string, src string) (ast.Seq, *SourceSet, error) {
+	sset, err := ConcatSource(filenames, src)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
-	p, err := Parse("", []byte(src))
+	p, err := Parse("", []byte(sset.Text))
 	if err != nil {
-		return nil, ImproveError(err, src, srcInfo)
+		return nil, nil, convertErrList(err, sset)
 	}
-	return sliceOf[ast.Op](p), nil
-}
-
-// SourceInfo holds source file offsets.
-type SourceInfo struct {
-	filename string
-	start    int
-	end      int
+	return sliceOf[ast.Op](p), sset, nil
 }
 
 // ConcatSource concatenates the source files in filenames followed by src,
-// returning the result and a corresponding slice of SourceInfos.
-func ConcatSource(filenames []string, src string) (string, []SourceInfo, error) {
+// returning a SourceSet.
+func ConcatSource(filenames []string, src string) (*SourceSet, error) {
 	var b strings.Builder
-	var sis []SourceInfo
+	var sis []*SourceInfo
 	for _, f := range filenames {
 		bb, err := os.ReadFile(f)
 		if err != nil {
-			return "", nil, err
+			return nil, err
 		}
-		start := b.Len()
+		sis = append(sis, newSourceInfo(f, b.Len(), bb))
 		b.Write(bb)
-		sis = append(sis, SourceInfo{f, start, b.Len()})
 		b.WriteByte('\n')
 	}
-	start := b.Len()
-	b.WriteString(src)
-	sis = append(sis, SourceInfo{"", start, b.Len()})
-	if b.Len() == 0 {
-		return "*", nil, nil
+	if b.Len() == 0 && src == "" {
+		src = "*"
 	}
-	return b.String(), sis, nil
+	sis = append(sis, newSourceInfo("", b.Len(), []byte(src)))
+	b.WriteString(src)
+	return &SourceSet{b.String(), sis}, nil
 }
 
-// ImproveError tries to improve an error from Parse.  err is the error.  src is
-// the source code for which Parse return err.  If src came from ConcatSource,
-// sis is the corresponding slice of SourceInfo; otherwise, sis is nil.
-func ImproveError(err error, src string, sis []SourceInfo) error {
-	el, ok := err.(errList)
-	if !ok || len(el) != 1 {
-		return err
-	}
-	pe, ok := el[0].(*parserError)
+func convertErrList(err error, sset *SourceSet) error {
+	errs, ok := err.(errList)
 	if !ok {
 		return err
 	}
-	return NewError(src, sis, pe.pos.offset)
+	var out ErrorList
+	for _, e := range errs {
+		pe, ok := e.(*parserError)
+		if !ok {
+			return err
+		}
+		out.Append("error parsing Zed", pe.pos.offset, -1)
+	}
+	out.SetSourceSet(sset)
+	return out
 }
 
-// Error is a parse error with nice formatting.  It includes the source code
-// line containing the error.
-type Error struct {
-	Offset int // offset into original source code
-
-	filename string // omitted from formatting if ""
-	LineNum  int    // zero-based; omitted from formatting if negative
+// ErrList is a list of Errors.
+type ErrorList []*Error
 
-	line   string // contains no newlines
-	Column int    // zero-based
+// Append appends an Error to e.
+func (e *ErrorList) Append(msg string, pos, end int) {
+	*e = append(*e, &Error{msg, pos, end, nil})
 }
 
-// NewError returns an Error.  src is the source code containing the error.  If
-// src came from ConcatSource, sis is the corresponding slice of SourceInfo;
-// otherwise, src is nil.  offset is the offset of the error within src.
-func NewError(src string, sis []SourceInfo, offset int) error {
-	var filename string
-	for _, si := range sis {
-		if offset < si.end {
-			filename = si.filename
-			offset -= si.start
-			src = src[si.start:si.end]
-			break
+// Error concatenates the errors in e with a newline between each.
+func (e ErrorList) Error() string {
+	var b strings.Builder
+	for i, err := range e {
+		if i > 0 {
+			b.WriteByte('\n')
 		}
+		b.WriteString(err.Error())
 	}
-	lineNum := -1
-	if filename != "" || strings.Count(src, "\n") > 0 {
-		lineNum = strings.Count(src[:offset], "\n")
-	}
-	column := offset
-	if i := strings.LastIndexByte(src[:offset], '\n'); i != -1 {
-		column -= i + 1
-		src = src[i+1:]
-	}
-	if i := strings.IndexByte(src, '\n'); i != -1 {
-		src = src[:i]
-	}
-	return &Error{
-		Offset:   offset,
-		LineNum:  lineNum,
-		Column:   column,
-		filename: filename,
-		line:     src,
+	return b.String()
+}
+
+// SetSourceSet sets the SourceSet for every Error in e.
+func (e ErrorList) SetSourceSet(sset *SourceSet) {
+	for i := range e {
+		e[i].sset = sset
 	}
 }
 
+type Error struct {
+	Msg  string
+	Pos  int
+	End  int
+	sset *SourceSet
+}
+
 func (e *Error) Error() string {
+	if e.sset == nil {
+		return e.Msg
+	}
+	src := e.sset.SourceOf(e.Pos)
+	start := src.Position(e.Pos)
+	end := src.Position(e.End)
 	var b strings.Builder
-	b.WriteString("error parsing Zed ")
-	if e.filename != "" {
-		fmt.Fprintf(&b, "in %s ", e.filename)
+	b.WriteString(e.Msg)
+	if src.Filename != "" {
+		fmt.Fprintf(&b, " in %s", src.Filename)
 	}
-	b.WriteString("at ")
-	if e.LineNum >= 0 {
-		fmt.Fprintf(&b, "line %d, ", e.LineNum+1)
+	line := src.LineOfPos(e.sset.Text, e.Pos)
+	fmt.Fprintf(&b, " at line %d, column %d:\n%s\n", start.Line, start.Column, line)
+	if end.IsValid() {
+		formatSpanError(&b, line, start, end)
+	} else {
+		formatPointError(&b, start)
 	}
-	fmt.Fprintf(&b, "column %d:\n", e.Column+1)
-	b.WriteString(e.ParseErrorContext())
 	return b.String()
 }
 
-func (e *Error) ParseErrorContext() string {
-	var b strings.Builder
-	b.WriteString(e.line + "\n")
-	for k := 0; k < e.Column; k++ {
-		if k >= e.Column-4 && k != e.Column-1 {
+func formatSpanError(b *strings.Builder, line string, start, end Position) {
+	col := start.Column - 1
+	b.WriteString(strings.Repeat(" ", col))
+	n := len(line) - col
+	if start.Line == end.Line {
+		n = end.Column - 1 - col
+	}
+	b.WriteString(strings.Repeat("~", n))
+}
+
+func formatPointError(b *strings.Builder, start Position) {
+	col := start.Column - 1
+	for k := 0; k < col; k++ {
+		if k >= col-4 && k != col-1 {
 			b.WriteByte('=')
 		} else {
 			b.WriteByte(' ')
 		}
 	}
-	b.WriteByte('^')
-	b.WriteString(" ===")
-	return b.String()
+	b.WriteString("^ ===")
 }