Repository: golang/mock
Pull request #157 (2018-03-05 20:11:30)

Fix mockgen for package name not matching import path
This PR provides a fix to mockgen so it runs successfully on files that have package imports with names not matching import path suffix, e.g. package named `client` under import path `github.com/golang/mock/mockgen/tests/custom_package_name/client/v1`.

Full description is in `README.md` below in this PR.


Patch:
diff --git a/mockgen/model/model.go b/mockgen/model/model.go
index c8af9a3..1df8ef3 100644
--- a/mockgen/model/model.go
+++ b/mockgen/model/model.go
@@ -354,13 +354,6 @@ func typeFromType(t reflect.Type) (Type, error) {
 	}
 
 	if imp := t.PkgPath(); imp != "" {
-		// PkgPath might return a path that includes "vendor"
-		// These paths do not compile, so we need to remove everything
-		// up to and including "/vendor/"
-		// see https://github.com/golang/go/issues/12019
-		if i := strings.LastIndex(imp, "/vendor/"); i != -1 {
-			imp = imp[i+len("/vendor/"):]
-		}
 		return &NamedType{
 			Package: imp,
 			Type:    t.Name(),
diff --git a/mockgen/parse.go b/mockgen/parse.go
index 8ea6280..2350952 100644
--- a/mockgen/parse.go
+++ b/mockgen/parse.go
@@ -119,12 +119,14 @@ func (p *fileParser) parseAuxFiles(auxFiles string) error {
 		if len(parts) != 2 {
 			return fmt.Errorf("bad aux file spec: %v", kv)
 		}
-		file, err := parser.ParseFile(p.fileSet, parts[1], nil, 0)
+		pkg, fpath := parts[0], parts[1]
+
+		file, err := parser.ParseFile(p.fileSet, fpath, nil, 0)
 		if err != nil {
 			return err
 		}
 		p.auxFiles = append(p.auxFiles, file)
-		p.addAuxInterfacesFromFile(parts[0], file)
+		p.addAuxInterfacesFromFile(pkg, file)
 	}
 	return nil
 }
@@ -138,6 +140,8 @@ func (p *fileParser) addAuxInterfacesFromFile(pkg string, file *ast.File) {
 	}
 }
 
+// parseFile loads all file imports and auxiliary files import into the
+// fileParser, parses all file interfaces and returns package model.
 func (p *fileParser) parseFile(file *ast.File) (*model.Package, error) {
 	allImports := importsOfFile(file)
 	// Don't stomp imports provided by -imports. Those should take precedence.
@@ -170,6 +174,8 @@ func (p *fileParser) parseFile(file *ast.File) (*model.Package, error) {
 	}, nil
 }
 
+// parsePackage loads package specified by path, parses it and populates
+// corresponding imports and importedInterfaces into the fileParser.
 func (p *fileParser) parsePackage(path string) error {
 	var pkgs map[string]*ast.Package
 	if imp, err := build.Import(path, p.srcDir, build.FindOnly); err != nil {
@@ -417,30 +423,34 @@ func (p *fileParser) parseType(pkg string, typ ast.Expr) (model.Type, error) {
 // importsOfFile returns a map of package name to import path
 // of the imports in file.
 func importsOfFile(file *ast.File) map[string]string {
-	/* We have to make guesses about some imports, because imports are not required
-	 * to have names. Named imports are always certain. Unnamed imports are guessed
-	 * to have a name of the last path component; if the last path component has dots,
-	 * the first dot-delimited field is used as the name.
-	 */
-
 	m := make(map[string]string)
 	for _, is := range file.Imports {
-		var pkg string
+		var pkgName string
 		importPath := is.Path.Value[1 : len(is.Path.Value)-1] // remove quotes
 
 		if is.Name != nil {
+			// Named imports are always certain.
 			if is.Name.Name == "_" {
 				continue
 			}
-			pkg = removeDot(is.Name.Name)
+			pkgName = removeDot(is.Name.Name)
 		} else {
-			_, last := path.Split(importPath)
-			pkg = strings.SplitN(last, ".", 2)[0]
+			pkg, err := build.Import(importPath, "", 0)
+			if err != nil {
+				// Fallback to import path suffix. Note that this is uncertain.
+				log.Printf("failed to import package by path %s: %s - fallback to import path suffix", importPath, err.Error())
+				_, last := path.Split(importPath)
+				// If the last path component has dots, the first dot-delimited
+				// field is used as the name.
+				pkgName = strings.SplitN(last, ".", 2)[0]
+			}
+			pkgName = pkg.Name
 		}
-		if _, ok := m[pkg]; ok {
-			log.Fatalf("imported package collision: %q imported twice", pkg)
+
+		if _, ok := m[pkgName]; ok {
+			log.Fatalf("imported package collision: %q imported twice", pkgName)
 		}
-		m[pkg] = importPath
+		m[pkgName] = importPath
 	}
 	return m
 }


Test patch:
diff --git a/mockgen/parse_test.go b/mockgen/parse_test.go
new file mode 100644
index 0000000..f455ade
--- /dev/null
+++ b/mockgen/parse_test.go
@@ -0,0 +1,108 @@
+package main
+
+import (
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"testing"
+)
+
+func TestFileParser_ParseFile(t *testing.T) {
+	fs := token.NewFileSet()
+	file, err := parser.ParseFile(fs, "tests/custom_package_name/greeter/greeter.go", nil, 0)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	p := fileParser{
+		fileSet:            fs,
+		imports:            make(map[string]string),
+		importedInterfaces: make(map[string]map[string]*ast.InterfaceType),
+	}
+
+	pkg, err := p.parseFile(file)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	checkGreeterImports(t, p.imports)
+
+	expectedName := "greeter"
+	if pkg.Name != expectedName {
+		t.Fatalf("Expected name to be %v but got %v", expectedName, pkg.Name)
+	}
+
+	expectedInterfaceName := "InputMaker"
+	if pkg.Interfaces[0].Name != expectedInterfaceName {
+		t.Fatalf("Expected interface name to be %v but got %v", expectedInterfaceName, pkg.Interfaces[0].Name)
+	}
+}
+
+func TestFileParser_ParsePackage(t *testing.T) {
+	fs := token.NewFileSet()
+	_, err := parser.ParseFile(fs, "tests/custom_package_name/greeter/greeter.go", nil, 0)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	p := fileParser{
+		fileSet:            fs,
+		imports:            make(map[string]string),
+		importedInterfaces: make(map[string]map[string]*ast.InterfaceType),
+	}
+
+	err = p.parsePackage("github.com/golang/mock/mockgen/tests/custom_package_name/greeter")
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	checkGreeterImports(t, p.imports)
+}
+
+func TestImportsOfFile(t *testing.T) {
+	fs := token.NewFileSet()
+	file, err := parser.ParseFile(fs, "tests/custom_package_name/greeter/greeter.go", nil, 0)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	imports := importsOfFile(file)
+	checkGreeterImports(t, imports)
+}
+
+func checkGreeterImports(t *testing.T, imports map[string]string) {
+	// check that imports have stdlib package "fmt"
+	if fmtPackage, ok := imports["fmt"]; !ok {
+		t.Errorf("Expected imports to have key \"fmt\"")
+	} else {
+		expectedFmtPackage := "fmt"
+		if fmtPackage != expectedFmtPackage {
+			t.Errorf("Expected fmt key to have value %s but got %s", expectedFmtPackage, fmtPackage)
+		}
+	}
+
+	// check that imports have package named "validator"
+	if validatorPackage, ok := imports["validator"]; !ok {
+		t.Errorf("Expected imports to have key \"fmt\"")
+	} else {
+		expectedValidatorPackage := "github.com/golang/mock/mockgen/tests/custom_package_name/validator"
+		if validatorPackage != expectedValidatorPackage {
+			t.Errorf("Expected validator key to have value %s but got %s", expectedValidatorPackage, validatorPackage)
+		}
+	}
+
+	// check that imports have package named "client"
+	if clientPackage, ok := imports["client"]; !ok {
+		t.Errorf("Expected imports to have key \"client\"")
+	} else {
+		expectedClientPackage := "github.com/golang/mock/mockgen/tests/custom_package_name/client/v1"
+		if clientPackage != expectedClientPackage {
+			t.Errorf("Expected client key to have value %s but got %s", expectedClientPackage, clientPackage)
+		}
+	}
+
+	// check that imports don't have package named "v1"
+	if _, ok := imports["v1"]; ok {
+		t.Errorf("Expected import not to have key \"v1\"")
+	}
+}
diff --git a/mockgen/tests/custom_package_name/README.md b/mockgen/tests/custom_package_name/README.md
new file mode 100644
index 0000000..b16da21
--- /dev/null
+++ b/mockgen/tests/custom_package_name/README.md
@@ -0,0 +1,22 @@
+# Tests for custom package names
+
+This directory contains test for mockgen generating mocks when imported package
+name does not match import path suffix. For example, package with name "client"
+is located under import path "github.com/golang/mock/mockgen/tests/custom_package_name/client/v1".
+
+Prior to this patch:
+
+    $ go generate greeter/greeter.go
+    2018/03/05 22:44:52 Loading input failed: greeter.go:17:11: failed parsing returns: greeter.go:17:14: unknown package "client"
+    greeter/greeter.go:1: running "mockgen": exit status 1
+
+This can be fixed by manually providing `-imports` flag, like `-imports client=github.com/golang/mock/mockgen/tests/custom_package_name/client/v1`.
+But, mockgen should be able to automatically resolve package names in such situations.
+
+With this patch applied:
+
+    $ go generate greeter/greeter.go
+    $ echo $?
+    0
+
+Mockgen runs successfully, produced output is equal to [greeter_mock_test.go](greeter/greeter_mock_test.go) content.
diff --git a/mockgen/tests/custom_package_name/client/v1/client.go b/mockgen/tests/custom_package_name/client/v1/client.go
new file mode 100644
index 0000000..fc799d9
--- /dev/null
+++ b/mockgen/tests/custom_package_name/client/v1/client.go
@@ -0,0 +1,13 @@
+package client
+
+import "fmt"
+
+type Client struct{}
+
+func (c *Client) Greet(in GreetInput) string {
+	return fmt.Sprintf("Hello, %s!", in.Name)
+}
+
+type GreetInput struct {
+	Name string
+}
diff --git a/mockgen/tests/custom_package_name/greeter/greeter.go b/mockgen/tests/custom_package_name/greeter/greeter.go
new file mode 100644
index 0000000..8c2d293
--- /dev/null
+++ b/mockgen/tests/custom_package_name/greeter/greeter.go
@@ -0,0 +1,31 @@
+//go:generate mockgen -source greeter.go -destination greeter_mock_test.go -package greeter
+
+package greeter
+
+import (
+	// stdlib import
+	"fmt"
+
+	// non-matching import suffix and package name
+	"github.com/golang/mock/mockgen/tests/custom_package_name/client/v1"
+
+	//  matching import suffix and package name
+	"github.com/golang/mock/mockgen/tests/custom_package_name/validator"
+)
+
+type InputMaker interface {
+	MakeInput() client.GreetInput
+}
+
+type Greeter struct {
+	InputMaker InputMaker
+	Client     *client.Client
+}
+
+func (g *Greeter) Greet() (string, error) {
+	in := g.InputMaker.MakeInput()
+	if err := validator.Validate(in.Name); err != nil {
+		return "", fmt.Errorf("validation failed: %v", err)
+	}
+	return g.Client.Greet(in), nil
+}
diff --git a/mockgen/tests/custom_package_name/greeter/greeter_mock_test.go b/mockgen/tests/custom_package_name/greeter/greeter_mock_test.go
new file mode 100644
index 0000000..0bf0f46
--- /dev/null
+++ b/mockgen/tests/custom_package_name/greeter/greeter_mock_test.go
@@ -0,0 +1,46 @@
+// Code generated by MockGen. DO NOT EDIT.
+// Source: greeter.go
+
+// Package greeter is a generated GoMock package.
+package greeter
+
+import (
+	gomock "github.com/golang/mock/gomock"
+	v1 "github.com/golang/mock/mockgen/tests/custom_package_name/client/v1"
+	reflect "reflect"
+)
+
+// MockInputMaker is a mock of InputMaker interface
+type MockInputMaker struct {
+	ctrl     *gomock.Controller
+	recorder *MockInputMakerMockRecorder
+}
+
+// MockInputMakerMockRecorder is the mock recorder for MockInputMaker
+type MockInputMakerMockRecorder struct {
+	mock *MockInputMaker
+}
+
+// NewMockInputMaker creates a new mock instance
+func NewMockInputMaker(ctrl *gomock.Controller) *MockInputMaker {
+	mock := &MockInputMaker{ctrl: ctrl}
+	mock.recorder = &MockInputMakerMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use
+func (m *MockInputMaker) EXPECT() *MockInputMakerMockRecorder {
+	return m.recorder
+}
+
+// MakeInput mocks base method
+func (m *MockInputMaker) MakeInput() v1.GreetInput {
+	ret := m.ctrl.Call(m, "MakeInput")
+	ret0, _ := ret[0].(v1.GreetInput)
+	return ret0
+}
+
+// MakeInput indicates an expected call of MakeInput
+func (mr *MockInputMakerMockRecorder) MakeInput() *gomock.Call {
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MakeInput", reflect.TypeOf((*MockInputMaker)(nil).MakeInput))
+}
diff --git a/mockgen/tests/custom_package_name/greeter/greeter_test.go b/mockgen/tests/custom_package_name/greeter/greeter_test.go
new file mode 100644
index 0000000..f056ec8
--- /dev/null
+++ b/mockgen/tests/custom_package_name/greeter/greeter_test.go
@@ -0,0 +1,37 @@
+package greeter
+
+import (
+	"testing"
+
+	"github.com/golang/mock/gomock"
+	"github.com/golang/mock/mockgen/tests/custom_package_name/client/v1"
+)
+
+func TestGreeter_Greet(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	input := client.GreetInput{
+		Name: "Foo",
+	}
+
+	inputMaker := NewMockInputMaker(ctrl)
+	inputMaker.EXPECT().
+		MakeInput().
+		Return(input)
+
+	g := &Greeter{
+		InputMaker: inputMaker,
+		Client:     &client.Client{},
+	}
+
+	greeting, err := g.Greet()
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+
+	expected := "Hello, Foo!"
+	if greeting != expected {
+		t.Fatalf("Expected greeting to be %v but got %v", expected, greeting)
+	}
+}
diff --git a/mockgen/tests/custom_package_name/validator/validate.go b/mockgen/tests/custom_package_name/validator/validate.go
new file mode 100644
index 0000000..0617c4b
--- /dev/null
+++ b/mockgen/tests/custom_package_name/validator/validate.go
@@ -0,0 +1,5 @@
+package validator
+
+func Validate(s string) error {
+	return nil
+}
diff --git a/mockgen/tests/vendor_dep/README.md b/mockgen/tests/vendor_dep/README.md
deleted file mode 100644
index 9f9217d..0000000
--- a/mockgen/tests/vendor_dep/README.md
+++ /dev/null
@@ -1,2 +0,0 @@
-Test for [Issue#4](https://github.com/golang/mock/issues/4).
-Also see discussion on [#28](https://github.com/golang/mock/pull/28).
diff --git a/mockgen/tests/vendor_dep/doc.go b/mockgen/tests/vendor_dep/doc.go
deleted file mode 100644
index 595f679..0000000
--- a/mockgen/tests/vendor_dep/doc.go
+++ /dev/null
@@ -1,3 +0,0 @@
-package vendor_dep
-
-//go:generate mockgen -package vendor_dep -destination mock.go github.com/golang/mock/mockgen/tests/vendor_dep VendorsDep
diff --git a/mockgen/tests/vendor_dep/mock.go b/mockgen/tests/vendor_dep/mock.go
deleted file mode 100644
index aace06e..0000000
--- a/mockgen/tests/vendor_dep/mock.go
+++ /dev/null
@@ -1,46 +0,0 @@
-// Code generated by MockGen. DO NOT EDIT.
-// Source: github.com/golang/mock/mockgen/tests/vendor_dep (interfaces: VendorsDep)
-
-// Package vendor_dep is a generated GoMock package.
-package vendor_dep
-
-import (
-	a "a"
-	gomock "github.com/golang/mock/gomock"
-	reflect "reflect"
-)
-
-// MockVendorsDep is a mock of VendorsDep interface
-type MockVendorsDep struct {
-	ctrl     *gomock.Controller
-	recorder *MockVendorsDepMockRecorder
-}
-
-// MockVendorsDepMockRecorder is the mock recorder for MockVendorsDep
-type MockVendorsDepMockRecorder struct {
-	mock *MockVendorsDep
-}
-
-// NewMockVendorsDep creates a new mock instance
-func NewMockVendorsDep(ctrl *gomock.Controller) *MockVendorsDep {
-	mock := &MockVendorsDep{ctrl: ctrl}
-	mock.recorder = &MockVendorsDepMockRecorder{mock}
-	return mock
-}
-
-// EXPECT returns an object that allows the caller to indicate expected use
-func (m *MockVendorsDep) EXPECT() *MockVendorsDepMockRecorder {
-	return m.recorder
-}
-
-// Foo mocks base method
-func (m *MockVendorsDep) Foo() a.Ifc {
-	ret := m.ctrl.Call(m, "Foo")
-	ret0, _ := ret[0].(a.Ifc)
-	return ret0
-}
-
-// Foo indicates an expected call of Foo
-func (mr *MockVendorsDepMockRecorder) Foo() *gomock.Call {
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Foo", reflect.TypeOf((*MockVendorsDep)(nil).Foo))
-}
diff --git a/mockgen/tests/vendor_dep/vendor/a/a.go b/mockgen/tests/vendor_dep/vendor/a/a.go
deleted file mode 100644
index 156f70b..0000000
--- a/mockgen/tests/vendor_dep/vendor/a/a.go
+++ /dev/null
@@ -1,10 +0,0 @@
-package a
-
-type Ifc interface {
-	A(string) string
-	B(int) int
-	C(chan int) chan int
-	D(interface{})
-	E(map[string]interface{})
-	F([]float64)
-}
diff --git a/mockgen/tests/vendor_dep/vendor_dep.go b/mockgen/tests/vendor_dep/vendor_dep.go
deleted file mode 100644
index 412636a..0000000
--- a/mockgen/tests/vendor_dep/vendor_dep.go
+++ /dev/null
@@ -1,7 +0,0 @@
-package vendor_dep
-
-import "a"
-
-type VendorsDep interface {
-	Foo() a.Ifc
-}