cue/ast: move WalkVisitor and Visitor to astutil and unexport them

https://cuelang.org/cl/1194006 added ast.WalkVisitor and ast.Visitor
so that the cue/ast and cue/ast/astutil packages could share the code
rather than duplicating it.

Roger correctly pointed out that ast.Walk is equally as powerful,
one just needs to keep enough state somewhere like a method receiver.
The new ast.stackVisitor type just added does exactly that.

We can now move WalkVisitor and stackVisitor into cue/ast/astutil
and unexport them, as they are the only users across the repository.
The APIs were just exposed in master and not included in any release.

Later on I would like to remove the needed for his walkVisitor layer,
instead having the Sanitize and Resolve APIs keep their own state
on top of the ast.Walk API. However, that work is not urgent,
and would very easily be non-trivial, so leave a TODO for now.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: Ic35b997645da193aae42bd0489d5f5b1a0cdd071
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194084
Reviewed-by: Roger Peppe <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>

Author: mvdan

cue/ast/astutil/resolve.go

@@ -75,14 +75,14 @@ type ErrFunc func(pos token.Pos, msg string, args ...interface{})
 // Resolve resolves all identifiers in a file. Unresolved identifiers are
 // recorded in Unresolved. It will not overwrite already resolved values.
 func Resolve(f *ast.File, errFn ErrFunc) {
-	ast.WalkVisitor(f, &scope{errFn: errFn, identFn: resolveIdent})
+	walkVisitor(f, &scope{errFn: errFn, identFn: resolveIdent})
 }
 
 // Resolve resolves all identifiers in an expression.
 // It will not overwrite already resolved values.
 func ResolveExpr(e ast.Expr, errFn ErrFunc) {
 	f := &ast.File{}
-	ast.WalkVisitor(e, &scope{file: f, errFn: errFn, identFn: resolveIdent})
+	walkVisitor(e, &scope{file: f, errFn: errFn, identFn: resolveIdent})
 }
 
 // A Scope maintains the set of named language entities declared
@@ -250,13 +250,13 @@ func (s *scope) lookup(name string) (p *scope, obj ast.Node, node entry) {
 }
 
 func (s *scope) After(n ast.Node) {}
-func (s *scope) Before(n ast.Node) (w ast.Visitor) {
+func (s *scope) Before(n ast.Node) (w visitor) {
 	switch x := n.(type) {
 	case *ast.File:
 		s := newScope(x, s, x, x.Decls)
 		// Support imports.
 		for _, d := range x.Decls {
-			ast.WalkVisitor(d, s)
+			walkVisitor(d, s)
 		}
 		return nil
 
@@ -265,7 +265,7 @@ func (s *scope) Before(n ast.Node) (w ast.Visitor) {
 
 	case *ast.Comprehension:
 		s = scopeClauses(s, x.Clauses)
-		ast.WalkVisitor(x.Value, s)
+		walkVisitor(x.Value, s)
 		return nil
 
 	case *ast.Field:
@@ -277,10 +277,10 @@ func (s *scope) Before(n ast.Node) (w ast.Visitor) {
 
 		switch label := n.(type) {
 		case *ast.ParenExpr:
-			ast.WalkVisitor(label, s)
+			walkVisitor(label, s)
 
 		case *ast.Interpolation:
-			ast.WalkVisitor(label, s)
+			walkVisitor(label, s)
 
 		case *ast.ListLit:
 			if len(label.Elts) != 1 {
@@ -317,7 +317,7 @@ func (s *scope) Before(n ast.Node) (w ast.Visitor) {
 					}
 				}
 			})
-			ast.WalkVisitor(expr, s)
+			walkVisitor(expr, s)
 		}
 
 		if n := x.Value; n != nil {
@@ -329,7 +329,7 @@ func (s *scope) Before(n ast.Node) (w ast.Visitor) {
 				n = alias.Expr
 			}
 			s.inField = true
-			ast.WalkVisitor(n, s)
+			walkVisitor(n, s)
 			s.inField = false
 		}
 
@@ -342,7 +342,7 @@ func (s *scope) Before(n ast.Node) (w ast.Visitor) {
 		delete(s.index, name) // The same name may still appear in another scope
 
 		if x.Expr != nil {
-			ast.WalkVisitor(x.Expr, s)
+			walkVisitor(x.Expr, s)
 		}
 		s.index[name] = saved
 		return nil
@@ -354,7 +354,7 @@ func (s *scope) Before(n ast.Node) (w ast.Visitor) {
 		delete(s.index, name) // The same name may still appear in another scope
 
 		if x.Expr != nil {
-			ast.WalkVisitor(x.Expr, s)
+			walkVisitor(x.Expr, s)
 		}
 		s.index[name] = saved
 		return nil
@@ -367,7 +367,7 @@ func (s *scope) Before(n ast.Node) (w ast.Visitor) {
 		// that resolve in a list.
 
 	case *ast.SelectorExpr:
-		ast.WalkVisitor(x.X, s)
+		walkVisitor(x.X, s)
 		return nil
 
 	case *ast.Ident:
@@ -410,20 +410,20 @@ func scopeClauses(s *scope, clauses []ast.Clause) *scope {
 	for _, c := range clauses {
 		switch x := c.(type) {
 		case *ast.ForClause:
-			ast.WalkVisitor(x.Source, s)
+			walkVisitor(x.Source, s)
 			s = newScope(s.file, s, x, nil)
 			if x.Key != nil {
 				s.insert(x.Key.Name, x.Key, x)
 			}
 			s.insert(x.Value.Name, x.Value, x)
 
 		case *ast.LetClause:
-			ast.WalkVisitor(x.Expr, s)
+			walkVisitor(x.Expr, s)
 			s = newScope(s.file, s, x, nil)
 			s.insert(x.Ident.Name, x.Ident, x)
 
 		default:
-			ast.WalkVisitor(c, s)
+			walkVisitor(c, s)
 		}
 	}
 	return s

cue/ast/astutil/sanitize.go

@@ -50,7 +50,7 @@ func Sanitize(f *ast.File) error {
 	}
 
 	// Gather all names.
-	ast.WalkVisitor(f, &scope{
+	walkVisitor(f, &scope{
 		errFn:   z.errf,
 		nameFn:  z.addName,
 		identFn: z.markUsed,
@@ -67,7 +67,7 @@ func Sanitize(f *ast.File) error {
 		index:   make(map[string]entry),
 	}
 	z.fileScope = s
-	ast.WalkVisitor(f, s)
+	walkVisitor(f, s)
 	if z.errs != nil {
 		return z.errs
 	}

cue/ast/astutil/walk.go

@@ -0,0 +1,54 @@
+// Copyright 2018 The 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
+//
+//     http://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 astutil
+
+import "cuelang.org/go/cue/ast"
+
+// walkVisitor traverses an AST in depth-first order with a [visitor].
+//
+// TODO(mvdan): refactor away the need for walkVisitor;
+// Resolve and Sanitize should be able to use ast.Walk directly.
+func walkVisitor(node ast.Node, v visitor) {
+	sv := &stackVisitor{stack: []visitor{v}}
+	ast.Walk(node, sv.Before, sv.After)
+}
+
+// stackVisitor helps implement visitor support on top of ast.Walk.
+type stackVisitor struct {
+	stack []visitor
+}
+
+func (v *stackVisitor) Before(node ast.Node) bool {
+	current := v.stack[len(v.stack)-1]
+	next := current.Before(node)
+	if next == nil {
+		return false
+	}
+	v.stack = append(v.stack, next)
+	return true
+}
+
+func (v *stackVisitor) After(node ast.Node) {
+	v.stack[len(v.stack)-1] = nil // set visitor to nil so it can be garbage collected
+	v.stack = v.stack[:len(v.stack)-1]
+}
+
+// A visitor's before method is invoked for each node encountered by Walk.
+// If the result visitor w is true, Walk visits each of the children
+// of node with the visitor w, followed by a call of w.After.
+type visitor interface {
+	Before(node ast.Node) (w visitor)
+	After(node ast.Node)
+}

cue/ast/walk.go

@@ -18,40 +18,6 @@ import (
 	"fmt"
 )
 
-// WalkVisitor traverses an AST in depth-first order with a [Visitor].
-func WalkVisitor(node Node, visitor Visitor) {
-	v := &stackVisitor{stack: []Visitor{visitor}}
-	Walk(node, v.Before, v.After)
-}
-
-// stackVisitor helps implement Visitor support on top of Walk.
-type stackVisitor struct {
-	stack []Visitor
-}
-
-func (v *stackVisitor) Before(node Node) bool {
-	current := v.stack[len(v.stack)-1]
-	next := current.Before(node)
-	if next == nil {
-		return false
-	}
-	v.stack = append(v.stack, next)
-	return true
-}
-
-func (v *stackVisitor) After(node Node) {
-	v.stack[len(v.stack)-1] = nil // set visitor to nil so it can be garbage collected
-	v.stack = v.stack[:len(v.stack)-1]
-}
-
-// A Visitor's before method is invoked for each node encountered by Walk.
-// If the result Visitor w is true, Walk visits each of the children
-// of node with the Visitor w, followed by a call of w.After.
-type Visitor interface {
-	Before(node Node) (w Visitor)
-	After(node Node)
-}
-
 func walkList[N Node](list []N, before func(Node) bool, after func(Node)) {
 	for _, node := range list {
 		Walk(node, before, after)