cue/errors: use Pos instead of Position in Error
Issue #52
This simplifies code and allows optimizations down
the road.
Change-Id: Ifd6c946a7f22f84c561a3ecae935e7e8f7560353
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/2205
Reviewed-by: Marcel van Lohuizen <mpvl@google.com>
diff --git a/cue/build/instance.go b/cue/build/instance.go
index da3eb24..f0407c0 100644
--- a/cue/build/instance.go
+++ b/cue/build/instance.go
@@ -140,7 +140,7 @@
}
func (inst *Instance) errorf(pos token.Pos, format string, args ...interface{}) error {
- return inst.chkErr(errors.E(pos.Position(), fmt.Sprintf(format, args...)))
+ return inst.chkErr(errors.E(pos, fmt.Sprintf(format, args...)))
}
// Context defines the build context for this instance. All files defined
diff --git a/cue/errors.go b/cue/errors.go
index bc1e2ae..afd89ae 100644
--- a/cue/errors.go
+++ b/cue/errors.go
@@ -24,6 +24,8 @@
"golang.org/x/xerrors"
)
+var _ errors.Error = &nodeError{}
+
// A nodeError is an error associated with processing an AST node.
type nodeError struct {
path []string // optional
@@ -35,12 +37,12 @@
func nodeErrorf(n ast.Node, format string, args ...interface{}) *nodeError {
return &nodeError{
n: n,
- Message: errors.Message{Format: format, Args: args},
+ Message: errors.NewMessage(format, args),
}
}
-func (e *nodeError) Position() token.Position {
- return e.n.Pos().Position()
+func (e *nodeError) Position() token.Pos {
+ return e.n.Pos()
}
func (e *nodeError) Path() []string {
@@ -49,15 +51,14 @@
func (v Value) toErr(b *bottom) errors.Error {
return &valueError{
- Message: errors.Message{
- Format: b.msg,
- Args: nil,
- },
- v: v,
- err: b,
+ Message: errors.NewMessage(b.msg, nil),
+ v: v,
+ err: b,
}
}
+var _ errors.Error = &valueError{}
+
// A valueError is returned as a result of evaluating a value.
type valueError struct {
errors.Message
@@ -66,8 +67,8 @@
err *bottom
}
-func (e *valueError) Position() token.Position {
- return e.err.pos.Pos().Position()
+func (e *valueError) Position() token.Pos {
+ return e.err.pos.Pos()
}
func (e *valueError) Path() (a []string) {
diff --git a/cue/errors/errors.go b/cue/errors/errors.go
index 1531f51..019c71d 100644
--- a/cue/errors/errors.go
+++ b/cue/errors/errors.go
@@ -34,7 +34,7 @@
// A Handler is a generic error handler used throughout CUE packages.
//
// The position points to the beginning of the offending value.
-type Handler func(pos token.Position, msg string)
+type Handler func(pos token.Pos, msg string)
// A Message implements the error interface as well as Message to allow
// internationalized messages.
@@ -62,7 +62,7 @@
// Error is the common error message.
type Error interface {
- Position() token.Position
+ Position() token.Pos
// Error reports the error message without position information.
Error() string
@@ -71,11 +71,7 @@
// // TODO: make Error an interface that returns a list of positions.
// Newf creates an Error with the associated position and message.
-func Newf(pos token.Pos, format string, args ...interface{}) Error {
- var p token.Position
- if pos.IsValid() {
- p = pos.Position()
- }
+func Newf(p token.Pos, format string, args ...interface{}) Error {
return &posError{
pos: p,
Message: NewMessage(format, args),
@@ -84,11 +80,7 @@
// Wrapf creates an Error with the associated position and message. The provided
// error is added for inspection context.
-func Wrapf(err error, pos token.Pos, format string, args ...interface{}) Error {
- var p token.Position
- if pos.IsValid() {
- p = pos.Position()
- }
+func Wrapf(err error, p token.Pos, format string, args ...interface{}) Error {
return &posError{
pos: p,
Message: NewMessage(format, args),
@@ -96,12 +88,14 @@
}
}
+var _ Error = &posError{}
+
// In an List, an error is represented by an *posError.
// The position Pos, if valid, points to the beginning of
// the offending token, and the error condition is described
// by Msg.
type posError struct {
- pos token.Position
+ pos token.Pos
Message
// The underlying error that triggered this one, if any.
@@ -121,9 +115,9 @@
switch x := a.(type) {
case string:
e.Message = NewMessage("%s", []interface{}{x})
- case token.Position:
+ case token.Pos:
e.pos = x
- case []token.Position:
+ case []token.Pos:
// TODO: do something more clever
if len(x) > 0 {
e.pos = x[0]
@@ -157,7 +151,7 @@
return &posError{err: err}
}
-func (e *posError) Position() token.Position {
+func (e *posError) Position() token.Pos {
return e.pos
}
@@ -171,7 +165,7 @@
} else {
p.Printf(format, args...)
}
- if p.Detail() && e.pos.Filename != "" || e.pos.IsValid() {
+ if p.Detail() && e.pos.Filename() != "" || e.pos.IsValid() {
p.Printf("%s", e.pos.String())
}
return next
@@ -204,7 +198,7 @@
}
// AddNew adds an Error with given position and error message to an List.
-func (p *List) AddNew(pos token.Position, msg string) {
+func (p *List) AddNew(pos token.Pos, msg string) {
p.add(&posError{pos: pos, Message: Message{format: msg}})
}
@@ -226,14 +220,14 @@
// Note that it is not sufficient to simply compare file offsets because
// the offsets do not reflect modified line information (through //line
// comments).
- if e.Filename != f.Filename {
- return e.Filename < f.Filename
+ if e.Filename() != f.Filename() {
+ return e.Filename() < f.Filename()
}
- if e.Line != f.Line {
- return e.Line < f.Line
+ if e.Line() != f.Line() {
+ return e.Line() < f.Line()
}
- if e.Column != f.Column {
- return e.Column < f.Column
+ if e.Column() != f.Column() {
+ return e.Column() < f.Column()
}
return p[i].Error() < p[j].Error()
}
@@ -249,11 +243,11 @@
// RemoveMultiples sorts an List and removes all but the first error per line.
func (p *List) RemoveMultiples() {
sort.Sort(p)
- var last token.Position // initial last.Line is != any legal error line
+ var last token.Pos // initial last.Line is != any legal error line
i := 0
for _, e := range *p {
pos := e.Position()
- if pos.Filename != last.Filename || pos.Line != last.Line {
+ if pos.Filename() != last.Filename() || pos.Line() != last.Line() {
last = pos
(*p)[i] = e
i++
@@ -305,7 +299,7 @@
for e := err; e != nil; e = xerrors.Unwrap(e) {
switch x := e.(type) {
- case interface{ Position() token.Position }:
+ case interface{ Position() token.Pos }:
if pos := x.Position().String(); pos != "-" {
positions = append(positions, pos)
}
diff --git a/cue/errors/errors_test.go b/cue/errors/errors_test.go
index fb8e7ff..2f124a4 100644
--- a/cue/errors/errors_test.go
+++ b/cue/errors/errors_test.go
@@ -38,7 +38,7 @@
func TestErrorList_Add(t *testing.T) {
type args struct {
- pos token.Position
+ pos token.Pos
msg string
}
tests := []struct {
diff --git a/cue/load/errors.go b/cue/load/errors.go
index 6f77495..6196e2c 100644
--- a/cue/load/errors.go
+++ b/cue/load/errors.go
@@ -53,14 +53,14 @@
// A packageError describes an error loading information about a package.
type packageError struct {
- ImportStack []string // shortest path from package named on command line to this one
- Pos string // position of error
- Err string // the error itself
- IsImportCycle bool `json:"-"` // the error is an import cycle
- Hard bool `json:"-"` // whether the error is soft or hard; soft errors are ignored in some places
+ ImportStack []string // shortest path from package named on command line to this one
+ Pos token.Pos // position of error
+ Err string // the error itself
+ IsImportCycle bool `json:"-"` // the error is an import cycle
+ Hard bool `json:"-"` // whether the error is soft or hard; soft errors are ignored in some places
}
-func (l *loader) errPkgf(importPos []token.Position, format string, args ...interface{}) *packageError {
+func (l *loader) errPkgf(importPos []token.Pos, format string, args ...interface{}) *packageError {
err := &packageError{
ImportStack: l.stk.Copy(),
Err: fmt.Sprintf(format, args...),
@@ -69,11 +69,9 @@
return err
}
-func (p *packageError) fillPos(cwd string, positions []token.Position) {
- if len(positions) > 0 && p.Pos == "" {
- pos := positions[0]
- pos.Filename = shortPath(cwd, pos.Filename)
- p.Pos = pos.String()
+func (p *packageError) fillPos(cwd string, positions []token.Pos) {
+ if len(positions) > 0 && !p.Pos.IsValid() {
+ p.Pos = positions[0]
}
}
@@ -82,10 +80,10 @@
if p.IsImportCycle {
return fmt.Sprintf("%s\npackage %s\n", p.Err, strings.Join(p.ImportStack, "\n\timports "))
}
- if p.Pos != "" {
+ if p.Pos.IsValid() {
// Omit import stack. The full path to the file where the error
// is the most important thing.
- return p.Pos + ": " + p.Err
+ return p.Pos.String() + ": " + p.Err
}
if len(p.ImportStack) == 0 {
return p.Err
diff --git a/cue/load/loader.go b/cue/load/loader.go
index eb4597d..c9a3c78 100644
--- a/cue/load/loader.go
+++ b/cue/load/loader.go
@@ -89,13 +89,6 @@
getTestDeps
)
-func firstPos(p []token.Position) token.Position {
- if len(p) == 0 {
- return token.Position{}
- }
- return p[0]
-}
-
type loader struct {
cfg *Config
stk importStack
diff --git a/cue/parser/error_test.go b/cue/parser/error_test.go
index eb0cb1f..628c9c6 100644
--- a/cue/parser/error_test.go
+++ b/cue/parser/error_test.go
@@ -114,7 +114,7 @@
// a Pos so we can do a map lookup
ePos := error.Position()
eMsg := error.Error()
- pos := getPos(file, ePos.Offset).WithRel(0)
+ pos := getPos(file, ePos.Offset()).WithRel(0)
if msg, found := expected[pos]; found {
// we expect a message at pos; check if it matches
rx, err := regexp.Compile(msg)
diff --git a/cue/parser/parser.go b/cue/parser/parser.go
index 20e63c4..27b4e5b 100644
--- a/cue/parser/parser.go
+++ b/cue/parser/parser.go
@@ -70,7 +70,7 @@
if p.mode&parseCommentsMode != 0 {
m = scanner.ScanComments
}
- eh := func(pos token.Position, msg string) { p.errors.AddNew(pos, msg) }
+ eh := func(pos token.Pos, msg string) { p.errors.AddNew(pos, msg) }
p.scanner.Init(p.file, src, eh, m)
p.trace = p.mode&traceMode != 0 // for convenience (p.trace is used frequently)
@@ -343,14 +343,15 @@
}
func (p *parser) error(pos token.Pos, msg string) {
- ePos := p.file.Position(pos)
+ // ePos := p.file.Position(pos)
+ ePos := pos
// If AllErrors is not set, discard errors reported on the same line
// 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 {
+ if n > 0 && p.errors[n-1].Position().Line() == ePos.Line() {
return // discard - likely a spurious error
}
if n > 10 {
diff --git a/cue/scanner/scanner.go b/cue/scanner/scanner.go
index 03c9c19..c0617d4 100644
--- a/cue/scanner/scanner.go
+++ b/cue/scanner/scanner.go
@@ -147,7 +147,7 @@
func (s *Scanner) error(offs int, msg string) {
if s.err != nil {
- s.err(s.file.Position(s.file.Pos(offs, 0)), msg)
+ s.err(s.file.Pos(offs, 0), msg)
}
s.ErrorCount++
}
diff --git a/cue/scanner/scanner_test.go b/cue/scanner/scanner_test.go
index 9b0a8f7..18cbd3f 100644
--- a/cue/scanner/scanner_test.go
+++ b/cue/scanner/scanner_test.go
@@ -208,7 +208,7 @@
whitespace_linecount := newlineCount(whitespace)
// error handler
- eh := func(_ token.Position, msg string) {
+ eh := func(_ token.Pos, msg string) {
t.Errorf("error handler called (msg = %s)", msg)
}
@@ -591,7 +591,7 @@
func TestScanInterpolation(t *testing.T) {
// error handler
- eh := func(pos token.Position, msg string) {
+ eh := func(pos token.Pos, msg string) {
t.Errorf("error handler called (pos = %v, msg = %s)", pos, msg)
}
trim := func(s string) string { return strings.Trim(s, `#"\\()`) }
@@ -646,7 +646,7 @@
"~ ~ ~" // original file, line 1 again
var list errors.List
- eh := func(pos token.Position, msg string) { list.AddNew(pos, msg) }
+ eh := func(pos token.Pos, msg string) { list.AddNew(pos, msg) }
var s Scanner
s.Init(token.NewFile("File1", 1, len(src)), []byte(src), eh, dontInsertCommas)
@@ -679,16 +679,16 @@
}
type errorCollector struct {
- cnt int // number of errors encountered
- msg string // last error message encountered
- pos token.Position // last error position encountered
+ cnt int // number of errors encountered
+ msg string // last error message encountered
+ pos token.Pos // last error position encountered
}
func checkError(t *testing.T, src string, tok token.Token, pos int, lit, err string) {
t.Helper()
var s Scanner
var h errorCollector
- eh := func(pos token.Position, msg string) {
+ eh := func(pos token.Pos, msg string) {
h.cnt++
h.msg = msg
h.pos = pos
@@ -711,8 +711,8 @@
if h.msg != err {
t.Errorf("%q: got msg %q, expected %q", src, h.msg, err)
}
- if h.pos.Offset != pos {
- t.Errorf("%q: got offset %d, expected %d", src, h.pos.Offset, pos)
+ if h.pos.Offset() != pos {
+ t.Errorf("%q: got offset %d, expected %d", src, h.pos.Offset(), pos)
}
}
diff --git a/cue/token/position.go b/cue/token/position.go
index a3bded3..9371083 100644
--- a/cue/token/position.go
+++ b/cue/token/position.go
@@ -84,6 +84,20 @@
return p.Position().Line
}
+func (p Pos) Column() int {
+ if p.file == nil {
+ return 0
+ }
+ return p.Position().Column
+}
+
+func (p Pos) Filename() string {
+ if p.file == nil {
+ return ""
+ }
+ return p.Position().Filename
+}
+
func (p Pos) Position() Position {
if p.file == nil {
return Position{}
@@ -150,7 +164,7 @@
// Offset reports the byte offset relative to the file.
func (p Pos) Offset() int {
- return p.offset >> relShift
+ return p.Position().Offset
}
// Add creates a new position relative to the p offset by n.
diff --git a/cue/types.go b/cue/types.go
index 810753c..f168f35 100644
--- a/cue/types.go
+++ b/cue/types.go
@@ -796,12 +796,12 @@
}
// Pos returns position information.
-func (v Value) Pos() token.Position {
+func (v Value) Pos() token.Pos {
if v.path == nil || v.Source() == nil {
- return token.Position{}
+ return token.NoPos
}
pos := v.Source().Pos()
- return pos.Position()
+ return pos
}
// IsConcrete reports whether the current value is a concrete scalar value,
