Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't let goimports guess import paths #365

Merged
merged 1 commit into from
Oct 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gometalinter.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@
"PartitionStrategy": "packages"
}
},
"Skip": ["internal/imports/testdata"],
"Disable": ["gas","golint","gocyclo","goconst", "gotype", "maligned", "gosec"]
}
7 changes: 7 additions & 0 deletions codegen/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
1 change: 1 addition & 0 deletions codegen/import_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ var ambientImports = []string{
"time",
"sync",
"errors",
"bytes",

"github.com/vektah/gqlparser",
"github.com/vektah/gqlparser/ast",
Expand Down
5 changes: 2 additions & 3 deletions codegen/templates/inliner/inliner.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ package main

import (
"bytes"
"go/format"
"io/ioutil"
"strconv"
"strings"

"golang.org/x/tools/imports"
)

func main() {
Expand Down Expand Up @@ -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)
}
Expand Down
17 changes: 4 additions & 13 deletions codegen/templates/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"bytes"
"fmt"
"io/ioutil"
"log"
"os"
"path/filepath"
"sort"
Expand All @@ -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) {
Expand Down Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion codegen/testserver/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion example/chat/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion example/config/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion example/dataloader/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion example/scalars/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion example/selection/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion example/starwars/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion example/todo/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion integration/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

119 changes: 119 additions & 0 deletions internal/imports/prune.go
Original file line number Diff line number Diff line change
@@ -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.Node, 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
}
22 changes: 22 additions & 0 deletions internal/imports/prune_test.go
Original file line number Diff line number Diff line change
@@ -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
}
20 changes: 20 additions & 0 deletions internal/imports/testdata/unused.expected.go
Original file line number Diff line number Diff line change
@@ -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"`
}
21 changes: 21 additions & 0 deletions internal/imports/testdata/unused.go
Original file line number Diff line number Diff line change
@@ -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"`
}