diff --git a/cmd/bundle/main.go b/cmd/bundle/main.go index 7cd31c98b6..b53ced4d02 100644 --- a/cmd/bundle/main.go +++ b/cmd/bundle/main.go @@ -96,6 +96,7 @@ import ( "log" "os" "path" + "strconv" "strings" "golang.org/x/tools/go/loader" @@ -257,46 +258,61 @@ func bundle(src, dst, dstpkg, prefix string) ([]byte, error) { fmt.Fprintf(&out, "package %s\n\n", dstpkg) - // Print a single declaration that imports all necessary packages. - // TODO(adonovan): - // - support renaming imports. + // BUG(adonovan,shurcooL): bundle may generate incorrect code + // due to shadowing between identifiers and imported package names. + // + // The generated code will either fail to compile or + // (unlikely) compile successfully but have different behavior + // than the original package. The risk of this happening is higher + // when the original package has renamed imports (they're typically + // renamed in order to resolve a shadow inside that particular .go file). + + // TODO(adonovan,shurcooL): + // - detect shadowing issues, and either return error or resolve them // - preserve comments from the original import declarations. + + // pkgStd and pkgExt are sets of printed import specs. This is done + // to deduplicate instances of the same import name and path. + var pkgStd = make(map[string]bool) + var pkgExt = make(map[string]bool) for _, f := range info.Files { for _, imp := range f.Imports { + path, err := strconv.Unquote(imp.Path.Value) + if err != nil { + log.Fatalf("invalid import path string: %v", err) // Shouldn't happen here since conf.Load succeeded. + } + if path == dst { + continue + } + if newPath, ok := importMap[path]; ok { + path = newPath + } + + var name string if imp.Name != nil { - return nil, fmt.Errorf("%s: renaming imports not supported", - lprog.Fset.Position(imp.Pos())) + name = imp.Name.Name + } + spec := fmt.Sprintf("%s %q", name, path) + if isStandardImportPath(path) { + pkgStd[spec] = true + } else { + pkgExt[spec] = true } } } - var pkgStd, pkgExt []string - for _, p := range info.Pkg.Imports() { - if p.Path() == dst { - continue - } - x, ok := importMap[p.Path()] - if !ok { - x = p.Path() - } - if isStandardImportPath(x) { - pkgStd = append(pkgStd, x) - } else { - pkgExt = append(pkgExt, x) - } - } - + // Print a single declaration that imports all necessary packages. fmt.Fprintln(&out, "import (") - for _, p := range pkgStd { - fmt.Fprintf(&out, "\t%q\n", p) + for p := range pkgStd { + fmt.Fprintf(&out, "\t%s\n", p) } if len(pkgExt) > 0 { fmt.Fprintln(&out) - for _, p := range pkgExt { - fmt.Fprintf(&out, "\t%q\n", p) - } } - fmt.Fprintln(&out, ")\n") + for p := range pkgExt { + fmt.Fprintf(&out, "\t%s\n", p) + } + fmt.Fprint(&out, ")\n\n") // Modify and print each file. for _, f := range info.Files { diff --git a/cmd/bundle/main_test.go b/cmd/bundle/main_test.go index 96c8cf707d..4db8aa7776 100644 --- a/cmd/bundle/main_test.go +++ b/cmd/bundle/main_test.go @@ -28,6 +28,7 @@ func TestBundle(t *testing.T) { "initial": { "a.go": load("testdata/src/initial/a.go"), "b.go": load("testdata/src/initial/b.go"), + "c.go": load("testdata/src/initial/c.go"), }, "domain.name/importdecl": { "p.go": load("testdata/src/domain.name/importdecl/p.go"), diff --git a/cmd/bundle/testdata/out.golden b/cmd/bundle/testdata/out.golden index 59f98fe9d6..0ca96c9d59 100644 --- a/cmd/bundle/testdata/out.golden +++ b/cmd/bundle/testdata/out.golden @@ -8,6 +8,10 @@ package dest import ( "fmt" + . "fmt" + _ "fmt" + renamedfmt "fmt" + renamedfmt2 "fmt" "domain.name/importdecl" ) @@ -31,3 +35,9 @@ const prefixc = 1 func prefixfoo() { fmt.Println(importdecl.F()) } + +func prefixbaz() { + renamedfmt.Println() + renamedfmt2.Println() + Println() +} diff --git a/cmd/bundle/testdata/src/initial/c.go b/cmd/bundle/testdata/src/initial/c.go new file mode 100644 index 0000000000..6a9cbc6914 --- /dev/null +++ b/cmd/bundle/testdata/src/initial/c.go @@ -0,0 +1,12 @@ +package initial + +import _ "fmt" +import renamedfmt "fmt" +import renamedfmt2 "fmt" +import . "fmt" + +func baz() { + renamedfmt.Println() + renamedfmt2.Println() + Println() +}