Skip to content

Commit 76ab8bb

Browse files
committed
cmd/stringer: revert back to source importer
Roll back my two recent changes. Stringer is now very slow again, but works in most use cases. My git foo is insufficient to do this as a revert, but it is a by-hand reversion of CLs https://go-review.googlesource.com/121884 https://go-review.googlesource.com/121995 See the issue for a long conversation about the general problem. Update golang/go#10249 Update golang/go#25650 Change-Id: I7b6ce352a4c7ebf0977883509e9d7189aaac1251 Reviewed-on: https://go-review.googlesource.com/122535 Run-TryBot: Rob Pike <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent dd9bb31 commit 76ab8bb

File tree

6 files changed

+50
-110
lines changed

6 files changed

+50
-110
lines changed

endtoend_test.go

Lines changed: 10 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
package main
1010

1111
import (
12-
"bytes"
1312
"fmt"
1413
"go/build"
1514
"io"
@@ -27,8 +26,17 @@ import (
2726
// binary panics if the String method for X is not correct, including for error cases.
2827

2928
func TestEndToEnd(t *testing.T) {
30-
dir, stringer := buildStringer(t)
29+
dir, err := ioutil.TempDir("", "stringer")
30+
if err != nil {
31+
t.Fatal(err)
32+
}
3133
defer os.RemoveAll(dir)
34+
// Create stringer in temporary directory.
35+
stringer := filepath.Join(dir, "stringer.exe")
36+
err = run("go", "build", "-o", stringer)
37+
if err != nil {
38+
t.Fatalf("building stringer: %s", err)
39+
}
3240
// Read the testdata directory.
3341
fd, err := os.Open("testdata")
3442
if err != nil {
@@ -45,10 +53,6 @@ func TestEndToEnd(t *testing.T) {
4553
t.Errorf("%s is not a Go file", name)
4654
continue
4755
}
48-
if strings.HasPrefix(name, "tag_") {
49-
// This file is used for tag processing in TestTags, below.
50-
continue
51-
}
5256
if name == "cgo.go" && !build.Default.CgoEnabled {
5357
t.Logf("cgo is not enabled for %s", name)
5458
continue
@@ -59,69 +63,9 @@ func TestEndToEnd(t *testing.T) {
5963
}
6064
}
6165

62-
// TestTags verifies that the -tags flag works as advertised.
63-
func TestTags(t *testing.T) {
64-
dir, stringer := buildStringer(t)
65-
defer os.RemoveAll(dir)
66-
var (
67-
protectedConst = []byte("TagProtected")
68-
output = filepath.Join(dir, "const_string.go")
69-
)
70-
for _, file := range []string{"tag_main.go", "tag_tag.go"} {
71-
72-
err := copy(filepath.Join(dir, file), filepath.Join("testdata", file))
73-
if err != nil {
74-
t.Fatal(err)
75-
}
76-
77-
}
78-
err := run(stringer, "-type", "Const", dir)
79-
if err != nil {
80-
t.Fatal(err)
81-
}
82-
result, err := ioutil.ReadFile(output)
83-
if err != nil {
84-
t.Fatal(err)
85-
}
86-
if bytes.Contains(result, protectedConst) {
87-
t.Fatal("tagged variable appears in untagged run")
88-
}
89-
err = os.Remove(output)
90-
if err != nil {
91-
t.Fatal(err)
92-
}
93-
err = run(stringer, "-type", "Const", "-tags", "tag", dir)
94-
if err != nil {
95-
t.Fatal(err)
96-
}
97-
result, err = ioutil.ReadFile(output)
98-
if err != nil {
99-
t.Fatal(err)
100-
}
101-
if !bytes.Contains(result, protectedConst) {
102-
t.Fatal("tagged variable does not appear in tagged run")
103-
}
104-
}
105-
106-
// buildStringer creates a temporary directory and installs stringer there.
107-
func buildStringer(t *testing.T) (dir string, stringer string) {
108-
t.Helper()
109-
dir, err := ioutil.TempDir("", "stringer")
110-
if err != nil {
111-
t.Fatal(err)
112-
}
113-
stringer = filepath.Join(dir, "stringer.exe")
114-
err = run("go", "build", "-o", stringer, "stringer.go")
115-
if err != nil {
116-
t.Fatalf("building stringer: %s", err)
117-
}
118-
return dir, stringer
119-
}
120-
12166
// stringerCompileAndRun runs stringer for the named file and compiles and
12267
// runs the target binary in directory dir. That binary will panic if the String method is incorrect.
12368
func stringerCompileAndRun(t *testing.T, dir, stringer, typeName, fileName string) {
124-
t.Helper()
12569
t.Logf("run: %s %s\n", fileName, typeName)
12670
source := filepath.Join(dir, fileName)
12771
err := copy(source, filepath.Join("testdata", fileName))

importer18.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2017 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// +build !go1.9
6+
7+
package main
8+
9+
import (
10+
"go/importer"
11+
"go/types"
12+
)
13+
14+
func defaultImporter() types.Importer {
15+
return importer.Default()
16+
}

importer19.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2017 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// +build go1.9
6+
7+
package main
8+
9+
import (
10+
"go/importer"
11+
"go/types"
12+
)
13+
14+
func defaultImporter() types.Importer {
15+
return importer.For("source", nil)
16+
}

stringer.go

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ import (
6666
"go/build"
6767
exact "go/constant"
6868
"go/format"
69-
"go/importer"
7069
"go/parser"
7170
"go/token"
7271
"go/types"
@@ -83,7 +82,6 @@ var (
8382
output = flag.String("output", "", "output file name; default srcdir/<type>_string.go")
8483
trimprefix = flag.String("trimprefix", "", "trim the `prefix` from the generated constant names")
8584
linecomment = flag.Bool("linecomment", false, "use line comment text as printed text when present")
86-
buildTags = flag.String("tags", "", "comma-separated list of build tags to apply")
8785
)
8886

8987
// Usage is a replacement usage function for the flags package.
@@ -107,10 +105,6 @@ func main() {
107105
os.Exit(2)
108106
}
109107
types := strings.Split(*typeNames, ",")
110-
var tags []string
111-
if len(*buildTags) > 0 {
112-
tags = strings.Split(*buildTags, ",")
113-
}
114108

115109
// We accept either one directory or a list of files. Which do we have?
116110
args := flag.Args()
@@ -127,11 +121,8 @@ func main() {
127121
}
128122
if len(args) == 1 && isDirectory(args[0]) {
129123
dir = args[0]
130-
g.parsePackageDir(args[0], tags)
124+
g.parsePackageDir(args[0])
131125
} else {
132-
if len(tags) != 0 {
133-
log.Fatal("-tags option applies only to directories, not when files are specified")
134-
}
135126
dir = filepath.Dir(args[0])
136127
g.parsePackageFiles(args)
137128
}
@@ -206,15 +197,9 @@ type Package struct {
206197
typesPkg *types.Package
207198
}
208199

209-
func buildContext(tags []string) *build.Context {
210-
ctx := build.Default
211-
ctx.BuildTags = tags
212-
return &ctx
213-
}
214-
215200
// parsePackageDir parses the package residing in the directory.
216-
func (g *Generator) parsePackageDir(directory string, tags []string) {
217-
pkg, err := buildContext(tags).ImportDir(directory, 0)
201+
func (g *Generator) parsePackageDir(directory string) {
202+
pkg, err := build.Default.ImportDir(directory, 0)
218203
if err != nil {
219204
log.Fatalf("cannot process directory %s: %s", directory, err)
220205
}
@@ -276,17 +261,14 @@ func (g *Generator) parsePackage(directory string, names []string, text interfac
276261
g.pkg.name = astFiles[0].Name.Name
277262
g.pkg.files = files
278263
g.pkg.dir = directory
279-
g.pkg.typeCheck(fs, astFiles)
264+
// Type check the package.
265+
g.pkg.check(fs, astFiles)
280266
}
281267

282-
// check type-checks the package so we can evaluate contants whose values we are printing.
283-
func (pkg *Package) typeCheck(fs *token.FileSet, astFiles []*ast.File) {
268+
// check type-checks the package. The package must be OK to proceed.
269+
func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) {
284270
pkg.defs = make(map[*ast.Ident]types.Object)
285-
config := types.Config{
286-
IgnoreFuncBodies: true, // We only need to evaluate constants.
287-
Importer: importer.Default(),
288-
FakeImportC: true,
289-
}
271+
config := types.Config{Importer: defaultImporter(), FakeImportC: true}
290272
info := &types.Info{
291273
Defs: pkg.defs,
292274
}

testdata/tag_main.go

Lines changed: 0 additions & 11 deletions
This file was deleted.

testdata/tag_tag.go

Lines changed: 0 additions & 7 deletions
This file was deleted.

0 commit comments

Comments
 (0)