feat: add noloopclosure

.golangci.reference.yml

@@ -1148,6 +1148,11 @@ linters-settings:
     # Default: 1
     block-size: 2
 
+  noloopclosure:
+    # Enable to lint test files (files that has _test.go suffix).
+    # Default: false
+    include-test-files: true
+
   nolintlint:
     # Disable to ensure that all nolint directives actually have an effect.
     # Default: false
@@ -1904,6 +1909,7 @@ linters:
     - nlreturn
     - noctx
     - nolintlint
+    - noloopclosure
     - nonamedreturns
     - nosnakecase
     - nosprintfhostport
@@ -2005,6 +2011,7 @@ linters:
     - nlreturn
     - noctx
     - nolintlint
+    - noloopclosure
     - nonamedreturns
     - nosnakecase
     - nosprintfhostport

go.mod

@@ -24,6 +24,7 @@ require (
 	github.com/daixiang0/gci v0.4.3
 	github.com/denis-tingaikin/go-header v0.4.3
 	github.com/esimonov/ifshort v1.0.4
+	github.com/fatanugraha/noloopclosure v0.1.1
 	github.com/fatih/color v1.13.0
 	github.com/firefart/nonamedreturns v1.0.4
 	github.com/fzipp/gocyclo v0.6.0
@@ -101,7 +102,7 @@ require (
 	github.com/yagipy/maintidx v1.0.0
 	github.com/yeya24/promlinter v0.2.0
 	gitlab.com/bosi/decorder v0.2.2
-	golang.org/x/tools v0.1.12-0.20220628192153-7743d1d949f1
+	golang.org/x/tools v0.1.12
 	gopkg.in/yaml.v3 v3.0.1
 	honnef.co/go/tools v0.3.2
 	mvdan.cc/gofumpt v0.3.1
@@ -171,8 +172,8 @@ require (
 	go.uber.org/zap v1.17.0 // indirect
 	golang.org/x/exp/typeparams v0.0.0-20220613132600-b0d781184e0d // indirect
 	golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
-	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect
-	golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 // indirect
+	golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 // indirect
+	golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f // indirect
 	golang.org/x/text v0.3.7 // indirect
 	google.golang.org/protobuf v1.28.0 // indirect
 	gopkg.in/ini.v1 v1.66.6 // indirect

pkg/config/linters_settings.go

@@ -75,6 +75,9 @@ var defaultLintersSettings = LintersSettings{
 	Nestif: NestifSettings{
 		MinComplexity: 5,
 	},
+	Noloopclosure: NoloopclosureSettings{
+		IncludeTestFiles: false,
+	},
 	NoLintLint: NoLintLintSettings{
 		RequireExplanation: false,
 		AllowLeadingSpace:  true,
@@ -162,6 +165,7 @@ type LintersSettings struct {
 	Nestif           NestifSettings
 	NilNil           NilNilSettings
 	Nlreturn         NlreturnSettings
+	Noloopclosure    NoloopclosureSettings
 	NoLintLint       NoLintLintSettings
 	NoNamedReturns   NoNamedReturnsSettings
 	ParallelTest     ParallelTestSettings
@@ -486,6 +490,10 @@ type NlreturnSettings struct {
 	BlockSize int `mapstructure:"block-size"`
 }
 
+type NoloopclosureSettings struct {
+	IncludeTestFiles bool `mapstructure:"include-test-files"`
+}
+
 type NoLintLintSettings struct {
 	RequireExplanation bool     `mapstructure:"require-explanation"`
 	AllowLeadingSpace  bool     `mapstructure:"allow-leading-space"`

pkg/golinters/noloopclosure.go

@@ -0,0 +1,30 @@
+package golinters
+
+import (
+	"golang.org/x/tools/go/analysis"
+
+	"github.com/golangci/golangci-lint/pkg/config"
+	"github.com/golangci/golangci-lint/pkg/golinters/goanalysis"
+
+	"github.com/fatanugraha/noloopclosure"
+)
+
+func NewNoLoopClosure(settings *config.NoloopclosureSettings) *goanalysis.Linter {
+	analyzer := noloopclosure.Analyzer
+
+	var cfg map[string]map[string]interface{}
+	if settings != nil {
+		cfg = map[string]map[string]interface{}{
+			analyzer.Name: {
+				"t": settings.IncludeTestFiles,
+			},
+		}
+	}
+
+	return goanalysis.NewLinter(
+		analyzer.Name,
+		analyzer.Doc,
+		[]*analysis.Analyzer{analyzer},
+		cfg,
+	).WithLoadMode(goanalysis.LoadModeTypesInfo)
+}

pkg/lint/lintersdb/manager.go

@@ -147,6 +147,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
 		nestifCfg           *config.NestifSettings
 		nilNilCfg           *config.NilNilSettings
 		nlreturnCfg         *config.NlreturnSettings
+		noloopclosureCfg    *config.NoloopclosureSettings
 		noLintLintCfg       *config.NoLintLintSettings
 		noNamedReturnsCfg   *config.NoNamedReturnsSettings
 		parallelTestCfg     *config.ParallelTestSettings
@@ -218,6 +219,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
 		nestifCfg = &m.cfg.LintersSettings.Nestif
 		nilNilCfg = &m.cfg.LintersSettings.NilNil
 		nlreturnCfg = &m.cfg.LintersSettings.Nlreturn
+		noloopclosureCfg = &m.cfg.LintersSettings.Noloopclosure
 		noLintLintCfg = &m.cfg.LintersSettings.NoLintLint
 		noNamedReturnsCfg = &m.cfg.LintersSettings.NoNamedReturns
 		preallocCfg = &m.cfg.LintersSettings.Prealloc
@@ -628,6 +630,12 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
 			WithPresets(linter.PresetPerformance, linter.PresetBugs).
 			WithURL("https://github.com/sonatard/noctx"),
 
+		linter.NewConfig(golinters.NewNoLoopClosure(noloopclosureCfg)).
+			WithSince("v1.48.0").
+			WithLoadForGoAnalysis().
+			WithPresets(linter.PresetBugs).
+			WithURL("https://github.com/fatanugraha/noloopclosure"),
+
 		linter.NewConfig(golinters.NewNoNamedReturns(noNamedReturnsCfg)).
 			WithSince("v1.46.0").
 			WithLoadForGoAnalysis().

test/testdata/noloopclosure.go

@@ -0,0 +1,152 @@
+//golangcitest:args -Enoloopclosure
+package testdata
+
+import _ "fmt"
+
+func noloopclosureForLoop() {
+	for {
+
+	}
+
+	for false {
+		// noop
+	}
+
+	for i := 0; ; i++ {
+		_ = func() {
+			_ = i // ERROR "found captured reference to loop variable inside a closure"
+		}
+	}
+
+	for i := 0; ; {
+		_ = i
+	}
+
+	for i := 0; i < 5; i++ {
+		_ = i
+	}
+
+	var i int
+	for i < 5 {
+		// Note: we ignore the condition clause to reduce false positive.
+		// Because it's hard to check which variable inside the condition that will be mutated.
+		_ = func() {
+			_ = i
+		}
+	}
+
+	for i := 0; i < 5; i++ {
+		_ = func() {
+			_ = i // ERROR "found captured reference to loop variable inside a closure"
+		}
+	}
+
+	for i := 0; i < 5; i++ {
+		for j := 0; j < 5; j++ {
+			_ = func() {
+				_ = i // ERROR "found captured reference to loop variable inside a closure"
+				_ = j // ERROR "found captured reference to loop variable inside a closure"
+			}
+		}
+
+		var j int
+		_ = func() {
+			_ = j
+		}
+	}
+
+	k := 5
+	for i, j := 0, 0; i < j; i, k = i+1, k+1 {
+		_ = func() {
+			// Not okay since it's listed in the PostInit.
+			_ = k // ERROR "found captured reference to loop variable inside a closure"
+		}
+
+		_ = func() {
+			_ = i // ERROR "found captured reference to loop variable inside a closure"
+
+			// Not okay, even if it's not part of the PostInit, as it's very likely that dev will mutate it somehow
+			// inside the for loop.
+			_ = j // ERROR "found captured reference to loop variable inside a closure"
+		}
+	}
+
+	// Can handle ast.SelectorExpr properly.
+	x := struct {
+		A int
+		B int
+	}{}
+	for x.A = 0; x.A < 5; x.A++ {
+		_ = func() {
+			_ = x.A // ERROR "found captured reference to loop variable inside a closure"
+			_ = x.B
+		}
+	}
+
+	// Can handle ast.ParenExpr properly.
+	for (k) = 0; k < 5; k++ {
+		_ = func() {
+			_ = k // ERROR "found captured reference to loop variable inside a closure"
+		}
+	}
+
+	// Can handle ast.StarExpr properly.
+	var p *int = new(int)
+	for *p = 0; *p < 5; (*p)++ {
+		_ = func() {
+			_ = *p // ERROR "found captured reference to loop variable inside a closure"
+			_ = p  // ERROR "found captured reference to loop variable inside a closure"
+		}
+	}
+
+	// Can handle ast.IndexExpr properly.
+	arrayOfInt := []int{1, 2, 3}
+	for arrayOfInt[1] = 0; arrayOfInt[1] < 5; arrayOfInt[1]++ {
+		_ = func() {
+			// Note: we will disallow any captured reference to the array, not just arrayOfInt[1]
+			// because we cant accurately know which index that is being mutated in compile time.
+			//
+			// Generally it's a good practice to not do this anyway hence we raise it as an issue.
+			_ = arrayOfInt[1] // ERROR "found captured reference to loop variable inside a closure"
+		}
+	}
+
+	mapOfInt := map[int]int{1: 1}
+	for mapOfInt[1] = 0; mapOfInt[1] < 5; mapOfInt[1]++ {
+		_ = func() {
+			// Note: disallow any captured reference to the map.
+			_ = mapOfInt[1] // ERROR "found captured reference to loop variable inside a closure"
+		}
+	}
+}
+
+func noloopclosureRangeLoop() {
+	for k, v := range map[string]int{} {
+		_ = func() {
+			_ = k // ERROR "found captured reference to loop variable inside a closure"
+			_ = v // ERROR "found captured reference to loop variable inside a closure"
+		}
+	}
+
+	for _, v := range map[string]int{} {
+		_ = func() {
+			_ = v // ERROR "found captured reference to loop variable inside a closure"
+		}
+	}
+
+	for k := range map[string]int{} {
+		_ = func() {
+			_ = k // ERROR "found captured reference to loop variable inside a closure"
+		}
+	}
+
+	for range map[string]int{} {
+	}
+
+	x := struct{ A string }{}
+	for x.A = range map[string]int{} {
+		_ = func() {
+			_ = x.A // ERROR "found captured reference to loop variable inside a closure"
+		}
+	}
+}