plugin: Imports should be qualified when the base name doesn't match (#…

…233)

Imports in plugin templates previously had this logic: If the package can be
imported (it exists on the `GOPATH`), import it and determine its name.
Otherwise, use the base path. This doesn't work if the package is not
available on the `GOPATH` and has a `-` in the name.

This commit makes two fixes to this behavior:

-   Always strip hyphens out of the import name.
-   If the base name and the package name do not match, always use a qualified
    import regardless of whether the package name was determined by importing
    it or guessed based on its base name.

This should ensure that even if we guess the package name wrong, the code
still works.

CHANGELOG.md

@@ -1,7 +1,17 @@
 Releases
 ========
 
-v0.2.0 (unreleased)
+v0.2.1 (2016-09-27)
+-------------------
+
+-   Plugin templates: Fixed a bug where imports in templates would use the base
+    name of the package even if it had a hyphen in it if it wasn't available on
+    the `GOPATH`.
+-   Plugin templates: Imports in generated code are now always qualified if the
+    package name doesn't match the base name.
+
+
+v0.2.0 (2016-09-09)
 -------------------
 
 -   Added a `-v`/`--version` flag.

internal/goast/package.go

@@ -23,6 +23,7 @@ package goast
 import (
 	"go/build"
 	"path/filepath"
+	"strings"
 )
 
 // DeterminePackageName determines the name of the package at the given import
@@ -35,7 +36,15 @@ func DeterminePackageName(importPath string) string {
 
 	pkg, err := build.Import(importPath, "", 0)
 	if err != nil {
-		return filepath.Base(importPath)
+		return guessPackageName(importPath)
 	}
 	return pkg.Name
 }
+
+func guessPackageName(importPath string) string {
+	packageName := filepath.Base(importPath)
+	if strings.HasSuffix(packageName, "-go") {
+		packageName = packageName[:len(packageName)-3]
+	}
+	return strings.Replace(packageName, "-", "_", -1)
+}

plugin/template.go

@@ -26,6 +26,7 @@ import (
 	"go/parser"
 	"go/printer"
 	"go/token"
+	"path/filepath"
 	"sort"
 	"text/template"
 
@@ -124,8 +125,9 @@ func (g *goFileGenerator) Import(path string) string {
 		importedName = fmt.Sprintf("%s%d", name, i)
 	}
 
-	if importedName == name {
-		// Package name is available so no named import
+	if importedName == name && name == filepath.Base(path) {
+		// Package name is available and matches the base name, so we won't do
+		// a named import. We'll use named imports for all other cases.
 		g.imports[path] = ""
 	} else {
 		g.imports[path] = importedName

plugin/template_test.go

@@ -182,6 +182,48 @@ func TestGoFileFromTemplate(t *testing.T) {
 				`}`,
 			),
 		},
+		{
+			desc: "import with dash",
+			template: `
+				package hello
+
+				<$yarpc := import "github.com/yarpc/yarpc-go">
+
+				func Hello() <$yarpc>.ReqMeta {
+					return nil
+				}
+			`,
+			wantBody: unlines(
+				`package hello`,
+				``,
+				`import yarpc "github.com/yarpc/yarpc-go"`,
+				``,
+				`func Hello() yarpc.ReqMeta {`,
+				`	return nil`,
+				`}`,
+			),
+		},
+		{
+			desc: "import with dash and conflict",
+			template: `
+				package hello
+
+				<$foo1 := import "github.com/thriftrw/thriftrw-go/foo-bar">
+				<$foo2 := import "github.com/thriftrw/thriftrw-go/foo_bar">
+
+				var x <$foo1>.Foo1 = <$foo2>.Foo2
+			`,
+			wantBody: unlines(
+				`package hello`,
+				``,
+				`import (`,
+				`	foo_bar "github.com/thriftrw/thriftrw-go/foo-bar"`,
+				`	foo_bar2 "github.com/thriftrw/thriftrw-go/foo_bar"`,
+				`)`,
+				``,
+				`var x foo_bar.Foo1 = foo_bar2.Foo2`,
+			),
+		},
 	}
 
 	for _, tt := range tests {

version.go

@@ -20,4 +20,4 @@
 
 package main
 
-var version = "v0.2.0"
+var version = "v0.2.1"