From 486fe6340c8acc600a44a3a698becb9725d578fb Mon Sep 17 00:00:00 2001
From: Abhinav Gupta <mail@abhinavg.net>
Date: Tue, 27 Sep 2016 10:01:38 -0700
Subject: [PATCH] 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              | 12 ++++++++++-
 internal/goast/package.go | 11 +++++++++-
 plugin/template.go        |  6 ++++--
 plugin/template_test.go   | 42 +++++++++++++++++++++++++++++++++++++++
 version.go                |  2 +-
 5 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f3eab7be..3cda16fc 100644
--- a/CHANGELOG.md
+++ b/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.
diff --git a/internal/goast/package.go b/internal/goast/package.go
index fc74c297..8fa50c26 100644
--- a/internal/goast/package.go
+++ b/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)
+}
diff --git a/plugin/template.go b/plugin/template.go
index 73238815..d6f0517c 100644
--- a/plugin/template.go
+++ b/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
diff --git a/plugin/template_test.go b/plugin/template_test.go
index 3d68a5c9..cc4d4323 100644
--- a/plugin/template_test.go
+++ b/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 {
diff --git a/version.go b/version.go
index a4bc1d95..3ac86f9f 100644
--- a/version.go
+++ b/version.go
@@ -20,4 +20,4 @@
 
 package main
 
-var version = "v0.2.0"
+var version = "v0.2.1"