diff --git a/cue/errors/errors.go b/cue/errors/errors.go index 708091095..26c99a7c5 100644 --- a/cue/errors/errors.go +++ b/cue/errors/errors.go @@ -29,6 +29,7 @@ import ( "strings" "cuelang.org/go/cue/token" + "cuelang.org/go/internal/core/format" ) // New is a convenience wrapper for [errors.New] in the core library. @@ -495,6 +496,12 @@ type Config struct { // ToSlash sets whether to use Unix paths. Mostly used for testing. ToSlash bool + + // OmitPath removes the path prefix from error messages. + OmitPath bool + + // Printer is used internally to detect printing cycles. + Printer format.Printer } var zeroConfig = &Config{} @@ -526,10 +533,20 @@ func String(err Error) string { return b.String() } +// StringWithConfig generates a short message from a given Error, using the +// provided configuration. +func StringWithConfig(err Error, cfg *Config) string { + var b strings.Builder + writeErr(&b, err, cfg) + return b.String() +} + func writeErr(w io.Writer, err Error, cfg *Config) { - if path := strings.Join(err.Path(), "."); path != "" { - _, _ = io.WriteString(w, path) - _, _ = io.WriteString(w, ": ") + if !cfg.OmitPath { + if path := strings.Join(err.Path(), "."); path != "" { + _, _ = io.WriteString(w, path) + _, _ = io.WriteString(w, ": ") + } } for { @@ -544,21 +561,33 @@ func writeErr(w io.Writer, err Error, cfg *Config) { // so we make a copy if we need to replace any arguments. didCopy := false for i, arg := range args { - var pos token.Position + var alt any switch arg := arg.(type) { case token.Pos: - pos = arg.Position() + pos := arg.Position() + pos.Filename = relPath(pos.Filename, cfg) + alt = pos case token.Position: - pos = arg + pos := arg + pos.Filename = relPath(pos.Filename, cfg) + alt = pos default: - continue + if cfg.Printer == nil { + // We should always do something. Consider replacing + // vertices with a path if this is not set. + continue + } + var replaced bool + alt, replaced = cfg.Printer.ReplaceArg(arg) + if !replaced { + continue + } } if !didCopy { args = slices.Clone(args) didCopy = true } - pos.Filename = relPath(pos.Filename, cfg) - args[i] = pos + args[i] = alt } n, _ := fmt.Fprintf(w, msg, args...) diff --git a/cue/testdata/eval/dynamic_field.txtar b/cue/testdata/eval/dynamic_field.txtar index 3831437c9..60eacc310 100644 --- a/cue/testdata/eval/dynamic_field.txtar +++ b/cue/testdata/eval/dynamic_field.txtar @@ -187,7 +187,7 @@ Result: // [incomplete] noCycleError.foo.baz.#ID: invalid interpolation: non-concrete value string (type string): // ./in.cue:59:8 // ./in.cue:59:11 - // noCycleError.foo.bar.entries: key value of dynamic field must be concrete, found _|_(invalid interpolation: noCycleError.foo.baz.#ID: non-concrete value string (type string)): + // noCycleError.foo.bar.entries: key value of dynamic field must be concrete, found _|_(noCycleError.foo.baz.#ID: invalid interpolation: non-concrete value string (type string)): // ./in.cue:61:22 } #ID: (_|_){ @@ -261,7 +261,7 @@ diff old new // [incomplete] noCycleError.foo.baz.#ID: invalid interpolation: non-concrete value string (type string): // ./in.cue:59:8 // ./in.cue:59:11 -+ // noCycleError.foo.bar.entries: key value of dynamic field must be concrete, found _|_(invalid interpolation: noCycleError.foo.baz.#ID: non-concrete value string (type string)): ++ // noCycleError.foo.bar.entries: key value of dynamic field must be concrete, found _|_(noCycleError.foo.baz.#ID: invalid interpolation: non-concrete value string (type string)): + // ./in.cue:61:22 } #ID: (_|_){ diff --git a/cue/testdata/eval/issue2235.txtar b/cue/testdata/eval/issue2235.txtar index 613ee27b2..7517c00ee 100644 --- a/cue/testdata/eval/issue2235.txtar +++ b/cue/testdata/eval/issue2235.txtar @@ -233,16 +233,16 @@ NumCloseIDs: 14 } } #GlobalIngressController: (_|_){ - // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: key value of dynamic field must be concrete, found _|_(invalid interpolation: #GlobalIngressController.objects.namespaced.ingress.Service: non-concrete value string (type string)): + // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: key value of dynamic field must be concrete, found _|_(#GlobalIngressController.objects.namespaced.ingress.Service: invalid interpolation: non-concrete value string (type string)): // ./issue2235.cue:43:12 class: (string){ string } objects: (#struct){ namespaced: (#struct){ ingress: (_|_){ - // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: key value of dynamic field must be concrete, found _|_(invalid interpolation: #GlobalIngressController.objects.namespaced.ingress.Service: non-concrete value string (type string)): + // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: key value of dynamic field must be concrete, found _|_(#GlobalIngressController.objects.namespaced.ingress.Service: invalid interpolation: non-concrete value string (type string)): // ./issue2235.cue:43:12 Service: (_|_){ - // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: key value of dynamic field must be concrete, found _|_(invalid interpolation: #GlobalIngressController.objects.namespaced.ingress.Service: non-concrete value string (type string)): + // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: key value of dynamic field must be concrete, found _|_(#GlobalIngressController.objects.namespaced.ingress.Service: invalid interpolation: non-concrete value string (type string)): // ./issue2235.cue:43:12 } } @@ -343,7 +343,7 @@ diff old new } #GlobalIngressController: (_|_){ - // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: invalid interpolation: non-concrete value string (type string): -+ // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: key value of dynamic field must be concrete, found _|_(invalid interpolation: #GlobalIngressController.objects.namespaced.ingress.Service: non-concrete value string (type string)): ++ // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: key value of dynamic field must be concrete, found _|_(#GlobalIngressController.objects.namespaced.ingress.Service: invalid interpolation: non-concrete value string (type string)): // ./issue2235.cue:43:12 - // ./issue2235.cue:40:9 class: (string){ string } @@ -351,12 +351,12 @@ diff old new namespaced: (#struct){ ingress: (_|_){ - // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: invalid interpolation: non-concrete value string (type string): -+ // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: key value of dynamic field must be concrete, found _|_(invalid interpolation: #GlobalIngressController.objects.namespaced.ingress.Service: non-concrete value string (type string)): ++ // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: key value of dynamic field must be concrete, found _|_(#GlobalIngressController.objects.namespaced.ingress.Service: invalid interpolation: non-concrete value string (type string)): // ./issue2235.cue:43:12 - // ./issue2235.cue:40:9 Service: (_|_){ - // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: invalid interpolation: non-concrete value string (type string): -+ // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: key value of dynamic field must be concrete, found _|_(invalid interpolation: #GlobalIngressController.objects.namespaced.ingress.Service: non-concrete value string (type string)): ++ // [incomplete] #GlobalIngressController.objects.namespaced.ingress.Service: key value of dynamic field must be concrete, found _|_(#GlobalIngressController.objects.namespaced.ingress.Service: invalid interpolation: non-concrete value string (type string)): // ./issue2235.cue:43:12 - // ./issue2235.cue:40:9 } diff --git a/cue/testdata/references/value.txtar b/cue/testdata/references/value.txtar index 3917496f4..eb3ad055e 100644 --- a/cue/testdata/references/value.txtar +++ b/cue/testdata/references/value.txtar @@ -10,6 +10,14 @@ valueCycle: b: X=3 + X // Issue #1003 listValueAlias: X = [1, 2, X[0]] + +-- err.cue -- +@experiment(structcmp) + +cycleErr: X={ + err3: == (X + 1) + err3: 4 +} -- out/eval/stats -- Leaks: 0 Freed: 13 @@ -21,7 +29,24 @@ Unifications: 13 Conjuncts: 20 Disjuncts: 13 -- out/evalalpha -- -(struct){ +Errors: +cycleErr.err3: invalid operands {err3:_|_(cycleErr.err3: invalid operands value at path 'cycleErr' and 1 to '+' (type struct and int))} and 1 to '+' (type struct and int): + ./err.cue:4:12 + ./err.cue:3:13 + ./err.cue:4:16 + +Result: +(_|_){ + // [eval] + cycleErr: (_|_){ + // [eval] + err3: (_|_){ + // [eval] cycleErr.err3: invalid operands {err3:_|_(cycleErr.err3: invalid operands value at path 'cycleErr' and 1 to '+' (type struct and int))} and 1 to '+' (type struct and int): + // ./err.cue:4:12 + // ./err.cue:3:13 + // ./err.cue:4:16 + } + } structShorthand: (struct){ b: (int){ 3 } c: (int){ 3 } @@ -48,7 +73,30 @@ Disjuncts: 13 diff old new --- old +++ new -@@ -11,8 +11,8 @@ +@@ -1,4 +1,21 @@ +-(struct){ ++Errors: ++cycleErr.err3: invalid operands {err3:_|_(cycleErr.err3: invalid operands value at path 'cycleErr' and 1 to '+' (type struct and int))} and 1 to '+' (type struct and int): ++ ./err.cue:4:12 ++ ./err.cue:3:13 ++ ./err.cue:4:16 ++ ++Result: ++(_|_){ ++ // [eval] ++ cycleErr: (_|_){ ++ // [eval] ++ err3: (_|_){ ++ // [eval] cycleErr.err3: invalid operands {err3:_|_(cycleErr.err3: invalid operands value at path 'cycleErr' and 1 to '+' (type struct and int))} and 1 to '+' (type struct and int): ++ // ./err.cue:4:12 ++ // ./err.cue:3:13 ++ // ./err.cue:4:16 ++ } ++ } + structShorthand: (struct){ + b: (int){ 3 } + c: (int){ 3 } +@@ -11,8 +28,8 @@ } valueCycle: (struct){ b: (_|_){ @@ -84,6 +132,13 @@ diff old new } } -- out/compile -- +--- err.cue +{ + cycleErr: { + err3: ==(〈1〉 + 1) + err3: 4 + } +} --- in.cue { structShorthand: { diff --git a/cue/types_test.go b/cue/types_test.go index 16e3f7bd0..9666e958f 100644 --- a/cue/types_test.go +++ b/cue/types_test.go @@ -3183,7 +3183,7 @@ func TestMarshalJSON(t *testing.T) { }, { // Issue #326 value: `x: "\(string)": "v"`, - err: `x: key value of dynamic field must be concrete, found _|_(invalid interpolation: x: non-concrete value string (type string)) (and 1 more errors)`, + err: `x: key value of dynamic field must be concrete, found _|_(x: invalid interpolation: non-concrete value string (type string)) (and 1 more errors)`, }, { // Issue #326 value: `x: "\(bool)": "v"`, diff --git a/internal/core/adt/context.go b/internal/core/adt/context.go index cba0a0801..76a9f36eb 100644 --- a/internal/core/adt/context.go +++ b/internal/core/adt/context.go @@ -1327,15 +1327,31 @@ func (c *OpContext) String(x Node) string { return c.Format(c.Runtime, x) } -type stringerFunc func() string +// Formatter wraps an adt.Node with the necessary information to print it. +// +// TODO: we could eliminate the need for this by ensuring that errors are +// _always_ formatted with a printer. We are not far off from this goal, but +// we need to verify several things. +// This is mainly possible because we intend to have a global string index +// using weak references. It also assumes that errors are always printed +// equally. +type Formatter struct { + X Node + + // F formats Node, resolving references as needed.using Runtime. + // TODO: only used for cases where the debug printer is somehow + // circumvented. Verify this no longer happens. + F func(Runtime, Node) string -func (f stringerFunc) String() string { return f() } + // TODO: is runtime needed? Probably not if we have a global string index. + R Runtime +} + +func (f Formatter) String() string { return f.F(f.R, f.X) } // Str reports a string of x via a [fmt.Stringer], for use in errors or debugging. func (c *OpContext) Str(x Node) fmt.Stringer { - return stringerFunc(func() string { - return c.String(x) - }) + return Formatter{X: x, F: c.Format, R: c.Runtime} } // NewList returns a new list for the given values. diff --git a/internal/core/debug/compact.go b/internal/core/debug/compact.go index 0d9629e0a..2402d7693 100644 --- a/internal/core/debug/compact.go +++ b/internal/core/debug/compact.go @@ -45,6 +45,11 @@ func (w *printer) compactNode(n adt.Node) { switch v := x.BaseValue.(type) { case *adt.StructMarker: + if !w.pushVertex(x) { + return + } + defer w.popVertex() + w.string("{") for i, a := range x.Arcs { if i > 0 { @@ -72,6 +77,11 @@ func (w *printer) compactNode(n adt.Node) { w.string("}") case *adt.ListMarker: + if !w.pushVertex(x) { + return + } + defer w.popVertex() + w.string("[") for i, a := range x.Arcs { if i > 0 { @@ -82,6 +92,8 @@ func (w *printer) compactNode(n adt.Node) { w.string("]") case *adt.Vertex: + // Disjunction, structure shared, etc. + if v, ok := w.printShared(x); !ok { w.node(v) w.popVertex() @@ -154,7 +166,7 @@ func (w *printer) compactNode(n adt.Node) { w.string(`_|_`) if x.Err != nil { w.string("(") - w.string(x.Err.Error()) + w.shortError(x.Err, false) w.string(")") } diff --git a/internal/core/debug/debug.go b/internal/core/debug/debug.go index e255e72cf..6cc55c7ee 100644 --- a/internal/core/debug/debug.go +++ b/internal/core/debug/debug.go @@ -81,6 +81,49 @@ type printer struct { // - auto } +// ReplaceArg implements the format.Printer interface. It wraps Vertex arguments +// with a formatter value, that holds a pointer to w. This allows the stack +// of processed vertices to be passed down, which in turn is used for cycle +// detection. +func (w *printer) ReplaceArg(arg any) (replacement any, replaced bool) { + var x adt.Node + var r adt.Runtime + switch v := arg.(type) { + case adt.Node: + x = v + case adt.Formatter: + x = v.X + r = v.R + } + + switch x := x.(type) { + default: + return arg, false + case *adt.Vertex: + // We replace the formatter (or node) with our own formatter that is + // capable of detecting cycles. + return formatter{p: w, x: x, r: r}, true + } +} + +type formatter struct { + p *printer + x adt.Node + r adt.Runtime `` +} + +func (f formatter) String() string { + p := printer{ + dst: make([]byte, 0, 128), + index: f.r, + cfg: f.p.cfg, + compact: true, // Always compact for error arguments. + stack: f.p.stack, + } + p.node(f.x) + return string(p.dst) +} + func (w *printer) string(s string) { if !w.compact && len(w.indent) > 0 { s = strings.Replace(s, "\n", "\n"+w.indent, -1) @@ -163,7 +206,9 @@ func (w *printer) printShared(v0 *adt.Vertex) (x *adt.Vertex, ok bool) { func (w *printer) pushVertex(v *adt.Vertex) bool { for _, x := range w.stack { if x == v { - w.string("") + w.string("value at path '") + w.path(v) + w.string("'") return false } } @@ -175,21 +220,15 @@ func (w *printer) popVertex() { w.stack = w.stack[:len(w.stack)-1] } -func (w *printer) shortError(errs errors.Error) { - for { - msg, args := errs.Msg() - w.dst = fmt.Appendf(w.dst, msg, args...) - - err := errors.Unwrap(errs) - if err == nil { - break - } - - if errs, _ = err.(errors.Error); errs != nil { - w.string(err.Error()) - break - } - } +// TODO: always print path? We allow a choice for keeping the error diff at a +// minimum. +func (w *printer) shortError(errs errors.Error, omitPath bool) { + w.string(errors.StringWithConfig(errs, &errors.Config{ + Cwd: w.cfg.Cwd, + ToSlash: true, + OmitPath: omitPath, + Printer: w, + })) } func (w *printer) interpolation(x *adt.Interpolation) { @@ -277,6 +316,7 @@ func (w *printer) node(n adt.Node) { msg := errors.Details(v.Err, &errors.Config{ Cwd: w.cfg.Cwd, ToSlash: true, + Printer: w, }) msg = strings.TrimSpace(msg) if msg != "" { @@ -438,7 +478,7 @@ func (w *printer) node(n adt.Node) { w.string(`_|_`) if x.Err != nil { w.string("(") - w.shortError(x.Err) + w.shortError(x.Err, true) w.string(")") } diff --git a/internal/core/export/testdata/main/let.txtar b/internal/core/export/testdata/main/let.txtar index 1d5c8c436..771cd4878 100644 --- a/internal/core/export/testdata/main/let.txtar +++ b/internal/core/export/testdata/main/let.txtar @@ -598,8 +598,8 @@ y: Y & Y_1 a: "foo" } } - comprehension: _|_ // comprehension: key value of dynamic field must be concrete, found _|_(invalid interpolation: invalid interpolation: comprehension.filepath: undefined field: name) (and 1 more errors) - files: _|_ // files: key value of dynamic field must be concrete, found _|_(invalid interpolation: invalid interpolation: filepath: undefined field: name) (and 3 more errors) + comprehension: _|_ // comprehension: key value of dynamic field must be concrete, found _|_(comprehension.filepath: invalid interpolation: invalid interpolation: undefined field: name) (and 1 more errors) + files: _|_ // files: key value of dynamic field must be concrete, found _|_(filepath: invalid interpolation: invalid interpolation: undefined field: name) (and 3 more errors) incomplete: { a: { x: _|_ // invalid interpolation: incomplete.a.x: non-concrete value string (type string) @@ -901,8 +901,8 @@ diff old new } - comprehension: _|_ // invalid interpolation: cycle error - files: _|_ // invalid interpolation: cycle error (and 1 more errors) -+ comprehension: _|_ // comprehension: key value of dynamic field must be concrete, found _|_(invalid interpolation: invalid interpolation: comprehension.filepath: undefined field: name) (and 1 more errors) -+ files: _|_ // files: key value of dynamic field must be concrete, found _|_(invalid interpolation: invalid interpolation: filepath: undefined field: name) (and 3 more errors) ++ comprehension: _|_ // comprehension: key value of dynamic field must be concrete, found _|_(comprehension.filepath: invalid interpolation: invalid interpolation: undefined field: name) (and 1 more errors) ++ files: _|_ // files: key value of dynamic field must be concrete, found _|_(filepath: invalid interpolation: invalid interpolation: undefined field: name) (and 3 more errors) incomplete: { a: { x: _|_ // invalid interpolation: incomplete.a.x: non-concrete value string (type string) diff --git a/internal/core/format/printer.go b/internal/core/format/printer.go new file mode 100644 index 000000000..a907a73b0 --- /dev/null +++ b/internal/core/format/printer.go @@ -0,0 +1,26 @@ +// Copyright 2025 CUE Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package format provides functionality for pretty-printing CUE values. +// These types need to be in a separate package to avoid import cycles. +package format + +// Printer is the interface used to print CUE values. The only implementation so +// far is the one in internal/core/debug. Note that most packages cannot +// directly import the debug package. +type Printer interface { + // ReplaceArg is a function that may be called to replace arguments to + // errors. This is mostly used for cycle detection. + ReplaceArg(x any) (r any, wasReplaced bool) +}