From f9d2ec05b7b18a07e333b7d36cae999f23cc7dc6 Mon Sep 17 00:00:00 2001 From: Adam Scarr Date: Tue, 2 Oct 2018 17:16:36 +1000 Subject: [PATCH] Don't let goimports guess import paths --- codegen/build.go | 7 ++ codegen/import_build.go | 1 + codegen/templates/inliner/inliner.go | 5 +- codegen/templates/templates.go | 17 +-- codegen/testserver/generated.go | 2 +- example/chat/generated.go | 2 +- example/config/generated.go | 2 +- example/dataloader/generated.go | 2 +- example/scalars/generated.go | 2 +- example/selection/generated.go | 2 +- example/starwars/generated.go | 2 +- example/todo/generated.go | 2 +- integration/generated.go | 2 +- internal/imports/prune.go | 119 +++++++++++++++++++ internal/imports/prune_test.go | 22 ++++ internal/imports/testdata/unused.expected.go | 20 ++++ internal/imports/testdata/unused.go | 21 ++++ 17 files changed, 205 insertions(+), 25 deletions(-) create mode 100644 internal/imports/prune.go create mode 100644 internal/imports/prune_test.go create mode 100644 internal/imports/testdata/unused.expected.go create mode 100644 internal/imports/testdata/unused.go diff --git a/codegen/build.go b/codegen/build.go index 42dedbf872b..f6d2962344c 100644 --- a/codegen/build.go +++ b/codegen/build.go @@ -112,6 +112,13 @@ func (cfg *Config) server(destDir string) *ServerBuild { imports.add(cfg.Exec.ImportPath()) imports.add(cfg.Resolver.ImportPath()) + // extra imports only used by the server template + imports.add("context") + imports.add("log") + imports.add("net/http") + imports.add("os") + imports.add("github.com/99designs/gqlgen/handler") + return &ServerBuild{ PackageName: cfg.Resolver.Package, Imports: imports.finalize(), diff --git a/codegen/import_build.go b/codegen/import_build.go index d634834eb27..67ae251324d 100644 --- a/codegen/import_build.go +++ b/codegen/import_build.go @@ -26,6 +26,7 @@ var ambientImports = []string{ "time", "sync", "errors", + "bytes", "github.com/vektah/gqlparser", "github.com/vektah/gqlparser/ast", diff --git a/codegen/templates/inliner/inliner.go b/codegen/templates/inliner/inliner.go index cece75c1d90..3c5a0a81d01 100644 --- a/codegen/templates/inliner/inliner.go +++ b/codegen/templates/inliner/inliner.go @@ -2,11 +2,10 @@ package main import ( "bytes" + "go/format" "io/ioutil" "strconv" "strings" - - "golang.org/x/tools/imports" ) func main() { @@ -39,7 +38,7 @@ func main() { out.WriteString("}\n") - formatted, err2 := imports.Process(dir+"data.go", out.Bytes(), nil) + formatted, err2 := format.Source(out.Bytes()) if err2 != nil { panic(err2) } diff --git a/codegen/templates/templates.go b/codegen/templates/templates.go index df909cb5747..64cef2d8191 100644 --- a/codegen/templates/templates.go +++ b/codegen/templates/templates.go @@ -6,6 +6,7 @@ import ( "bytes" "fmt" "io/ioutil" + "log" "os" "path/filepath" "sort" @@ -14,10 +15,8 @@ import ( "text/template" "unicode" - "log" - + "github.com/99designs/gqlgen/internal/imports" "github.com/pkg/errors" - "golang.org/x/tools/imports" ) func Run(name string, tpldata interface{}) (*bytes.Buffer, error) { @@ -164,23 +163,15 @@ func RenderToFile(tpl string, filename string, data interface{}) error { return nil } -func gofmt(filename string, b []byte) ([]byte, error) { - out, err := imports.Process(filename, b, nil) - if err != nil { - return b, errors.Wrap(err, "unable to gofmt") - } - return out, nil -} - func write(filename string, b []byte) error { err := os.MkdirAll(filepath.Dir(filename), 0755) if err != nil { return errors.Wrap(err, "failed to create directory") } - formatted, err := gofmt(filename, b) + formatted, err := imports.Prune(filename, b) if err != nil { - fmt.Fprintf(os.Stderr, "gofmt failed: %s\n", err.Error()) + fmt.Fprintf(os.Stderr, "gofmt failed on %s: %s\n", filepath.Base(filename), err.Error()) formatted = b } diff --git a/codegen/testserver/generated.go b/codegen/testserver/generated.go index c8e7721c028..92a73ab92d8 100644 --- a/codegen/testserver/generated.go +++ b/codegen/testserver/generated.go @@ -3,7 +3,7 @@ package testserver import ( - "bytes" + bytes "bytes" context "context" fmt "fmt" strconv "strconv" diff --git a/example/chat/generated.go b/example/chat/generated.go index 33d96b52a4d..00e2aaef9cf 100644 --- a/example/chat/generated.go +++ b/example/chat/generated.go @@ -3,7 +3,7 @@ package chat import ( - "bytes" + bytes "bytes" context "context" strconv "strconv" sync "sync" diff --git a/example/config/generated.go b/example/config/generated.go index f6e6d5cce90..9efb8d01a7c 100644 --- a/example/config/generated.go +++ b/example/config/generated.go @@ -3,7 +3,7 @@ package config import ( - "bytes" + bytes "bytes" context "context" strconv "strconv" sync "sync" diff --git a/example/dataloader/generated.go b/example/dataloader/generated.go index 4ba9adab5ad..543274b512d 100644 --- a/example/dataloader/generated.go +++ b/example/dataloader/generated.go @@ -3,7 +3,7 @@ package dataloader import ( - "bytes" + bytes "bytes" context "context" strconv "strconv" sync "sync" diff --git a/example/scalars/generated.go b/example/scalars/generated.go index 58c21d09621..da4ffeddb69 100644 --- a/example/scalars/generated.go +++ b/example/scalars/generated.go @@ -3,7 +3,7 @@ package scalars import ( - "bytes" + bytes "bytes" context "context" external "external" strconv "strconv" diff --git a/example/selection/generated.go b/example/selection/generated.go index 7ce421db180..ca8cfbe47c1 100644 --- a/example/selection/generated.go +++ b/example/selection/generated.go @@ -3,7 +3,7 @@ package selection import ( - "bytes" + bytes "bytes" context "context" fmt "fmt" strconv "strconv" diff --git a/example/starwars/generated.go b/example/starwars/generated.go index fdde36622ff..d92f231bce8 100644 --- a/example/starwars/generated.go +++ b/example/starwars/generated.go @@ -3,7 +3,7 @@ package starwars import ( - "bytes" + bytes "bytes" context "context" fmt "fmt" strconv "strconv" diff --git a/example/todo/generated.go b/example/todo/generated.go index 11e59c7ef63..94faf5fcebf 100644 --- a/example/todo/generated.go +++ b/example/todo/generated.go @@ -3,7 +3,7 @@ package todo import ( - "bytes" + bytes "bytes" context "context" strconv "strconv" sync "sync" diff --git a/integration/generated.go b/integration/generated.go index f2bd6a787f2..7dbde0b2955 100644 --- a/integration/generated.go +++ b/integration/generated.go @@ -3,7 +3,7 @@ package integration import ( - "bytes" + bytes "bytes" context "context" remote_api "remote_api" strconv "strconv" diff --git a/internal/imports/prune.go b/internal/imports/prune.go new file mode 100644 index 00000000000..6b696bb818f --- /dev/null +++ b/internal/imports/prune.go @@ -0,0 +1,119 @@ +// Wrapper around x/tools/imports that only removes imports, never adds new ones. + +package imports + +import ( + "bytes" + "go/ast" + "go/build" + "go/parser" + "go/printer" + "go/token" + "path/filepath" + "strings" + + "golang.org/x/tools/imports" + + "golang.org/x/tools/go/ast/astutil" +) + +type visitFn func(node ast.Node) + +func (fn visitFn) Visit(node ast.Node) ast.Visitor { + fn(node) + return fn +} + +// Prune removes any unused imports +func Prune(filename string, src []byte) ([]byte, error) { + fset := token.NewFileSet() + + file, err := parser.ParseFile(fset, filename, src, parser.ParseComments|parser.AllErrors) + if err != nil { + return nil, err + } + + unused, err := getUnusedImports(file, filename) + if err != nil { + return nil, err + } + for ipath, name := range unused { + astutil.DeleteNamedImport(fset, file, name, ipath) + } + printConfig := &printer.Config{Mode: printer.TabIndent, Tabwidth: 8} + + var buf bytes.Buffer + if err := printConfig.Fprint(&buf, fset, file); err != nil { + return nil, err + } + + return imports.Process(filename, buf.Bytes(), &imports.Options{FormatOnly: true, Comments: true, TabIndent: true, TabWidth: 8}) +} + +func getUnusedImports(file *ast.File, filename string) (map[string]string, error) { + imported := map[string]*ast.ImportSpec{} + used := map[string]bool{} + + abs, err := filepath.Abs(filename) + if err != nil { + return nil, err + } + srcDir := filepath.Dir(abs) + + ast.Walk(visitFn(func(node ast.Node) { + if node == nil { + return + } + switch v := node.(type) { + case *ast.ImportSpec: + if v.Name != nil { + imported[v.Name.Name] = v + break + } + ipath := strings.Trim(v.Path.Value, `"`) + if ipath == "C" { + break + } + + local := importPathToName(ipath, srcDir) + + imported[local] = v + case *ast.SelectorExpr: + xident, ok := v.X.(*ast.Ident) + if !ok { + break + } + if xident.Obj != nil { + // if the parser can resolve it, it's not a package ref + break + } + used[xident.Name] = true + } + }), file) + + for pkg := range used { + delete(imported, pkg) + } + + unusedImport := map[string]string{} + for pkg, is := range imported { + if !used[pkg] && pkg != "_" && pkg != "." { + name := "" + if is.Name != nil { + name = is.Name.Name + } + unusedImport[strings.Trim(is.Path.Value, `"`)] = name + } + } + + return unusedImport, nil +} + +func importPathToName(importPath, srcDir string) (packageName string) { + pkg, err := build.Default.Import(importPath, srcDir, 0) + if err != nil { + return "" + } + + return pkg.Name +} diff --git a/internal/imports/prune_test.go b/internal/imports/prune_test.go new file mode 100644 index 00000000000..d0691bf242e --- /dev/null +++ b/internal/imports/prune_test.go @@ -0,0 +1,22 @@ +package imports + +import ( + "io/ioutil" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPrune(t *testing.T) { + b, err := Prune("testdata/unused.go", mustReadFile("testdata/unused.go")) + require.NoError(t, err) + require.Equal(t, string(mustReadFile("testdata/unused.expected.go")), string(b)) +} + +func mustReadFile(filename string) []byte { + b, err := ioutil.ReadFile(filename) + if err != nil { + panic(err) + } + return b +} diff --git a/internal/imports/testdata/unused.expected.go b/internal/imports/testdata/unused.expected.go new file mode 100644 index 00000000000..db4c9b4d278 --- /dev/null +++ b/internal/imports/testdata/unused.expected.go @@ -0,0 +1,20 @@ +package testdata + +import _ "underscore" +import a "fmt" +import "time" + +type foo struct { + Time time.Time `json:"text"` +} + +func fn() { + a.Println("hello") +} + +type Message struct { + ID string `json:"id"` + Text string `json:"text"` + CreatedBy string `json:"createdBy"` + CreatedAt time.Time `json:"createdAt"` +} diff --git a/internal/imports/testdata/unused.go b/internal/imports/testdata/unused.go new file mode 100644 index 00000000000..63ea8d7f84b --- /dev/null +++ b/internal/imports/testdata/unused.go @@ -0,0 +1,21 @@ +package testdata + +import "unused" +import _ "underscore" +import a "fmt" +import "time" + +type foo struct { + Time time.Time `json:"text"` +} + +func fn() { + a.Println("hello") +} + +type Message struct { + ID string `json:"id"` + Text string `json:"text"` + CreatedBy string `json:"createdBy"` + CreatedAt time.Time `json:"createdAt"` +}