go/analysis/passes/unusedwrite: silence if unsafe is imported

As reported in golang/go#67684, the unusedwrite analyzer may have false
positives when used with unsafe, so silence it if unsafe is imported.

This is a rather large hammer, but there is a high likelihood of such
false positives, and they significantly detracting from the usefulness
of the analysis. Specifically: this noise causes Go developers to
disable the analyzer.

Fixes golang/go#67684

Change-Id: I855a5da6124a0495c7eb11722b466824689780a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/621817
Auto-Submit: Robert Findley <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: findleyr

go/analysis/passes/unusedwrite/testdata/src/a/unusedwrite.go

@@ -30,6 +30,15 @@ func BadWrites() {
 		v.x = i // want "unused write to field x"
 		_ = v.y
 	}
+
+	// The analyzer can handle only simple control flow.
+	type T struct{ x, y int }
+	t := new(T)
+	if true {
+		t = new(T)
+	} // causes t below to become phi(alloc, alloc), not a simple alloc
+	t.x = 1 // false negative
+	print(t.y)
 }
 
 func (t T1) BadValueReceiverWrite(v T2) {

go/analysis/passes/unusedwrite/testdata/src/importsunsafe/i.go

@@ -0,0 +1,20 @@
+package importsunsafe
+
+import "unsafe"
+
+type S struct {
+	F, G int
+}
+
+func _() {
+	var s S
+	s.F = 1
+	// This write to G is used below, because &s.F allows access to all of s, but
+	// the analyzer would naively report it as unused. For this reason, we
+	// silence the analysis if unsafe is imported.
+	s.G = 2
+
+	ptr := unsafe.Pointer(&s.F)
+	t := (*S)(ptr)
+	println(t.G)
+}

go/analysis/passes/unusedwrite/unusedwrite.go

@@ -28,7 +28,15 @@ var Analyzer = &analysis.Analyzer{
 	Run:      run,
 }
 
-func run(pass *analysis.Pass) (interface{}, error) {
+func run(pass *analysis.Pass) (any, error) {
+	for _, pkg := range pass.Pkg.Imports() {
+		if pkg.Path() == "unsafe" {
+			// See golang/go#67684, or testdata/src/importsunsafe: the unusedwrite
+			// analyzer may have false positives when used with unsafe.
+			return nil, nil
+		}
+	}
+
 	ssainput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
 	for _, fn := range ssainput.SrcFuncs {
 		reports := checkStores(fn)

go/analysis/passes/unusedwrite/unusedwrite_test.go

@@ -13,5 +13,5 @@ import (
 
 func Test(t *testing.T) {
 	testdata := analysistest.TestData()
-	analysistest.Run(t, testdata, unusedwrite.Analyzer, "a")
+	analysistest.Run(t, testdata, unusedwrite.Analyzer, "a", "importsunsafe")
 }

gopls/internal/test/marker/testdata/diagnostics/analyzers.txt

@@ -19,6 +19,7 @@ package analyzer
 
 import (
 	"fmt"
+	"log/slog"
 	"sync"
 	"testing"
 	"time"
@@ -61,9 +62,16 @@ func _(s struct{x int}) {
 	s.x = 1  //@diag("x", re"unused write to field x")
 }
 
--- cgocall.go --
-package analyzer
+// slog
+func _() {
+	slog.Info("msg", 1) //@diag("1", re`slog.Info arg "1" should be a string or a slog.Attr`)
+}
+
+-- cgocall/cgocall.go --
+package cgocall
 
+// Note: this test must be in a separate package, as the unsafe import
+// silences the unusedwrite analyzer.
 import "unsafe"
 
 // void f(void *ptr) {}
@@ -73,15 +81,3 @@ import "C"
 func _(c chan bool) {
 	C.f(unsafe.Pointer(&c)) //@ diag("unsafe", re"passing Go type with embedded pointer to C")
 }
-
--- bad_test_go121.go --
-//go:build go1.21
-
-package analyzer
-
-import "log/slog"
-
-// slog
-func _() {
-	slog.Info("msg", 1) //@diag("1", re`slog.Info arg "1" should be a string or a slog.Attr`)
-}