From f063a61302a4d9df979ac75ec2a614cbe3d1be18 Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Thu, 8 Apr 2021 11:35:55 +0200 Subject: [PATCH] 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 Reviewed-by: Paul Jolly --- cue/path.go | 2 +- cue/path_test.go | 2 +- cue/query.go | 2 +- cue/query_test.go | 44 ++++++++++++++++++++++++++++++-------------- cue/types.go | 35 +++++++---------------------------- cue/types_test.go | 4 ++-- 6 files changed, 42 insertions(+), 47 deletions(-) diff --git a/cue/path.go b/cue/path.go index 4ffb2dbef..4f57056d3 100644 --- a/cue/path.go +++ b/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) { diff --git a/cue/path_test.go b/cue/path_test.go index b1264384d..334bb3950 100644 --- a/cue/path_test.go +++ b/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]`, diff --git a/cue/query.go b/cue/query.go index 542a9af5c..41ee6b967 100644 --- a/cue/query.go +++ b/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) diff --git a/cue/query_test.go b/cue/query_test.go index 5301be2d3..5f147d60d 100644 --- a/cue/query_test.go +++ b/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,8 +99,8 @@ 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) { @@ -98,8 +108,14 @@ func TestLookupPath(t *testing.T) { 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 } diff --git a/cue/types.go b/cue/types.go index bfcc02c15..2038a70c8 100644 --- a/cue/types.go +++ b/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,35 +1500,11 @@ 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") @@ -1535,6 +1512,8 @@ 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 { diff --git a/cue/types_test.go b/cue/types_test.go index 51d84a2e7..afb38486d 100644 --- a/cue/types_test.go +++ b/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",