Skip to content

Commit

Permalink
Node traversal(->lookup) method renames.
Browse files Browse the repository at this point in the history
Most important things first!  To follow this refactor:

```
sed s/TraverseField/LookupString/g
sed s/TraverseIndex/LookupIndex/g
```

It is *literally* a sed-refactor in complexity.

---

Now, details.

This has been pending for a while, and there is some discussion in
#22 .

In short, "Traversal" seemed like a mouthful;
"Field" was always a misnomer here;
and we've discovered several other methods that we *should* have
in the area as well, which necessitated a thought about placement.

In this commit, only the renames are applied, but you can also see
the outlines of two new methods in the Node interface, as comments.
These will be added in future commits.

Signed-off-by: Eric Myhre <[email protected]>
  • Loading branch information
warpfork committed Aug 12, 2019
1 parent 256c605 commit 2e3868c
Show file tree
Hide file tree
Showing 28 changed files with 120 additions and 105 deletions.
2 changes: 1 addition & 1 deletion doc/nodes.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ kind of data this node contains in terms of the IPLD Data Model.

The validity of many other methods can be anticipated by switching on the kind:
for example, `AsString` is definitely going to error if `ReprKind() == ipld.ReprKind_Map`,
and `TraverseField` is definitely going to error if `ReprKind() == ipld.ReprKind_String`.
and `LookupString` is definitely going to error if `ReprKind() == ipld.ReprKind_String`.

See the [godocs for ReprKind](https://godoc.org/github.com/ipld/go-ipld-prime#ReprKind).

Expand Down
2 changes: 1 addition & 1 deletion encoding/dagcbor/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func Marshal(n ipld.Node, sink shared.TokenSink) error {
}
// Emit list contents (and recurse).
for i := 0; i < l; i++ {
v, err := n.TraverseIndex(i)
v, err := n.LookupIndex(i)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion encoding/dagjson/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func Marshal(n ipld.Node, sink shared.TokenSink) error {
}
// Emit list contents (and recurse).
for i := 0; i < l; i++ {
v, err := n.TraverseIndex(i)
v, err := n.LookupIndex(i)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion encoding/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func Marshal(n ipld.Node, sink shared.TokenSink) error {
}
// Emit list contents (and recurse).
for i := 0; i < l; i++ {
v, err := n.TraverseIndex(i)
v, err := n.LookupIndex(i)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
// that node concretely contains.
//
// For example, calling AsString on a map will return ErrWrongKind.
// Calling TraverseField on an int will similarly return ErrWrongKind.
// Calling LookupString on an int will similarly return ErrWrongKind.
type ErrWrongKind struct {
// CONSIDER: if we should add a `TypeName string` here as well?
// It seems to be useful information, and in many places we've shoved it
Expand Down
12 changes: 6 additions & 6 deletions fluent/fluentNode.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import (
// (The fluent.Recover function can be used to nicely gather these panics.)
type Node interface {
ReprKind() ipld.ReprKind
TraverseField(path string) Node
TraverseIndex(idx int) Node
LookupString(path string) Node
LookupIndex(idx int) Node
MapIterator() MapIterator
ListIterator() ListIterator
Length() int
Expand Down Expand Up @@ -56,21 +56,21 @@ func (n node) ReprKind() ipld.ReprKind {
}
return n.n.ReprKind()
}
func (n node) TraverseField(path string) Node {
func (n node) LookupString(path string) Node {
if n.err != nil {
return n
}
v, err := n.n.TraverseField(path)
v, err := n.n.LookupString(path)
if err != nil {
return node{nil, err}
}
return node{v, nil}
}
func (n node) TraverseIndex(idx int) Node {
func (n node) LookupIndex(idx int) Node {
if n.err != nil {
return n
}
v, err := n.n.TraverseIndex(idx)
v, err := n.n.LookupIndex(idx)
if err != nil {
return node{nil, err}
}
Expand Down
4 changes: 2 additions & 2 deletions fluent/fluentRecover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ func TestRecover(t *testing.T) {
t.Run("simple traversal error should capture", func(t *testing.T) {
Wish(t,
Recover(func() {
WrapNode(&ipldfree.Node{}).TraverseIndex(0).AsString()
WrapNode(&ipldfree.Node{}).LookupIndex(0).AsString()
t.Fatal("should not be reached")
}),
ShouldEqual,
Error{ipld.ErrWrongKind{MethodName: "TraverseIndex", AppropriateKind: ipld.ReprKindSet_JustList, ActualKind: ipld.ReprKind_Invalid}},
Error{ipld.ErrWrongKind{MethodName: "LookupIndex", AppropriateKind: ipld.ReprKindSet_JustList, ActualKind: ipld.ReprKind_Invalid}},
)
})
t.Run("correct traversal should return nil", func(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions impl/bind/boundNode.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (n Node) Length() int {
panic("NYI")
}

func (n Node) TraverseField(pth string) (ipld.Node, error) {
func (n Node) LookupString(pth string) (ipld.Node, error) {
v := n.bound

// Traverse.
Expand Down Expand Up @@ -214,6 +214,6 @@ func (n Node) TraverseField(pth string) (ipld.Node, error) {
}
}

func (n Node) TraverseIndex(idx int) (ipld.Node, error) {
func (n Node) LookupIndex(idx int) (ipld.Node, error) {
panic("NYI")
}
8 changes: 4 additions & 4 deletions impl/free/freeNode.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func (n *Node) Length() int {
}
}

func (n *Node) TraverseField(pth string) (ipld.Node, error) {
func (n *Node) LookupString(pth string) (ipld.Node, error) {
switch n.kind {
case ipld.ReprKind_Map:
v, exists := n._map[pth]
Expand All @@ -178,13 +178,13 @@ func (n *Node) TraverseField(pth string) (ipld.Node, error) {
ipld.ReprKind_Int,
ipld.ReprKind_Float,
ipld.ReprKind_Link:
return nil, ipld.ErrWrongKind{MethodName: "TraverseField", AppropriateKind: ipld.ReprKindSet_JustMap, ActualKind: n.kind}
return nil, ipld.ErrWrongKind{MethodName: "LookupString", AppropriateKind: ipld.ReprKindSet_JustMap, ActualKind: n.kind}
default:
panic("unreachable")
}
}

func (n *Node) TraverseIndex(idx int) (ipld.Node, error) {
func (n *Node) LookupIndex(idx int) (ipld.Node, error) {
switch n.kind {
case ipld.ReprKind_List:
if idx >= len(n._arr) {
Expand All @@ -203,7 +203,7 @@ func (n *Node) TraverseIndex(idx int) (ipld.Node, error) {
ipld.ReprKind_Int,
ipld.ReprKind_Float,
ipld.ReprKind_Link:
return nil, ipld.ErrWrongKind{MethodName: "TraverseIndex", AppropriateKind: ipld.ReprKindSet_JustList, ActualKind: n.kind}
return nil, ipld.ErrWrongKind{MethodName: "LookupIndex", AppropriateKind: ipld.ReprKindSet_JustList, ActualKind: n.kind}
default:
panic("unreachable")
}
Expand Down
8 changes: 4 additions & 4 deletions impl/free/justString.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ type justString string
func (justString) ReprKind() ipld.ReprKind {
return ipld.ReprKind_String
}
func (justString) TraverseField(string) (ipld.Node, error) {
return nil, ipld.ErrWrongKind{MethodName: "TraverseField", AppropriateKind: ipld.ReprKindSet_JustMap, ActualKind: ipld.ReprKind_String}
func (justString) LookupString(string) (ipld.Node, error) {
return nil, ipld.ErrWrongKind{MethodName: "LookupString", AppropriateKind: ipld.ReprKindSet_JustMap, ActualKind: ipld.ReprKind_String}
}
func (justString) TraverseIndex(idx int) (ipld.Node, error) {
return nil, ipld.ErrWrongKind{MethodName: "TraverseIndex", AppropriateKind: ipld.ReprKindSet_JustList, ActualKind: ipld.ReprKind_String}
func (justString) LookupIndex(idx int) (ipld.Node, error) {
return nil, ipld.ErrWrongKind{MethodName: "LookupIndex", AppropriateKind: ipld.ReprKindSet_JustList, ActualKind: ipld.ReprKind_String}
}
func (justString) MapIterator() ipld.MapIterator {
return nodeutil.MapIteratorErrorThunk(ipld.ErrWrongKind{MethodName: "MapIterator", AppropriateKind: ipld.ReprKindSet_JustMap, ActualKind: ipld.ReprKind_String})
Expand Down
6 changes: 3 additions & 3 deletions impl/typed/wrapStruct.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ func (tn wrapnodeStruct) Type() schema.Type {
return tn.typ
}

func (tn wrapnodeStruct) TraverseField(key string) (ipld.Node, error) {
func (tn wrapnodeStruct) LookupString(key string) (ipld.Node, error) {
for _, field := range tn.typ.Fields() {
if field.Name() != key {
continue
}
v, e1 := tn.Node.TraverseField(key)
v, e1 := tn.Node.LookupString(key)
if e1 == nil {
return v, nil // null or set both flow through here
}
Expand All @@ -60,7 +60,7 @@ func (itr *wrapnodeStruct_Iterator) Next() (k ipld.Node, v ipld.Node, _ error) {
}
field := itr.node.typ.Fields()[itr.idx]
k = ipldfree.String(field.Name())
v, e1 := itr.node.TraverseField(field.Name())
v, e1 := itr.node.LookupString(field.Name())
if e1 != nil {
if _, ok := e1.(ipld.ErrNotExists); ok {
v = ipld.Undef // we assume the type allows this, or this node shouldn't have been possible to construct in the first place
Expand Down
25 changes: 20 additions & 5 deletions node.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,40 @@ type Node interface {
// Most other handling of a node requires first switching upon the kind.
ReprKind() ReprKind

// TraverseField looks up a child object in this node and returns it.
// LookupString looks up a child object in this node and returns it.
// The returned Node may be any of the ReprKind:
// a primitive (string, int, etc), a map, a list, or a link.
//
// If the Kind of this Node is not ReprKind_Map, a nil node and an error
// will be returned.
//
// If the key does not exist, a nil node and an error will be returned.
TraverseField(key string) (Node, error)
LookupString(key string) (Node, error)

// TraverseIndex is the equivalent of TraverseField but for indexing into a list.
// As with TraverseField, the returned Node may be any of the ReprKind:
// Lookup is the equivalent of LookupString, but takes a reified Node
// as a parameter instead of a plain string.
// This mechanism is useful if working with typed maps (if the key types
// have constraints, and you already have a reified `typed.Node` value,
// using that value can save parsing and validation costs);
// and may simply be convenient if you already have a Node value in hand.
/// Lookup(key Node) (Node, error)

// LookupIndex is the equivalent of LookupString but for indexing into a list.
// As with LookupString, the returned Node may be any of the ReprKind:
// a primitive (string, int, etc), a map, a list, or a link.
//
// If the Kind of this Node is not ReprKind_List, a nil node and an error
// will be returned.
//
// If idx is out of range, a nil node and an error will be returned.
TraverseIndex(idx int) (Node, error)
LookupIndex(idx int) (Node, error)

// LookupSegment is a convenience method that might exist... if we moved
// the PathSegment type up here to the main package!
/// LookupSegment(seg PathSegment) (Node, error)

// Note that when using codegenerated types, there may be a fifth variant
// of lookup method on maps: `Get($GeneratedTypeKey) $GeneratedTypeValue`!

// MapIterator returns an iterator which yields key-value pairs
// traversing the node.
Expand Down
4 changes: 2 additions & 2 deletions schema/gen/go/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ type typeGenerator interface {

EmitNodeType(io.Writer)
EmitNodeMethodReprKind(io.Writer)
EmitNodeMethodTraverseField(io.Writer)
EmitNodeMethodTraverseIndex(io.Writer)
EmitNodeMethodLookupString(io.Writer)
EmitNodeMethodLookupIndex(io.Writer)
EmitNodeMethodMapIterator(io.Writer)
EmitNodeMethodListIterator(io.Writer)
EmitNodeMethodLength(io.Writer)
Expand Down
24 changes: 12 additions & 12 deletions schema/gen/go/genCommon.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,18 @@ import (

type generateKindedRejections struct{}

func (generateKindedRejections) emitNodeMethodTraverseField(w io.Writer, t schema.Type) {
func (generateKindedRejections) emitNodeMethodLookupString(w io.Writer, t schema.Type) {
doTemplate(`
func ({{ .Name }}) TraverseField(string) (ipld.Node, error) {
return nil, ipld.ErrWrongKind{MethodName: "{{ .Name }}.TraverseField", AppropriateKind: ipld.ReprKindSet_JustMap, ActualKind: {{ .Kind.ActsLike | ReprKindConst }}}
func ({{ .Name }}) LookupString(string) (ipld.Node, error) {
return nil, ipld.ErrWrongKind{MethodName: "{{ .Name }}.LookupString", AppropriateKind: ipld.ReprKindSet_JustMap, ActualKind: {{ .Kind.ActsLike | ReprKindConst }}}
}
`, w, t)
}

func (generateKindedRejections) emitNodeMethodTraverseIndex(w io.Writer, t schema.Type) {
func (generateKindedRejections) emitNodeMethodLookupIndex(w io.Writer, t schema.Type) {
doTemplate(`
func ({{ .Name }}) TraverseIndex(idx int) (ipld.Node, error) {
return nil, ipld.ErrWrongKind{MethodName: "{{ .Name }}.TraverseIndex", AppropriateKind: ipld.ReprKindSet_JustList, ActualKind: {{ .Kind.ActsLike | ReprKindConst }}}
func ({{ .Name }}) LookupIndex(idx int) (ipld.Node, error) {
return nil, ipld.ErrWrongKind{MethodName: "{{ .Name }}.LookupIndex", AppropriateKind: ipld.ReprKindSet_JustList, ActualKind: {{ .Kind.ActsLike | ReprKindConst }}}
}
`, w, t)
}
Expand Down Expand Up @@ -117,11 +117,11 @@ type generateKindedRejections_String struct {
Type schema.Type // used so we can generate error messages with the type name.
}

func (gk generateKindedRejections_String) EmitNodeMethodTraverseField(w io.Writer) {
generateKindedRejections{}.emitNodeMethodTraverseField(w, gk.Type)
func (gk generateKindedRejections_String) EmitNodeMethodLookupString(w io.Writer) {
generateKindedRejections{}.emitNodeMethodLookupString(w, gk.Type)
}
func (gk generateKindedRejections_String) EmitNodeMethodTraverseIndex(w io.Writer) {
generateKindedRejections{}.emitNodeMethodTraverseIndex(w, gk.Type)
func (gk generateKindedRejections_String) EmitNodeMethodLookupIndex(w io.Writer) {
generateKindedRejections{}.emitNodeMethodLookupIndex(w, gk.Type)
}
func (gk generateKindedRejections_String) EmitNodeMethodMapIterator(w io.Writer) {
generateKindedRejections{}.emitNodeMethodMapIterator(w, gk.Type)
Expand Down Expand Up @@ -161,8 +161,8 @@ type generateKindedRejections_Map struct {
Type schema.Type // used so we can generate error messages with the type name.
}

func (gk generateKindedRejections_Map) EmitNodeMethodTraverseIndex(w io.Writer) {
generateKindedRejections{}.emitNodeMethodTraverseIndex(w, gk.Type)
func (gk generateKindedRejections_Map) EmitNodeMethodLookupIndex(w io.Writer) {
generateKindedRejections{}.emitNodeMethodLookupIndex(w, gk.Type)
}
func (gk generateKindedRejections_Map) EmitNodeMethodListIterator(w io.Writer) {
generateKindedRejections{}.emitNodeMethodListIterator(w, gk.Type)
Expand Down
4 changes: 2 additions & 2 deletions schema/gen/go/genKindStruct.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ func (gk generateKindStruct) EmitNodeMethodReprKind(w io.Writer) {
`, w, gk)
}

func (gk generateKindStruct) EmitNodeMethodTraverseField(w io.Writer) {
func (gk generateKindStruct) EmitNodeMethodLookupString(w io.Writer) {
doTemplate(`
func (x {{ .Type.Name }}) TraverseField(key string) (ipld.Node, error) {
func (x {{ .Type.Name }}) LookupString(key string) (ipld.Node, error) {
switch key {
{{- range $field := .Type.Fields }}
case "{{ $field.Name }}":
Expand Down
4 changes: 2 additions & 2 deletions schema/gen/go/gen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ func TestNuevo(t *testing.T) {
emitType := func(tg typeGenerator, w io.Writer) {
tg.EmitNodeType(w)
tg.EmitNodeMethodReprKind(w)
tg.EmitNodeMethodTraverseField(w)
tg.EmitNodeMethodTraverseIndex(w)
tg.EmitNodeMethodLookupString(w)
tg.EmitNodeMethodLookupIndex(w)
tg.EmitNodeMethodMapIterator(w)
tg.EmitNodeMethodListIterator(w)
tg.EmitNodeMethodLength(w)
Expand Down
2 changes: 1 addition & 1 deletion schema/tests/testsForStructs.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestFoo(t *testing.T, newNb func(typ schema.Type) ipld.NodeBuilder) {
})
t.Run("reading", func(t *testing.T) {
t.Run("regular fields", func(t *testing.T) {
v, err := n1.TraverseField("f1")
v, err := n1.LookupString("f1")
Wish(t, err, ShouldEqual, nil)
v2, _ := nbString.CreateString("asdf")
Wish(t, v, ShouldEqual, v2)
Expand Down
10 changes: 5 additions & 5 deletions tests/building.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestBuildingRecursives(t *testing.T, nb ipld.NodeBuilder) {
n := nb.CreateList(func(lb fluent.ListBuilder, vnb fluent.NodeBuilder) {
lb.Append(vnb.CreateString("asdf"))
})
Wish(t, fluent.WrapNode(n).TraverseIndex(0).AsString(), ShouldEqual, "asdf")
Wish(t, fluent.WrapNode(n).LookupIndex(0).AsString(), ShouldEqual, "asdf")
})
t.Run("nested list node", func(t *testing.T) {
nb := fluent.WrapNodeBuilder(nb)
Expand All @@ -65,8 +65,8 @@ func TestBuildingRecursives(t *testing.T, nb ipld.NodeBuilder) {
})
nf := fluent.WrapNode(n)
Wish(t, nf.ReprKind(), ShouldEqual, ipld.ReprKind_List)
Wish(t, nf.TraverseIndex(0).TraverseIndex(0).AsString(), ShouldEqual, "asdf")
Wish(t, nf.TraverseIndex(1).AsString(), ShouldEqual, "quux")
Wish(t, nf.LookupIndex(0).LookupIndex(0).AsString(), ShouldEqual, "asdf")
Wish(t, nf.LookupIndex(1).AsString(), ShouldEqual, "quux")
})
t.Run("long list node", func(t *testing.T) {
nb := fluent.WrapNodeBuilder(nb)
Expand All @@ -77,8 +77,8 @@ func TestBuildingRecursives(t *testing.T, nb ipld.NodeBuilder) {
nf := fluent.WrapNode(n)
Wish(t, nf.ReprKind(), ShouldEqual, ipld.ReprKind_List)
Wish(t, nf.Length(), ShouldEqual, 20)
Wish(t, nf.TraverseIndex(9).AsString(), ShouldEqual, "quux")
Wish(t, nf.TraverseIndex(19).AsString(), ShouldEqual, "quuux")
Wish(t, nf.LookupIndex(9).AsString(), ShouldEqual, "quux")
Wish(t, nf.LookupIndex(19).AsString(), ShouldEqual, "quuux")
})

// todo map tests
Expand Down
Loading

0 comments on commit 2e3868c

Please sign in to comment.