Use stricter check for controller type (#8)

pkg/analyzer/analyzer.go

@@ -10,11 +10,18 @@ import (
 )
 
 const (
-	gomockControllerType = "mock/gomock.Controller"
-	finish               = "Finish"
-	reportMsg            = "calling Finish on gomock.Controller is no longer needed"
+	reportMsg  = "calling Finish on gomock.Controller is no longer needed"
+	mock       = "mock"
+	controller = "gomock.Controller"
+	pkgLen     = 3
+	finish     = "Finish"
 )
 
+var pkgSourcesMap = map[string]bool{
+	"golang":      true,
+	"go.uber.org": true,
+}
+
 // New returns new gomockcontrollerfinish analyzer.
 func New() *analysis.Analyzer {
 	return &analysis.Analyzer{
@@ -46,10 +53,31 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		}
 
 		// check for unnecessary call to gomock.Controller.Finish()
-		if strings.HasSuffix(pass.TypesInfo.TypeOf(selIdent).String(), gomockControllerType) && selectorExpr.Sel.Name == finish {
+		if isValidType(pass.TypesInfo.TypeOf(selIdent).String()) && selectorExpr.Sel.Name == finish {
 			pass.Reportf(callExpr.Pos(), reportMsg)
 		}
 	})
 
 	return nil, nil
 }
+
+// isValidType checks whether t is a valid package source for gomock controller or not
+// currently supports golang/mock/gomock.Controller and go.uber.org/mock/gomock.Controller
+//
+// value of t can be *examples/vendor/go.uber.org/mock/gomock.Controller
+// hence the checking is only the last 3 part.
+func isValidType(t string) bool {
+	strs := strings.Split(t, "/")
+
+	if len(strs) < pkgLen {
+		return false
+	}
+
+	// get the last 3 elements
+	strs = strs[len(strs)-pkgLen:]
+
+	// first element has to be either golang or go.uber.org
+	// second element has to be mock
+	// third element has to be gomock.Controller
+	return pkgSourcesMap[strs[0]] && strs[1] == mock && strs[2] == controller
+}

pkg/analyzer/testdata/src/examples/gomock/controller.go

@@ -0,0 +1,20 @@
+package gomock
+
+// TestReporter is something that can be used to report test failures.
+// It imitates TestReporter from original gomock package.
+type TestReporter interface {
+	Errorf(format string, args ...interface{})
+	Fatalf(format string, args ...interface{})
+}
+
+// Controller imitates the original Controller from gomock package
+type Controller struct{}
+
+// Controller imitates the original Finish() on Controller from gomock package
+func (td *Controller) Finish() {}
+
+// NewController imitates the original NewController from gomock package.
+// It returns Controller imitation.
+func NewController(t TestReporter) *Controller {
+	return &Controller{}
+}

pkg/analyzer/testdata/src/examples/original_pkg_test.go

@@ -8,6 +8,8 @@ import (
 	"github.com/golang/mock/gomock"
 )
 
+// This file tests that the linter will still work if the gomock is from the original golang package
+
 func TestFinishCall(t *testing.T) {
 	mock := gomock.NewController(t)
 	mock.Finish() // want "calling Finish on gomock.Controller is no longer needed"

pkg/analyzer/testdata/src/examples/regular.go

@@ -8,6 +8,7 @@ func New() *testDummy {
 	return &testDummy{}
 }
 
+// finish call from the package that defines it
 func Finish() {
 	td := New()
 	td.Finish()

pkg/analyzer/testdata/src/examples/renamed_pkg_test.go

@@ -6,6 +6,8 @@ import (
 	gomick "github.com/golang/mock/gomock"
 )
 
+// This file tests that the linter will still work even if the gomock from the original golang package is renamed
+
 func TestRenamedFinishCall(t *testing.T) {
 	mock := gomick.NewController(t)
 	mock.Finish() // want "calling Finish on gomock.Controller is no longer needed"

pkg/analyzer/testdata/src/examples/similar_gomock_test.go

@@ -0,0 +1,37 @@
+package examples_test
+
+import (
+	"testing"
+
+	"examples/gomock" // gomock imitation
+)
+
+// This file tests that the linter won't be tricked by another package source that has the same Controller object as the original package.
+
+func TestSimilarFinishCall(t *testing.T) {
+	mock := gomock.NewController(t)
+	mock.Finish()
+}
+
+func TestSimilarFinishCallDefer(t *testing.T) {
+	mock := gomock.NewController(t)
+	defer mock.Finish()
+}
+
+func TestSimilarFinishCallWithoutT(t *testing.T) {
+	mock := gomock.NewController(nil)
+	mock.Finish()
+}
+
+func TestSimilarFinsihCallInAnotherFunction(t *testing.T) {
+	mock := gomock.NewController(t)
+	callSimilarFinish(mock)
+}
+
+func callSimilarFinish(mock *gomock.Controller) {
+	mock.Finish()
+}
+
+func TestSimilarNoFinishCall(t *testing.T) {
+	gomock.NewController(t)
+}

pkg/analyzer/testdata/src/examples/uber_mock_pkg_test.go

@@ -7,6 +7,8 @@ import (
 	"go.uber.org/mock/gomock"
 )
 
+// This file tests that the linter will still work even if the gomock is from the go.uber.org fork
+
 func TestUberFinishCall(t *testing.T) {
 	mock := gomock.NewController(t)
 	mock.Finish() // want "calling Finish on gomock.Controller is no longer needed"

pkg/analyzer/testdata/src/examples/uber_mock_renamed_pkg_test.go

@@ -6,6 +6,8 @@ import (
 	gomick "go.uber.org/mock/gomock"
 )
 
+// This file tests that the linter will still work even if the gomock that is from the go.uber.org fork is renamed to something else
+
 func TestUberRenamedFinishCall(t *testing.T) {
 	mock := gomick.NewController(t)
 	mock.Finish() // want "calling Finish on gomock.Controller is no longer needed"