Merge pull request #157 from hypnoglow/fix-mockgen-custom-import-paths

Fix mockgen for package name not matching import path

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
 }

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\"")
+	}
+}

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.

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
+}

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
+}

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)
+	}
+}

mockgen/tests/custom_package_name/validator/validate.go

@@ -0,0 +1,5 @@
+package validator
+
+func Validate(s string) error {
+	return nil
+}