Repository: golang/mock
Pull request #376 (2019-12-31 00:52:47)

refactor method names for reflect/source
The reflect method should be renamed so it does not collide with
the reflect package name. As is, it makes import reflect in the
code non-idiomatic. Also renamed sourceMode to stay consistent.

Noticed this here: #371


Patch:
diff --git a/ci/check_go_generate.sh b/ci/check_go_generate.sh
index a0212e2..d906bd5 100755
--- a/ci/check_go_generate.sh
+++ b/ci/check_go_generate.sh
@@ -9,6 +9,7 @@
 
 set -euo pipefail
 
+BASE_DIR="$PWD"
 TEMP_DIR=$( mktemp -d )
 function cleanup() {
     rm -rf "${TEMP_DIR}"
@@ -16,8 +17,9 @@ function cleanup() {
 trap cleanup EXIT
 
 cp -r . "${TEMP_DIR}/"
+cd $TEMP_DIR
 go generate ./...
-if ! diff -r . "${TEMP_DIR}"; then
+if ! diff -r . "${BASE_DIR}"; then
     echo
     echo "The generated files aren't up to date."
     echo "Update them with the 'go generate ./...' command."
diff --git a/mockgen/mockgen.go b/mockgen/mockgen.go
index 18aeb3d..d5090c0 100644
--- a/mockgen/mockgen.go
+++ b/mockgen/mockgen.go
@@ -62,13 +62,13 @@ func main() {
 	var pkg *model.Package
 	var err error
 	if *source != "" {
-		pkg, err = parseFile(*source)
+		pkg, err = sourceMode(*source)
 	} else {
 		if flag.NArg() != 2 {
 			usage()
 			log.Fatal("Expected exactly two arguments")
 		}
-		pkg, err = reflect(flag.Arg(0), strings.Split(flag.Arg(1), ","))
+		pkg, err = reflectMode(flag.Arg(0), strings.Split(flag.Arg(1), ","))
 	}
 	if err != nil {
 		log.Fatalf("Loading input failed: %v", err)
@@ -92,11 +92,11 @@ func main() {
 		dst = f
 	}
 
-	packageName := *packageOut
-	if packageName == "" {
+	outputPackageName := *packageOut
+	if outputPackageName == "" {
 		// pkg.Name in reflect mode is the base name of the import path,
 		// which might have characters that are illegal to have in package names.
-		packageName = "mock_" + sanitize(pkg.Name)
+		outputPackageName = "mock_" + sanitize(pkg.Name)
 	}
 
 	// outputPackagePath represents the fully qualified name of the package of
@@ -137,7 +137,7 @@ func main() {
 
 		g.copyrightHeader = string(header)
 	}
-	if err := g.Generate(pkg, packageName, outputPackagePath); err != nil {
+	if err := g.Generate(pkg, outputPackageName, outputPackagePath); err != nil {
 		log.Fatalf("Failed generating mock: %v", err)
 	}
 	if _, err := dst.Write(g.Output()); err != nil {
@@ -234,8 +234,8 @@ func sanitize(s string) string {
 	return t
 }
 
-func (g *generator) Generate(pkg *model.Package, pkgName string, outputPackagePath string) error {
-	if pkgName != pkg.Name {
+func (g *generator) Generate(pkg *model.Package, outputPkgName string, outputPackagePath string) error {
+	if outputPkgName != pkg.Name {
 		outputPackagePath = ""
 	}
 
@@ -294,22 +294,27 @@ func (g *generator) Generate(pkg *model.Package, pkgName string, outputPackagePa
 			i++
 		}
 
+		// Avoid importing package if source pkg == output pkg
+		if pth == pkg.PkgPath && outputPkgName == pkg.Name {
+			continue
+		}
+
 		g.packageMap[pth] = pkgName
 		localNames[pkgName] = true
 	}
 
 	if *writePkgComment {
-		g.p("// Package %v is a generated GoMock package.", pkgName)
+		g.p("// Package %v is a generated GoMock package.", outputPkgName)
 	}
-	g.p("package %v", pkgName)
+	g.p("package %v", outputPkgName)
 	g.p("")
 	g.p("import (")
 	g.in()
-	for pkgPath, pkg := range g.packageMap {
+	for pkgPath, pkgName := range g.packageMap {
 		if pkgPath == outputPackagePath {
 			continue
 		}
-		g.p("%v %q", pkg, pkgPath)
+		g.p("%v %q", pkgName, pkgPath)
 	}
 	for _, pkgPath := range pkg.DotImports {
 		g.p(". %q", pkgPath)
diff --git a/mockgen/model/model.go b/mockgen/model/model.go
index 9c143cf..88783cf 100644
--- a/mockgen/model/model.go
+++ b/mockgen/model/model.go
@@ -29,6 +29,7 @@ const pkgPath = "github.com/golang/mock/mockgen/model"
 // Package is a Go package. It may be a subset.
 type Package struct {
 	Name       string
+	PkgPath    string
 	Interfaces []*Interface
 	DotImports []string
 }
diff --git a/mockgen/parse.go b/mockgen/parse.go
index b0403e5..e35d16f 100644
--- a/mockgen/parse.go
+++ b/mockgen/parse.go
@@ -41,7 +41,8 @@ var (
 
 // TODO: simplify error reporting
 
-func parseFile(source string) (*model.Package, error) {
+// sourceMode generates mocks via source file.
+func sourceMode(source string) (*model.Package, error) {
 	srcDir, err := filepath.Abs(filepath.Dir(source))
 	if err != nil {
 		return nil, fmt.Errorf("failed getting source directory: %v", err)
@@ -187,6 +188,7 @@ func (p *fileParser) parseFile(importPath string, file *ast.File) (*model.Packag
 	}
 	return &model.Package{
 		Name:       file.Name.String(),
+		PkgPath:    importPath,
 		Interfaces: is,
 		DotImports: dotImports,
 	}, nil
diff --git a/mockgen/reflect.go b/mockgen/reflect.go
index 15f819c..47f95ef 100644
--- a/mockgen/reflect.go
+++ b/mockgen/reflect.go
@@ -132,7 +132,8 @@ func runInDir(program []byte, dir string) (*model.Package, error) {
 	return run(filepath.Join(tmpDir, progBinary))
 }
 
-func reflect(importPath string, symbols []string) (*model.Package, error) {
+// reflectMode generates mocks via reflection on an interface.
+func reflectMode(importPath string, symbols []string) (*model.Package, error) {
 	// TODO: sanity check arguments
 
 	if *execOnly != "" {


Test patch:
diff --git a/mockgen/mockgen_test.go b/mockgen/mockgen_test.go
index 8d926db..1c139d7 100644
--- a/mockgen/mockgen_test.go
+++ b/mockgen/mockgen_test.go
@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+	"reflect"
 	"regexp"
 	"strings"
 	"testing"
@@ -327,29 +328,9 @@ func TestGetArgNames(t *testing.T) {
 			g := generator{}
 
 			result := g.getArgNames(testCase.method)
-			if !testEqSliceStr(t, result, testCase.expected) {
+			if !reflect.DeepEqual(result, testCase.expected) {
 				t.Fatalf("expected %s, got %s", result, testCase.expected)
 			}
 		})
 	}
 }
-
-func testEqSliceStr(t *testing.T, a, b []string) bool {
-	t.Helper()
-
-	if a == nil || b == nil {
-		return false
-	}
-
-	if len(a) != len(b) {
-		return false
-	}
-
-	for i := range a {
-		if a[i] != b[i] {
-			return false
-		}
-	}
-
-	return true
-}
diff --git a/mockgen/parse_test.go b/mockgen/parse_test.go
index a41fa0c..dab22fa 100644
--- a/mockgen/parse_test.go
+++ b/mockgen/parse_test.go
@@ -110,6 +110,6 @@ func checkGreeterImports(t *testing.T, imports map[string]string) {
 func Benchmark_parseFile(b *testing.B) {
 	source := "internal/tests/performance/big_interface/big_interface.go"
 	for n := 0; n < b.N; n++ {
-		parseFile(source)
+		sourceMode(source)
 	}
 }