Skip to content

Commit 904c6ba

Browse files
committed
gopls/internal/settings: enable "unusedwrite" analyzer
This analyzer shows a very high rate of true positives, indicating that code is (at best) unnecessarily complex and confusing but, often, not working as intended. + release note Fixes golang/go#44461 Change-Id: I1c86e7305a8d512d7e4d1bbec3b2b44ce3a9c9ab Reviewed-on: https://go-review.googlesource.com/c/tools/+/575061 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent 9663999 commit 904c6ba

File tree

7 files changed

+66
-7
lines changed

7 files changed

+66
-7
lines changed
+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2024 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
//go:build ignore
6+
7+
// The unusedwrite command runs the unusedwrite analyzer
8+
// on the specified packages.
9+
package main
10+
11+
import (
12+
"golang.org/x/tools/go/analysis/passes/unusedwrite"
13+
"golang.org/x/tools/go/analysis/singlechecker"
14+
)
15+
16+
func main() { singlechecker.Main(unusedwrite.Analyzer) }

gopls/doc/analyzers.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -1022,7 +1022,7 @@ Another example is about non-pointer receiver:
10221022

10231023
[Full documentation](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/unusedwrite)
10241024

1025-
**Disabled by default. Enable it by setting `"analyses": {"unusedwrite": true}`.**
1025+
**Enabled by default.**
10261026

10271027
## **useany**
10281028

gopls/doc/release/v0.16.0.md

+33
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,36 @@ cause your editor to navigate to the declaration.
2424
TODO: demo of source linking.
2525
TODO: appeal to VS Code users to upvote https://github.com/microsoft/vscode/issues/208093?
2626

27+
### `unusedwrite` analyzer
28+
29+
The new
30+
[unusedwrite](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/unusedwrite)
31+
analyzer reports assignments, often to fields of structs, that have no
32+
effect because, for example, the struct is never used again:
33+
34+
```go
35+
func scheme(host string) string {
36+
u := &url.URL{
37+
Host: host, // "unused write to field Host" (no need to construct a URL)
38+
Scheme: "https:",
39+
}
40+
return u.Scheme
41+
}
42+
```
43+
44+
This is at best an indication that the code is unnecessarily complex
45+
(for instance, some dead code could be removed), but often indicates a
46+
bug, as in this example:
47+
48+
```go
49+
type S struct { x int }
50+
51+
func (s S) set(x int) {
52+
s.x = x // "unused write to field x" (s should be a *S pointer)
53+
}
54+
```
55+
56+
2757
### Hover shows size/offset info
2858

2959
Hovering over the identifier that declares a type or struct field now
@@ -36,3 +66,6 @@ TODO: example hover image.
3666
## Bugs fixed
3767

3868
## Thank you to our contributors!
69+
70+
@guodongli-google for the `unusedwrite` analyzer.
71+
TODO: they're a xoogler; is there a more current GH account?

gopls/internal/settings/api_json.go

+5-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gopls/internal/settings/settings.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -1436,6 +1436,9 @@ func analyzers() map[string]*Analyzer {
14361436
unusedresult.Analyzer.Name: {Analyzer: unusedresult.Analyzer, Enabled: true},
14371437

14381438
// Non-vet analyzers:
1439+
// - some (nilness, unusedwrite) use go/ssa;
1440+
// - some (unusedwrite) report bad code but not always a bug,
1441+
// so are not suitable for vet.
14391442
atomicalign.Analyzer.Name: {Analyzer: atomicalign.Analyzer, Enabled: true},
14401443
deepequalerrors.Analyzer.Name: {Analyzer: deepequalerrors.Analyzer, Enabled: true},
14411444
fieldalignment.Analyzer.Name: {Analyzer: fieldalignment.Analyzer, Enabled: false},
@@ -1444,7 +1447,7 @@ func analyzers() map[string]*Analyzer {
14441447
sortslice.Analyzer.Name: {Analyzer: sortslice.Analyzer, Enabled: true},
14451448
testinggoroutine.Analyzer.Name: {Analyzer: testinggoroutine.Analyzer, Enabled: true},
14461449
unusedparams.Analyzer.Name: {Analyzer: unusedparams.Analyzer, Enabled: true},
1447-
unusedwrite.Analyzer.Name: {Analyzer: unusedwrite.Analyzer, Enabled: false},
1450+
unusedwrite.Analyzer.Name: {Analyzer: unusedwrite.Analyzer, Enabled: true},
14481451
useany.Analyzer.Name: {Analyzer: useany.Analyzer, Enabled: false},
14491452
infertypeargs.Analyzer.Name: {
14501453
Analyzer: infertypeargs.Analyzer,

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

+5
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ func _(ptr *int) {
5050
}
5151
}
5252

53+
// unusedwrite
54+
func _(s struct{x int}) {
55+
s.x = 1 //@diag("x", re"unused write to field x")
56+
}
57+
5358
-- bad_test_go121.go --
5459
//go:build go1.21
5560

gopls/internal/test/marker/testdata/references/issue60676.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,11 @@ import "mod.test/b"
5555

5656
func _() {
5757
x := b.B{
58-
F: 42, //@refs("F", FDef, "F")
58+
F: 42, //@refs("F", FDef, "F", Fuse)
5959
}
6060
x.G = "hi" //@refs("G", GDef, "G")
6161
_ = x.E //@refs("E", EDef, "E")
62+
_ = x.F //@loc(Fuse, "F")
6263
}
6364

6465
func _(y b.BI) {

0 commit comments

Comments
 (0)