From b9fbbb8e31c4009aa2d3a848240c61625a0a0bdc Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 1 Jun 2015 15:21:03 -0400 Subject: [PATCH] refactor/eg: refactor to break dependency on go/loader Change-Id: I0a865a694b911437944743f60bfaa2a937d85c9a Reviewed-on: https://go-review.googlesource.com/14133 Reviewed-by: David Crawshaw --- cmd/eg/eg.go | 2 +- refactor/eg/eg.go | 40 +++++++++++++++------------------------- refactor/eg/eg_test.go | 2 +- refactor/eg/match.go | 7 +++---- refactor/eg/rewrite.go | 6 +++--- 5 files changed, 23 insertions(+), 34 deletions(-) diff --git a/cmd/eg/eg.go b/cmd/eg/eg.go index 5970c1ac89..c3cee7df31 100644 --- a/cmd/eg/eg.go +++ b/cmd/eg/eg.go @@ -90,7 +90,7 @@ func doMain() error { // Analyze the template. template := iprog.Created[0] - xform, err := eg.NewTransformer(iprog.Fset, template, *verboseFlag) + xform, err := eg.NewTransformer(iprog.Fset, template.Pkg, template.Files[0], &template.Info, *verboseFlag) if err != nil { return err } diff --git a/refactor/eg/eg.go b/refactor/eg/eg.go index a609255568..12f9b0d71a 100644 --- a/refactor/eg/eg.go +++ b/refactor/eg/eg.go @@ -11,7 +11,6 @@ import ( "go/token" "os" - "golang.org/x/tools/go/loader" "golang.org/x/tools/go/types" ) @@ -131,19 +130,13 @@ changing the arguments as needed; (3) change the declaration of f to match f'; (4) use eg to rename f' to f in all calls; (5) delete f'. ` -// TODO(adonovan): allow the tool to be invoked using relative package -// directory names (./foo). Requires changes to go/loader. - // TODO(adonovan): expand upon the above documentation as an HTML page. -// TODO(adonovan): eliminate dependency on loader.PackageInfo. -// Move its TypeOf method into go/types. - // A Transformer represents a single example-based transformation. type Transformer struct { fset *token.FileSet verbose bool - info loader.PackageInfo // combined type info for template/input/output ASTs + info *types.Info // combined type info for template/input/output ASTs seenInfos map[*types.Info]bool wildcards map[*types.Var]bool // set of parameters in func before() env map[string]ast.Expr // maps parameter name to wildcard binding @@ -157,16 +150,17 @@ type Transformer struct { } // NewTransformer returns a transformer based on the specified template, -// a package containing "before" and "after" functions as described -// in the package documentation. +// a single-file package containing "before" and "after" functions as +// described in the package documentation. +// tmplInfo is the type information for tmplFile. // -func NewTransformer(fset *token.FileSet, template *loader.PackageInfo, verbose bool) (*Transformer, error) { +func NewTransformer(fset *token.FileSet, tmplPkg *types.Package, tmplFile *ast.File, tmplInfo *types.Info, verbose bool) (*Transformer, error) { // Check the template. - beforeSig := funcSig(template.Pkg, "before") + beforeSig := funcSig(tmplPkg, "before") if beforeSig == nil { return nil, fmt.Errorf("no 'before' func found in template") } - afterSig := funcSig(template.Pkg, "after") + afterSig := funcSig(tmplPkg, "after") if afterSig == nil { return nil, fmt.Errorf("no 'after' func found in template") } @@ -177,18 +171,17 @@ func NewTransformer(fset *token.FileSet, template *loader.PackageInfo, verbose b beforeSig, afterSig) } - templateFile := template.Files[0] - for _, imp := range templateFile.Imports { + for _, imp := range tmplFile.Imports { if imp.Name != nil && imp.Name.Name == "." { // Dot imports are currently forbidden. We // make the simplifying assumption that all // imports are regular, without local renames. - //TODO document + // TODO(adonovan): document return nil, fmt.Errorf("dot-import (of %s) in template", imp.Path.Value) } } var beforeDecl, afterDecl *ast.FuncDecl - for _, decl := range templateFile.Decls { + for _, decl := range tmplFile.Decls { if decl, ok := decl.(*ast.FuncDecl); ok { switch decl.Name.Name { case "before": @@ -228,8 +221,8 @@ func NewTransformer(fset *token.FileSet, template *loader.PackageInfo, verbose b // of the replacement. (Consider the rule that array literal keys // must be unique.) So we cannot hope to prove the safety of a // transformation in general. - Tb := template.TypeOf(before) - Ta := template.TypeOf(after) + Tb := tmplInfo.TypeOf(before) + Ta := tmplInfo.TypeOf(after) if types.AssignableTo(Tb, Ta) { // safe: replacement is assignable to pattern. } else if tuple, ok := Tb.(*types.Tuple); ok && tuple.Len() == 0 { @@ -253,19 +246,16 @@ func NewTransformer(fset *token.FileSet, template *loader.PackageInfo, verbose b // type info for the synthesized ASTs too. This saves us // having to book-keep where each ast.Node originated as we // construct the resulting hybrid AST. - // - // TODO(adonovan): move type utility methods of PackageInfo to - // types.Info, or at least into go/types.typeutil. - tr.info.Info = types.Info{ + tr.info = &types.Info{ Types: make(map[ast.Expr]types.TypeAndValue), Defs: make(map[*ast.Ident]types.Object), Uses: make(map[*ast.Ident]types.Object), Selections: make(map[*ast.SelectorExpr]*types.Selection), } - mergeTypeInfo(&tr.info.Info, &template.Info) + mergeTypeInfo(tr.info, tmplInfo) // Compute set of imported objects required by after(). - // TODO reject dot-imports in pattern + // TODO(adonovan): reject dot-imports in pattern ast.Inspect(after, func(n ast.Node) bool { if n, ok := n.(*ast.SelectorExpr); ok { if _, ok := tr.info.Selections[n]; !ok { diff --git a/refactor/eg/eg_test.go b/refactor/eg/eg_test.go index 295e842b57..50e84535d9 100644 --- a/refactor/eg/eg_test.go +++ b/refactor/eg/eg_test.go @@ -100,7 +100,7 @@ func Test(t *testing.T) { if strings.HasSuffix(filename, "template") { // a new template shouldFail, _ := info.Pkg.Scope().Lookup("shouldFail").(*types.Const) - xform, err = eg.NewTransformer(iprog.Fset, info, *verboseFlag) + xform, err = eg.NewTransformer(iprog.Fset, info.Pkg, file, &info.Info, *verboseFlag) if err != nil { if shouldFail == nil { t.Errorf("NewTransformer(%s): %s", filename, err) diff --git a/refactor/eg/match.go b/refactor/eg/match.go index d8590d4f7c..52127eda1c 100644 --- a/refactor/eg/match.go +++ b/refactor/eg/match.go @@ -10,7 +10,6 @@ import ( "golang.org/x/tools/go/ast/astutil" "golang.org/x/tools/go/exact" - "golang.org/x/tools/go/loader" "golang.org/x/tools/go/types" ) @@ -42,8 +41,8 @@ func (tr *Transformer) matchExpr(x, y ast.Expr) bool { // Object identifiers (including pkg-qualified ones) // are handled semantically, not syntactically. - xobj := isRef(x, &tr.info) - yobj := isRef(y, &tr.info) + xobj := isRef(x, tr.info) + yobj := isRef(y, tr.info) if xobj != nil { return xobj == yobj } @@ -231,7 +230,7 @@ func unparen(e ast.Expr) ast.Expr { return astutil.Unparen(e) } // isRef returns the object referred to by this (possibly qualified) // identifier, or nil if the node is not a referring identifier. -func isRef(n ast.Node, info *loader.PackageInfo) types.Object { +func isRef(n ast.Node, info *types.Info) types.Object { switch n := n.(type) { case *ast.Ident: return info.Uses[n] diff --git a/refactor/eg/rewrite.go b/refactor/eg/rewrite.go index db9c693b6f..7e81a1724e 100644 --- a/refactor/eg/rewrite.go +++ b/refactor/eg/rewrite.go @@ -31,7 +31,7 @@ import ( func (tr *Transformer) Transform(info *types.Info, pkg *types.Package, file *ast.File) int { if !tr.seenInfos[info] { tr.seenInfos[info] = true - mergeTypeInfo(&tr.info.Info, info) + mergeTypeInfo(tr.info, info) } tr.currentPkg = pkg tr.nsubsts = 0 @@ -234,7 +234,7 @@ func (tr *Transformer) subst(env map[string]ast.Expr, pattern, pos reflect.Value // denoted by unqualified identifiers. // if tr.importedObjs != nil && pattern.Type() == selectorExprType { - obj := isRef(pattern.Interface().(*ast.SelectorExpr), &tr.info) + obj := isRef(pattern.Interface().(*ast.SelectorExpr), tr.info) if obj != nil { if sel, ok := tr.importedObjs[obj]; ok { var id ast.Expr @@ -288,7 +288,7 @@ func (tr *Transformer) subst(env map[string]ast.Expr, pattern, pos reflect.Value // All ast.Node implementations are *structs, // so this case catches them all. if e := rvToExpr(v); e != nil { - updateTypeInfo(&tr.info.Info, e, p.Interface().(ast.Expr)) + updateTypeInfo(tr.info, e, p.Interface().(ast.Expr)) } return v