feat: add goconcurrencylint

.golangci.next.reference.yml

@@ -57,6 +57,7 @@ linters:
     - gochecknoinits
     - gochecksumtype
     - gocognit
+    - goconcurrencylint
     - goconst
     - gocritic
     - gocyclo
@@ -174,6 +175,7 @@ linters:
     - gochecknoinits
     - gochecksumtype
     - gocognit
+    - goconcurrencylint
     - goconst
     - gocritic
     - gocyclo

go.mod

@@ -114,6 +114,7 @@ require (
 	github.com/ryancurrah/gomodguard v1.4.1
 	github.com/ryancurrah/gomodguard/v2 v2.1.3
 	github.com/ryanrolds/sqlclosecheck v0.6.0
+	github.com/sanbricio/goconcurrencylint v0.3.0
 	github.com/sanposhiho/wastedassign/v2 v2.1.0
 	github.com/santhosh-tekuri/jsonschema/v6 v6.0.2
 	github.com/sashamelentyev/interfacebloat v1.1.0

jsonschema/golangci.next.jsonschema.json

@@ -865,6 +865,7 @@
             "gochecknoinits",
             "gochecksumtype",
             "gocognit",
+            "goconcurrencylint",
             "goconst",
             "gocritic",
             "gocyclo",

pkg/golinters/goconcurrencylint/goconcurrencylint.go

@@ -0,0 +1,14 @@
+package goconcurrencylint
+
+import (
+	concurrencyanalyzer "github.com/sanbricio/goconcurrencylint/pkg/analyzer"
+
+	"github.com/golangci/golangci-lint/v2/pkg/goanalysis"
+)
+
+func New() *goanalysis.Linter {
+	return goanalysis.
+		NewLinterFromAnalyzer(concurrencyanalyzer.Analyzer).
+		WithDesc("Detects incorrect sync.Mutex, sync.RWMutex, and sync.WaitGroup usage").
+		WithLoadMode(goanalysis.LoadModeTypesInfo)
+}

pkg/golinters/goconcurrencylint/goconcurrencylint_integration_test.go

@@ -0,0 +1,11 @@
+package goconcurrencylint
+
+import (
+	"testing"
+
+	"github.com/golangci/golangci-lint/v2/test/testshared/integration"
+)
+
+func TestFromTestdata(t *testing.T) {
+	integration.RunTestdata(t)
+}

pkg/golinters/goconcurrencylint/goconcurrencylint_packagelevel_integration_test.go

@@ -0,0 +1,29 @@
+package goconcurrencylint
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/golangci/golangci-lint/v2/pkg/exitcodes"
+	"github.com/golangci/golangci-lint/v2/test/testshared"
+)
+
+func TestPackageLevelAcrossFiles(t *testing.T) {
+	binPath := testshared.InstallGolangciLint(t)
+
+	target := filepath.Join("testdata", "packagelevel")
+
+	testshared.NewRunnerBuilder(t).
+		WithBinPath(binPath).
+		WithNoConfig().
+		WithArgs("--default=none", "--show-stats=false", "-Egoconcurrencylint").
+		WithTargetPath(target).
+		Runner().
+		Run().
+		ExpectExitCode(exitcodes.IssuesFound).
+		ExpectOutputContains(
+			`goconcurrencylint_packagelevel_usage.go:4:2: mutex 'sharedPackageMu' is locked but not unlocked`,
+			`goconcurrencylint_packagelevel_usage.go:8:2: waitgroup 'sharedPackageWG' has Add without corresponding Done`,
+		).
+		ExpectOutputNotContains("undefined: sharedPackage")
+}

pkg/golinters/goconcurrencylint/testdata/goconcurrencylint.go

@@ -0,0 +1,28 @@
+//golangcitest:args -Egoconcurrencylint
+package testdata
+
+import "sync"
+
+var packageMu1 sync.Mutex
+var packageWG1 sync.WaitGroup
+
+func badPackageMutex() {
+	packageMu1.Lock() // want "mutex 'packageMu1' is locked but not unlocked"
+}
+
+func badPackageWaitGroup() {
+	packageWG1.Add(1) // want "waitgroup 'packageWG1' has Add without corresponding Done"
+	packageWG1.Wait()
+}
+
+func badWaitGroupGoAfterWait() {
+	var wg sync.WaitGroup
+	wg.Wait()
+	wg.Go(func() {}) // want "waitgroup 'wg' Go called after Wait"
+}
+
+func goodWaitGroupGo() {
+	var wg sync.WaitGroup
+	wg.Go(func() {})
+	wg.Wait()
+}

pkg/golinters/goconcurrencylint/testdata/goconcurrencylint_mutex.go

@@ -0,0 +1,96 @@
+//golangcitest:args -Egoconcurrencylint
+package testdata
+
+import "sync"
+
+type safeMap struct {
+	mu sync.Mutex
+	rw sync.RWMutex
+}
+
+func badDoubleUnlockCase() {
+	var mu sync.Mutex
+	mu.Lock()
+	mu.Unlock()
+	mu.Unlock() // want "mutex 'mu' is unlocked but not locked"
+}
+
+func badConditionalMissingUnlockCase() {
+	var mu sync.Mutex
+	cond := true
+	if cond {
+		mu.Lock() // want "mutex 'mu' is locked but not unlocked in if"
+	}
+}
+
+func badElseIfMissingUnlockCase() {
+	var mu sync.Mutex
+	cond1 := false
+	cond2 := true
+
+	if cond1 {
+		mu.Lock()
+		mu.Unlock()
+	} else if cond2 {
+		mu.Lock() // want "mutex 'mu' is locked but not unlocked in if"
+	}
+}
+
+func badGoroutineMutexCase() {
+	var mu sync.Mutex
+	ch := make(chan struct{})
+	go func() {
+		mu.Lock() // want "mutex 'mu' is locked but not unlocked in goroutine"
+		<-ch
+	}()
+}
+
+func badStructFieldMutexCase() {
+	var sm safeMap
+	sm.mu.Lock() // want "mutex 'sm.mu' is locked but not unlocked"
+}
+
+func badGoroutineDeferUnlockWithoutLockCase() {
+	var mu sync.Mutex
+	ch := make(chan struct{})
+	go func() {
+		defer mu.Unlock() // want "mutex 'mu' has defer unlock but no corresponding lock"
+		<-ch
+	}()
+}
+
+func badRLockWithoutRUnlockCase() {
+	var rw sync.RWMutex
+	rw.RLock() // want "rwmutex 'rw' is rlocked but not runlocked"
+}
+
+func badRWUnlockCase() {
+	var rw sync.RWMutex
+	rw.RUnlock() // want "rwmutex 'rw' is runlocked but not rlocked"
+}
+
+func goodMutexDeferCase() {
+	var mu sync.Mutex
+	mu.Lock()
+	defer mu.Unlock()
+}
+
+func goodConditionalBothBranchesCase() {
+	var mu sync.Mutex
+	cond := true
+	if cond {
+		mu.Lock()
+		defer mu.Unlock()
+	} else {
+		mu.Lock()
+		defer mu.Unlock()
+	}
+}
+
+func goodRWMultipleOperationsCase() {
+	var rw sync.RWMutex
+	rw.RLock()
+	rw.RUnlock()
+	rw.Lock()
+	rw.Unlock()
+}

pkg/golinters/goconcurrencylint/testdata/goconcurrencylint_waitgroup.go

@@ -0,0 +1,138 @@
+//golangcitest:args -Egoconcurrencylint
+package testdata
+
+import "sync"
+
+type workerPool struct {
+	wg sync.WaitGroup
+}
+
+func handleExternalWorkForTest(wg *sync.WaitGroup) {
+	defer wg.Done()
+}
+
+func runWithDoneCallbackForTest(done func()) {
+	defer done()
+}
+
+func badExtraDoneCase() {
+	var wg sync.WaitGroup
+	wg.Add(1)
+	wg.Done()
+	wg.Done() // want "waitgroup 'wg' has Done without corresponding Add"
+	wg.Wait()
+}
+
+func badAddAfterWaitCase() {
+	var wg sync.WaitGroup
+	wg.Wait()
+	go func() {
+		wg.Add(1) // want "waitgroup 'wg' Add called after Wait"
+		wg.Done()
+	}()
+}
+
+func badAddAfterWaitMainFlowCase() {
+	var wg sync.WaitGroup
+	wg.Wait()
+	wg.Add(1) // want "waitgroup 'wg' Add called after Wait"
+	wg.Done()
+}
+
+func badLoopMissingDoneCase() {
+	var wg sync.WaitGroup
+	for i := 0; i < 2; i++ {
+		wg.Add(1) // want "waitgroup 'wg' has Add without corresponding Done"
+		if i == 0 {
+			go func() {
+				wg.Done()
+			}()
+		}
+	}
+	wg.Wait()
+}
+
+func badPrematureReturnCase() {
+	var wg sync.WaitGroup
+	wg.Add(1) // want "waitgroup 'wg' has Add without corresponding Done"
+	go func() {
+		return
+		wg.Done()
+	}()
+	wg.Wait()
+}
+
+func badMethodWaitGroupCase() {
+	var wp workerPool
+	wp.wg.Add(1) // want "waitgroup 'wp.wg' has Add without corresponding Done"
+	wp.wg.Wait()
+}
+
+func badSwitchDefaultOnlyDoneCase() {
+	var wg sync.WaitGroup
+	wg.Add(1) // want "waitgroup 'wg' has Add without corresponding Done"
+	go func() {
+		x := 1
+		switch x {
+		case 2:
+			// missing Done in this branch
+		default:
+			wg.Done()
+		}
+	}()
+	wg.Wait()
+}
+
+func goodDeferredDoneCase() {
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+	}()
+	wg.Wait()
+}
+
+func goodSwitchWithDefaultCase() {
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		x := 1
+		switch x {
+		case 2:
+			wg.Done()
+		default:
+			wg.Done()
+		}
+	}()
+	wg.Wait()
+}
+
+func goodWaitGroupPassedToHelperCase() {
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go handleExternalWorkForTest(&wg)
+	wg.Wait()
+}
+
+func goodWaitGroupMethodPassedCase() {
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go runWithDoneCallbackForTest(wg.Done)
+	wg.Wait()
+}
+
+func goodReuseWaitGroupCase() {
+	var wg sync.WaitGroup
+
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+	}()
+	wg.Wait()
+
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+	}()
+	wg.Wait()
+}

pkg/golinters/goconcurrencylint/testdata/packagelevel/goconcurrencylint_packagelevel_globals.go

@@ -0,0 +1,6 @@
+package packagelevel
+
+import "sync"
+
+var sharedPackageMu sync.Mutex
+var sharedPackageWG sync.WaitGroup

pkg/golinters/goconcurrencylint/testdata/packagelevel/goconcurrencylint_packagelevel_usage.go

@@ -0,0 +1,10 @@
+package packagelevel
+
+func badSplitPackageMutexCase() {
+	sharedPackageMu.Lock()
+}
+
+func badSplitPackageWaitGroupCase() {
+	sharedPackageWG.Add(1)
+	sharedPackageWG.Wait()
+}