cue/errors: remove List from public API
and use Errors, Sanitize and Promote to replace
the missing functionality.
Removing List greatly simplifies the API and also avoid
the nil-interface issue, making it easier to keep code
correct.
Change-Id: Idd3feedc6efe7ad17c104ee1bedea615ee15383d
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/2501
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cue/ast.go b/cue/ast.go
index d203675..d64a5af 100644
--- a/cue/ast.go
+++ b/cue/ast.go
@@ -45,7 +45,7 @@
// inserting. For now, we accept errors that did not make it up to the tree.
result := v.walk(f)
if isBottom(result) {
- if v.errors.Len() > 0 {
+ if v.errors != nil {
return v.errors
}
v := newValueRoot(v.ctx(), result)
@@ -87,7 +87,7 @@
// make unique per level to avoid reuse of structs being an issue.
astMap map[ast.Node]scope
- errors errors.List
+ errors errors.Error
}
func (s *astState) mapScope(n ast.Node) (m scope) {
@@ -126,7 +126,7 @@
}
func (v *astVisitor) errf(n ast.Node, format string, args ...interface{}) evaluated {
- v.astState.errors.Add(&nodeError{
+ v.astState.errors = errors.Append(v.astState.errors, &nodeError{
path: v.appendPath(nil),
n: n,
Message: errors.NewMessage(format, args),
diff --git a/cue/ast_test.go b/cue/ast_test.go
index 95b96ca..96ea535 100644
--- a/cue/ast_test.go
+++ b/cue/ast_test.go
@@ -250,10 +250,10 @@
}}
for _, tc := range testCases {
t.Run("", func(t *testing.T) {
- ctx, root, errs := compileFileWithErrors(t, tc.in)
+ ctx, root, err := compileFileWithErrors(t, tc.in)
buf := &bytes.Buffer{}
- if len(errs) > 0 {
- errors.Print(buf, errs, nil)
+ if err != nil {
+ errors.Print(buf, err, nil)
}
buf.WriteString(debugStr(ctx, root))
got := buf.String()
diff --git a/cue/errors/errors.go b/cue/errors/errors.go
index 7b9277b..f5c5619 100644
--- a/cue/errors/errors.go
+++ b/cue/errors/errors.go
@@ -148,7 +148,7 @@
}
}
-// Promote converts a regular Go error to an Error if it isn't already so.
+// Promote converts a regular Go error to an Error if it isn't already one.
func Promote(err error, msg string) Error {
switch x := err.(type) {
case Error:
@@ -195,7 +195,7 @@
switch x := a.(type) {
case nil:
return b
- case List:
+ case list:
return appendToList(x, b)
}
// Preserve order of errors.
@@ -212,7 +212,7 @@
switch x := err.(type) {
case nil:
return nil
- case List:
+ case list:
return []Error(x)
case Error:
return []Error{x}
@@ -221,43 +221,43 @@
}
}
-func appendToList(list List, err Error) List {
+func appendToList(a list, err Error) list {
switch x := err.(type) {
case nil:
- return list
- case List:
- if list == nil {
+ return a
+ case list:
+ if a == nil {
return x
}
- return append(list, x...)
+ return append(a, x...)
default:
- return append(list, err)
+ return append(a, err)
}
}
-// List is a list of Errors.
-// The zero value for an List is an empty List ready to use.
-type List []Error
+// list is a list of Errors.
+// The zero value for an list is an empty list ready to use.
+type list []Error
// AddNewf adds an Error with given position and error message to an List.
-func (p *List) AddNewf(pos token.Pos, msg string, args ...interface{}) {
+func (p *list) AddNewf(pos token.Pos, msg string, args ...interface{}) {
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) {
+func (p *list) Add(err Error) {
*p = appendToList(*p, err)
}
// Reset resets an List to no errors.
-func (p *List) Reset() { *p = (*p)[0:0] }
+func (p *list) Reset() { *p = (*p)[0:0] }
// List implements the sort Interface.
-func (p List) Len() int { return len(p) }
-func (p List) Swap(i, j int) { p[i], p[j] = p[j], p[i] }
+func (p list) Len() int { return len(p) }
+func (p list) Swap(i, j int) { p[i], p[j] = p[j], p[i] }
-func (p List) Less(i, j int) bool {
+func (p list) Less(i, j int) bool {
if c := comparePos(p[i].Position(), p[j].Position()); c != 0 {
return c == -1
}
@@ -315,16 +315,29 @@
return true
}
+// Sanitize sorts multiple errors and removes duplicates on a best effort basis.
+// If err represents a single or no error, it returns the error as is.
+func Sanitize(err Error) Error {
+ if l, ok := err.(list); ok && err != nil {
+ a := make(list, len(l))
+ copy(a, l)
+ a.Sort()
+ a.RemoveMultiples()
+ return a
+ }
+ return err
+}
+
// Sort sorts an List. *posError entries are sorted by position,
// other errors are sorted by error message, and before any *posError
// entry.
//
-func (p List) Sort() {
+func (p list) Sort() {
sort.Sort(p)
}
// RemoveMultiples sorts an List and removes all but the first error per line.
-func (p *List) RemoveMultiples() {
+func (p *list) RemoveMultiples() {
p.Sort()
var last Error
i := 0
@@ -343,13 +356,13 @@
}
// An List implements the error interface.
-func (p List) Error() string {
+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{}) {
+func (p list) Msg() (format string, args []interface{}) {
switch len(p) {
case 0:
return "no errors", nil
@@ -361,7 +374,7 @@
}
// Position reports the primary position for the first error, if any.
-func (p List) Position() token.Pos {
+func (p list) Position() token.Pos {
if len(p) == 0 {
return token.NoPos
}
@@ -369,7 +382,7 @@
}
// InputPositions reports the input positions for the first error, if any.
-func (p List) InputPositions() []token.Pos {
+func (p list) InputPositions() []token.Pos {
if len(p) == 0 {
return nil
}
@@ -377,7 +390,7 @@
}
// Path reports the path location of the first error, if any.
-func (p List) Path() []string {
+func (p list) Path() []string {
if len(p) == 0 {
return nil
}
@@ -386,7 +399,7 @@
// Err returns an error equivalent to this error list.
// If the list is empty, Err returns nil.
-func (p List) Err() error {
+func (p list) Err() error {
if len(p) == 0 {
return nil
}
diff --git a/cue/errors/errors_test.go b/cue/errors/errors_test.go
index 76c7e68..f6bdc35 100644
--- a/cue/errors/errors_test.go
+++ b/cue/errors/errors_test.go
@@ -43,7 +43,7 @@
}
tests := []struct {
name string
- p *List
+ p *list
args args
}{
// TODO: Add test cases.
@@ -56,7 +56,7 @@
func TestErrorList_Reset(t *testing.T) {
tests := []struct {
name string
- p *List
+ p *list
}{
// TODO: Add test cases.
}
@@ -68,14 +68,14 @@
func TestErrorList_Len(t *testing.T) {
tests := []struct {
name string
- p List
+ p list
want int
}{
// TODO: Add test cases.
}
for _, tt := range tests {
if got := tt.p.Len(); got != tt.want {
- t.Errorf("%q. List.Len() = %v, want %v", tt.name, got, tt.want)
+ t.Errorf("%q. list.Len() = %v, want %v", tt.name, got, tt.want)
}
}
}
@@ -87,7 +87,7 @@
}
tests := []struct {
name string
- p List
+ p list
args args
}{
// TODO: Add test cases.
@@ -104,7 +104,7 @@
}
tests := []struct {
name string
- p List
+ p list
args args
want bool
}{
@@ -112,7 +112,7 @@
}
for _, tt := range tests {
if got := tt.p.Less(tt.args.i, tt.args.j); got != tt.want {
- t.Errorf("%q. List.Less() = %v, want %v", tt.name, got, tt.want)
+ t.Errorf("%q. list.Less() = %v, want %v", tt.name, got, tt.want)
}
}
}
@@ -120,7 +120,7 @@
func TestErrorList_Sort(t *testing.T) {
tests := []struct {
name string
- p List
+ p list
}{
// TODO: Add test cases.
}
@@ -132,7 +132,7 @@
func TestErrorList_RemoveMultiples(t *testing.T) {
tests := []struct {
name string
- p *List
+ p *list
}{
// TODO: Add test cases.
}
@@ -144,14 +144,14 @@
func TestErrorList_Error(t *testing.T) {
tests := []struct {
name string
- p List
+ p list
want string
}{
// TODO: Add test cases.
}
for _, tt := range tests {
if got := tt.p.Error(); got != tt.want {
- t.Errorf("%q. List.Error() = %v, want %v", tt.name, got, tt.want)
+ t.Errorf("%q. list.Error() = %v, want %v", tt.name, got, tt.want)
}
}
}
@@ -159,14 +159,14 @@
func TestErrorList_Err(t *testing.T) {
tests := []struct {
name string
- p List
+ p list
wantErr bool
}{
// TODO: Add test cases.
}
for _, tt := range tests {
if err := tt.p.Err(); (err != nil) != tt.wantErr {
- t.Errorf("%q. List.Err() error = %v, wantErr %v", tt.name, err, tt.wantErr)
+ t.Errorf("%q. list.Err() error = %v, wantErr %v", tt.name, err, tt.wantErr)
}
}
}
diff --git a/cue/load/import.go b/cue/load/import.go
index b011fcb..e0618cb 100644
--- a/cue/load/import.go
+++ b/cue/load/import.go
@@ -147,7 +147,7 @@
rewriteFiles(p, root, false)
if errs := fp.finalize(); errs != nil {
- for _, e := range errs {
+ for _, e := range errors.Errors(errs) {
p.ReportError(e)
}
return p
@@ -293,7 +293,7 @@
c *Config
pkg *build.Instance
- err errors.List
+ err errors.Error
}
func newFileProcessor(c *Config, p *build.Instance) *fileProcessor {
@@ -305,13 +305,13 @@
}
}
-func (fp *fileProcessor) finalize() errors.List {
+func (fp *fileProcessor) finalize() errors.Error {
p := fp.pkg
if fp.err != nil {
return fp.err
}
if len(p.CUEFiles) == 0 && !fp.c.DataFiles {
- fp.err.Add(&noCUEError{Package: p, Dir: p.Dir, Ignored: len(p.IgnoredCUEFiles) > 0})
+ fp.err = errors.Append(fp.err, &noCUEError{Package: p, Dir: p.Dir, Ignored: len(p.IgnoredCUEFiles) > 0})
return fp.err
}
@@ -337,7 +337,7 @@
p := fp.pkg
badFile := func(err errors.Error) bool {
- fp.err.Add(err)
+ fp.err = errors.Append(fp.err, err)
p.InvalidCUEFiles = append(p.InvalidCUEFiles, fullPath)
return true
}
@@ -357,7 +357,6 @@
pf, perr := parser.ParseFile(filename, data, parser.ImportsOnly, parser.ParseComments)
if perr != nil {
- // should always be an errors.List, but just in case.
badFile(errors.Promote(perr, "add failed"))
return true
}
diff --git a/cue/load/loader.go b/cue/load/loader.go
index aa8e38b..7dd8e90 100644
--- a/cue/load/loader.go
+++ b/cue/load/loader.go
@@ -133,7 +133,7 @@
}
pkg.Dir = cfg.Dir
rewriteFiles(pkg, pkg.Dir, true)
- for _, err := range fp.finalize() { // ImportDir(&ctxt, dir, 0)
+ for _, err := range errors.Errors(fp.finalize()) { // ImportDir(&ctxt, dir, 0)
pkg.ReportError(err)
}
// TODO: Support module importing.
diff --git a/cue/parser/error_test.go b/cue/parser/error_test.go
index 0118acb..e434d05 100644
--- a/cue/parser/error_test.go
+++ b/cue/parser/error_test.go
@@ -108,7 +108,7 @@
// compareErrors compares the map of expected error messages with the list
// of found errors and reports discrepancies.
//
-func compareErrors(t *testing.T, file *token.File, expected map[token.Pos]string, found errors.List) {
+func compareErrors(t *testing.T, file *token.File, expected map[token.Pos]string, found []errors.Error) {
t.Helper()
for _, error := range found {
// error.Pos is a Position, but we want
@@ -157,12 +157,7 @@
f, err := ParseFile(filename, src, DeclarationErrors, AllErrors)
file := f.Pos().File()
- found, ok := err.(errors.List)
- if err != nil && !ok {
- t.Error(err)
- return
- }
- found.RemoveMultiples()
+ found := errors.Errors(err)
// we are expecting the following errors
// (collect these after parsing a file so that it is found in the file set)
diff --git a/cue/parser/interface.go b/cue/parser/interface.go
index 980cb2e..0711b20 100644
--- a/cue/parser/interface.go
+++ b/cue/parser/interface.go
@@ -18,6 +18,7 @@
import (
"cuelang.org/go/cue/ast"
+ "cuelang.org/go/cue/errors"
"cuelang.org/go/cue/token"
"cuelang.org/go/internal/source"
)
@@ -127,8 +128,7 @@
}
}
- pp.errors.Sort()
- err = pp.errors.Err()
+ err = errors.Sanitize(pp.errors)
}()
// parse source
@@ -159,8 +159,7 @@
if p.panicking {
_ = recover()
}
- p.errors.Sort()
- err = p.errors.Err()
+ err = errors.Sanitize(p.errors)
}()
// parse expr
@@ -180,9 +179,8 @@
p.expect(token.EOF)
}
- if p.errors.Len() > 0 {
- p.errors.Sort()
- return nil, p.errors.Err()
+ if p.errors != nil {
+ return nil, p.errors
}
resolveExpr(e, p.errf)
diff --git a/cue/parser/parser.go b/cue/parser/parser.go
index b84f6b7..404d830 100644
--- a/cue/parser/parser.go
+++ b/cue/parser/parser.go
@@ -29,7 +29,7 @@
// The parser structure holds the parser's internal state.
type parser struct {
file *token.File
- errors errors.List
+ errors errors.Error
scanner scanner.Scanner
// Tracing/debugging
@@ -71,7 +71,7 @@
m = scanner.ScanComments
}
eh := func(pos token.Pos, msg string, args []interface{}) {
- p.errors.AddNewf(pos, msg, args...)
+ p.errors = errors.Append(p.errors, errors.Newf(pos, msg, args...))
}
p.scanner.Init(p.file, src, eh, m)
@@ -345,8 +345,9 @@
// as the last recorded error and stop parsing if there are more than
// 10 errors.
if p.mode&allErrorsMode == 0 {
- n := len(p.errors)
- if n > 0 && p.errors[n-1].Position().Line() == ePos.Line() {
+ errors := errors.Errors(p.errors)
+ n := len(errors)
+ if n > 0 && errors[n-1].Position().Line() == ePos.Line() {
return // discard - likely a spurious error
}
if n > 10 {
@@ -355,7 +356,7 @@
}
}
- p.errors.AddNewf(ePos, msg, args...)
+ p.errors = errors.Append(p.errors, errors.Newf(ePos, msg, args...))
}
func (p *parser) errorExpected(pos token.Pos, obj string) {
@@ -1314,7 +1315,7 @@
// Don't bother parsing the rest if we had errors scanning the first
// Likely not a Go source file at all.
- if p.errors.Len() != 0 {
+ if p.errors != nil {
return nil
}
diff --git a/cue/resolve_test.go b/cue/resolve_test.go
index 94e2ab4..193a6ef 100644
--- a/cue/resolve_test.go
+++ b/cue/resolve_test.go
@@ -17,16 +17,14 @@
import (
"flag"
"testing"
-
- "cuelang.org/go/cue/errors"
)
var traceOn = flag.Bool("debug", false, "enable tracing")
-func compileFileWithErrors(t *testing.T, body string) (*context, *structLit, errors.List) {
+func compileFileWithErrors(t *testing.T, body string) (*context, *structLit, error) {
t.Helper()
- ctx, inst, errs := compileInstance(t, body)
- return ctx, inst.rootValue.evalPartial(ctx).(*structLit), errs
+ ctx, inst, err := compileInstance(t, body)
+ return ctx, inst.rootValue.evalPartial(ctx).(*structLit), err
}
func compileFile(t *testing.T, body string) (*context, *structLit) {
@@ -38,27 +36,19 @@
return ctx, inst.rootValue.evalPartial(ctx).(*structLit)
}
-func compileInstance(t *testing.T, body string) (*context, *Instance, errors.List) {
+func compileInstance(t *testing.T, body string) (*context, *Instance, error) {
t.Helper()
var r Runtime
- x, err := r.Parse("test", body)
- ctx := r.index().newContext()
+ inst, err := r.Parse("test", body)
- switch errs := err.(type) {
- case nil:
- var r Runtime
- inst, _ := r.Parse("test", body)
- return r.index().newContext(), inst, nil
- case errors.List:
+ if err != nil {
x := newIndex().NewInstance(nil)
ctx := x.newContext()
-
- return ctx, x, errs
- default:
- t.Fatal(err)
+ return ctx, x, err
}
- return ctx, x, nil
+
+ return r.index().newContext(), inst, nil
}
func rewriteHelper(t *testing.T, cases []testCase, r rewriteMode) {
diff --git a/cue/scanner/scanner_test.go b/cue/scanner/scanner_test.go
index a1b8741..6748ce2 100644
--- a/cue/scanner/scanner_test.go
+++ b/cue/scanner/scanner_test.go
@@ -651,9 +651,9 @@
"//line File1:1\n" +
"~ ~ ~" // original file, line 1 again
- var list errors.List
+ var list errors.Error
eh := func(pos token.Pos, msg string, args []interface{}) {
- list.AddNewf(pos, msg, args...)
+ list = errors.Append(list, errors.Newf(pos, msg, args...))
}
var s Scanner
@@ -664,24 +664,19 @@
}
}
- if len(list) != s.ErrorCount {
- t.Errorf("found %d errors, expected %d", len(list), s.ErrorCount)
+ n := len(errors.Errors(list))
+ if n != s.ErrorCount {
+ t.Errorf("found %d errors, expected %d", n, s.ErrorCount)
}
- if len(list) != 9 {
- t.Errorf("found %d raw errors, expected 9", len(list))
+ if n != 9 {
+ t.Errorf("found %d raw errors, expected 9", n)
errors.Print(os.Stderr, list, nil)
}
- list.Sort()
- if len(list) != 9 {
- t.Errorf("found %d sorted errors, expected 9", len(list))
- errors.Print(os.Stderr, list, nil)
- }
-
- list.RemoveMultiples()
- if len(list) != 4 {
- t.Errorf("found %d one-per-line errors, expected 4", len(list))
+ n = len(errors.Errors(errors.Sanitize(list)))
+ if n != 4 {
+ t.Errorf("found %d one-per-line errors, expected 4", n)
errors.Print(os.Stderr, list, nil)
}
}
diff --git a/cue/types.go b/cue/types.go
index ee68d3b..38e6a70 100644
--- a/cue/types.go
+++ b/cue/types.go
@@ -1339,35 +1339,30 @@
}
}
-// Validate reports any errors, recursively. The returned error may be an
-// errors.List reporting multiple errors, where the total number of errors
-// reported may be less than the actual number.
+// Validate reports any errors, recursively. The returned error may represent
+// more than one error, retrievable with errors.Errors, if more than one
+// exists.
func (v Value) Validate(opts ...Option) error {
o := getOptions(opts)
- list := errors.List{}
+ var errs errors.Error
v.Walk(func(v Value) bool {
if err := v.checkKind(v.ctx(), bottomKind); err != nil {
if !o.concrete && isIncomplete(v.eval(v.ctx())) {
return false
}
- list.Add(v.toErr(err))
- if len(list) > 50 {
+ errs = errors.Append(errs, v.toErr(err))
+ if len(errors.Errors(errs)) > 50 {
return false // mostly to avoid some hypothetical cycle issue
}
}
if o.concrete {
if err := isGroundRecursive(v.ctx(), v.eval(v.ctx())); err != nil {
- list.Add(v.toErr(err))
+ errs = errors.Append(errs, v.toErr(err))
}
}
return true
}, nil)
- if len(list) > 0 {
- list.Sort()
- list.RemoveMultiples() // TODO: use RemoveMultiples when it is fixed
- return list
- }
- return nil
+ return errors.Sanitize(errs)
}
func isGroundRecursive(ctx *context, v value) *bottom {
diff --git a/encoding/protobuf/protobuf.go b/encoding/protobuf/protobuf.go
index 8d180d9..d813438 100644
--- a/encoding/protobuf/protobuf.go
+++ b/encoding/protobuf/protobuf.go
@@ -81,7 +81,7 @@
instCache map[string]*build.Instance
imports map[string]*build.Instance
- errs errors.List
+ errs errors.Error
done bool
}
@@ -114,11 +114,11 @@
// Err returns the errors accumulated during testing. The returned error may be
// of type cuelang.org/go/cue/errors.List.
func (b *Extractor) Err() error {
- return b.errs.Err()
+ return b.errs
}
func (b *Extractor) addErr(err error) {
- b.errs.Add(errors.Promote(err, "unknown error"))
+ b.errs = errors.Append(b.errs, errors.Promote(err, "unknown error"))
}
// AddFile adds a proto definition file to be converted into CUE by the builder.
@@ -131,8 +131,10 @@
//
func (b *Extractor) AddFile(filename string, src interface{}) error {
if b.done {
- b.errs.Add(errors.Newf(token.NoPos, "protobuf: cannot call AddFile: Instances was already called"))
- return b.Err()
+ err := errors.Newf(token.NoPos,
+ "protobuf: cannot call AddFile: Instances was already called")
+ b.errs = errors.Append(b.errs, err)
+ return err
}
if b.root != b.cwd && !filepath.IsAbs(filename) {
filename = filepath.Join(b.root, filename)
@@ -235,7 +237,7 @@
return instances[i].ImportPath < instances[j].ImportPath
})
- if err := b.errs.Err(); err != nil {
+ if err != nil {
return instances, err
}
return instances, nil
@@ -247,8 +249,10 @@
}
importPath := p.goPkgPath
if importPath == "" {
- b.errs.AddNewf(token.NoPos, "no go_package for proto package %q in file %s", p.id, p.file.Filename)
- // TODO: fine an alternative. Is proto package good enough?
+ err := errors.Newf(token.NoPos,
+ "no go_package for proto package %q in file %s", p.id, p.file.Filename)
+ b.errs = errors.Append(b.errs, err)
+ // TODO: find an alternative. Is proto package good enough?
return nil
}
@@ -263,10 +267,11 @@
want = filepath.Join(b.root, want)
}
if dir != want {
- b.errs.AddNewf(token.NoPos,
+ err := errors.Newf(token.NoPos,
"file %s mapped to inconsistent path %s; module name %q may be inconsistent with root dir %s",
want, dir, b.module, b.root,
)
+ b.errs = errors.Append(b.errs, err)
}
}
