code review 2220045: Add option to gofmt to help build tools that need packa... (issue2220045)

[ful...@gmail.com]

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golan...@googlegroups.com (cc: golan...@googlegroups.com),

I'd like you to review this change.


Description:
Add option to gofmt to help build tools that need package name and
imports.

Please review this at http://codereview.appspot.com/2220045/

Affected files:
M src/cmd/gofmt/gofmt.go


Index: src/cmd/gofmt/gofmt.go
===================================================================
--- a/src/cmd/gofmt/gofmt.go
+++ b/src/cmd/gofmt/gofmt.go
@@ -12,6 +12,7 @@
"go/parser"
"go/printer"
"go/scanner"
+ "go/token"
"io/ioutil"
"os"
pathutil "path"
@@ -26,6 +27,7 @@
rewriteRule = flag.String("r", "", "rewrite rule (e.g., 'α[β:len(α)] ->
α[β:]')")

// debugging support
+ build = flag.Bool("build", false, "print code useful for build tools")
comments = flag.Bool("comments", true, "print comments")
trace = flag.Bool("trace", false, "print parse trace")
printAST = flag.Bool("ast", false, "print AST (before rewrites)")
@@ -66,6 +68,9 @@
if *trace {
parserMode |= parser.Trace
}
+ if *build && !*write {
+ parserMode |= parser.ImportsOnly
+ }
}


@@ -86,6 +91,22 @@
}


+func extractImports(file *ast.File) <-chan *ast.ImportSpec {
+ ch := make(chan *ast.ImportSpec)
+ go func() {
+ defer close(ch)
+ for _, decl := range file.Decls {
+ if gd, ok := decl.(*ast.GenDecl); ok && gd.Tok == token.IMPORT {
+ for _, spec := range gd.Specs {
+ ch <- spec.(*ast.ImportSpec)
+ }
+ }
+ }
+ }()
+ return ch
+}
+
+
func processFile(f *os.File) os.Error {
src, err := ioutil.ReadAll(f)
if err != nil {
@@ -98,6 +119,14 @@
return err
}

+ if *build && !*write {
+ fmt.Printf("package %s\n", file.Name.Name)
+ for spec := range extractImports(file) {
+ fmt.Printf("import %s\n", string(spec.Path.Value))
+ }
+ return err
+ }
+
if *printAST {
ast.Print(file)
}

[Robert Griesemer]

Thanks, but this code (as is) doesn't belong into gofmt.

(If it were simply exposing all of the three parser modes (parse package clause, parse imports, parse all) and use the normal formatting I could perhaps be talked into it. But I think that would defeat the purpose as imports would show up in various forms.)

It is very tempting to put this into gofmt because it's just a few lines and all the infrastructure is there, but it opens the door to all kinds of other things that have nothing to do with formatting. We don't want gofmt to become the Swiss Army Knife of tools. It is intentionally kept super-simple (and arguably, some of the other debug flags shouldn't be there, either).

Thanks.

- gri

[r...@google.com]

Closing review.

http://codereview.appspot.com/2220045/

[Albert Strasheim]

Hello all

On Sep 16, 7:07 pm, Robert Griesemer <g...@golang.org> wrote:
> Thanks, but this code (as is) doesn't belong into gofmt.

Thanks for the review.

> (If it were simply exposing all of the three parser modes (parse package
> clause, parse imports, parse all) and use the normal formatting I could
> perhaps be talked into it. But I think that would defeat the purpose as
> imports would show up in various forms.)

Yes, to simplify parsing one wants an import per line, which is going
to be different from normal gofmt output.

> It is very tempting to put this into gofmt because it's just a few lines and
> all the infrastructure is there, but it opens the door to all kinds of other
> things that have nothing to do with formatting. We don't want gofmt to
> become the Swiss Army Knife of tools. It is intentionally kept super-simple
> (and arguably, some of the other debug flags shouldn't be there, either).

Okay, understood.

When I asked about this a few weeks ago, Andrew Gerrand suggested
putting it in godoc or goinstall, but after looking at those tools,
gofmt still looked like the best (but not ideal) place.

How about making a new tool, perhaps called goparse or gotool, as a
place for doing things with the Go parser?

I can think of two use-cases at present:

1. Parse go files and print package name and imports, useful for build
tools

2. Parse a _test.go file and output _testmain.go, like gotest does

Both of these will help any third party tool to build Go code.

Thanks again.

Cheers

Albert