[tools] cmd/stringer: add a -tags flag that supports build tags
98 views
Skip to first unread message
Rob Pike (Gerrit)
Jul 8, 2018, 10:27:24 PM7/8/18
to Ian Lance Taylor, goph...@pubsubhelper.golang.org, Rob Pike, golang-co...@googlegroups.com
Rob Pike has uploaded this change for review.
cmd/stringer: add a -tags flag that supports build tags
This is reapplying CL121995 after rolling back the change to the importing
methods. There is still a need for a flag to control tags.
The original CL decription:
The feature has been requested but, like build tags in general,
only works in a directory, not when files are specified explicitly.
Unlike the build tools, report when the feature is misused like this
to avoid confusion.
Fixes golang/go#9449
Change-Id: I732627d5f2e6323367e3bdd5de746923868890a9
---
M cmd/stringer/endtoend_test.go
M cmd/stringer/stringer.go
A cmd/stringer/testdata/tag_main.go
A cmd/stringer/testdata/tag_tag.go
4 files changed, 110 insertions(+), 18 deletions(-)
diff --git a/cmd/stringer/endtoend_test.go b/cmd/stringer/endtoend_test.go
index 273f29b..91f3409 100644
--- a/cmd/stringer/endtoend_test.go
+++ b/cmd/stringer/endtoend_test.go
@@ -9,6 +9,7 @@
package main
import (
+ "bytes"
"fmt"
"go/build"
"io"
@@ -26,17 +27,8 @@
// binary panics if the String method for X is not correct, including for error cases.
func TestEndToEnd(t *testing.T) {
- dir, err := ioutil.TempDir("", "stringer")
- if err != nil {
- t.Fatal(err)
- }
+ dir, stringer := buildStringer(t)
defer os.RemoveAll(dir)
- // Create stringer in temporary directory.
- stringer := filepath.Join(dir, "stringer.exe")
- err = run("go", "build", "-o", stringer)
- if err != nil {
- t.Fatalf("building stringer: %s", err)
- }
// Read the testdata directory.
fd, err := os.Open("testdata")
if err != nil {
@@ -53,6 +45,10 @@
t.Errorf("%s is not a Go file", name)
continue
}
+ if strings.HasPrefix(name, "tag_") {
+ // This file is used for tag processing in TestTags, below.
+ continue
+ }
if name == "cgo.go" && !build.Default.CgoEnabled {
t.Logf("cgo is not enabled for %s", name)
continue
@@ -63,9 +59,69 @@
}
}
+// TestTags verifies that the -tags flag works as advertised.
+func TestTags(t *testing.T) {
+ dir, stringer := buildStringer(t)
+ defer os.RemoveAll(dir)
+ var (
+ protectedConst = []byte("TagProtected")
+ output = filepath.Join(dir, "const_string.go")
+ )
+ for _, file := range []string{"tag_main.go", "tag_tag.go"} {
+
+ err := copy(filepath.Join(dir, file), filepath.Join("testdata", file))
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ }
+ err := run(stringer, "-type", "Const", dir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ result, err := ioutil.ReadFile(output)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if bytes.Contains(result, protectedConst) {
+ t.Fatal("tagged variable appears in untagged run")
+ }
+ err = os.Remove(output)
+ if err != nil {
+ t.Fatal(err)
+ }
+ err = run(stringer, "-type", "Const", "-tags", "tag", dir)
+ if err != nil {
+ t.Fatal(err)
+ }
+ result, err = ioutil.ReadFile(output)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !bytes.Contains(result, protectedConst) {
+ t.Fatal("tagged variable does not appear in tagged run")
+ }
+}
+
+// buildStringer creates a temporary directory and installs stringer there.
+func buildStringer(t *testing.T) (dir string, stringer string) {
+ t.Helper()
+ dir, err := ioutil.TempDir("", "stringer")
+ if err != nil {
+ t.Fatal(err)
+ }
+ stringer = filepath.Join(dir, "stringer.exe")
+ err = run("go", "build", "-o", stringer, "stringer.go")
+ if err != nil {
+ t.Fatalf("building stringer: %s", err)
+ }
+ return dir, stringer
+}
+
// stringerCompileAndRun runs stringer for the named file and compiles and
// runs the target binary in directory dir. That binary will panic if the String method is incorrect.
func stringerCompileAndRun(t *testing.T, dir, stringer, typeName, fileName string) {
+ t.Helper()
t.Logf("run: %s %s\n", fileName, typeName)
source := filepath.Join(dir, fileName)
err := copy(source, filepath.Join("testdata", fileName))
diff --git a/cmd/stringer/stringer.go b/cmd/stringer/stringer.go
index 4b8d1ba..bed6b6b 100644
--- a/cmd/stringer/stringer.go
+++ b/cmd/stringer/stringer.go
@@ -66,6 +66,7 @@
"go/build"
exact "go/constant"
"go/format"
+ "go/importer"
"go/parser"
"go/token"
"go/types"
@@ -82,6 +83,7 @@
output = flag.String("output", "", "output file name; default srcdir/<type>_string.go")
trimprefix = flag.String("trimprefix", "", "trim the `prefix` from the generated constant names")
linecomment = flag.Bool("linecomment", false, "use line comment text as printed text when present")
+ buildTags = flag.String("tags", "", "comma-separated list of build tags to apply")
)
// Usage is a replacement usage function for the flags package.
@@ -105,6 +107,10 @@
os.Exit(2)
}
types := strings.Split(*typeNames, ",")
+ var tags []string
+ if len(*buildTags) > 0 {
+ tags = strings.Split(*buildTags, ",")
+ }
// We accept either one directory or a list of files. Which do we have?
args := flag.Args()
@@ -121,8 +127,11 @@
}
if len(args) == 1 && isDirectory(args[0]) {
dir = args[0]
- g.parsePackageDir(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)
}
@@ -197,9 +206,15 @@
typesPkg *types.Package
}
+func buildContext(tags []string) *build.Context {
+ ctx := build.Default
+ ctx.BuildTags = tags
+ return &ctx
+}
+
// parsePackageDir parses the package residing in the directory.
-func (g *Generator) parsePackageDir(directory string) {
- pkg, err := build.Default.ImportDir(directory, 0)
+func (g *Generator) parsePackageDir(directory string, tags []string) {
+ pkg, err := buildContext(tags).ImportDir(directory, 0)
if err != nil {
log.Fatalf("cannot process directory %s: %s", directory, err)
}
@@ -261,14 +276,17 @@
g.pkg.name = astFiles[0].Name.Name
g.pkg.files = files
g.pkg.dir = directory
- // Type check the package.
- g.pkg.check(fs, astFiles)
+ g.pkg.typeCheck(fs, astFiles)
}
-// check type-checks the package. The package must be OK to proceed.
-func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) {
+// 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{Importer: defaultImporter(), FakeImportC: true}
+ config := types.Config{
+ IgnoreFuncBodies: true, // We only need to evaluate constants.
+ Importer: importer.Default(),
+ FakeImportC: true,
+ }
info := &types.Info{
Defs: pkg.defs,
}
diff --git a/cmd/stringer/testdata/tag_main.go b/cmd/stringer/testdata/tag_main.go
new file mode 100644
index 0000000..449ea0d
--- /dev/null
+++ b/cmd/stringer/testdata/tag_main.go
@@ -0,0 +1,11 @@
+// No build tag in this file.
+
+package main
+
+type Const int
+
+const (
+ A Const = iota
+ B
+ C
+)
diff --git a/cmd/stringer/testdata/tag_tag.go b/cmd/stringer/testdata/tag_tag.go
new file mode 100644
index 0000000..6844cb0
--- /dev/null
+++ b/cmd/stringer/testdata/tag_tag.go
@@ -0,0 +1,7 @@
+// This file has a build tag "tag"
+
+// +build tag
+
+package main
+
+const TagProtected Const = C + 1
To view, visit change 122537. To unsubscribe, or for help writing mail filters, visit settings.
Rob Pike (Gerrit)
Jul 8, 2018, 10:27:46 PM7/8/18
to Rob Pike, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Patch set 1:Run-TryBot +1
Gobot Gobot (Gerrit)
Jul 8, 2018, 10:27:48 PM7/8/18
to Rob Pike, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
TryBots beginning. Status page: https://farmer.golang.org/try?commit=418f3f2e
Gobot Gobot (Gerrit)
Jul 8, 2018, 10:30:19 PM7/8/18
to Rob Pike, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
TryBots are happy.
Patch set 1:TryBot-Result +1
Ian Lance Taylor (Gerrit)
Jul 9, 2018, 7:02:19 PM7/9/18
to Rob Pike, goph...@pubsubhelper.golang.org, Gobot Gobot, golang-co...@googlegroups.com
1 comment:
File cmd/stringer/stringer.go:
Patch Set #1, Line 287:
Importer: importer.Default(),Shouldn't you be calling defaultImporter here, rather than importer.Default?
To view, visit change 122537. To unsubscribe, or for help writing mail filters, visit settings.
Rob Pike (Gerrit)
Jul 9, 2018, 10:42:02 PM7/9/18
to Rob Pike, Gobot Gobot, goph...@pubsubhelper.golang.org, Ian Lance Taylor, golang-co...@googlegroups.com
Rob Pike uploaded patch set #2 to this change.
cmd/stringer: add a -tags flag that supports build tags
This is reapplying CL121995 after rolling back the change to the importing
methods. There is still a need for a flag to control tags.
The original CL decription:
The feature has been requested but, like build tags in general,
only works in a directory, not when files are specified explicitly.
Unlike the build tools, report when the feature is misused like this
to avoid confusion.
Fixes golang/go#9449
Change-Id: I732627d5f2e6323367e3bdd5de746923868890a9
---
M cmd/stringer/endtoend_test.go
M cmd/stringer/stringer.go
A cmd/stringer/testdata/tag_main.go
A cmd/stringer/testdata/tag_tag.go
4 files changed, 109 insertions(+), 18 deletions(-)
To view, visit change 122537. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Jul 10, 2018, 12:21:57 AM7/10/18
to Rob Pike, goph...@pubsubhelper.golang.org, Gobot Gobot, golang-co...@googlegroups.com
Patch set 2:Run-TryBot +1Code-Review +2
Gobot Gobot (Gerrit)
Jul 10, 2018, 12:22:00 AM7/10/18
to Rob Pike, goph...@pubsubhelper.golang.org, Ian Lance Taylor, golang-co...@googlegroups.com
Gobot Gobot (Gerrit)
Jul 10, 2018, 12:26:13 AM7/10/18
to Rob Pike, goph...@pubsubhelper.golang.org, Ian Lance Taylor, golang-co...@googlegroups.com
TryBots are happy.
Patch set 2:TryBot-Result +1
Rob Pike (Gerrit)
Jul 10, 2018, 1:22:44 AM7/10/18
to Rob Pike, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Gobot Gobot, Ian Lance Taylor, golang-co...@googlegroups.com
Rob Pike merged this change.
Approvals:
Ian Lance Taylor: Looks good to me, approved; Run TryBots
Gobot Gobot: TryBots succeeded
cmd/stringer: add a -tags flag that supports build tags
This is reapplying CL121995 after rolling back the change to the importing
methods. There is still a need for a flag to control tags.
The original CL decription:
The feature has been requested but, like build tags in general,
only works in a directory, not when files are specified explicitly.
Unlike the build tools, report when the feature is misused like this
to avoid confusion.
Fixes golang/go#9449
Change-Id: I732627d5f2e6323367e3bdd5de746923868890a9
Reviewed-on: https://go-review.googlesource.com/122537
Reviewed-by: Ian Lance Taylor <ia...@golang.org>
Run-TryBot: Ian Lance Taylor <ia...@golang.org>
TryBot-Result: Gobot Gobot <go...@golang.org>
---
M cmd/stringer/endtoend_test.go
M cmd/stringer/stringer.go
A cmd/stringer/testdata/tag_main.go
A cmd/stringer/testdata/tag_tag.go
4 files changed, 109 insertions(+), 18 deletions(-)
diff --git a/cmd/stringer/endtoend_test.go b/cmd/stringer/endtoend_test.go
index 273f29b..f8530a6 100644
+ err = run("go", "build", "-o", stringer)
+ if err != nil {
+ t.Fatalf("building stringer: %s", err)
+ }
+ return dir, stringer
+}
+
// stringerCompileAndRun runs stringer for the named file and compiles and
// runs the target binary in directory dir. That binary will panic if the String method is incorrect.
func stringerCompileAndRun(t *testing.T, dir, stringer, typeName, fileName string) {
+ t.Helper()
t.Logf("run: %s %s\n", fileName, typeName)
source := filepath.Join(dir, fileName)
err := copy(source, filepath.Join("testdata", fileName))
diff --git a/cmd/stringer/stringer.go b/cmd/stringer/stringer.go
index 4b8d1ba..5e660fd 100644
--- a/cmd/stringer/stringer.go
+++ b/cmd/stringer/stringer.go
@@ -82,6 +82,7 @@
output = flag.String("output", "", "output file name; default srcdir/<type>_string.go")
trimprefix = flag.String("trimprefix", "", "trim the `prefix` from the generated constant names")
linecomment = flag.Bool("linecomment", false, "use line comment text as printed text when present")
+ buildTags = flag.String("tags", "", "comma-separated list of build tags to apply")
)
// Usage is a replacement usage function for the flags package.
@@ -105,6 +106,10 @@
os.Exit(2)
}
types := strings.Split(*typeNames, ",")
+ var tags []string
+ if len(*buildTags) > 0 {
+ tags = strings.Split(*buildTags, ",")
+ }
// We accept either one directory or a list of files. Which do we have?
args := flag.Args()
@@ -121,8 +126,11 @@
}
if len(args) == 1 && isDirectory(args[0]) {
dir = args[0]
- g.parsePackageDir(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)
}
@@ -197,9 +205,15 @@
typesPkg *types.Package
}
+func buildContext(tags []string) *build.Context {
+ ctx := build.Default
+ ctx.BuildTags = tags
+ return &ctx
+}
+
// parsePackageDir parses the package residing in the directory.
-func (g *Generator) parsePackageDir(directory string) {
- pkg, err := build.Default.ImportDir(directory, 0)
+func (g *Generator) parsePackageDir(directory string, tags []string) {
+ pkg, err := buildContext(tags).ImportDir(directory, 0)
if err != nil {
log.Fatalf("cannot process directory %s: %s", directory, err)
}
@@ -261,14 +275,17 @@
g.pkg.name = astFiles[0].Name.Name
g.pkg.files = files
g.pkg.dir = directory
- // Type check the package.
- g.pkg.check(fs, astFiles)
+ g.pkg.typeCheck(fs, astFiles)
}
-// check type-checks the package. The package must be OK to proceed.
-func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) {
+// 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{Importer: defaultImporter(), FakeImportC: true}
+ config := types.Config{
+ IgnoreFuncBodies: true, // We only need to evaluate constants.
+ Importer: defaultImporter(),Reply all
Reply to author
Forward
0 new messages