cue/errors: make List implement Error

The semants is for List to adopt the features of the first
error. This is consistent with the default error message
being printed.

Advantages:
- Allows improved type checking.
- Avoids the nil-error return issues for List.
- Simplifies combining code.

Also:
- get rid of E
- Implement: Promote, Append
- clean up code accordingly
- tighten type checking

Change-Id: Iea90ab001111d914aad93731d4ebb50e91865e1d
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/2480
Reviewed-by: Marcel van Lohuizen <[email protected]>

Author: mpvl

cue/ast.go

@@ -35,7 +35,7 @@ import (
 //
 // There should be no unresolved identifiers in file, meaning the Node field
 // of all identifiers should be set to a non-nil value.
-func (inst *Instance) insertFile(f *ast.File) error {
+func (inst *Instance) insertFile(f *ast.File) errors.Error {
 	// TODO: insert by converting to value first so that the trim command can
 	// also remove top-level fields.
 	// First process single file.
@@ -48,7 +48,8 @@ func (inst *Instance) insertFile(f *ast.File) error {
 		if v.errors.Len() > 0 {
 			return v.errors
 		}
-		return &callError{result.(*bottom)}
+		v := newValueRoot(v.ctx(), result)
+		return v.toErr(result.(*bottom))
 	}
 
 	return nil

cue/build/import.go

@@ -26,6 +26,17 @@ import (
 
 type LoadFunc func(pos token.Pos, path string) *Instance
 
+type cueError = errors.Error
+
+type buildError struct {
+	cueError
+	inputs []token.Pos
+}
+
+func (e *buildError) InputPositions() []token.Pos {
+	return e.inputs
+}
+
 func (inst *Instance) complete() errors.Error {
 	// TODO: handle case-insensitive collisions.
 	// dir := inst.Dir
@@ -65,7 +76,10 @@ func (inst *Instance) complete() errors.Error {
 	for path := range imported {
 		paths = append(paths, path)
 		if path == "" {
-			return errors.E(imported[path], "empty import path")
+			return &buildError{
+				errors.Newf(token.NoPos, "empty import path"),
+				imported[path],
+			}
 		}
 	}
 

cue/build/instance.go

@@ -59,7 +59,7 @@ type Instance struct {
 	// The Err for loading this package or nil on success. This does not
 	// include any errors of dependencies. Incomplete will be set if there
 	// were any errors in dependencies.
-	Err error
+	Err errors.Error
 
 	// Incomplete reports whether any dependencies had an error.
 	Incomplete bool
@@ -125,22 +125,7 @@ func (inst *Instance) setPkg(pkg string) bool {
 
 // ReportError reports an error processing this instance.
 func (inst *Instance) ReportError(err errors.Error) {
-	var list errors.List
-	switch x := inst.Err.(type) {
-	default:
-		// Should never happen, but in the worst case at least one error is
-		// recorded.
-		return
-	case nil:
-		inst.Err = err
-		return
-	case errors.List:
-		list = x
-	case errors.Error:
-		list.Add(x)
-	}
-	list.Add(err)
-	inst.Err = list
+	inst.Err = errors.Append(inst.Err, err)
 }
 
 // Context defines the build context for this instance. All files defined
@@ -181,18 +166,8 @@ func (inst *Instance) AddFile(filename string, src interface{}) error {
 	file, err := parser.ParseFile(filename, src, parser.ParseComments)
 	if err != nil {
 		// should always be an errors.List, but just in case.
-		switch x := err.(type) {
-		case errors.List:
-			for _, e := range x {
-				inst.ReportError(e)
-			}
-		case errors.Error:
-			inst.ReportError(x)
-		default:
-			e := errors.Wrapf(err, token.NoPos, "error adding file")
-			inst.ReportError(e)
-			err = e
-		}
+		err := errors.Promote(err, "error adding file")
+		inst.ReportError(err)
 		return err
 	}
 

cue/errors/errors.go

@@ -148,31 +148,14 @@ func Wrapf(err error, p token.Pos, format string, args ...interface{}) Error {
 	}
 }
 
-// E creates a new error. The following types correspond to the following
-// methods:
-//     arg type     maps to
-//     Message      Msg
-//     string       shorthand for a format message with no arguments
-//     token.Pos    Position
-//     []token.Pos  Positions
-//     error        Unwrap/Cause
-func E(args ...interface{}) Error {
-	e := &posError{}
-	for _, a := range args {
-		switch x := a.(type) {
-		case string:
-			e.Message = NewMessage(x, nil)
-		case Message:
-			e.Message = x
-		case token.Pos:
-			e.pos = x
-		case []token.Pos:
-			e.inputs = x
-		case error:
-			e.err = x
-		}
+// Promote converts a regular Go error to an Error if it isn't already so.
+func Promote(err error, msg string) Error {
+	switch x := err.(type) {
+	case Error:
+		return x
+	default:
+		return Wrapf(err, token.NoPos, msg)
 	}
-	return e
 }
 
 var _ Error = &posError{}
@@ -201,25 +184,70 @@ func (e *posError) Error() string {
 	if e.err == nil {
 		return e.Message.Error()
 	}
+	if e.Message.format == "" {
+		return e.err.Error()
+	}
 	return fmt.Sprintf("%s: %s", e.Message.Error(), e.err)
 }
 
+// Append combines two errors, flattening Lists as necessary.
+func Append(a, b Error) Error {
+	switch x := a.(type) {
+	case nil:
+		return b
+	case List:
+		return appendToList(x, b)
+	}
+	// Preserve order of errors.
+	list := appendToList(nil, a)
+	list = appendToList(list, b)
+	return list
+}
+
+// Errors reports the individual errors associated with an error, which is
+// the error itself if there is only one or, if the underlying type is List,
+// its individual elements. If the given error is not an Error, it will be
+// promoted to one.
+func Errors(err error) []Error {
+	switch x := err.(type) {
+	case nil:
+		return nil
+	case List:
+		return []Error(x)
+	case Error:
+		return []Error{x}
+	default:
+		return []Error{Promote(err, "")}
+	}
+}
+
+func appendToList(list List, err Error) List {
+	switch x := err.(type) {
+	case nil:
+		return list
+	case List:
+		if list == nil {
+			return x
+		}
+		return append(list, x...)
+	default:
+		return append(list, err)
+	}
+}
+
 // List is a list of Errors.
 // The zero value for an List is an empty List ready to use.
 type List []Error
 
-func (p *List) add(err Error) {
-	*p = append(*p, err)
-}
-
 // AddNewf adds an Error with given position and error message to an List.
 func (p *List) AddNewf(pos token.Pos, msg string, args ...interface{}) {
-	p.add(&posError{pos: pos, Message: Message{format: msg, args: args}})
+	err := &posError{pos: pos, Message: Message{format: msg, args: args}}
+	*p = append(*p, err)
 }
 
 // Add adds an Error with given position and error message to an List.
 func (p *List) Add(err Error) {
-	p.add(err)
+	*p = appendToList(*p, err)
 }
 
 // Reset resets an List to no errors.
@@ -316,13 +344,44 @@ func (p *List) RemoveMultiples() {
 
 // An List implements the error interface.
 func (p List) Error() string {
+	format, args := p.Msg()
+	return fmt.Sprintf(format, args...)
+}
+
+// Msg reports the unformatted error message for the first error, if any.
+func (p List) Msg() (format string, args []interface{}) {
 	switch len(p) {
 	case 0:
-		return "no errors"
+		return "no errors", nil
 	case 1:
-		return p[0].Error()
+		return p[0].Msg()
 	}
-	return fmt.Sprintf("%s (and %d more errors)", p[0], len(p)-1)
+	format, args = p[0].Msg()
+	return "%s (and %d more errors)", append(args, len(p)-1)
+}
+
+// Position reports the primary position for the first error, if any.
+func (p List) Position() token.Pos {
+	if len(p) == 0 {
+		return token.NoPos
+	}
+	return p[0].Position()
+}
+
+// InputPositions reports the input positions for the first error, if any.
+func (p List) InputPositions() []token.Pos {
+	if len(p) == 0 {
+		return nil
+	}
+	return p[0].InputPositions()
+}
+
+// Path reports the path location of the first error, if any.
+func (p List) Path() []string {
+	if len(p) == 0 {
+		return nil
+	}
+	return p[0].Path()
 }
 
 // Err returns an error equivalent to this error list.
@@ -356,12 +415,8 @@ func Print(w io.Writer, err error, cfg *Config) {
 	if cfg == nil {
 		cfg = &Config{}
 	}
-	if list, ok := err.(List); ok {
-		for _, e := range list {
-			printError(w, e, cfg)
-		}
-	} else if err != nil {
-		printError(w, err, cfg)
+	for _, e := range Errors(err) {
+		printError(w, e, cfg)
 	}
 }
 

cue/instance.go

@@ -20,7 +20,6 @@ import (
 	"cuelang.org/go/cue/ast"
 	"cuelang.org/go/cue/build"
 	"cuelang.org/go/cue/errors"
-	"cuelang.org/go/cue/token"
 	"cuelang.org/go/internal"
 )
 
@@ -40,8 +39,8 @@ type Instance struct {
 	Dir        string
 	Name       string
 
-	Incomplete bool  // true if Pkg and all its dependencies are free of errors
-	Err        error // non-nil if the package had errors
+	Incomplete bool         // true if Pkg and all its dependencies are free of errors
+	Err        errors.Error // non-nil if the package had errors
 
 	inst *build.Instance
 
@@ -84,41 +83,14 @@ func (x *index) NewInstance(p *build.Instance) *Instance {
 	return i
 }
 
-func (inst *Instance) setListOrError(err error) {
+func (inst *Instance) setListOrError(err errors.Error) {
 	inst.Incomplete = true
-	switch x := err.(type) {
-	case errors.List:
-		if inst.Err == nil {
-			inst.Err = x
-			return
-		}
-		for _, e := range x {
-			inst.setError(e)
-		}
-	case errors.Error:
-		inst.setError(x)
-	default:
-		inst.setError(errors.Wrapf(err, token.NoPos, "unknown error"))
-	}
+	inst.Err = errors.Append(inst.Err, err)
 }
 
 func (inst *Instance) setError(err errors.Error) {
 	inst.Incomplete = true
-	var list errors.List
-	switch x := inst.Err.(type) {
-	default:
-		// Should never happen, but in the worst case at least one error is
-		// recorded.
-		return
-	case nil:
-		inst.Err = err
-		return
-	case errors.List:
-		list = x
-	case errors.Error:
-		list.Add(err)
-	}
-	inst.Err = list
+	inst.Err = errors.Append(inst.Err, err)
 }
 
 func (inst *Instance) eval(ctx *context) evaluated {

cue/load/config.go

@@ -144,16 +144,7 @@ func (c Config) newInstance(path string) *build.Instance {
 func (c Config) newErrInstance(m *match, path string, err error) *build.Instance {
 	i := c.Context.NewInstance(path, nil)
 	i.DisplayPath = path
-	switch x := err.(type) {
-	case errors.Error:
-		i.ReportError(x)
-	case errors.List:
-		for _, e := range x {
-			i.ReportError(e)
-		}
-	default:
-		i.ReportError(errors.Wrapf(err, token.NoPos, "instance"))
-	}
+	i.ReportError(errors.Promote(err, "instance"))
 	return i
 }
 

cue/load/import.go

@@ -358,16 +358,7 @@ func (fp *fileProcessor) add(pos token.Pos, root, path string, mode importMode)
 	pf, perr := parser.ParseFile(filename, data, parser.ImportsOnly, parser.ParseComments)
 	if perr != nil {
 		// should always be an errors.List, but just in case.
-		switch x := perr.(type) {
-		case errors.List:
-			for _, e := range x {
-				badFile(e)
-			}
-		case errors.Error:
-			badFile(x)
-		default:
-			badFile(errors.Wrapf(err, token.NoPos, "add failed"))
-		}
+		badFile(errors.Promote(perr, "add failed"))
 		return true
 	}
 

encoding/protobuf/protobuf.go

@@ -118,12 +118,7 @@ func (b *Extractor) Err() error {
 }
 
 func (b *Extractor) addErr(err error) {
-	switch err := err.(type) {
-	case errors.Error:
-		b.errs.Add(err)
-	default:
-		b.errs.AddNewf(token.NoPos, "unknown error: %v", err)
-	}
+	b.errs.Add(errors.Promote(err, "unknown error"))
 }
 
 // AddFile adds a proto definition file to be converted into CUE by the builder.