internal/core/adt: get rid of NotExistError

And replace it with a field.

Note that this does not fully fix the below error, as the error
is still incorrectly marked as an incomplete error.

Issue #1509

Signed-off-by: Marcel van Lohuizen <mpvl@golang.org>

Change-Id: I419705b0e67fd960c65f67befd0611542edbb503
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/532871
Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Paul Jolly <paul@myitcv.io>
Reviewed-by: Marcel van Lohuizen <mpvl@gmail.com>

Author: mpvl

cmd/cue/cmd/testdata/script/eval_notexistfatal.txt

@@ -0,0 +1,34 @@
+#Issue: 1509
+
+# TODO: recognize when an unresolved value
+
+# export
+! exec cue export
+cmp stderr expect-stderr1
+cmp stdout expect-stdout1
+
+# eval
+exec cue eval
+cmp stderr expect-stderr2
+cmp stdout expect-stdout2
+
+-- x.cue --
+package x
+
+import "list"
+
+x: [1, 2, 3]
+// This should be a fatal error, as the passed struct will never be
+// able to contain `less` by making the configuration more concrete.
+sortedx: list.Sort(x, {})
+
+-- expect-stdout1 --
+-- expect-stderr1 --
+error in call to list.Sort: field not found: less:
+    ./x.cue:8:10
+-- expect-stdout2 --
+import "list"
+
+x: [1, 2, 3]
+sortedx: list.Sort(x, {})
+-- expect-stderr2 --

cmd/cue/cmd/testdata/script/eval_notexistincomplete.txt

@@ -0,0 +1,33 @@
+#Issue: 1509
+
+# export
+! exec cue export
+cmp stderr expect-stderr1
+cmp stdout expect-stdout1
+
+# eval
+exec cue eval
+cmp stderr expect-stderr2
+cmp stdout expect-stdout2
+
+-- x.cue --
+package x
+
+import "list"
+
+x: [1, 2, 3]
+sortedx: list.Sort(x, cmp)
+
+cmp: {}
+
+-- expect-stdout1 --
+-- expect-stderr1 --
+error in call to list.Sort: field not found: less:
+    ./x.cue:6:10
+-- expect-stdout2 --
+import "list"
+
+x: [1, 2, 3]
+sortedx: list.Sort(x, cmp)
+cmp: {}
+-- expect-stderr2 --

cue/errors.go

@@ -91,8 +91,9 @@ func (e *valueError) Path() (a []string) {
 }
 
 var errNotExists = &adt.Bottom{
-	Code: adt.NotExistError,
-	Err:  errors.Newf(token.NoPos, "undefined value"),
+	Code:      adt.IncompleteError,
+	NotExists: true,
+	Err:       errors.Newf(token.NoPos, "undefined value"),
 }
 
 func mkErr(idx *runtime.Runtime, src adt.Node, args ...interface{}) *adt.Bottom {

cue/path_test.go

@@ -77,7 +77,7 @@ func TestPaths(t *testing.T) {
 	}, {
 		path: ParsePath("#Foo.a.c"),
 		str:  "#Foo.a.c",
-		out:  `_|_ // field "c" not found`,
+		out:  `_|_ // field not found: c`,
 	}, {
 		path: ParsePath(`b[2]`),
 		str:  `b[2]`,

cue/query.go

@@ -71,8 +71,11 @@ outer:
 		if err, ok := sel.sel.(pathError); ok {
 			x = &adt.Bottom{Err: err.Error}
 		} else {
-			// TODO: better message.
-			x = mkErr(v.idx, n, adt.NotExistError, "field %q not found", sel.sel)
+			x = mkErr(v.idx, n, adt.EvalError, "field not found: %v", sel.sel)
+			if n.Accept(ctx, f) {
+				x.Code = adt.IncompleteError
+			}
+			x.NotExists = true
 		}
 		v := makeValue(v.idx, n, parent)
 		return newErrValue(v, x)

cue/query_test.go

@@ -53,11 +53,11 @@ func TestLookupPath(t *testing.T) {
 	}, {
 		in:   `_foo: 3`,
 		path: cue.MakePath(cue.Def("_foo")),
-		err:  `field "#_foo" not found`,
+		err:  `field not found: #_foo`,
 	}, {
 		in:   `_#foo: 3`,
 		path: cue.MakePath(cue.Def("_#foo")),
-		err:  `field "_#foo" not found`,
+		err:  `field not found: _#foo`,
 	}, {
 		in:   `"foo", #foo: 3`,
 		path: cue.ParsePath("#foo"),
@@ -104,7 +104,7 @@ func TestLookupPath(t *testing.T) {
 		[Name=string]: { a: Name }
 		`,
 		path: cue.MakePath(cue.Str("a")),
-		err:  `field "a" not found`,
+		err:  `field not found: a`,
 	}}
 	for _, tc := range testCases {
 		t.Run(tc.path.String(), func(t *testing.T) {

cue/types.go

@@ -137,8 +137,15 @@ func (o *hiddenStructValue) Lookup(key string) Value {
 		}
 	}
 	if i == len {
-		// TODO: better message.
-		x := mkErr(o.v.idx, o.obj, adt.NotExistError, "value %q not found", key)
+		x := mkErr(o.v.idx, o.obj, 0, "field not found: %v", key)
+		x.NotExists = true
+		// TODO: more specifically we should test whether the values that
+		// are addressable from the root of the configuration can support the
+		// looked up value. This will avoid false positives such as when
+		// an open literal struct is passed to a builtin.
+		if o.obj.Accept(o.ctx, f) {
+			x.Code = adt.IncompleteError
+		}
 		return newErrValue(o.v, x)
 	}
 	return newChildValue(o, i)
@@ -1165,7 +1172,7 @@ func (v Value) Exists() bool {
 		return false
 	}
 	if err, ok := v.v.BaseValue.(*adt.Bottom); ok {
-		return err.Code != adt.NotExistError
+		return !err.NotExists
 	}
 	return true
 }

cue/types_test.go

@@ -1516,11 +1516,11 @@ func TestValue_LookupDef(t *testing.T) {
 	}, {
 		in:  `_foo: 3`,
 		def: "_foo",
-		out: `_|_ // field "#_foo" not found`,
+		out: `_|_ // field not found: #_foo`,
 	}, {
 		in:  `_#foo: 3`,
 		def: "_#foo",
-		out: `_|_ // field "_#foo" not found`,
+		out: `_|_ // field not found: _#foo`,
 	}, {
 		in:  `"foo", #foo: 3`,
 		def: "#foo",

internal/core/adt/errors.go

@@ -49,10 +49,6 @@ const (
 	// A UserError is a fatal error originating from the user.
 	UserError
 
-	// NotExistError is used to indicate a value does not exist.
-	// Mostly used for legacy reasons.
-	NotExistError
-
 	// StructuralCycleError means a structural cycle was found. Structural
 	// cycles are permanent errors, but they are not passed up recursively,
 	// as a unification of a value with a structural cycle with one that
@@ -96,6 +92,7 @@ type Bottom struct {
 	Code         ErrorCode
 	HasRecursive bool
 	ChildError   bool // Err is the error of the child
+	NotExists    bool // This error originated from a failed lookup.
 	// Value holds the computed value so far in case
 	Value Value
 }

internal/core/validate/validate.go

@@ -74,7 +74,7 @@ func (v *validator) validate(x *adt.Vertex) {
 				v.add(b)
 			}
 
-		case adt.IncompleteError, adt.NotExistError:
+		case adt.IncompleteError:
 			if v.checkConcrete() {
 				v.add(b)
 			}