Skip to content

Commit

Permalink
cue/errors: remove List from public API
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@mpvl
mpvl committed Jul 9, 2019
1 parent 561b39a commit a68a786
Show file tree
Hide file tree
Showing 13 changed files with 120 additions and 129 deletions.
6 changes: 3 additions & 3 deletions cue/ast.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (inst *Instance) insertFile(f *ast.File) errors.Error {
// 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)
Expand Down Expand Up @@ -87,7 +87,7 @@ type astState struct {
// 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) {
Expand Down Expand Up @@ -126,7 +126,7 @@ func newVisitorCtx(ctx *context, inst *build.Instance, obj, resolveRoot *structL
}

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),
Expand Down
6 changes: 3 additions & 3 deletions cue/ast_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,10 @@ func TestCompile(t *testing.T) {
}}
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()
Expand Down
65 changes: 39 additions & 26 deletions cue/errors/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func Wrapf(err error, p token.Pos, format string, args ...interface{}) Error {
}
}

// 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:
Expand Down Expand Up @@ -195,7 +195,7 @@ func Append(a, b Error) Error {
switch x := a.(type) {
case nil:
return b
case List:
case list:
return appendToList(x, b)
}
// Preserve order of errors.
Expand All @@ -212,7 +212,7 @@ func Errors(err error) []Error {
switch x := err.(type) {
case nil:
return nil
case List:
case list:
return []Error(x)
case Error:
return []Error{x}
Expand All @@ -221,43 +221,43 @@ func Errors(err error) []Error {
}
}

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
}
Expand Down Expand Up @@ -315,16 +315,29 @@ func equalPath(a, b []string) bool {
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
Expand All @@ -343,13 +356,13 @@ func (p *List) RemoveMultiples() {
}

// 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
Expand All @@ -361,23 +374,23 @@ func (p List) Msg() (format string, args []interface{}) {
}

// 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
}
return p[0].Position()
}

// 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
}
return p[0].InputPositions()
}

// 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
}
Expand All @@ -386,7 +399,7 @@ func (p List) Path() []string {

// 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
}
Expand Down
26 changes: 13 additions & 13 deletions cue/errors/errors_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestErrorList_Add(t *testing.T) {
}
tests := []struct {
name string
p *List
p *list
args args
}{
// TODO: Add test cases.
Expand All @@ -56,7 +56,7 @@ func TestErrorList_Add(t *testing.T) {
func TestErrorList_Reset(t *testing.T) {
tests := []struct {
name string
p *List
p *list
}{
// TODO: Add test cases.
}
Expand All @@ -68,14 +68,14 @@ func TestErrorList_Reset(t *testing.T) {
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)
}
}
}
Expand All @@ -87,7 +87,7 @@ func TestErrorList_Swap(t *testing.T) {
}
tests := []struct {
name string
p List
p list
args args
}{
// TODO: Add test cases.
Expand All @@ -104,23 +104,23 @@ func TestErrorList_Less(t *testing.T) {
}
tests := []struct {
name string
p List
p list
args args
want bool
}{
// TODO: Add test cases.
}
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)
}
}
}

func TestErrorList_Sort(t *testing.T) {
tests := []struct {
name string
p List
p list
}{
// TODO: Add test cases.
}
Expand All @@ -132,7 +132,7 @@ func TestErrorList_Sort(t *testing.T) {
func TestErrorList_RemoveMultiples(t *testing.T) {
tests := []struct {
name string
p *List
p *list
}{
// TODO: Add test cases.
}
Expand All @@ -144,29 +144,29 @@ func TestErrorList_RemoveMultiples(t *testing.T) {
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)
}
}
}

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)
}
}
}
Expand Down
11 changes: 5 additions & 6 deletions cue/load/import.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func (l *loader) importPkg(pos token.Pos, path, srcDir string) *build.Instance {

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
Expand Down Expand Up @@ -293,7 +293,7 @@ type fileProcessor struct {
c *Config
pkg *build.Instance

err errors.List
err errors.Error
}

func newFileProcessor(c *Config, p *build.Instance) *fileProcessor {
Expand All @@ -305,13 +305,13 @@ func newFileProcessor(c *Config, p *build.Instance) *fileProcessor {
}
}

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
}

Expand All @@ -337,7 +337,7 @@ func (fp *fileProcessor) add(pos token.Pos, root, path string, mode importMode)
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
}
Expand All @@ -357,7 +357,6 @@ 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.
badFile(errors.Promote(perr, "add failed"))
return true
}
Expand Down
2 changes: 1 addition & 1 deletion cue/load/loader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (l *loader) cueFilesPackage(files []string) *build.Instance {
}
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.
Expand Down
Loading

0 comments on commit a68a786

Please sign in to comment.