cmd/stringer: type check using export data

Use go/packages to find and type check packages using export data.

In addition to type checking with export data, this should also enable
cmd/stringer to work correctly when using go modules.

jba: took over CL, tweaked package.Config use.

Change-Id: Ie253378b52fbd909f7194dfd09c039aab63dd8f0
Reviewed-on: https://go-review.googlesource.com/c/126535
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

cmd/stringer/endtoend_test.go

@@ -75,7 +75,12 @@ func TestTags(t *testing.T) {
 		}
 
 	}
-	err := run(stringer, "-type", "Const", dir)
+	// Run stringer in the directory that contains the package files.
+	// We cannot run stringer in the current directory for the following reasons:
+	// - Versions of Go earlier than Go 1.11, do not support absolute directories as a pattern.
+	// - When the current directory is inside a go module, the path will not be considered
+	//   a valid path to a package.
+	err := runInDir(dir, stringer, "-type", "Const", ".")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -90,7 +95,7 @@ func TestTags(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	err = run(stringer, "-type", "Const", "-tags", "tag", dir)
+	err = runInDir(dir, stringer, "-type", "Const", "-tags", "tag", ".")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -160,7 +165,14 @@ func copy(to, from string) error {
 // run runs a single command and returns an error if it does not succeed.
 // os/exec should have this function, to be honest.
 func run(name string, arg ...string) error {
+	return runInDir(".", name, arg...)
+}
+
+// runInDir runs a single command in directory dir and returns an error if
+// it does not succeed.
+func runInDir(dir, name string, arg ...string) error {
 	cmd := exec.Command(name, arg...)
+	cmd.Dir = dir
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
 	return cmd.Run()

cmd/stringer/golden_test.go

@@ -10,6 +10,9 @@
 package main
 
 import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -297,6 +300,12 @@ func (i Token) String() string {
 `
 
 func TestGolden(t *testing.T) {
+	dir, err := ioutil.TempDir("", "stringer")
+	if err != nil {
+		t.Error(err)
+	}
+	defer os.RemoveAll(dir)
+
 	for _, test := range golden {
 		g := Generator{
 			trimPrefix:  test.trimPrefix,
@@ -304,7 +313,13 @@ func TestGolden(t *testing.T) {
 		}
 		input := "package test\n" + test.input
 		file := test.name + ".go"
-		g.parsePackage(".", []string{file}, input)
+		absFile := filepath.Join(dir, file)
+		err := ioutil.WriteFile(absFile, []byte(input), 0644)
+		if err != nil {
+			t.Error(err)
+		}
+
+		g.parsePackage([]string{absFile}, nil)
 		// Extract the name and type of the constant from the first line.
 		tokens := strings.SplitN(test.input, " ", 3)
 		if len(tokens) != 3 {

cmd/stringer/importer18.go

@@ -1,16 +0,0 @@
-// Copyright 2017 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build !go1.9
-
-package main
-
-import (
-	"go/importer"
-	"go/types"
-)
-
-func defaultImporter() types.Importer {
-	return importer.Default()
-}

cmd/stringer/importer19.go

@@ -1,16 +0,0 @@
-// Copyright 2017 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build go1.9
-
-package main
-
-import (
-	"go/importer"
-	"go/types"
-)
-
-func defaultImporter() types.Importer {
-	return importer.For("source", nil)
-}

cmd/stringer/stringer.go

@@ -63,10 +63,8 @@ import (
 	"flag"
 	"fmt"
 	"go/ast"
-	"go/build"
 	"go/constant"
 	"go/format"
-	"go/parser"
 	"go/token"
 	"go/types"
 	"io/ioutil"
@@ -75,6 +73,8 @@ import (
 	"path/filepath"
 	"sort"
 	"strings"
+
+	"golang.org/x/tools/go/packages"
 )
 
 var (
@@ -124,17 +124,18 @@ func main() {
 		trimPrefix:  *trimprefix,
 		lineComment: *linecomment,
 	}
+	// TODO(suzmue): accept other patterns for packages (directories, list of files, import paths, etc).
 	if len(args) == 1 && isDirectory(args[0]) {
 		dir = args[0]
-		g.parsePackageDir(args[0], tags)
 	} else {
 		if len(tags) != 0 {
 			log.Fatal("-tags option applies only to directories, not when files are specified")
 		}
 		dir = filepath.Dir(args[0])
-		g.parsePackageFiles(args)
 	}
 
+	g.parsePackage(args, tags)
+
 	// Print the header and package clause.
 	g.Printf("// Code generated by \"stringer %s\"; DO NOT EDIT.\n", strings.Join(os.Args[1:], " "))
 	g.Printf("\n")
@@ -198,102 +199,47 @@ type File struct {
 }
 
 type Package struct {
-	dir      string
+	name  string
-	name     string
+	defs  map[*ast.Ident]types.Object
-	defs     map[*ast.Ident]types.Object
+	files []*File
-	files    []*File
-	typesPkg *types.Package
 }
 
-func buildContext(tags []string) *build.Context {
+// parsePackage analyzes the single package constructed from the patterns and tags.
-	ctx := build.Default
+// parsePackage exits if there is an error.
-	ctx.BuildTags = tags
+func (g *Generator) parsePackage(patterns []string, tags []string) {
-	return &ctx
+	cfg := &packages.Config{
-}
+		Mode: packages.LoadSyntax,
-
+		// TODO: Need to think about constants in test files. Maybe write type_string_test.go
-// parsePackageDir parses the package residing in the directory.
+		// in a separate pass? For later.
-func (g *Generator) parsePackageDir(directory string, tags []string) {
+		Tests:      false,
-	pkg, err := buildContext(tags).ImportDir(directory, 0)
+		BuildFlags: []string{fmt.Sprintf("-tags=%s", strings.Join(tags, " "))},
+	}
+	pkgs, err := packages.Load(cfg, patterns...)
 	if err != nil {
-		log.Fatalf("cannot process directory %s: %s", directory, err)
+		log.Fatal(err)
 	}
-	var names []string
+	if len(pkgs) != 1 {
-	names = append(names, pkg.GoFiles...)
+		log.Fatalf("error: %d packages found", len(pkgs))
-	names = append(names, pkg.CgoFiles...)
+	}
-	// TODO: Need to think about constants in test files. Maybe write type_string_test.go
+	g.addPackage(pkgs[0])
-	// in a separate pass? For later.
-	// names = append(names, pkg.TestGoFiles...) // These are also in the "foo" package.
-	names = append(names, pkg.SFiles...)
-	names = prefixDirectory(directory, names)
-	g.parsePackage(directory, names, nil)
 }
 
-// parsePackageFiles parses the package occupying the named files.
+// addPackage adds a type checked Package and its syntax files to the generator.
-func (g *Generator) parsePackageFiles(names []string) {
+func (g *Generator) addPackage(pkg *packages.Package) {
-	g.parsePackage(".", names, nil)
+	g.pkg = &Package{
-}
+		name:  pkg.Name,
-
+		defs:  pkg.TypesInfo.Defs,
-// prefixDirectory places the directory name on the beginning of each name in the list.
+		files: make([]*File, len(pkg.Syntax)),
-func prefixDirectory(directory string, names []string) []string {
-	if directory == "." {
-		return names
 	}
-	ret := make([]string, len(names))
-	for i, name := range names {
-		ret[i] = filepath.Join(directory, name)
-	}
-	return ret
-}
 
-// parsePackage analyzes the single package constructed from the named files.
+	for i, file := range pkg.Syntax {
-// If text is non-nil, it is a string to be used instead of the content of the file,
+		g.pkg.files[i] = &File{
-// to be used for testing. parsePackage exits if there is an error.
+			file:        file,
-func (g *Generator) parsePackage(directory string, names []string, text interface{}) {
-	var files []*File
-	var astFiles []*ast.File
-	g.pkg = new(Package)
-	fs := token.NewFileSet()
-	for _, name := range names {
-		if !strings.HasSuffix(name, ".go") {
-			continue
-		}
-		parsedFile, err := parser.ParseFile(fs, name, text, parser.ParseComments)
-		if err != nil {
-			log.Fatalf("parsing package: %s: %s", name, err)
-		}
-		astFiles = append(astFiles, parsedFile)
-		files = append(files, &File{
-			file:        parsedFile,
 			pkg:         g.pkg,
 			trimPrefix:  g.trimPrefix,
 			lineComment: g.lineComment,
-		})
+		}
 	}
-	if len(astFiles) == 0 {
-		log.Fatalf("%s: no buildable Go files", directory)
-	}
-	g.pkg.name = astFiles[0].Name.Name
-	g.pkg.files = files
-	g.pkg.dir = directory
-	g.pkg.typeCheck(fs, astFiles)
-}
-
-// check type-checks the package so we can evaluate contants whose values we are printing.
-func (pkg *Package) typeCheck(fs *token.FileSet, astFiles []*ast.File) {
-	pkg.defs = make(map[*ast.Ident]types.Object)
-	config := types.Config{
-		IgnoreFuncBodies: true, // We only need to evaluate constants.
-		Importer:         defaultImporter(),
-		FakeImportC:      true,
-	}
-	info := &types.Info{
-		Defs: pkg.defs,
-	}
-	typesPkg, err := config.Check(pkg.dir, fs, astFiles, info)
-	if err != nil {
-		log.Fatalf("checking package: %s", err)
-	}
-	pkg.typesPkg = typesPkg
 }
 
 // generate produces the String method for the named type.