Fix var name generation to avoid conflict (#145)

When the type and the package name is the same for an anonymous
parameter (ex: time.Time), and there are more than 1 such parameters,
the generated name for both was the same. And the generated code would
not be valid.

Fix the bug by ensuring the parameter name does not conflict with
package imports first before checking against other parameter names.

internal/registry/method_scope.go

@@ -23,51 +23,18 @@ type MethodScope struct {
 // without conflict with other variables and imported packages. It also
 // adds the relevant imports to the registry for each added variable.
 func (m *MethodScope) AddVar(vr *types.Var, suffix string) *Var {
-	name := vr.Name()
-	if name == "" || name == "_" {
-		name = generateVarName(vr.Type())
-	}
-
-	name += suffix
-
-	switch name {
-	case "mock", "callInfo", "break", "default", "func", "interface", "select", "case", "defer", "go", "map", "struct",
-		"chan", "else", "goto", "package", "switch", "const", "fallthrough", "if", "range", "type", "continue", "for",
-		"import", "return", "var":
-		name += "MoqParam"
-	}
-
-	if _, ok := m.searchVar(name); ok || m.conflicted[name] {
-		return m.addDisambiguatedVar(vr, name)
-	}
-
-	return m.addVar(vr, name)
-}
-
-func (m *MethodScope) addDisambiguatedVar(vr *types.Var, suggested string) *Var {
-	n := 1
-	for {
-		// Keep incrementing the suffix until we find a name which is unused.
-		if _, ok := m.searchVar(suggested + strconv.Itoa(n)); !ok {
-			break
-		}
-		n++
-	}
-
-	name := suggested + strconv.Itoa(n)
-	if n == 1 {
-		conflict, _ := m.searchVar(suggested)
-		conflict.Name += "1"
-		name = suggested + "2"
-		m.conflicted[suggested] = true
-	}
-
-	return m.addVar(vr, name)
-}
-
-func (m *MethodScope) addVar(vr *types.Var, name string) *Var {
 	imports := make(map[string]*Package)
 	m.populateImports(vr.Type(), imports)
+	m.resolveImportVarConflicts(imports)
+
+	name := varName(vr, suffix)
+	// Ensure that the var name does not conflict with a package import.
+	if _, ok := m.registry.searchImport(name); ok {
+		name += "MoqParam"
+	}
+	if _, ok := m.searchVar(name); ok || m.conflicted[name] {
+		name = m.resolveVarNameConflict(name)
+	}
 
 	v := Var{
 		vr:         vr,
@@ -76,10 +43,26 @@ func (m *MethodScope) addVar(vr *types.Var, name string) *Var {
 		Name:       name,
 	}
 	m.vars = append(m.vars, &v)
-	m.resolveImportVarConflicts(&v)
 	return &v
 }
 
+func (m *MethodScope) resolveVarNameConflict(suggested string) string {
+	for n := 1; ; n++ {
+		_, ok := m.searchVar(suggested + strconv.Itoa(n))
+		if ok {
+			continue
+		}
+
+		if n == 1 {
+			conflict, _ := m.searchVar(suggested)
+			conflict.Name += "1"
+			m.conflicted[suggested] = true
+			n++
+		}
+		return suggested + strconv.Itoa(n)
+	}
+}
+
 func (m MethodScope) searchVar(name string) (*Var, bool) {
 	for _, v := range m.vars {
 		if v.Name == name {
@@ -139,15 +122,12 @@ func (m MethodScope) populateImports(t types.Type, imports map[string]*Package)
 	}
 }
 
-func (m MethodScope) resolveImportVarConflicts(v *Var) {
+// resolveImportVarConflicts ensures that all the newly added imports do not
-	// Ensure that the newly added var does not conflict with a package import
+// conflict with any of the existing vars.
-	// which was added earlier.
+func (m MethodScope) resolveImportVarConflicts(imports map[string]*Package) {
-	if _, ok := m.registry.searchImport(v.Name); ok {
-		v.Name += "MoqParam"
-	}
 	// Ensure that all the newly added imports do not conflict with any of the
 	// existing vars.
-	for _, imprt := range v.imports {
+	for _, imprt := range imports {
 		if v, ok := m.searchVar(imprt.Qualifier()); ok {
 			v.Name += "MoqParam"
 		}

internal/registry/var.go

@@ -38,7 +38,25 @@ func (v Var) packageQualifier(pkg *types.Package) string {
 	return v.imports[path].Qualifier()
 }
 
-// generateVarName generates a name for the variable using the type
+func varName(vr *types.Var, suffix string) string {
+	name := vr.Name()
+	if name != "" && name != "_" {
+		return name + suffix
+	}
+
+	name = varNameForType(vr.Type()) + suffix
+
+	switch name {
+	case "mock", "callInfo", "break", "default", "func", "interface", "select", "case", "defer", "go", "map", "struct",
+		"chan", "else", "goto", "package", "switch", "const", "fallthrough", "if", "range", "type", "continue", "for",
+		"import", "return", "var":
+		name += "MoqParam"
+	}
+
+	return name
+}
+
+// varNameForType generates a name for the variable using the type
 // information.
 //
 // Examples:
@@ -49,12 +67,12 @@ func (v Var) packageQualifier(pkg *types.Package) string {
 // - map[string]int -> stringToInt
 // - error -> err
 // - a.MyType -> myType
-func generateVarName(t types.Type) string {
+func varNameForType(t types.Type) string {
 	nestedType := func(t types.Type) string {
 		if t, ok := t.(*types.Basic); ok {
 			return deCapitalise(t.String())
 		}
-		return generateVarName(t)
+		return varNameForType(t)
 	}
 
 	switch t := t.(type) {
@@ -83,7 +101,7 @@ func generateVarName(t types.Type) string {
 		return "val"
 
 	case *types.Pointer:
-		return generateVarName(t.Elem())
+		return varNameForType(t.Elem())
 
 	case *types.Signature:
 		return "fn"

pkg/moq/testpackages/paramconflict/iface.go

@@ -1,5 +1,7 @@
 package paramconflict
 
+import "time"
+
 type Interface interface {
-	Method(string, bool, string, bool, int, int32, int64, float32, float64)
+	Method(string, bool, string, bool, int, int32, int64, float32, float64, time.Time, time.Time)
 }

pkg/moq/testpackages/paramconflict/iface_moq.golden.go

@@ -5,6 +5,7 @@ package paramconflict
 
 import (
 	"sync"
+	"time"
 )
 
 // Ensure, that InterfaceMock does implement Interface.
@@ -17,7 +18,7 @@ var _ Interface = &InterfaceMock{}
 //
 // 		// make and configure a mocked Interface
 // 		mockedInterface := &InterfaceMock{
-// 			MethodFunc: func(s1 string, b1 bool, s2 string, b2 bool, n1 int, n2 int32, n3 int64, f1 float32, f2 float64)  {
+// 			MethodFunc: func(s1 string, b1 bool, s2 string, b2 bool, n1 int, n2 int32, n3 int64, f1 float32, f2 float64, timeMoqParam1 time.Time, timeMoqParam2 time.Time)  {
 // 				panic("mock out the Method method")
 // 			},
 // 		}
@@ -28,7 +29,7 @@ var _ Interface = &InterfaceMock{}
 // 	}
 type InterfaceMock struct {
 	// MethodFunc mocks the Method method.
-	MethodFunc func(s1 string, b1 bool, s2 string, b2 bool, n1 int, n2 int32, n3 int64, f1 float32, f2 float64)
+	MethodFunc func(s1 string, b1 bool, s2 string, b2 bool, n1 int, n2 int32, n3 int64, f1 float32, f2 float64, timeMoqParam1 time.Time, timeMoqParam2 time.Time)
 
 	// calls tracks calls to the methods.
 	calls struct {
@@ -52,67 +53,79 @@ type InterfaceMock struct {
 			F1 float32
 			// F2 is the f2 argument value.
 			F2 float64
+			// TimeMoqParam1 is the timeMoqParam1 argument value.
+			TimeMoqParam1 time.Time
+			// TimeMoqParam2 is the timeMoqParam2 argument value.
+			TimeMoqParam2 time.Time
 		}
 	}
 	lockMethod sync.RWMutex
 }
 
 // Method calls MethodFunc.
-func (mock *InterfaceMock) Method(s1 string, b1 bool, s2 string, b2 bool, n1 int, n2 int32, n3 int64, f1 float32, f2 float64) {
+func (mock *InterfaceMock) Method(s1 string, b1 bool, s2 string, b2 bool, n1 int, n2 int32, n3 int64, f1 float32, f2 float64, timeMoqParam1 time.Time, timeMoqParam2 time.Time) {
 	if mock.MethodFunc == nil {
 		panic("InterfaceMock.MethodFunc: method is nil but Interface.Method was just called")
 	}
 	callInfo := struct {
-		S1 string
+		S1            string
-		B1 bool
+		B1            bool
-		S2 string
+		S2            string
-		B2 bool
+		B2            bool
-		N1 int
+		N1            int
-		N2 int32
+		N2            int32
-		N3 int64
+		N3            int64
-		F1 float32
+		F1            float32
-		F2 float64
+		F2            float64
+		TimeMoqParam1 time.Time
+		TimeMoqParam2 time.Time
 	}{
-		S1: s1,
+		S1:            s1,
-		B1: b1,
+		B1:            b1,
-		S2: s2,
+		S2:            s2,
-		B2: b2,
+		B2:            b2,
-		N1: n1,
+		N1:            n1,
-		N2: n2,
+		N2:            n2,
-		N3: n3,
+		N3:            n3,
-		F1: f1,
+		F1:            f1,
-		F2: f2,
+		F2:            f2,
+		TimeMoqParam1: timeMoqParam1,
+		TimeMoqParam2: timeMoqParam2,
 	}
 	mock.lockMethod.Lock()
 	mock.calls.Method = append(mock.calls.Method, callInfo)
 	mock.lockMethod.Unlock()
-	mock.MethodFunc(s1, b1, s2, b2, n1, n2, n3, f1, f2)
+	mock.MethodFunc(s1, b1, s2, b2, n1, n2, n3, f1, f2, timeMoqParam1, timeMoqParam2)
 }
 
 // MethodCalls gets all the calls that were made to Method.
 // Check the length with:
 //     len(mockedInterface.MethodCalls())
 func (mock *InterfaceMock) MethodCalls() []struct {
-	S1 string
+	S1            string
-	B1 bool
+	B1            bool
-	S2 string
+	S2            string
-	B2 bool
+	B2            bool
-	N1 int
+	N1            int
-	N2 int32
+	N2            int32
-	N3 int64
+	N3            int64
-	F1 float32
+	F1            float32
-	F2 float64
+	F2            float64
+	TimeMoqParam1 time.Time
+	TimeMoqParam2 time.Time
 } {
 	var calls []struct {
-		S1 string
+		S1            string
-		B1 bool
+		B1            bool
-		S2 string
+		S2            string
-		B2 bool
+		B2            bool
-		N1 int
+		N1            int
-		N2 int32
+		N2            int32
-		N3 int64
+		N3            int64
-		F1 float32
+		F1            float32
-		F2 float64
+		F2            float64
+		TimeMoqParam1 time.Time
+		TimeMoqParam2 time.Time
 	}
 	mock.lockMethod.RLock()
 	calls = mock.calls.Method