From 805ea0b301287a6b7122cd79f94f4c224ce2e5e1 Mon Sep 17 00:00:00 2001
From: seiya <20365512+seiyab@users.noreply.github.com>
Date: Sat, 20 Apr 2024 09:43:22 +0900
Subject: [PATCH 1/2] update test

---
 closecloser/testdata/{testing.go => testing_test.go} | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 rename closecloser/testdata/{testing.go => testing_test.go} (94%)

diff --git a/closecloser/testdata/testing.go b/closecloser/testdata/testing_test.go
similarity index 94%
rename from closecloser/testdata/testing.go
rename to closecloser/testdata/testing_test.go
index 6705078..df1e7c0 100644
--- a/closecloser/testdata/testing.go
+++ b/closecloser/testdata/testing_test.go
@@ -1,4 +1,4 @@
-package testdata
+package testdata_test
 
 import (
 	"os"

From 13b4005fa3f3abb5ffbdfa122202465193998dc7 Mon Sep 17 00:00:00 2001
From: seiya <20365512+seiyab@users.noreply.github.com>
Date: Sat, 20 Apr 2024 11:58:28 +0900
Subject: [PATCH 2/2] allow unclosed closer returned by method

---
 closecloser/analyzer.go | 12 +++-------
 closecloser/ignored.go  | 50 +++++++++--------------------------------
 closecloser/testing.go  | 46 -------------------------------------
 3 files changed, 14 insertions(+), 94 deletions(-)
 delete mode 100644 closecloser/testing.go

diff --git a/closecloser/analyzer.go b/closecloser/analyzer.go
index bccaa3d..b9aea2d 100644
--- a/closecloser/analyzer.go
+++ b/closecloser/analyzer.go
@@ -22,7 +22,7 @@ func run(pass *analysis.Pass) (any, error) {
 	c := newCloseCollector(pass, ipr)
 
 	ipr.Nodes([]ast.Node{
-		&ast.FuncDecl{},
+		&ast.File{},
 		&ast.AssignStmt{},
 		&ast.ValueSpec{},
 	},
@@ -32,14 +32,8 @@ func run(pass *analysis.Pass) (any, error) {
 			}
 			var ids []*ast.Ident
 			switch n := n.(type) {
-			case *ast.FuncDecl:
-				// NOTE:
-				// As of now, the analyzer does not report test functions.
-				// It is because:
-				//     - resources can be implicitly closed by t.Cleanup()
-				//     - people tend to write tests that do not close resources
-				// I think it is not desirable but this is practically helpful anyway.
-				if isTestOrTestHelper(n, pass) {
+			case *ast.File:
+				if utils.IsTestRelatedFile(n) {
 					return false
 				}
 			case *ast.AssignStmt:
diff --git a/closecloser/ignored.go b/closecloser/ignored.go
index 785d236..5d41e55 100644
--- a/closecloser/ignored.go
+++ b/closecloser/ignored.go
@@ -20,7 +20,9 @@ func shouldAllowUnclosed(expr ast.Expr, pass *analysis.Pass) bool {
 }
 
 func isAllowedCall(call *ast.CallExpr, pass *analysis.Pass) bool {
-	if isAllowedMethodCall(call, pass) {
+	if isMethodCall(call, pass) {
+		// closer returned by method call is allowed.
+		// its lifecycle might be managed by the receiver.
 		return true
 	}
 	if isAllowedFuncCall(call, pass) {
@@ -29,40 +31,22 @@ func isAllowedCall(call *ast.CallExpr, pass *analysis.Pass) bool {
 	return false
 }
 
-func isAllowedMethodCall(call *ast.CallExpr, pass *analysis.Pass) bool {
+func isMethodCall(call *ast.CallExpr, pass *analysis.Pass) bool {
 	fun, ok := call.Fun.(*ast.SelectorExpr)
 	if !ok {
 		return false
 	}
-	ty := pass.TypesInfo.TypeOf(fun.X)
-	if ty == nil {
-		return false
-	}
-
-	if implementsCloser(ty) {
-		return true
-	}
-
-	pt, ok := ty.(*types.Pointer)
-	if ok {
-		ty = pt.Elem()
-	}
-
-	nt, ok := ty.(*types.Named)
+	ident, ok := fun.X.(*ast.Ident)
 	if !ok {
-		return false
+		return true
 	}
-	obj := nt.Obj()
-	if obj.Pkg() == nil {
+	ty := pass.TypesInfo.ObjectOf(ident)
+	if ty == nil {
 		return false
 	}
-	m := method{
-		pkg:  obj.Pkg().Path(),
-		typ:  nt.Obj().Name(),
-		name: fun.Sel.Name,
-	}
-	_, ok = allowedMethods[m]
-	return ok
+
+	_, ok = ty.(*types.PkgName)
+	return !ok
 }
 
 func isAllowedFuncCall(call *ast.CallExpr, pass *analysis.Pass) bool {
@@ -86,18 +70,6 @@ func isAllowedFuncCall(call *ast.CallExpr, pass *analysis.Pass) bool {
 	return ok
 }
 
-var allowedMethods = map[method]struct{}{
-	{"os/exec", "Cmd", "StdinPipe"}:  {},
-	{"os/exec", "Cmd", "StdoutPipe"}: {},
-	{"os/exec", "Cmd", "StderrPipe"}: {},
-}
-
-type method struct {
-	pkg  string
-	typ  string
-	name string
-}
-
 var allowedFuns = map[fun]struct{}{}
 
 type fun struct {
diff --git a/closecloser/testing.go b/closecloser/testing.go
deleted file mode 100644
index 0ca9a08..0000000
--- a/closecloser/testing.go
+++ /dev/null
@@ -1,46 +0,0 @@
-package closecloser
-
-import (
-	"go/ast"
-	"go/types"
-
-	"golang.org/x/tools/go/analysis"
-)
-
-func isTestOrTestHelper(n *ast.FuncDecl, pass *analysis.Pass) bool {
-	ty := n.Type
-	if ty == nil || ty.Params == nil || len(ty.Params.List) == 0 {
-		return false
-	}
-	t := ty.Params.List[0].Type
-	return isTestingTB(t, pass)
-}
-
-func isTestingTB(t ast.Expr, pass *analysis.Pass) bool {
-	s, ok := t.(*ast.StarExpr)
-	if !ok {
-		return false
-	}
-	sel, ok := s.X.(*ast.SelectorExpr)
-	if !ok {
-		return false
-	}
-	pkg, ok := sel.X.(*ast.Ident)
-	if !ok {
-		return false
-	}
-	pkgObj := pass.TypesInfo.ObjectOf(pkg)
-	if pkgObj == nil {
-		return false
-	}
-	pn, ok := pkgObj.(*types.PkgName)
-	if !ok {
-		return false
-	}
-	if pn.Imported().Path() != "testing" {
-		return false
-	}
-
-	name := sel.Sel.Name
-	return name == "T" || name == "TB" || name == "B"
-}