diff --git a/go/analysis/passes/copylock/copylock_test.go b/go/analysis/passes/copylock/copylock_test.go index c22001ca3ea..7667f35b4b3 100644 --- a/go/analysis/passes/copylock/copylock_test.go +++ b/go/analysis/passes/copylock/copylock_test.go @@ -16,6 +16,10 @@ import ( func Test(t *testing.T) { testdata := analysistest.TestData() + // TODO(mknyszek): Add "unfortunate" package once CL 627777 lands. That CL changes + // the internals of the sync package structures to carry an explicit noCopy to prevent + // problems from changes to the implementations of those structures, such as these + // tests failing, or a bad user experience. analysistest.Run(t, testdata, copylock.Analyzer, "a", "typeparams", "issue67787") } diff --git a/go/analysis/passes/copylock/testdata/src/a/copylock_func.go b/go/analysis/passes/copylock/testdata/src/a/copylock_func.go index 0d3168f1ef1..c27862627b9 100644 --- a/go/analysis/passes/copylock/testdata/src/a/copylock_func.go +++ b/go/analysis/passes/copylock/testdata/src/a/copylock_func.go @@ -5,20 +5,25 @@ // This file contains tests for the copylock checker's // function declaration analysis. +// There are two cases missing from this file which +// are located in the "unfortunate" package in the +// testdata directory. Once the go.mod >= 1.26 for this +// repository, merge local_go124.go back into this file. + package a import "sync" func OkFunc(*sync.Mutex) {} func BadFunc(sync.Mutex) {} // want "BadFunc passes lock by value: sync.Mutex" -func BadFunc2(sync.Map) {} // want "BadFunc2 passes lock by value: sync.Map contains sync.Mutex" +func BadFunc2(sync.Map) {} // want "BadFunc2 passes lock by value: sync.Map contains sync.(Mutex|noCopy)" func OkRet() *sync.Mutex {} func BadRet() sync.Mutex {} // Don't warn about results var ( OkClosure = func(*sync.Mutex) {} BadClosure = func(sync.Mutex) {} // want "func passes lock by value: sync.Mutex" - BadClosure2 = func(sync.Map) {} // want "func passes lock by value: sync.Map contains sync.Mutex" + BadClosure2 = func(sync.Map) {} // want "func passes lock by value: sync.Map contains sync.(Mutex|noCopy)" ) type EmbeddedRWMutex struct { @@ -118,19 +123,3 @@ func AcceptedCases() { x = BadRet() // function call on RHS is OK (#16227) x = *OKRet() // indirection of function call on RHS is OK (#16227) } - -// TODO: Unfortunate cases - -// Non-ideal error message: -// Since we're looking for Lock methods, sync.Once's underlying -// sync.Mutex gets called out, but without any reference to the sync.Once. -type LocalOnce sync.Once - -func (LocalOnce) Bad() {} // want `Bad passes lock by value: a.LocalOnce contains sync.\b.*` - -// False negative: -// LocalMutex doesn't have a Lock method. -// Nevertheless, it is probably a bad idea to pass it by value. -type LocalMutex sync.Mutex - -func (LocalMutex) Bad() {} // WANTED: An error here :( diff --git a/go/analysis/passes/copylock/testdata/src/unfortunate/local_go123.go b/go/analysis/passes/copylock/testdata/src/unfortunate/local_go123.go new file mode 100644 index 00000000000..1aa3abc788d --- /dev/null +++ b/go/analysis/passes/copylock/testdata/src/unfortunate/local_go123.go @@ -0,0 +1,25 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build !go1.24 + +package unfortunate + +import "sync" + +// TODO: Unfortunate cases + +// Non-ideal error message: +// Since we're looking for Lock methods, sync.Once's underlying +// sync.Mutex gets called out, but without any reference to the sync.Once. +type LocalOnce sync.Once + +func (LocalOnce) Bad() {} // want `Bad passes lock by value: a.LocalOnce contains sync.\b.*` + +// False negative: +// LocalMutex doesn't have a Lock method. +// Nevertheless, it is probably a bad idea to pass it by value. +type LocalMutex sync.Mutex + +func (LocalMutex) Bad() {} // WANTED: An error here :( diff --git a/go/analysis/passes/copylock/testdata/src/unfortunate/local_go124.go b/go/analysis/passes/copylock/testdata/src/unfortunate/local_go124.go new file mode 100644 index 00000000000..505b105a9b1 --- /dev/null +++ b/go/analysis/passes/copylock/testdata/src/unfortunate/local_go124.go @@ -0,0 +1,19 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build go1.24 + +package unfortunate + +import "sync" + +// Cases where the interior sync.noCopy shows through. + +type LocalOnce sync.Once + +func (LocalOnce) Bad() {} // want "Bad passes lock by value: a.LocalOnce contains sync.noCopy" + +type LocalMutex sync.Mutex + +func (LocalMutex) Bad() {} // want "Bad passes lock by value: a.LocalMutex contains sync.noCopy"