cue: deprecate Value.FieldByName

Can be replaced by LookupPath.

Also
- make Def behave like LookupDef
- update some other deprecation comments
- copy over tests

Change-Id: If7fa60fda28c9fea1b5328a41d79f52fafde394b
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/9349
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>

cue/path.go

@@ -334,7 +334,7 @@ func (s scopedSelector) feature(r adt.Runtime) adt.Feature {
 // not prefixed with a #. It will panic if s cannot be written as a valid
 // identifier.
 func Def(s string) Selector {
-	if !strings.HasPrefix(s, "#") {
+	if !strings.HasPrefix(s, "#") && !strings.HasPrefix(s, "_#") {
 		s = "#" + s
 	}
 	if !ast.IsValidIdent(s) {

cue/path_test.go

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

cue/query.go

@@ -88,7 +88,7 @@ outer:
 			x = &adt.Bottom{Err: err.Error}
 		} else {
 			// TODO: better message.
-			x = v.idx.mkErr(n, adt.NotExistError, "value %q not found", sel.sel)
+			x = v.idx.mkErr(n, adt.NotExistError, "field %q not found", sel.sel)
 		}
 		v := makeValue(v.idx, n)
 		return newErrValue(v, x)

cue/query_test.go

@@ -26,17 +26,11 @@ func TestLookupPath(t *testing.T) {
 	r := &cue.Runtime{}
 
 	testCases := []struct {
-		in       string
-		path     cue.Path
-		out      string `test:"update"` // :nerdSnipe:
-		notExist bool   `test:"update"` // :nerdSnipe:
+		in   string
+		path cue.Path
+		out  string `test:"update"` // :nerdSnipe:
+		err  string `test:"update"` // :nerdSnipe:
 	}{{
-		in: `
-		[Name=string]: { a: Name }
-		`,
-		path:     cue.MakePath(cue.Str("a")),
-		notExist: true,
-	}, {
 		in: `
 		#V: {
 			x: int
@@ -48,6 +42,22 @@ func TestLookupPath(t *testing.T) {
 		`,
 		path: cue.ParsePath("v.x"),
 		out:  `int64`,
+	}, {
+		in:   `#foo: 3`,
+		path: cue.ParsePath("#foo"),
+		out:  `3`,
+	}, {
+		in:   `_foo: 3`,
+		path: cue.MakePath(cue.Def("_foo")),
+		err:  `field "#_foo" not found`,
+	}, {
+		in:   `_#foo: 3`,
+		path: cue.MakePath(cue.Def("_#foo")),
+		err:  `field "_#foo" not found`,
+	}, {
+		in:   `"foo", #foo: 3`,
+		path: cue.ParsePath("#foo"),
+		out:  `3`,
 	}, {
 		in: `
 		a: [...int]
@@ -89,17 +99,23 @@ func TestLookupPath(t *testing.T) {
 		in: `
 		[Name=string]: { a: Name }
 		`,
-		path:     cue.MakePath(cue.Str("a")),
-		notExist: true,
+		path: cue.MakePath(cue.Str("a")),
+		err:  `field "a" not found`,
 	}}
 	for _, tc := range testCases {
 		t.Run(tc.path.String(), func(t *testing.T) {
 			v := compileT(t, r, tc.in)
 
 			v = v.LookupPath(tc.path)
 
-			if exists := v.Exists(); exists != !tc.notExist {
-				t.Fatalf("exists: got %v; want: %v", exists, !tc.notExist)
+			if err := v.Err(); err != nil || tc.err != "" {
+				if got := err.Error(); got != tc.err {
+					t.Errorf("error: got %v; want %v", got, tc.err)
+				}
+			}
+
+			if exists := v.Exists(); exists != (tc.err == "") {
+				t.Fatalf("exists: got %v; want: %v", exists, tc.err == "")
 			} else if !exists {
 				return
 			}

cue/types.go

@@ -1362,6 +1362,7 @@ func (v Value) structValOpts(ctx *context, o options) (s structValue, err *adt.B
 // Struct returns the underlying struct of a value or an error if the value
 // is not a struct.
 func (v Value) Struct() (*Struct, error) {
+	// TODO: deprecate
 	ctx := v.ctx()
 	obj, err := v.structValOpts(ctx, options{})
 	if err != nil {
@@ -1451,7 +1452,7 @@ func (v Value) Fields(opts ...Option) (*Iterator, error) {
 }
 
 // Lookup reports the value at a path starting from v. The empty path returns v
-// itself. Use LookupDef for definitions or LookupField for any kind of field.
+// itself.
 //
 // The Exists() method can be used to verify if the returned value existed.
 // Lookup cannot be used to look up hidden or optional fields or definitions.
@@ -1499,42 +1500,20 @@ func (v Value) Path() Path {
 	return Path{path: a}
 }
 
-// LookupDef reports the definition with the given name within struct v. The
-// Exists method of the returned value will report false if the definition did
-// not exist. The Err method reports if any error occurred during evaluation.
+// LookupDef is equal to LookupPath(MakePath(Def(name))).
 //
 // Deprecated: use LookupPath.
 func (v Value) LookupDef(name string) Value {
-	ctx := v.ctx()
-	o, err := v.structValFull(ctx)
-	if err != nil {
-		return newErrValue(v, err)
-	}
-
-	f := v.ctx().Label(name, true)
-	for i, a := range o.features {
-		if a == f {
-			if f.IsHidden() || !f.IsDef() { // optional not possible for now
-				break
-			}
-			return newChildValue(&o, i)
-		}
-	}
-	if !strings.HasPrefix(name, "#") {
-		alt := v.LookupDef("#" + name)
-		// Use the original error message if this resulted in an error as well.
-		if alt.Err() == nil {
-			return alt
-		}
-	}
-	return newErrValue(v, ctx.mkErr(v.v, "definition %q not found", name))
+	return v.LookupPath(MakePath(Def(name)))
 }
 
 var errNotFound = errors.Newf(token.NoPos, "field not found")
 
 // FieldByName looks up a field for the given name. If isIdent is true, it will
 // look up a definition or hidden field (starting with `_` or `_#`). Otherwise
 // it interprets name as an arbitrary string for a regular field.
+//
+// Deprecated: use LookupPath.
 func (v Value) FieldByName(name string, isIdent bool) (f FieldInfo, err error) {
 	s, err := v.Struct()
 	if err != nil {
@@ -1545,7 +1524,7 @@ func (v Value) FieldByName(name string, isIdent bool) (f FieldInfo, err error) {
 
 // LookupField reports information about a field of v.
 //
-// Deprecated: this API does not work with new-style definitions. Use FieldByName.
+// Deprecated: use LookupPath
 func (v Value) LookupField(name string) (FieldInfo, error) {
 	s, err := v.Struct()
 	if err != nil {

cue/types_test.go

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