Simplify linter logic (#4)

pkg/analyzer/analyzer.go

@@ -2,22 +2,17 @@ package analyzer
 
 import (
 	"go/ast"
-	"go/token"
-	"go/types"
 	"strings"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
-	"golang.org/x/tools/go/ast/astutil"
 	"golang.org/x/tools/go/ast/inspector"
 )
 
 const (
-	gomockControllerType = "github.com/golang/mock/gomock.Controller"
-	gomockPkg            = "github.com/golang/mock/gomock"
+	gomockControllerType = "mock/gomock.Controller"
 	finish               = "Finish"
-	newControllerMethod  = "NewController"
-	testingType          = "*testing.T"
+	reportMsg            = "calling Finish on gomock.Controller is no longer needed"
 )
 
 // New returns new gomockcontrollerfinish analyzer.
@@ -34,9 +29,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	inspector := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
 	nodeFilter := []ast.Node{(*ast.CallExpr)(nil)}
 
-	// map to track whether NewController is called for each testing function
-	newControllerCalledMap := make(map[string]bool)
-
 	inspector.Preorder(nodeFilter, func(n ast.Node) {
 		callExpr, ok := n.(*ast.CallExpr)
 		if !ok {
@@ -48,12 +40,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
 			return
 		}
 
-		// Check if it's a testing function
-		funcDecl, ok := enclosingFunction(pass, callExpr.Pos())
-		if !ok {
-			return
-		}
-
 		selectorExpr, ok := callExpr.Fun.(*ast.SelectorExpr)
 		if !ok {
 			return
@@ -64,24 +50,9 @@ func run(pass *analysis.Pass) (interface{}, error) {
 			return
 		}
 
-		// get pkg name if expression is from Go package
-		pkg, ok := pass.TypesInfo.ObjectOf(selIdent).(*types.PkgName)
-		// check if it's gomock pkg and method is NewController
-		if ok && strings.HasSuffix(pkg.Imported().Path(), gomockPkg) && selectorExpr.Sel.Name == newControllerMethod {
-			if len(callExpr.Args) == 1 {
-				if argType := pass.TypesInfo.TypeOf(callExpr.Args[0]); argType.String() == testingType {
-					// set newControllerCalled state for current testing function to true
-					newControllerCalledMap[funcDecl.Name.Name] = true
-				}
-			}
-		}
-
 		// check for unnecessary call to gomock.Controller.Finish()
 		if strings.HasSuffix(pass.TypesInfo.TypeOf(selIdent).String(), gomockControllerType) && selectorExpr.Sel.Name == finish {
-			// check if NewController is called for current testing function
-			if newControllerCalledMap[funcDecl.Name.Name] {
-				pass.Reportf(selectorExpr.Sel.Pos(), "since go1.14, if you are passing a testing.T to NewController then calling Finish on gomock.Controller is no longer needed")
-			}
+			pass.Reportf(callExpr.Pos(), reportMsg)
 		}
 	})
 
@@ -92,23 +63,3 @@ func run(pass *analysis.Pass) (interface{}, error) {
 func isTestFile(filename string) bool {
 	return strings.HasSuffix(filename, "_test.go")
 }
-
-// enclosingFunction returns the enclosing function declaration for a given position.
-func enclosingFunction(pass *analysis.Pass, pos token.Pos) (*ast.FuncDecl, bool) {
-	var file *ast.File
-
-	for _, f := range pass.Files {
-		if f.Pos() <= pos && pos <= f.End() {
-			file = f
-			break
-		}
-	}
-
-	path, _ := astutil.PathEnclosingInterval(file, pos, pos)
-	for _, node := range path {
-		if funcDecl, ok := node.(*ast.FuncDecl); ok {
-			return funcDecl, true
-		}
-	}
-	return nil, false
-}

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

@@ -8,19 +8,33 @@ import (
 
 func TestFinishCall(t *testing.T) {
 	mock := gomock.NewController(t)
-	mock.Finish() // want "since go1.14, if you are passing a testing.T to NewController then calling Finish on gomock.Controller is no longer needed"
+	mock.Finish() // want "calling Finish on gomock.Controller is no longer needed"
 }
 
 func TestFinishCallDefer(t *testing.T) {
 	mock := gomock.NewController(t)
-	defer mock.Finish() // want "since go1.14, if you are passing a testing.T to NewController then calling Finish on gomock.Controller is no longer needed"
+	defer mock.Finish() // want "calling Finish on gomock.Controller is no longer needed"
+}
+
+func TestFinishCallWithoutT(t *testing.T) {
+	mock := gomock.NewController(nil)
+	mock.Finish() // want "calling Finish on gomock.Controller is no longer needed"
+}
+
+func TestFinsihCallInAnotherFunction(t *testing.T) {
+	mock := gomock.NewController(t)
+	callFinish(mock)
+}
+
+func callFinish(mock *gomock.Controller) {
+	mock.Finish() // want "calling Finish on gomock.Controller is no longer needed"
 }
 
 func TestNoFinishCall(t *testing.T) {
 	gomock.NewController(t)
 }
 
-func TestFinishCallWithoutT(t *testing.T) {
-	mock := gomock.NewController(nil)
+func TestFinishCallOther(t *testing.T) {
+	mock := New()
 	mock.Finish()
 }

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

@@ -0,0 +1,14 @@
+package examples
+
+type testDummy struct{}
+
+func (td *testDummy) Finish() {}
+
+func New() *testDummy {
+	return &testDummy{}
+}
+
+func Finish() {
+	td := New()
+	td.Finish()
+}

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

@@ -9,19 +9,28 @@ import (
 
 func TestRenamedFinishCall(t *testing.T) {
 	mock := gomick.NewController(t)
-	mock.Finish() // want "since go1.14, if you are passing a testing.T to NewController then calling Finish on gomock.Controller is no longer needed"
+	mock.Finish() // want "calling Finish on gomock.Controller is no longer needed"
 }
 
 func TestRenamedFinishCallDefer(t *testing.T) {
 	mock := gomick.NewController(t)
-	defer mock.Finish() // want "since go1.14, if you are passing a testing.T to NewController then calling Finish on gomock.Controller is no longer needed"
+	defer mock.Finish() // want "calling Finish on gomock.Controller is no longer needed"
 }
 
-func TestRenamedNoFinishCall(t *testing.T) {
-	gomick.NewController(t)
+func TestRenamedFinishCallWithoutT(t *testing.T) {
+	mock := gomick.NewController(nil)
+	mock.Finish() // want "calling Finish on gomock.Controller is no longer needed"
 }
 
-func TestRenamedFinishCallWithoutT(t *testing.T) {
-	mock := gomock.NewController(nil)
-	mock.Finish()
+func TestRenamedFinsihCallInAnotherFunction(t *testing.T) {
+	mock := gomick.NewController(t)
+	renamedCallFinish(mock)
+}
+
+func renamedCallFinish(mock *gomock.Controller) {
+	mock.Finish() // want "calling Finish on gomock.Controller is no longer needed"
+}
+
+func TestRenamedNoFinishCall(t *testing.T) {
+	gomick.NewController(t)
 }