aboutsummaryrefslogtreecommitdiff
path: root/go/analysis/passes
diff options
context:
space:
mode:
authorAndroid Build Coastguard Worker <android-build-coastguard-worker@google.com>2023-03-16 14:15:45 +0000
committerAndroid Build Coastguard Worker <android-build-coastguard-worker@google.com>2023-03-16 14:15:45 +0000
commitcd3c7908c2ca750b27d330b4d257edc6818c4a5d (patch)
tree194d7b0e539d014393564a256bec571e18d6533a /go/analysis/passes
parent3225eca48f7ce16eb31b2dd5a170806c1214a49e (diff)
parent09c5a32afc5b66f28f166a68afe1fc71afbf9b73 (diff)
downloadgolang-x-tools-build-tools-release.tar.gz
Snap for 9757917 from 09c5a32afc5b66f28f166a68afe1fc71afbf9b73 to build-tools-releasebuild-tools-release
Change-Id: If48e809642d94de846f47e34b88e446095e21aa5
Diffstat (limited to 'go/analysis/passes')
-rw-r--r--go/analysis/passes/asmdecl/arches_go118.go12
-rw-r--r--go/analysis/passes/asmdecl/arches_go119.go14
-rw-r--r--go/analysis/passes/asmdecl/asmdecl.go5
-rw-r--r--go/analysis/passes/asmdecl/asmdecl_test.go19
-rw-r--r--go/analysis/passes/asmdecl/testdata/src/a/asm10.s192
-rw-r--r--go/analysis/passes/asmdecl/testdata/src/a/asm11.s13
-rw-r--r--go/analysis/passes/assign/assign.go15
-rw-r--r--go/analysis/passes/assign/testdata/src/a/a.go28
-rw-r--r--go/analysis/passes/assign/testdata/src/a/a.go.golden28
-rw-r--r--go/analysis/passes/bools/bools.go12
-rw-r--r--go/analysis/passes/buildssa/buildssa_test.go37
-rw-r--r--go/analysis/passes/buildssa/testdata/src/b/b.go20
-rw-r--r--go/analysis/passes/buildssa/testdata/src/c/c.go24
-rw-r--r--go/analysis/passes/buildtag/buildtag.go2
-rw-r--r--go/analysis/passes/buildtag/buildtag_old.go2
-rw-r--r--go/analysis/passes/cgocall/cgocall.go16
-rw-r--r--go/analysis/passes/composite/composite.go41
-rw-r--r--go/analysis/passes/composite/composite_test.go2
-rw-r--r--go/analysis/passes/composite/testdata/src/a/a.go17
-rw-r--r--go/analysis/passes/composite/testdata/src/a/a.go.golden144
-rw-r--r--go/analysis/passes/composite/testdata/src/a/a_fuzz_test.go.golden16
-rw-r--r--go/analysis/passes/composite/testdata/src/typeparams/typeparams.go10
-rw-r--r--go/analysis/passes/composite/testdata/src/typeparams/typeparams.go.golden27
-rw-r--r--go/analysis/passes/copylock/copylock.go2
-rw-r--r--go/analysis/passes/copylock/testdata/src/a/copylock.go42
-rw-r--r--go/analysis/passes/copylock/testdata/src/a/copylock_func.go2
-rw-r--r--go/analysis/passes/directive/directive.go216
-rw-r--r--go/analysis/passes/directive/directive_test.go39
-rw-r--r--go/analysis/passes/directive/testdata/src/a/badspace.go11
-rw-r--r--go/analysis/passes/directive/testdata/src/a/misplaced.go10
-rw-r--r--go/analysis/passes/directive/testdata/src/a/misplaced.s19
-rw-r--r--go/analysis/passes/directive/testdata/src/a/misplaced_test.go10
-rw-r--r--go/analysis/passes/directive/testdata/src/a/p.go11
-rw-r--r--go/analysis/passes/errorsas/errorsas.go28
-rw-r--r--go/analysis/passes/errorsas/testdata/src/a/a.go4
-rw-r--r--go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go2
-rw-r--r--go/analysis/passes/fieldalignment/fieldalignment.go7
-rw-r--r--go/analysis/passes/httpresponse/httpresponse.go27
-rw-r--r--go/analysis/passes/httpresponse/httpresponse_test.go3
-rw-r--r--go/analysis/passes/httpresponse/testdata/src/a/a.go27
-rw-r--r--go/analysis/passes/ifaceassert/parameterized.go1
-rw-r--r--go/analysis/passes/inspect/inspect.go15
-rw-r--r--go/analysis/passes/loopclosure/loopclosure.go408
-rw-r--r--go/analysis/passes/loopclosure/loopclosure_test.go4
-rw-r--r--go/analysis/passes/loopclosure/testdata/src/a/a.go131
-rw-r--r--go/analysis/passes/loopclosure/testdata/src/a/b.go9
-rw-r--r--go/analysis/passes/loopclosure/testdata/src/subtests/subtest.go202
-rw-r--r--go/analysis/passes/nilness/nilness.go18
-rw-r--r--go/analysis/passes/nilness/nilness_test.go17
-rw-r--r--go/analysis/passes/nilness/testdata/src/a/a.go41
-rw-r--r--go/analysis/passes/nilness/testdata/src/c/c.go14
-rw-r--r--go/analysis/passes/nilness/testdata/src/d/d.go55
-rw-r--r--go/analysis/passes/pkgfact/pkgfact.go8
-rw-r--r--go/analysis/passes/printf/printf.go27
-rw-r--r--go/analysis/passes/printf/testdata/src/a/a.go3
-rw-r--r--go/analysis/passes/printf/testdata/src/typeparams/diagnostics.go22
-rw-r--r--go/analysis/passes/printf/types.go10
-rw-r--r--go/analysis/passes/shadow/shadow.go1
-rw-r--r--go/analysis/passes/sigchanyzer/sigchanyzer.go2
-rw-r--r--go/analysis/passes/sortslice/analyzer.go9
-rw-r--r--go/analysis/passes/sortslice/testdata/src/a/a.go24
-rw-r--r--go/analysis/passes/stdmethods/stdmethods.go13
-rw-r--r--go/analysis/passes/stdmethods/testdata/src/a/a.go14
-rw-r--r--go/analysis/passes/stdmethods/testdata/src/typeparams/typeparams.go4
-rw-r--r--go/analysis/passes/tests/testdata/src/a/go118_test.go5
-rw-r--r--go/analysis/passes/tests/tests.go30
-rw-r--r--go/analysis/passes/timeformat/testdata/src/a/a.go50
-rw-r--r--go/analysis/passes/timeformat/testdata/src/a/a.go.golden50
-rw-r--r--go/analysis/passes/timeformat/testdata/src/b/b.go11
-rw-r--r--go/analysis/passes/timeformat/timeformat.go129
-rw-r--r--go/analysis/passes/timeformat/timeformat_test.go17
-rw-r--r--go/analysis/passes/unusedwrite/unusedwrite.go53
72 files changed, 2321 insertions, 232 deletions
diff --git a/go/analysis/passes/asmdecl/arches_go118.go b/go/analysis/passes/asmdecl/arches_go118.go
new file mode 100644
index 000000000..d8211afdc
--- /dev/null
+++ b/go/analysis/passes/asmdecl/arches_go118.go
@@ -0,0 +1,12 @@
+// Copyright 2022 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.19
+// +build !go1.19
+
+package asmdecl
+
+func additionalArches() []*asmArch {
+ return nil
+}
diff --git a/go/analysis/passes/asmdecl/arches_go119.go b/go/analysis/passes/asmdecl/arches_go119.go
new file mode 100644
index 000000000..3018383e7
--- /dev/null
+++ b/go/analysis/passes/asmdecl/arches_go119.go
@@ -0,0 +1,14 @@
+// Copyright 2022 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.19
+// +build go1.19
+
+package asmdecl
+
+var asmArchLoong64 = asmArch{name: "loong64", bigEndian: false, stack: "R3", lr: true}
+
+func additionalArches() []*asmArch {
+ return []*asmArch{&asmArchLoong64}
+}
diff --git a/go/analysis/passes/asmdecl/asmdecl.go b/go/analysis/passes/asmdecl/asmdecl.go
index b05ed5c15..7288559fc 100644
--- a/go/analysis/passes/asmdecl/asmdecl.go
+++ b/go/analysis/passes/asmdecl/asmdecl.go
@@ -92,7 +92,7 @@ var (
asmArchMips64LE = asmArch{name: "mips64le", bigEndian: false, stack: "R29", lr: true}
asmArchPpc64 = asmArch{name: "ppc64", bigEndian: true, stack: "R1", lr: true, retRegs: []string{"R3", "F1"}}
asmArchPpc64LE = asmArch{name: "ppc64le", bigEndian: false, stack: "R1", lr: true, retRegs: []string{"R3", "F1"}}
- asmArchRISCV64 = asmArch{name: "riscv64", bigEndian: false, stack: "SP", lr: true}
+ asmArchRISCV64 = asmArch{name: "riscv64", bigEndian: false, stack: "SP", lr: true, retRegs: []string{"X10", "F10"}}
asmArchS390X = asmArch{name: "s390x", bigEndian: true, stack: "R15", lr: true}
asmArchWasm = asmArch{name: "wasm", bigEndian: false, stack: "SP", lr: false}
@@ -114,6 +114,7 @@ var (
)
func init() {
+ arches = append(arches, additionalArches()...)
for _, arch := range arches {
arch.sizes = types.SizesFor("gc", arch.name)
if arch.sizes == nil {
@@ -731,7 +732,7 @@ func asmCheckVar(badf func(string, ...interface{}), fn *asmFunc, line, expr stri
src = 8
}
}
- case "mips", "mipsle", "mips64", "mips64le":
+ case "loong64", "mips", "mipsle", "mips64", "mips64le":
switch op {
case "MOVB", "MOVBU":
src = 1
diff --git a/go/analysis/passes/asmdecl/asmdecl_test.go b/go/analysis/passes/asmdecl/asmdecl_test.go
index f88c188b2..50938a075 100644
--- a/go/analysis/passes/asmdecl/asmdecl_test.go
+++ b/go/analysis/passes/asmdecl/asmdecl_test.go
@@ -14,14 +14,17 @@ import (
)
var goosarches = []string{
- "linux/amd64", // asm1.s, asm4.s
- "linux/386", // asm2.s
- "linux/arm", // asm3.s
- "linux/mips64", // asm5.s
- "linux/s390x", // asm6.s
- "linux/ppc64", // asm7.s
- "linux/mips", // asm8.s,
- "js/wasm", // asm9.s
+ "linux/amd64", // asm1.s, asm4.s
+ "linux/386", // asm2.s
+ "linux/arm", // asm3.s
+ // TODO: skip test on loong64 until go toolchain supported loong64.
+ // "linux/loong64", // asm10.s
+ "linux/mips64", // asm5.s
+ "linux/s390x", // asm6.s
+ "linux/ppc64", // asm7.s
+ "linux/mips", // asm8.s,
+ "js/wasm", // asm9.s
+ "linux/riscv64", // asm11.s
}
func Test(t *testing.T) {
diff --git a/go/analysis/passes/asmdecl/testdata/src/a/asm10.s b/go/analysis/passes/asmdecl/testdata/src/a/asm10.s
new file mode 100644
index 000000000..f0045882a
--- /dev/null
+++ b/go/analysis/passes/asmdecl/testdata/src/a/asm10.s
@@ -0,0 +1,192 @@
+// Copyright 2022 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 loong64
+
+TEXT ·arg1(SB),0,$0-2
+ MOVB x+0(FP), R19
+ MOVBU y+1(FP), R18
+ MOVH x+0(FP), R19 // want `\[loong64\] arg1: invalid MOVH of x\+0\(FP\); int8 is 1-byte value`
+ MOVHU y+1(FP), R19 // want `invalid MOVHU of y\+1\(FP\); uint8 is 1-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); int8 is 1-byte value`
+ MOVWU y+1(FP), R19 // want `invalid MOVWU of y\+1\(FP\); uint8 is 1-byte value`
+ MOVV x+0(FP), R19 // want `invalid MOVV of x\+0\(FP\); int8 is 1-byte value`
+ MOVV y+1(FP), R19 // want `invalid MOVV of y\+1\(FP\); uint8 is 1-byte value`
+ MOVB x+1(FP), R19 // want `invalid offset x\+1\(FP\); expected x\+0\(FP\)`
+ MOVBU y+2(FP), R19 // want `invalid offset y\+2\(FP\); expected y\+1\(FP\)`
+ MOVB 16(R3), R19 // want `16\(R3\) should be x\+0\(FP\)`
+ MOVB 17(R3), R19 // want `17\(R3\) should be y\+1\(FP\)`
+ MOVB 18(R3), R19 // want `use of 18\(R3\) points beyond argument frame`
+ RET
+
+TEXT ·arg2(SB),0,$0-4
+ MOVBU x+0(FP), R19 // want `arg2: invalid MOVBU of x\+0\(FP\); int16 is 2-byte value`
+ MOVB y+2(FP), R19 // want `invalid MOVB of y\+2\(FP\); uint16 is 2-byte value`
+ MOVHU x+0(FP), R19
+ MOVH y+2(FP), R18
+ MOVWU x+0(FP), R19 // want `invalid MOVWU of x\+0\(FP\); int16 is 2-byte value`
+ MOVW y+2(FP), R19 // want `invalid MOVW of y\+2\(FP\); uint16 is 2-byte value`
+ MOVV x+0(FP), R19 // want `invalid MOVV of x\+0\(FP\); int16 is 2-byte value`
+ MOVV y+2(FP), R19 // want `invalid MOVV of y\+2\(FP\); uint16 is 2-byte value`
+ MOVHU x+2(FP), R19 // want `invalid offset x\+2\(FP\); expected x\+0\(FP\)`
+ MOVH y+0(FP), R19 // want `invalid offset y\+0\(FP\); expected y\+2\(FP\)`
+ RET
+
+TEXT ·arg4(SB),0,$0-2 // want `arg4: wrong argument size 2; expected \$\.\.\.-8`
+ MOVB x+0(FP), R19 // want `invalid MOVB of x\+0\(FP\); int32 is 4-byte value`
+ MOVB y+4(FP), R18 // want `invalid MOVB of y\+4\(FP\); uint32 is 4-byte value`
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); int32 is 4-byte value`
+ MOVH y+4(FP), R19 // want `invalid MOVH of y\+4\(FP\); uint32 is 4-byte value`
+ MOVW x+0(FP), R19
+ MOVW y+4(FP), R19
+ MOVV x+0(FP), R19 // want `invalid MOVV of x\+0\(FP\); int32 is 4-byte value`
+ MOVV y+4(FP), R19 // want `invalid MOVV of y\+4\(FP\); uint32 is 4-byte value`
+ MOVW x+4(FP), R19 // want `invalid offset x\+4\(FP\); expected x\+0\(FP\)`
+ MOVW y+2(FP), R19 // want `invalid offset y\+2\(FP\); expected y\+4\(FP\)`
+ RET
+
+TEXT ·arg8(SB),7,$0-2 // want `wrong argument size 2; expected \$\.\.\.-16`
+ MOVB x+0(FP), R19 // want `invalid MOVB of x\+0\(FP\); int64 is 8-byte value`
+ MOVB y+8(FP), R18 // want `invalid MOVB of y\+8\(FP\); uint64 is 8-byte value`
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); int64 is 8-byte value`
+ MOVH y+8(FP), R19 // want `invalid MOVH of y\+8\(FP\); uint64 is 8-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); int64 is 8-byte value`
+ MOVW y+8(FP), R19 // want `invalid MOVW of y\+8\(FP\); uint64 is 8-byte value`
+ MOVV x+0(FP), R19
+ MOVV y+8(FP), R19
+ MOVV x+8(FP), R19 // want `invalid offset x\+8\(FP\); expected x\+0\(FP\)`
+ MOVV y+2(FP), R19 // want `invalid offset y\+2\(FP\); expected y\+8\(FP\)`
+ RET
+
+TEXT ·argint(SB),0,$0-2 // want `wrong argument size 2; expected \$\.\.\.-16`
+ MOVB x+0(FP), R19 // want `invalid MOVB of x\+0\(FP\); int is 8-byte value`
+ MOVB y+8(FP), R18 // want `invalid MOVB of y\+8\(FP\); uint is 8-byte value`
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); int is 8-byte value`
+ MOVH y+8(FP), R19 // want `invalid MOVH of y\+8\(FP\); uint is 8-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); int is 8-byte value`
+ MOVW y+8(FP), R19 // want `invalid MOVW of y\+8\(FP\); uint is 8-byte value`
+ MOVV x+0(FP), R19
+ MOVV y+8(FP), R19
+ MOVV x+8(FP), R19 // want `invalid offset x\+8\(FP\); expected x\+0\(FP\)`
+ MOVV y+2(FP), R19 // want `invalid offset y\+2\(FP\); expected y\+8\(FP\)`
+ RET
+
+TEXT ·argptr(SB),7,$0-2 // want `wrong argument size 2; expected \$\.\.\.-40`
+ MOVB x+0(FP), R19 // want `invalid MOVB of x\+0\(FP\); \*byte is 8-byte value`
+ MOVB y+8(FP), R18 // want `invalid MOVB of y\+8\(FP\); \*byte is 8-byte value`
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); \*byte is 8-byte value`
+ MOVH y+8(FP), R19 // want `invalid MOVH of y\+8\(FP\); \*byte is 8-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); \*byte is 8-byte value`
+ MOVW y+8(FP), R19 // want `invalid MOVW of y\+8\(FP\); \*byte is 8-byte value`
+ MOVV x+0(FP), R19
+ MOVV y+8(FP), R19
+ MOVV x+8(FP), R19 // want `invalid offset x\+8\(FP\); expected x\+0\(FP\)`
+ MOVV y+2(FP), R19 // want `invalid offset y\+2\(FP\); expected y\+8\(FP\)`
+ MOVW c+16(FP), R19 // want `invalid MOVW of c\+16\(FP\); chan int is 8-byte value`
+ MOVW m+24(FP), R19 // want `invalid MOVW of m\+24\(FP\); map\[int\]int is 8-byte value`
+ MOVW f+32(FP), R19 // want `invalid MOVW of f\+32\(FP\); func\(\) is 8-byte value`
+ RET
+
+TEXT ·argstring(SB),0,$32 // want `wrong argument size 0; expected \$\.\.\.-32`
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); string base is 8-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); string base is 8-byte value`
+ MOVV x+0(FP), R19
+ MOVH x_base+0(FP), R19 // want `invalid MOVH of x_base\+0\(FP\); string base is 8-byte value`
+ MOVW x_base+0(FP), R19 // want `invalid MOVW of x_base\+0\(FP\); string base is 8-byte value`
+ MOVV x_base+0(FP), R19
+ MOVH x_len+0(FP), R19 // want `invalid offset x_len\+0\(FP\); expected x_len\+8\(FP\)`
+ MOVW x_len+0(FP), R19 // want `invalid offset x_len\+0\(FP\); expected x_len\+8\(FP\)`
+ MOVV x_len+0(FP), R19 // want `invalid offset x_len\+0\(FP\); expected x_len\+8\(FP\)`
+ MOVH x_len+8(FP), R19 // want `invalid MOVH of x_len\+8\(FP\); string len is 8-byte value`
+ MOVW x_len+8(FP), R19 // want `invalid MOVW of x_len\+8\(FP\); string len is 8-byte value`
+ MOVV x_len+8(FP), R19
+ MOVV y+0(FP), R19 // want `invalid offset y\+0\(FP\); expected y\+16\(FP\)`
+ MOVV y_len+8(FP), R19 // want `invalid offset y_len\+8\(FP\); expected y_len\+24\(FP\)`
+ RET
+
+TEXT ·argslice(SB),0,$48 // want `wrong argument size 0; expected \$\.\.\.-48`
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); slice base is 8-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); slice base is 8-byte value`
+ MOVV x+0(FP), R19
+ MOVH x_base+0(FP), R19 // want `invalid MOVH of x_base\+0\(FP\); slice base is 8-byte value`
+ MOVW x_base+0(FP), R19 // want `invalid MOVW of x_base\+0\(FP\); slice base is 8-byte value`
+ MOVV x_base+0(FP), R19
+ MOVH x_len+0(FP), R19 // want `invalid offset x_len\+0\(FP\); expected x_len\+8\(FP\)`
+ MOVW x_len+0(FP), R19 // want `invalid offset x_len\+0\(FP\); expected x_len\+8\(FP\)`
+ MOVV x_len+0(FP), R19 // want `invalid offset x_len\+0\(FP\); expected x_len\+8\(FP\)`
+ MOVH x_len+8(FP), R19 // want `invalid MOVH of x_len\+8\(FP\); slice len is 8-byte value`
+ MOVW x_len+8(FP), R19 // want `invalid MOVW of x_len\+8\(FP\); slice len is 8-byte value`
+ MOVV x_len+8(FP), R19
+ MOVH x_cap+0(FP), R19 // want `invalid offset x_cap\+0\(FP\); expected x_cap\+16\(FP\)`
+ MOVW x_cap+0(FP), R19 // want `invalid offset x_cap\+0\(FP\); expected x_cap\+16\(FP\)`
+ MOVV x_cap+0(FP), R19 // want `invalid offset x_cap\+0\(FP\); expected x_cap\+16\(FP\)`
+ MOVH x_cap+16(FP), R19 // want `invalid MOVH of x_cap\+16\(FP\); slice cap is 8-byte value`
+ MOVW x_cap+16(FP), R19 // want `invalid MOVW of x_cap\+16\(FP\); slice cap is 8-byte value`
+ MOVV x_cap+16(FP), R19
+ MOVV y+0(FP), R19 // want `invalid offset y\+0\(FP\); expected y\+24\(FP\)`
+ MOVV y_len+8(FP), R19 // want `invalid offset y_len\+8\(FP\); expected y_len\+32\(FP\)`
+ MOVV y_cap+16(FP), R19 // want `invalid offset y_cap\+16\(FP\); expected y_cap\+40\(FP\)`
+ RET
+
+TEXT ·argiface(SB),0,$0-32
+ MOVH x+0(FP), R19 // want `invalid MOVH of x\+0\(FP\); interface type is 8-byte value`
+ MOVW x+0(FP), R19 // want `invalid MOVW of x\+0\(FP\); interface type is 8-byte value`
+ MOVV x+0(FP), R19
+ MOVH x_type+0(FP), R19 // want `invalid MOVH of x_type\+0\(FP\); interface type is 8-byte value`
+ MOVW x_type+0(FP), R19 // want `invalid MOVW of x_type\+0\(FP\); interface type is 8-byte value`
+ MOVV x_type+0(FP), R19
+ MOVV x_itable+0(FP), R19 // want `unknown variable x_itable; offset 0 is x_type\+0\(FP\)`
+ MOVV x_itable+1(FP), R19 // want `unknown variable x_itable; offset 1 is x_type\+0\(FP\)`
+ MOVH x_data+0(FP), R19 // want `invalid offset x_data\+0\(FP\); expected x_data\+8\(FP\)`
+ MOVW x_data+0(FP), R19 // want `invalid offset x_data\+0\(FP\); expected x_data\+8\(FP\)`
+ MOVV x_data+0(FP), R19 // want `invalid offset x_data\+0\(FP\); expected x_data\+8\(FP\)`
+ MOVH x_data+8(FP), R19 // want `invalid MOVH of x_data\+8\(FP\); interface data is 8-byte value`
+ MOVW x_data+8(FP), R19 // want `invalid MOVW of x_data\+8\(FP\); interface data is 8-byte value`
+ MOVV x_data+8(FP), R19
+ MOVH y+16(FP), R19 // want `invalid MOVH of y\+16\(FP\); interface itable is 8-byte value`
+ MOVW y+16(FP), R19 // want `invalid MOVW of y\+16\(FP\); interface itable is 8-byte value`
+ MOVV y+16(FP), R19
+ MOVH y_itable+16(FP), R19 // want `invalid MOVH of y_itable\+16\(FP\); interface itable is 8-byte value`
+ MOVW y_itable+16(FP), R19 // want `invalid MOVW of y_itable\+16\(FP\); interface itable is 8-byte value`
+ MOVV y_itable+16(FP), R19
+ MOVV y_type+16(FP), R19 // want `unknown variable y_type; offset 16 is y_itable\+16\(FP\)`
+ MOVH y_data+16(FP), R19 // want `invalid offset y_data\+16\(FP\); expected y_data\+24\(FP\)`
+ MOVW y_data+16(FP), R19 // want `invalid offset y_data\+16\(FP\); expected y_data\+24\(FP\)`
+ MOVV y_data+16(FP), R19 // want `invalid offset y_data\+16\(FP\); expected y_data\+24\(FP\)`
+ MOVH y_data+24(FP), R19 // want `invalid MOVH of y_data\+24\(FP\); interface data is 8-byte value`
+ MOVW y_data+24(FP), R19 // want `invalid MOVW of y_data\+24\(FP\); interface data is 8-byte value`
+ MOVV y_data+24(FP), R19
+ RET
+
+TEXT ·returnint(SB),0,$0-8
+ MOVB R19, ret+0(FP) // want `invalid MOVB of ret\+0\(FP\); int is 8-byte value`
+ MOVH R19, ret+0(FP) // want `invalid MOVH of ret\+0\(FP\); int is 8-byte value`
+ MOVW R19, ret+0(FP) // want `invalid MOVW of ret\+0\(FP\); int is 8-byte value`
+ MOVV R19, ret+0(FP)
+ MOVV R19, ret+1(FP) // want `invalid offset ret\+1\(FP\); expected ret\+0\(FP\)`
+ MOVV R19, r+0(FP) // want `unknown variable r; offset 0 is ret\+0\(FP\)`
+ RET
+
+TEXT ·returnbyte(SB),0,$0-9
+ MOVV x+0(FP), R19
+ MOVB R19, ret+8(FP)
+ MOVH R19, ret+8(FP) // want `invalid MOVH of ret\+8\(FP\); byte is 1-byte value`
+ MOVW R19, ret+8(FP) // want `invalid MOVW of ret\+8\(FP\); byte is 1-byte value`
+ MOVV R19, ret+8(FP) // want `invalid MOVV of ret\+8\(FP\); byte is 1-byte value`
+ MOVB R19, ret+7(FP) // want `invalid offset ret\+7\(FP\); expected ret\+8\(FP\)`
+ RET
+
+TEXT ·returnnamed(SB),0,$0-41
+ MOVB x+0(FP), R19
+ MOVV R19, r1+8(FP)
+ MOVH R19, r2+16(FP)
+ MOVV R19, r3+24(FP)
+ MOVV R19, r3_base+24(FP)
+ MOVV R19, r3_len+32(FP)
+ MOVB R19, r4+40(FP)
+ MOVW R19, r1+8(FP) // want `invalid MOVW of r1\+8\(FP\); int is 8-byte value`
+ RET
+
+TEXT ·returnintmissing(SB),0,$0-8
+ RET // want `RET without writing to 8-byte ret\+0\(FP\)`
diff --git a/go/analysis/passes/asmdecl/testdata/src/a/asm11.s b/go/analysis/passes/asmdecl/testdata/src/a/asm11.s
new file mode 100644
index 000000000..e81e8ee17
--- /dev/null
+++ b/go/analysis/passes/asmdecl/testdata/src/a/asm11.s
@@ -0,0 +1,13 @@
+// Copyright 2022 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.
+
+// +build riscv64
+
+// writing to result in ABIInternal function
+TEXT ·returnABIInternal<ABIInternal>(SB), NOSPLIT, $8
+ MOV $123, X10
+ RET
+TEXT ·returnmissingABIInternal<ABIInternal>(SB), NOSPLIT, $8
+ MOV $123, X20
+ RET // want `RET without writing to result register`
diff --git a/go/analysis/passes/assign/assign.go b/go/analysis/passes/assign/assign.go
index 3586638ef..89146b733 100644
--- a/go/analysis/passes/assign/assign.go
+++ b/go/analysis/passes/assign/assign.go
@@ -12,6 +12,7 @@ import (
"fmt"
"go/ast"
"go/token"
+ "go/types"
"reflect"
"golang.org/x/tools/go/analysis"
@@ -51,7 +52,8 @@ func run(pass *analysis.Pass) (interface{}, error) {
for i, lhs := range stmt.Lhs {
rhs := stmt.Rhs[i]
if analysisutil.HasSideEffects(pass.TypesInfo, lhs) ||
- analysisutil.HasSideEffects(pass.TypesInfo, rhs) {
+ analysisutil.HasSideEffects(pass.TypesInfo, rhs) ||
+ isMapIndex(pass.TypesInfo, lhs) {
continue // expressions may not be equal
}
if reflect.TypeOf(lhs) != reflect.TypeOf(rhs) {
@@ -74,3 +76,14 @@ func run(pass *analysis.Pass) (interface{}, error) {
return nil, nil
}
+
+// isMapIndex returns true if e is a map index expression.
+func isMapIndex(info *types.Info, e ast.Expr) bool {
+ if idx, ok := analysisutil.Unparen(e).(*ast.IndexExpr); ok {
+ if typ := info.Types[idx.X].Type; typ != nil {
+ _, ok := typ.Underlying().(*types.Map)
+ return ok
+ }
+ }
+ return false
+}
diff --git a/go/analysis/passes/assign/testdata/src/a/a.go b/go/analysis/passes/assign/testdata/src/a/a.go
index eaec634d1..f9663120b 100644
--- a/go/analysis/passes/assign/testdata/src/a/a.go
+++ b/go/analysis/passes/assign/testdata/src/a/a.go
@@ -29,3 +29,31 @@ func (s *ST) SetX(x int, ch chan int) {
}
func num() int { return 2 }
+
+func Index() {
+ s := []int{1}
+ s[0] = s[0] // want "self-assignment"
+
+ var a [5]int
+ a[0] = a[0] // want "self-assignment"
+
+ pa := &[2]int{1, 2}
+ pa[1] = pa[1] // want "self-assignment"
+
+ var pss *struct { // report self assignment despite nil dereference
+ s []int
+ }
+ pss.s[0] = pss.s[0] // want "self-assignment"
+
+ m := map[int]string{1: "a"}
+ m[0] = m[0] // bail on map self-assignments due to side effects
+ m[1] = m[1] // not modeling what elements must be in the map
+ (m[2]) = (m[2]) // even with parens
+ type Map map[string]bool
+ named := make(Map)
+ named["s"] = named["s"] // even on named maps.
+ var psm *struct {
+ m map[string]int
+ }
+ psm.m["key"] = psm.m["key"] // handles dereferences
+}
diff --git a/go/analysis/passes/assign/testdata/src/a/a.go.golden b/go/analysis/passes/assign/testdata/src/a/a.go.golden
index 6c91d3666..f45b7f208 100644
--- a/go/analysis/passes/assign/testdata/src/a/a.go.golden
+++ b/go/analysis/passes/assign/testdata/src/a/a.go.golden
@@ -29,3 +29,31 @@ func (s *ST) SetX(x int, ch chan int) {
}
func num() int { return 2 }
+
+func Index() {
+ s := []int{1}
+ // want "self-assignment"
+
+ var a [5]int
+ // want "self-assignment"
+
+ pa := &[2]int{1, 2}
+ // want "self-assignment"
+
+ var pss *struct { // report self assignment despite nil dereference
+ s []int
+ }
+ // want "self-assignment"
+
+ m := map[int]string{1: "a"}
+ m[0] = m[0] // bail on map self-assignments due to side effects
+ m[1] = m[1] // not modeling what elements must be in the map
+ (m[2]) = (m[2]) // even with parens
+ type Map map[string]bool
+ named := make(Map)
+ named["s"] = named["s"] // even on named maps.
+ var psm *struct {
+ m map[string]int
+ }
+ psm.m["key"] = psm.m["key"] // handles dereferences
+}
diff --git a/go/analysis/passes/bools/bools.go b/go/analysis/passes/bools/bools.go
index 5ae47d894..0d8b0bf4f 100644
--- a/go/analysis/passes/bools/bools.go
+++ b/go/analysis/passes/bools/bools.go
@@ -94,8 +94,10 @@ func (op boolOp) commutativeSets(info *types.Info, e *ast.BinaryExpr, seen map[*
}
// checkRedundant checks for expressions of the form
-// e && e
-// e || e
+//
+// e && e
+// e || e
+//
// Exprs must contain only side effect free expressions.
func (op boolOp) checkRedundant(pass *analysis.Pass, exprs []ast.Expr) {
seen := make(map[string]bool)
@@ -110,8 +112,10 @@ func (op boolOp) checkRedundant(pass *analysis.Pass, exprs []ast.Expr) {
}
// checkSuspect checks for expressions of the form
-// x != c1 || x != c2
-// x == c1 && x == c2
+//
+// x != c1 || x != c2
+// x == c1 && x == c2
+//
// where c1 and c2 are constant expressions.
// If c1 and c2 are the same then it's redundant;
// if c1 and c2 are different then it's always true or always false.
diff --git a/go/analysis/passes/buildssa/buildssa_test.go b/go/analysis/passes/buildssa/buildssa_test.go
index 0b381500b..52f7e7aa6 100644
--- a/go/analysis/passes/buildssa/buildssa_test.go
+++ b/go/analysis/passes/buildssa/buildssa_test.go
@@ -11,6 +11,7 @@ import (
"golang.org/x/tools/go/analysis/analysistest"
"golang.org/x/tools/go/analysis/passes/buildssa"
+ "golang.org/x/tools/internal/typeparams"
)
func Test(t *testing.T) {
@@ -27,3 +28,39 @@ func Test(t *testing.T) {
}
}
}
+
+func TestGenericDecls(t *testing.T) {
+ if !typeparams.Enabled {
+ t.Skip("TestGenericDecls requires type parameters.")
+ }
+ testdata := analysistest.TestData()
+ result := analysistest.Run(t, testdata, buildssa.Analyzer, "b")[0].Result
+
+ ssainfo := result.(*buildssa.SSA)
+ got := fmt.Sprint(ssainfo.SrcFuncs)
+ want := `[(*b.Pointer[T]).Load b.Load b.LoadPointer]`
+ if got != want {
+ t.Errorf("SSA.SrcFuncs = %s, want %s", got, want)
+ for _, f := range ssainfo.SrcFuncs {
+ f.WriteTo(os.Stderr)
+ }
+ }
+}
+
+func TestImporting(t *testing.T) {
+ if !typeparams.Enabled {
+ t.Skip("TestImporting depends on testdata/b/b/go which uses type parameters.")
+ }
+ testdata := analysistest.TestData()
+ result := analysistest.Run(t, testdata, buildssa.Analyzer, "c")[0].Result
+
+ ssainfo := result.(*buildssa.SSA)
+ got := fmt.Sprint(ssainfo.SrcFuncs)
+ want := `[c.A c.B]`
+ if got != want {
+ t.Errorf("SSA.SrcFuncs = %s, want %s", got, want)
+ for _, f := range ssainfo.SrcFuncs {
+ f.WriteTo(os.Stderr)
+ }
+ }
+}
diff --git a/go/analysis/passes/buildssa/testdata/src/b/b.go b/go/analysis/passes/buildssa/testdata/src/b/b.go
new file mode 100644
index 000000000..dd029cf60
--- /dev/null
+++ b/go/analysis/passes/buildssa/testdata/src/b/b.go
@@ -0,0 +1,20 @@
+// Package b contains declarations of generic functions.
+package b
+
+import "unsafe"
+
+type Pointer[T any] struct {
+ v unsafe.Pointer
+}
+
+func (x *Pointer[T]) Load() *T {
+ return (*T)(LoadPointer(&x.v))
+}
+
+func Load[T any](x *Pointer[T]) *T {
+ return x.Load()
+}
+
+func LoadPointer(addr *unsafe.Pointer) (val unsafe.Pointer)
+
+var G Pointer[int]
diff --git a/go/analysis/passes/buildssa/testdata/src/c/c.go b/go/analysis/passes/buildssa/testdata/src/c/c.go
new file mode 100644
index 000000000..387a3b0ed
--- /dev/null
+++ b/go/analysis/passes/buildssa/testdata/src/c/c.go
@@ -0,0 +1,24 @@
+// Package c is to test buildssa importing packages.
+package c
+
+import (
+ "a"
+ "b"
+ "unsafe"
+)
+
+func A() {
+ _ = a.Fib(10)
+}
+
+func B() {
+ var x int
+ ptr := unsafe.Pointer(&x)
+ _ = b.LoadPointer(&ptr)
+
+ m := b.G.Load()
+ f := b.Load(&b.G)
+ if f != m {
+ panic("loads of b.G are expected to be indentical")
+ }
+}
diff --git a/go/analysis/passes/buildtag/buildtag.go b/go/analysis/passes/buildtag/buildtag.go
index c4407ad91..775e507a3 100644
--- a/go/analysis/passes/buildtag/buildtag.go
+++ b/go/analysis/passes/buildtag/buildtag.go
@@ -20,7 +20,7 @@ import (
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
)
-const Doc = "check that +build tags are well-formed and correctly located"
+const Doc = "check //go:build and // +build directives"
var Analyzer = &analysis.Analyzer{
Name: "buildtag",
diff --git a/go/analysis/passes/buildtag/buildtag_old.go b/go/analysis/passes/buildtag/buildtag_old.go
index e9234925f..0001ba536 100644
--- a/go/analysis/passes/buildtag/buildtag_old.go
+++ b/go/analysis/passes/buildtag/buildtag_old.go
@@ -22,7 +22,7 @@ import (
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
)
-const Doc = "check that +build tags are well-formed and correctly located"
+const Doc = "check // +build directives"
var Analyzer = &analysis.Analyzer{
Name: "buildtag",
diff --git a/go/analysis/passes/cgocall/cgocall.go b/go/analysis/passes/cgocall/cgocall.go
index 5768d0b9b..b61ee5c3d 100644
--- a/go/analysis/passes/cgocall/cgocall.go
+++ b/go/analysis/passes/cgocall/cgocall.go
@@ -122,8 +122,8 @@ func checkCgo(fset *token.FileSet, f *ast.File, info *types.Info, reportf func(t
// For example, for each raw cgo source file in the original package,
// such as this one:
//
-// package p
-// import "C"
+// package p
+// import "C"
// import "fmt"
// type T int
// const k = 3
@@ -147,9 +147,9 @@ func checkCgo(fset *token.FileSet, f *ast.File, info *types.Info, reportf func(t
// the receiver into the first parameter;
// and all functions are renamed to "_".
//
-// package p
-// import . "·this·" // declares T, k, x, y, f, g, T.f
-// import "C"
+// package p
+// import . "·this·" // declares T, k, x, y, f, g, T.f
+// import "C"
// import "fmt"
// const _ = 3
// var _, _ = fmt.Println()
@@ -169,7 +169,6 @@ func checkCgo(fset *token.FileSet, f *ast.File, info *types.Info, reportf func(t
// C.f would resolve to "·this·"._C_func_f, for example. But we have
// limited ourselves here to preserving function bodies and initializer
// expressions since that is all that the cgocall analyzer needs.
-//
func typeCheckCgoSourceFiles(fset *token.FileSet, pkg *types.Package, files []*ast.File, info *types.Info, sizes types.Sizes) ([]*ast.File, *types.Info, error) {
const thispkg = "·this·"
@@ -284,8 +283,9 @@ func typeCheckCgoSourceFiles(fset *token.FileSet, pkg *types.Package, files []*a
// cgoBaseType tries to look through type conversions involving
// unsafe.Pointer to find the real type. It converts:
-// unsafe.Pointer(x) => x
-// *(*unsafe.Pointer)(unsafe.Pointer(&x)) => x
+//
+// unsafe.Pointer(x) => x
+// *(*unsafe.Pointer)(unsafe.Pointer(&x)) => x
func cgoBaseType(info *types.Info, arg ast.Expr) types.Type {
switch arg := arg.(type) {
case *ast.CallExpr:
diff --git a/go/analysis/passes/composite/composite.go b/go/analysis/passes/composite/composite.go
index d3670aca9..64e184d34 100644
--- a/go/analysis/passes/composite/composite.go
+++ b/go/analysis/passes/composite/composite.go
@@ -7,6 +7,7 @@
package composite
import (
+ "fmt"
"go/ast"
"go/types"
"strings"
@@ -83,7 +84,8 @@ func run(pass *analysis.Pass) (interface{}, error) {
}
for _, typ := range structuralTypes {
under := deref(typ.Underlying())
- if _, ok := under.(*types.Struct); !ok {
+ strct, ok := under.(*types.Struct)
+ if !ok {
// skip non-struct composite literals
continue
}
@@ -92,20 +94,47 @@ func run(pass *analysis.Pass) (interface{}, error) {
continue
}
- // check if the CompositeLit contains an unkeyed field
+ // check if the struct contains an unkeyed field
allKeyValue := true
- for _, e := range cl.Elts {
+ var suggestedFixAvailable = len(cl.Elts) == strct.NumFields()
+ var missingKeys []analysis.TextEdit
+ for i, e := range cl.Elts {
if _, ok := e.(*ast.KeyValueExpr); !ok {
allKeyValue = false
- break
+ if i >= strct.NumFields() {
+ break
+ }
+ field := strct.Field(i)
+ if !field.Exported() {
+ // Adding unexported field names for structs not defined
+ // locally will not work.
+ suggestedFixAvailable = false
+ break
+ }
+ missingKeys = append(missingKeys, analysis.TextEdit{
+ Pos: e.Pos(),
+ End: e.Pos(),
+ NewText: []byte(fmt.Sprintf("%s: ", field.Name())),
+ })
}
}
if allKeyValue {
- // all the composite literal fields are keyed
+ // all the struct fields are keyed
continue
}
- pass.ReportRangef(cl, "%s composite literal uses unkeyed fields", typeName)
+ diag := analysis.Diagnostic{
+ Pos: cl.Pos(),
+ End: cl.End(),
+ Message: fmt.Sprintf("%s struct literal uses unkeyed fields", typeName),
+ }
+ if suggestedFixAvailable {
+ diag.SuggestedFixes = []analysis.SuggestedFix{{
+ Message: "Add field names to struct literal",
+ TextEdits: missingKeys,
+ }}
+ }
+ pass.Report(diag)
return
}
})
diff --git a/go/analysis/passes/composite/composite_test.go b/go/analysis/passes/composite/composite_test.go
index 952de8bfd..7afaaa7ff 100644
--- a/go/analysis/passes/composite/composite_test.go
+++ b/go/analysis/passes/composite/composite_test.go
@@ -18,5 +18,5 @@ func Test(t *testing.T) {
if typeparams.Enabled {
pkgs = append(pkgs, "typeparams")
}
- analysistest.Run(t, testdata, composite.Analyzer, pkgs...)
+ analysistest.RunWithSuggestedFixes(t, testdata, composite.Analyzer, pkgs...)
}
diff --git a/go/analysis/passes/composite/testdata/src/a/a.go b/go/analysis/passes/composite/testdata/src/a/a.go
index 3a5bc203b..cd69d3951 100644
--- a/go/analysis/passes/composite/testdata/src/a/a.go
+++ b/go/analysis/passes/composite/testdata/src/a/a.go
@@ -11,6 +11,7 @@ import (
"go/scanner"
"go/token"
"image"
+ "sync"
"unicode"
)
@@ -79,6 +80,18 @@ var badStructLiteral = flag.Flag{ // want "unkeyed fields"
nil, // Value
"DefValue",
}
+var tooManyFieldsStructLiteral = flag.Flag{ // want "unkeyed fields"
+ "Name",
+ "Usage",
+ nil, // Value
+ "DefValue",
+ "Extra Field",
+}
+var tooFewFieldsStructLiteral = flag.Flag{ // want "unkeyed fields"
+ "Name",
+ "Usage",
+ nil, // Value
+}
var delta [3]rune
@@ -100,6 +113,10 @@ var badScannerErrorList = scanner.ErrorList{
&scanner.Error{token.Position{}, "foobar"}, // want "unkeyed fields"
}
+// sync.Mutex has unexported fields. We expect a diagnostic but no
+// suggested fix.
+var mu = sync.Mutex{0, 0} // want "unkeyed fields"
+
// Check whitelisted structs: if vet is run with --compositewhitelist=false,
// this line triggers an error.
var whitelistedPoint = image.Point{1, 2}
diff --git a/go/analysis/passes/composite/testdata/src/a/a.go.golden b/go/analysis/passes/composite/testdata/src/a/a.go.golden
new file mode 100644
index 000000000..fe73a2e0a
--- /dev/null
+++ b/go/analysis/passes/composite/testdata/src/a/a.go.golden
@@ -0,0 +1,144 @@
+// Copyright 2012 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.
+
+// This file contains the test for untagged struct literals.
+
+package a
+
+import (
+ "flag"
+ "go/scanner"
+ "go/token"
+ "image"
+ "sync"
+ "unicode"
+)
+
+var Okay1 = []string{
+ "Name",
+ "Usage",
+ "DefValue",
+}
+
+var Okay2 = map[string]bool{
+ "Name": true,
+ "Usage": true,
+ "DefValue": true,
+}
+
+var Okay3 = struct {
+ X string
+ Y string
+ Z string
+}{
+ "Name",
+ "Usage",
+ "DefValue",
+}
+
+var Okay4 = []struct {
+ A int
+ B int
+}{
+ {1, 2},
+ {3, 4},
+}
+
+type MyStruct struct {
+ X string
+ Y string
+ Z string
+}
+
+var Okay5 = &MyStruct{
+ "Name",
+ "Usage",
+ "DefValue",
+}
+
+var Okay6 = []MyStruct{
+ {"foo", "bar", "baz"},
+ {"aa", "bb", "cc"},
+}
+
+var Okay7 = []*MyStruct{
+ {"foo", "bar", "baz"},
+ {"aa", "bb", "cc"},
+}
+
+// Testing is awkward because we need to reference things from a separate package
+// to trigger the warnings.
+
+var goodStructLiteral = flag.Flag{
+ Name: "Name",
+ Usage: "Usage",
+}
+var badStructLiteral = flag.Flag{ // want "unkeyed fields"
+ Name: "Name",
+ Usage: "Usage",
+ Value: nil, // Value
+ DefValue: "DefValue",
+}
+var tooManyFieldsStructLiteral = flag.Flag{ // want "unkeyed fields"
+ "Name",
+ "Usage",
+ nil, // Value
+ "DefValue",
+ "Extra Field",
+}
+var tooFewFieldsStructLiteral = flag.Flag{ // want "unkeyed fields"
+ "Name",
+ "Usage",
+ nil, // Value
+}
+
+var delta [3]rune
+
+// SpecialCase is a named slice of CaseRange to test issue 9171.
+var goodNamedSliceLiteral = unicode.SpecialCase{
+ {Lo: 1, Hi: 2, Delta: delta},
+ unicode.CaseRange{Lo: 1, Hi: 2, Delta: delta},
+}
+var badNamedSliceLiteral = unicode.SpecialCase{
+ {Lo: 1, Hi: 2, Delta: delta}, // want "unkeyed fields"
+ unicode.CaseRange{Lo: 1, Hi: 2, Delta: delta}, // want "unkeyed fields"
+}
+
+// ErrorList is a named slice, so no warnings should be emitted.
+var goodScannerErrorList = scanner.ErrorList{
+ &scanner.Error{Msg: "foobar"},
+}
+var badScannerErrorList = scanner.ErrorList{
+ &scanner.Error{Pos: token.Position{}, Msg: "foobar"}, // want "unkeyed fields"
+}
+
+// sync.Mutex has unexported fields. We expect a diagnostic but no
+// suggested fix.
+var mu = sync.Mutex{0, 0} // want "unkeyed fields"
+
+// Check whitelisted structs: if vet is run with --compositewhitelist=false,
+// this line triggers an error.
+var whitelistedPoint = image.Point{1, 2}
+
+// Do not check type from unknown package.
+// See issue 15408.
+var unknownPkgVar = unicode.NoSuchType{"foo", "bar"}
+
+// A named pointer slice of CaseRange to test issue 23539. In
+// particular, we're interested in how some slice elements omit their
+// type.
+var goodNamedPointerSliceLiteral = []*unicode.CaseRange{
+ {Lo: 1, Hi: 2},
+ &unicode.CaseRange{Lo: 1, Hi: 2},
+}
+var badNamedPointerSliceLiteral = []*unicode.CaseRange{
+ {Lo: 1, Hi: 2, Delta: delta}, // want "unkeyed fields"
+ &unicode.CaseRange{Lo: 1, Hi: 2, Delta: delta}, // want "unkeyed fields"
+}
+
+// unicode.Range16 is whitelisted, so there'll be no vet error
+var range16 = unicode.Range16{0xfdd0, 0xfdef, 1}
+
+// unicode.Range32 is whitelisted, so there'll be no vet error
+var range32 = unicode.Range32{0x1fffe, 0x1ffff, 1}
diff --git a/go/analysis/passes/composite/testdata/src/a/a_fuzz_test.go.golden b/go/analysis/passes/composite/testdata/src/a/a_fuzz_test.go.golden
new file mode 100644
index 000000000..20b652e88
--- /dev/null
+++ b/go/analysis/passes/composite/testdata/src/a/a_fuzz_test.go.golden
@@ -0,0 +1,16 @@
+// Copyright 2022 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.18
+// +build go1.18
+
+package a
+
+import "testing"
+
+var fuzzTargets = []testing.InternalFuzzTarget{
+ {"Fuzz", Fuzz},
+}
+
+func Fuzz(f *testing.F) {}
diff --git a/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go b/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go
index dd5d57efe..f9a5e1fb1 100644
--- a/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go
+++ b/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go
@@ -6,7 +6,7 @@ package typeparams
import "typeparams/lib"
-type localStruct struct { F int }
+type localStruct struct{ F int }
func F[
T1 ~struct{ f int },
@@ -20,8 +20,8 @@ func F[
_ = T1{2}
_ = T2a{2}
_ = T2b{2} // want "unkeyed fields"
- _ = T3{1,2}
- _ = T4{1,2}
- _ = T5{1:2}
- _ = T6{1:2}
+ _ = T3{1, 2}
+ _ = T4{1, 2}
+ _ = T5{1: 2}
+ _ = T6{1: 2}
}
diff --git a/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go.golden b/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go.golden
new file mode 100644
index 000000000..66cd9158c
--- /dev/null
+++ b/go/analysis/passes/composite/testdata/src/typeparams/typeparams.go.golden
@@ -0,0 +1,27 @@
+// Copyright 2021 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.
+
+package typeparams
+
+import "typeparams/lib"
+
+type localStruct struct{ F int }
+
+func F[
+ T1 ~struct{ f int },
+ T2a localStruct,
+ T2b lib.Struct,
+ T3 ~[]int,
+ T4 lib.Slice,
+ T5 ~map[int]int,
+ T6 lib.Map,
+]() {
+ _ = T1{2}
+ _ = T2a{2}
+ _ = T2b{F: 2} // want "unkeyed fields"
+ _ = T3{1, 2}
+ _ = T4{1, 2}
+ _ = T5{1: 2}
+ _ = T6{1: 2}
+}
diff --git a/go/analysis/passes/copylock/copylock.go b/go/analysis/passes/copylock/copylock.go
index 350dc4e0f..8cc93e94d 100644
--- a/go/analysis/passes/copylock/copylock.go
+++ b/go/analysis/passes/copylock/copylock.go
@@ -128,7 +128,7 @@ func checkCopyLocksCallExpr(pass *analysis.Pass, ce *ast.CallExpr) {
}
if fun, ok := pass.TypesInfo.Uses[id].(*types.Builtin); ok {
switch fun.Name() {
- case "new", "len", "cap", "Sizeof":
+ case "new", "len", "cap", "Sizeof", "Offsetof", "Alignof":
return
}
}
diff --git a/go/analysis/passes/copylock/testdata/src/a/copylock.go b/go/analysis/passes/copylock/testdata/src/a/copylock.go
index e528280ab..4ab66dca1 100644
--- a/go/analysis/passes/copylock/testdata/src/a/copylock.go
+++ b/go/analysis/passes/copylock/testdata/src/a/copylock.go
@@ -50,27 +50,27 @@ func BadFunc() {
var t Tlock
var tp *Tlock
tp = &t
- *tp = t // want `assignment copies lock value to \*tp: a.Tlock contains sync.Once contains sync.Mutex`
- t = *tp // want "assignment copies lock value to t: a.Tlock contains sync.Once contains sync.Mutex"
+ *tp = t // want `assignment copies lock value to \*tp: a.Tlock contains sync.Once contains sync\b.*`
+ t = *tp // want `assignment copies lock value to t: a.Tlock contains sync.Once contains sync\b.*`
y := *x // want "assignment copies lock value to y: sync.Mutex"
- var z = t // want "variable declaration copies lock value to z: a.Tlock contains sync.Once contains sync.Mutex"
+ var z = t // want `variable declaration copies lock value to z: a.Tlock contains sync.Once contains sync\b.*`
w := struct{ L sync.Mutex }{
L: *x, // want `literal copies lock value from \*x: sync.Mutex`
}
var q = map[int]Tlock{
- 1: t, // want "literal copies lock value from t: a.Tlock contains sync.Once contains sync.Mutex"
- 2: *tp, // want `literal copies lock value from \*tp: a.Tlock contains sync.Once contains sync.Mutex`
+ 1: t, // want `literal copies lock value from t: a.Tlock contains sync.Once contains sync\b.*`
+ 2: *tp, // want `literal copies lock value from \*tp: a.Tlock contains sync.Once contains sync\b.*`
}
yy := []Tlock{
- t, // want "literal copies lock value from t: a.Tlock contains sync.Once contains sync.Mutex"
- *tp, // want `literal copies lock value from \*tp: a.Tlock contains sync.Once contains sync.Mutex`
+ t, // want `literal copies lock value from t: a.Tlock contains sync.Once contains sync\b.*`
+ *tp, // want `literal copies lock value from \*tp: a.Tlock contains sync.Once contains sync\b.*`
}
// override 'new' keyword
new := func(interface{}) {}
- new(t) // want "call of new copies lock value: a.Tlock contains sync.Once contains sync.Mutex"
+ new(t) // want `call of new copies lock value: a.Tlock contains sync.Once contains sync\b.*`
// copy of array of locks
var muA [5]sync.Mutex
@@ -124,6 +124,26 @@ func SizeofMutex() {
Sizeof(mu) // want "call of Sizeof copies lock value: sync.Mutex"
}
+func OffsetofMutex() {
+ type T struct {
+ f int
+ mu sync.Mutex
+ }
+ unsafe.Offsetof(T{}.mu) // OK
+ unsafe := struct{ Offsetof func(interface{}) }{}
+ unsafe.Offsetof(T{}.mu) // want "call of unsafe.Offsetof copies lock value: sync.Mutex"
+}
+
+func AlignofMutex() {
+ type T struct {
+ f int
+ mu sync.Mutex
+ }
+ unsafe.Alignof(T{}.mu) // OK
+ unsafe := struct{ Alignof func(interface{}) }{}
+ unsafe.Alignof(T{}.mu) // want "call of unsafe.Alignof copies lock value: sync.Mutex"
+}
+
// SyncTypesCheck checks copying of sync.* types except sync.Mutex
func SyncTypesCheck() {
// sync.RWMutex copying
@@ -173,9 +193,9 @@ func SyncTypesCheck() {
var onceX sync.Once
var onceXX = sync.Once{}
onceX1 := new(sync.Once)
- onceY := onceX // want "assignment copies lock value to onceY: sync.Once contains sync.Mutex"
- onceY = onceX // want "assignment copies lock value to onceY: sync.Once contains sync.Mutex"
- var onceYY = onceX // want "variable declaration copies lock value to onceYY: sync.Once contains sync.Mutex"
+ onceY := onceX // want `assignment copies lock value to onceY: sync.Once contains sync\b.*`
+ onceY = onceX // want `assignment copies lock value to onceY: sync.Once contains sync\b.*`
+ var onceYY = onceX // want `variable declaration copies lock value to onceYY: sync.Once contains sync\b.*`
onceP := &onceX
onceZ := &sync.Once{}
}
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 801bc6f24..0d3168f1e 100644
--- a/go/analysis/passes/copylock/testdata/src/a/copylock_func.go
+++ b/go/analysis/passes/copylock/testdata/src/a/copylock_func.go
@@ -126,7 +126,7 @@ func AcceptedCases() {
// 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.Mutex"
+func (LocalOnce) Bad() {} // want `Bad passes lock by value: a.LocalOnce contains sync.\b.*`
// False negative:
// LocalMutex doesn't have a Lock method.
diff --git a/go/analysis/passes/directive/directive.go b/go/analysis/passes/directive/directive.go
new file mode 100644
index 000000000..76d852cd0
--- /dev/null
+++ b/go/analysis/passes/directive/directive.go
@@ -0,0 +1,216 @@
+// Copyright 2023 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.
+
+// Package directive defines an Analyzer that checks known Go toolchain directives.
+package directive
+
+import (
+ "go/ast"
+ "go/parser"
+ "go/token"
+ "strings"
+ "unicode"
+ "unicode/utf8"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/passes/internal/analysisutil"
+)
+
+const Doc = `check Go toolchain directives such as //go:debug
+
+This analyzer checks for problems with known Go toolchain directives
+in all Go source files in a package directory, even those excluded by
+//go:build constraints, and all non-Go source files too.
+
+For //go:debug (see https://go.dev/doc/godebug), the analyzer checks
+that the directives are placed only in Go source files, only above the
+package comment, and only in package main or *_test.go files.
+
+Support for other known directives may be added in the future.
+
+This analyzer does not check //go:build, which is handled by the
+buildtag analyzer.
+`
+
+var Analyzer = &analysis.Analyzer{
+ Name: "directive",
+ Doc: Doc,
+ Run: runDirective,
+}
+
+func runDirective(pass *analysis.Pass) (interface{}, error) {
+ for _, f := range pass.Files {
+ checkGoFile(pass, f)
+ }
+ for _, name := range pass.OtherFiles {
+ if err := checkOtherFile(pass, name); err != nil {
+ return nil, err
+ }
+ }
+ for _, name := range pass.IgnoredFiles {
+ if strings.HasSuffix(name, ".go") {
+ f, err := parser.ParseFile(pass.Fset, name, nil, parser.ParseComments)
+ if err != nil {
+ // Not valid Go source code - not our job to diagnose, so ignore.
+ continue
+ }
+ checkGoFile(pass, f)
+ } else {
+ if err := checkOtherFile(pass, name); err != nil {
+ return nil, err
+ }
+ }
+ }
+ return nil, nil
+}
+
+func checkGoFile(pass *analysis.Pass, f *ast.File) {
+ check := newChecker(pass, pass.Fset.File(f.Package).Name(), f)
+
+ for _, group := range f.Comments {
+ // A +build comment is ignored after or adjoining the package declaration.
+ if group.End()+1 >= f.Package {
+ check.inHeader = false
+ }
+ // A //go:build comment is ignored after the package declaration
+ // (but adjoining it is OK, in contrast to +build comments).
+ if group.Pos() >= f.Package {
+ check.inHeader = false
+ }
+
+ // Check each line of a //-comment.
+ for _, c := range group.List {
+ check.comment(c.Slash, c.Text)
+ }
+ }
+}
+
+func checkOtherFile(pass *analysis.Pass, filename string) error {
+ // We cannot use the Go parser, since is not a Go source file.
+ // Read the raw bytes instead.
+ content, tf, err := analysisutil.ReadFile(pass.Fset, filename)
+ if err != nil {
+ return err
+ }
+
+ check := newChecker(pass, filename, nil)
+ check.nonGoFile(token.Pos(tf.Base()), string(content))
+ return nil
+}
+
+type checker struct {
+ pass *analysis.Pass
+ filename string
+ file *ast.File // nil for non-Go file
+ inHeader bool // in file header (before package declaration)
+ inStar bool // currently in a /* */ comment
+}
+
+func newChecker(pass *analysis.Pass, filename string, file *ast.File) *checker {
+ return &checker{
+ pass: pass,
+ filename: filename,
+ file: file,
+ inHeader: true,
+ }
+}
+
+func (check *checker) nonGoFile(pos token.Pos, fullText string) {
+ // Process each line.
+ text := fullText
+ inStar := false
+ for text != "" {
+ offset := len(fullText) - len(text)
+ var line string
+ line, text, _ = stringsCut(text, "\n")
+
+ if !inStar && strings.HasPrefix(line, "//") {
+ check.comment(pos+token.Pos(offset), line)
+ continue
+ }
+
+ // Skip over, cut out any /* */ comments,
+ // to avoid being confused by a commented-out // comment.
+ for {
+ line = strings.TrimSpace(line)
+ if inStar {
+ var ok bool
+ _, line, ok = stringsCut(line, "*/")
+ if !ok {
+ break
+ }
+ inStar = false
+ continue
+ }
+ line, inStar = stringsCutPrefix(line, "/*")
+ if !inStar {
+ break
+ }
+ }
+ if line != "" {
+ // Found non-comment non-blank line.
+ // Ends space for valid //go:build comments,
+ // but also ends the fraction of the file we can
+ // reliably parse. From this point on we might
+ // incorrectly flag "comments" inside multiline
+ // string constants or anything else (this might
+ // not even be a Go program). So stop.
+ break
+ }
+ }
+}
+
+func (check *checker) comment(pos token.Pos, line string) {
+ if !strings.HasPrefix(line, "//go:") {
+ return
+ }
+ // testing hack: stop at // ERROR
+ if i := strings.Index(line, " // ERROR "); i >= 0 {
+ line = line[:i]
+ }
+
+ verb := line
+ if i := strings.IndexFunc(verb, unicode.IsSpace); i >= 0 {
+ verb = verb[:i]
+ if line[i] != ' ' && line[i] != '\t' && line[i] != '\n' {
+ r, _ := utf8.DecodeRuneInString(line[i:])
+ check.pass.Reportf(pos, "invalid space %#q in %s directive", r, verb)
+ }
+ }
+
+ switch verb {
+ default:
+ // TODO: Use the go language version for the file.
+ // If that version is not newer than us, then we can
+ // report unknown directives.
+
+ case "//go:build":
+ // Ignore. The buildtag analyzer reports misplaced comments.
+
+ case "//go:debug":
+ if check.file == nil {
+ check.pass.Reportf(pos, "//go:debug directive only valid in Go source files")
+ } else if check.file.Name.Name != "main" && !strings.HasSuffix(check.filename, "_test.go") {
+ check.pass.Reportf(pos, "//go:debug directive only valid in package main or test")
+ } else if !check.inHeader {
+ check.pass.Reportf(pos, "//go:debug directive only valid before package declaration")
+ }
+ }
+}
+
+// Go 1.18 strings.Cut.
+func stringsCut(s, sep string) (before, after string, found bool) {
+ if i := strings.Index(s, sep); i >= 0 {
+ return s[:i], s[i+len(sep):], true
+ }
+ return s, "", false
+}
+
+// Go 1.20 strings.CutPrefix.
+func stringsCutPrefix(s, prefix string) (after string, found bool) {
+ if !strings.HasPrefix(s, prefix) {
+ return s, false
+ }
+ return s[len(prefix):], true
+}
diff --git a/go/analysis/passes/directive/directive_test.go b/go/analysis/passes/directive/directive_test.go
new file mode 100644
index 000000000..a526c0d74
--- /dev/null
+++ b/go/analysis/passes/directive/directive_test.go
@@ -0,0 +1,39 @@
+// Copyright 2023 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.
+
+package directive_test
+
+import (
+ "runtime"
+ "strings"
+ "testing"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/analysistest"
+ "golang.org/x/tools/go/analysis/passes/directive"
+)
+
+func Test(t *testing.T) {
+ if strings.HasPrefix(runtime.Version(), "go1.") && runtime.Version() < "go1.16" {
+ t.Skipf("skipping on %v", runtime.Version())
+ }
+ analyzer := *directive.Analyzer
+ analyzer.Run = func(pass *analysis.Pass) (interface{}, error) {
+ defer func() {
+ // The directive pass is unusual in that it checks the IgnoredFiles.
+ // After analysis, add IgnoredFiles to OtherFiles so that
+ // the test harness checks for expected diagnostics in those.
+ // (The test harness shouldn't do this by default because most
+ // passes can't do anything with the IgnoredFiles without type
+ // information, which is unavailable because they are ignored.)
+ var files []string
+ files = append(files, pass.OtherFiles...)
+ files = append(files, pass.IgnoredFiles...)
+ pass.OtherFiles = files
+ }()
+
+ return directive.Analyzer.Run(pass)
+ }
+ analysistest.Run(t, analysistest.TestData(), &analyzer, "a")
+}
diff --git a/go/analysis/passes/directive/testdata/src/a/badspace.go b/go/analysis/passes/directive/testdata/src/a/badspace.go
new file mode 100644
index 000000000..113139960
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/badspace.go
@@ -0,0 +1,11 @@
+// Copyright 2023 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 ignore
+
+// want +1 `invalid space '\\u00a0' in //go:debug directive`
+//go:debug 00a0
+
+package main
+
diff --git a/go/analysis/passes/directive/testdata/src/a/misplaced.go b/go/analysis/passes/directive/testdata/src/a/misplaced.go
new file mode 100644
index 000000000..db30ceb47
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/misplaced.go
@@ -0,0 +1,10 @@
+// Copyright 2023 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 ignore
+
+package main
+
+// want +1 `//go:debug directive only valid before package declaration`
+//go:debug panicnil=1
diff --git a/go/analysis/passes/directive/testdata/src/a/misplaced.s b/go/analysis/passes/directive/testdata/src/a/misplaced.s
new file mode 100644
index 000000000..9e26dbc52
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/misplaced.s
@@ -0,0 +1,19 @@
+// Copyright 2023 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.
+
+// want +1 `//go:debug directive only valid in Go source files`
+//go:debug panicnil=1
+
+/*
+can skip over comments
+//go:debug doesn't matter here
+*/
+
+// want +1 `//go:debug directive only valid in Go source files`
+//go:debug panicnil=1
+
+package a
+
+// no error here because we can't parse this far
+//go:debug panicnil=1
diff --git a/go/analysis/passes/directive/testdata/src/a/misplaced_test.go b/go/analysis/passes/directive/testdata/src/a/misplaced_test.go
new file mode 100644
index 000000000..6b4527a35
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/misplaced_test.go
@@ -0,0 +1,10 @@
+// Copyright 2023 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:debug panicnil=1
+
+package p_test
+
+// want +1 `//go:debug directive only valid before package declaration`
+//go:debug panicnil=1
diff --git a/go/analysis/passes/directive/testdata/src/a/p.go b/go/analysis/passes/directive/testdata/src/a/p.go
new file mode 100644
index 000000000..e1e3e6552
--- /dev/null
+++ b/go/analysis/passes/directive/testdata/src/a/p.go
@@ -0,0 +1,11 @@
+// Copyright 2023 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.
+
+// want +1 `//go:debug directive only valid in package main or test`
+//go:debug panicnil=1
+
+package p
+
+// want +1 `//go:debug directive only valid in package main or test`
+//go:debug panicnil=1
diff --git a/go/analysis/passes/errorsas/errorsas.go b/go/analysis/passes/errorsas/errorsas.go
index 384f02557..96adad3ee 100644
--- a/go/analysis/passes/errorsas/errorsas.go
+++ b/go/analysis/passes/errorsas/errorsas.go
@@ -7,6 +7,7 @@
package errorsas
import (
+ "errors"
"go/ast"
"go/types"
@@ -50,26 +51,39 @@ func run(pass *analysis.Pass) (interface{}, error) {
if len(call.Args) < 2 {
return // not enough arguments, e.g. called with return values of another function
}
- if fn.FullName() == "errors.As" && !pointerToInterfaceOrError(pass, call.Args[1]) {
- pass.ReportRangef(call, "second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type")
+ if fn.FullName() != "errors.As" {
+ return
+ }
+ if err := checkAsTarget(pass, call.Args[1]); err != nil {
+ pass.ReportRangef(call, "%v", err)
}
})
return nil, nil
}
-var errorType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface)
+var errorType = types.Universe.Lookup("error").Type()
// pointerToInterfaceOrError reports whether the type of e is a pointer to an interface or a type implementing error,
// or is the empty interface.
-func pointerToInterfaceOrError(pass *analysis.Pass, e ast.Expr) bool {
+
+// checkAsTarget reports an error if the second argument to errors.As is invalid.
+func checkAsTarget(pass *analysis.Pass, e ast.Expr) error {
t := pass.TypesInfo.Types[e].Type
if it, ok := t.Underlying().(*types.Interface); ok && it.NumMethods() == 0 {
- return true
+ // A target of interface{} is always allowed, since it often indicates
+ // a value forwarded from another source.
+ return nil
}
pt, ok := t.Underlying().(*types.Pointer)
if !ok {
- return false
+ return errors.New("second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type")
+ }
+ if pt.Elem() == errorType {
+ return errors.New("second argument to errors.As should not be *error")
}
_, ok = pt.Elem().Underlying().(*types.Interface)
- return ok || types.Implements(pt.Elem(), errorType)
+ if ok || types.Implements(pt.Elem(), errorType.Underlying().(*types.Interface)) {
+ return nil
+ }
+ return errors.New("second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type")
}
diff --git a/go/analysis/passes/errorsas/testdata/src/a/a.go b/go/analysis/passes/errorsas/testdata/src/a/a.go
index c987a8a65..7a9ae8976 100644
--- a/go/analysis/passes/errorsas/testdata/src/a/a.go
+++ b/go/analysis/passes/errorsas/testdata/src/a/a.go
@@ -28,10 +28,10 @@ func _() {
f iface
ei interface{}
)
- errors.As(nil, &e) // *error
+ errors.As(nil, &e) // want `second argument to errors.As should not be \*error`
errors.As(nil, &m) // *T where T implemements error
errors.As(nil, &f) // *interface
- errors.As(nil, perr()) // *error, via a call
+ errors.As(nil, perr()) // want `second argument to errors.As should not be \*error`
errors.As(nil, ei) // empty interface
errors.As(nil, nil) // want `second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type`
diff --git a/go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go b/go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go
index 5b9ec457c..4f7ae8491 100644
--- a/go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go
+++ b/go/analysis/passes/errorsas/testdata/src/typeparams/typeparams.go
@@ -28,7 +28,7 @@ func _[E error](e E) {
errors.As(nil, &e)
errors.As(nil, &m) // *T where T implemements error
errors.As(nil, &tw.t) // *T where T implements error
- errors.As(nil, perr[error]()) // *error, via a call
+ errors.As(nil, perr[error]()) // want `second argument to errors.As should not be \*error`
errors.As(nil, e) // want `second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type`
errors.As(nil, m) // want `second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type`
diff --git a/go/analysis/passes/fieldalignment/fieldalignment.go b/go/analysis/passes/fieldalignment/fieldalignment.go
index 78afe94ab..aff663046 100644
--- a/go/analysis/passes/fieldalignment/fieldalignment.go
+++ b/go/analysis/passes/fieldalignment/fieldalignment.go
@@ -23,7 +23,7 @@ import (
const Doc = `find structs that would use less memory if their fields were sorted
This analyzer find structs that can be rearranged to use less memory, and provides
-a suggested edit with the optimal order.
+a suggested edit with the most compact order.
Note that there are two different diagnostics reported. One checks struct size,
and the other reports "pointer bytes" used. Pointer bytes is how many bytes of the
@@ -41,6 +41,11 @@ has 24 pointer bytes because it has to scan further through the *uint32.
struct { string; uint32 }
has 8 because it can stop immediately after the string pointer.
+
+Be aware that the most compact order is not always the most efficient.
+In rare cases it may cause two variables each updated by its own goroutine
+to occupy the same CPU cache line, inducing a form of memory contention
+known as "false sharing" that slows down both goroutines.
`
var Analyzer = &analysis.Analyzer{
diff --git a/go/analysis/passes/httpresponse/httpresponse.go b/go/analysis/passes/httpresponse/httpresponse.go
index fd9e2af2b..3b9168c6c 100644
--- a/go/analysis/passes/httpresponse/httpresponse.go
+++ b/go/analysis/passes/httpresponse/httpresponse.go
@@ -62,15 +62,23 @@ func run(pass *analysis.Pass) (interface{}, error) {
// Find the innermost containing block, and get the list
// of statements starting with the one containing call.
- stmts := restOfBlock(stack)
+ stmts, ncalls := restOfBlock(stack)
if len(stmts) < 2 {
- return true // the call to the http function is the last statement of the block.
+ // The call to the http function is the last statement of the block.
+ return true
+ }
+
+ // Skip cases in which the call is wrapped by another (#52661).
+ // Example: resp, err := checkError(http.Get(url))
+ if ncalls > 1 {
+ return true
}
asg, ok := stmts[0].(*ast.AssignStmt)
if !ok {
return true // the first statement is not assignment.
}
+
resp := rootIdent(asg.Lhs[0])
if resp == nil {
return true // could not find the http.Response in the assignment.
@@ -130,20 +138,25 @@ func isHTTPFuncOrMethodOnClient(info *types.Info, expr *ast.CallExpr) bool {
}
// restOfBlock, given a traversal stack, finds the innermost containing
-// block and returns the suffix of its statements starting with the
-// current node (the last element of stack).
-func restOfBlock(stack []ast.Node) []ast.Stmt {
+// block and returns the suffix of its statements starting with the current
+// node, along with the number of call expressions encountered.
+func restOfBlock(stack []ast.Node) ([]ast.Stmt, int) {
+ var ncalls int
for i := len(stack) - 1; i >= 0; i-- {
if b, ok := stack[i].(*ast.BlockStmt); ok {
for j, v := range b.List {
if v == stack[i+1] {
- return b.List[j:]
+ return b.List[j:], ncalls
}
}
break
}
+
+ if _, ok := stack[i].(*ast.CallExpr); ok {
+ ncalls++
+ }
}
- return nil
+ return nil, 0
}
// rootIdent finds the root identifier x in a chain of selections x.y.z, or nil if not found.
diff --git a/go/analysis/passes/httpresponse/httpresponse_test.go b/go/analysis/passes/httpresponse/httpresponse_test.go
index 14e166789..34dc78ce2 100644
--- a/go/analysis/passes/httpresponse/httpresponse_test.go
+++ b/go/analysis/passes/httpresponse/httpresponse_test.go
@@ -5,10 +5,11 @@
package httpresponse_test
import (
+ "testing"
+
"golang.org/x/tools/go/analysis/analysistest"
"golang.org/x/tools/go/analysis/passes/httpresponse"
"golang.org/x/tools/internal/typeparams"
- "testing"
)
func Test(t *testing.T) {
diff --git a/go/analysis/passes/httpresponse/testdata/src/a/a.go b/go/analysis/passes/httpresponse/testdata/src/a/a.go
index df7703f41..de4121270 100644
--- a/go/analysis/passes/httpresponse/testdata/src/a/a.go
+++ b/go/analysis/passes/httpresponse/testdata/src/a/a.go
@@ -83,3 +83,30 @@ func badClientDo() {
log.Fatal(err)
}
}
+
+func goodUnwrapResp() {
+ unwrapResp := func(resp *http.Response, err error) *http.Response {
+ if err != nil {
+ panic(err)
+ }
+ return resp
+ }
+ resp := unwrapResp(http.Get("https://golang.org"))
+ // It is ok to call defer here immediately as err has
+ // been checked in unwrapResp (see #52661).
+ defer resp.Body.Close()
+}
+
+func badUnwrapResp() {
+ unwrapResp := func(resp *http.Response, err error) string {
+ if err != nil {
+ panic(err)
+ }
+ return "https://golang.org/" + resp.Status
+ }
+ resp, err := http.Get(unwrapResp(http.Get("https://golang.org")))
+ defer resp.Body.Close() // want "using resp before checking for errors"
+ if err != nil {
+ log.Fatal(err)
+ }
+}
diff --git a/go/analysis/passes/ifaceassert/parameterized.go b/go/analysis/passes/ifaceassert/parameterized.go
index 1285ecf13..b35f62dc7 100644
--- a/go/analysis/passes/ifaceassert/parameterized.go
+++ b/go/analysis/passes/ifaceassert/parameterized.go
@@ -1,6 +1,7 @@
// Copyright 2022 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.
+
package ifaceassert
import (
diff --git a/go/analysis/passes/inspect/inspect.go b/go/analysis/passes/inspect/inspect.go
index 4bb652a72..165c70cbd 100644
--- a/go/analysis/passes/inspect/inspect.go
+++ b/go/analysis/passes/inspect/inspect.go
@@ -19,14 +19,13 @@
// Requires: []*analysis.Analyzer{inspect.Analyzer},
// }
//
-// func run(pass *analysis.Pass) (interface{}, error) {
-// inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
-// inspect.Preorder(nil, func(n ast.Node) {
-// ...
-// })
-// return nil
-// }
-//
+// func run(pass *analysis.Pass) (interface{}, error) {
+// inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+// inspect.Preorder(nil, func(n ast.Node) {
+// ...
+// })
+// return nil, nil
+// }
package inspect
import (
diff --git a/go/analysis/passes/loopclosure/loopclosure.go b/go/analysis/passes/loopclosure/loopclosure.go
index 3ea91574d..ae5b4151d 100644
--- a/go/analysis/passes/loopclosure/loopclosure.go
+++ b/go/analysis/passes/loopclosure/loopclosure.go
@@ -18,19 +18,60 @@ import (
const Doc = `check references to loop variables from within nested functions
-This analyzer checks for references to loop variables from within a
-function literal inside the loop body. It checks only instances where
-the function literal is called in a defer or go statement that is the
-last statement in the loop body, as otherwise we would need whole
-program analysis.
+This analyzer reports places where a function literal references the
+iteration variable of an enclosing loop, and the loop calls the function
+in such a way (e.g. with go or defer) that it may outlive the loop
+iteration and possibly observe the wrong value of the variable.
-For example:
+In this example, all the deferred functions run after the loop has
+completed, so all observe the final value of v.
- for i, v := range s {
- go func() {
- println(i, v) // not what you might expect
- }()
- }
+ for _, v := range list {
+ defer func() {
+ use(v) // incorrect
+ }()
+ }
+
+One fix is to create a new variable for each iteration of the loop:
+
+ for _, v := range list {
+ v := v // new var per iteration
+ defer func() {
+ use(v) // ok
+ }()
+ }
+
+The next example uses a go statement and has a similar problem.
+In addition, it has a data race because the loop updates v
+concurrent with the goroutines accessing it.
+
+ for _, v := range elem {
+ go func() {
+ use(v) // incorrect, and a data race
+ }()
+ }
+
+A fix is the same as before. The checker also reports problems
+in goroutines started by golang.org/x/sync/errgroup.Group.
+A hard-to-spot variant of this form is common in parallel tests:
+
+ func Test(t *testing.T) {
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ t.Parallel()
+ use(test) // incorrect, and a data race
+ })
+ }
+ }
+
+The t.Parallel() call causes the rest of the function to execute
+concurrent with the loop.
+
+The analyzer reports references only in the last statement,
+as it is not deep enough to understand the effects of subsequent
+statements that might render the reference benign.
+("Last statement" is defined recursively in compound
+statements such as if, switch, and select.)
See: https://golang.org/doc/go_faq.html#closures_and_goroutines`
@@ -50,10 +91,12 @@ func run(pass *analysis.Pass) (interface{}, error) {
}
inspect.Preorder(nodeFilter, func(n ast.Node) {
// Find the variables updated by the loop statement.
- var vars []*ast.Ident
+ var vars []types.Object
addVar := func(expr ast.Expr) {
- if id, ok := expr.(*ast.Ident); ok {
- vars = append(vars, id)
+ if id, _ := expr.(*ast.Ident); id != nil {
+ if obj := pass.TypesInfo.ObjectOf(id); obj != nil {
+ vars = append(vars, obj)
+ }
}
}
var body *ast.BlockStmt
@@ -79,87 +122,312 @@ func run(pass *analysis.Pass) (interface{}, error) {
return
}
- // Inspect a go or defer statement
- // if it's the last one in the loop body.
- // (We give up if there are following statements,
- // because it's hard to prove go isn't followed by wait,
- // or defer by return.)
- if len(body.List) == 0 {
- return
- }
- // The function invoked in the last return statement.
- var fun ast.Expr
- switch s := body.List[len(body.List)-1].(type) {
- case *ast.GoStmt:
- fun = s.Call.Fun
- case *ast.DeferStmt:
- fun = s.Call.Fun
- case *ast.ExprStmt: // check for errgroup.Group.Go()
- if call, ok := s.X.(*ast.CallExpr); ok {
- fun = goInvokes(pass.TypesInfo, call)
- }
- }
- lit, ok := fun.(*ast.FuncLit)
- if !ok {
- return
- }
- ast.Inspect(lit.Body, func(n ast.Node) bool {
- id, ok := n.(*ast.Ident)
- if !ok || id.Obj == nil {
- return true
+ // Inspect statements to find function literals that may be run outside of
+ // the current loop iteration.
+ //
+ // For go, defer, and errgroup.Group.Go, we ignore all but the last
+ // statement, because it's hard to prove go isn't followed by wait, or
+ // defer by return. "Last" is defined recursively.
+ //
+ // TODO: consider allowing the "last" go/defer/Go statement to be followed by
+ // N "trivial" statements, possibly under a recursive definition of "trivial"
+ // so that that checker could, for example, conclude that a go statement is
+ // followed by an if statement made of only trivial statements and trivial expressions,
+ // and hence the go statement could still be checked.
+ forEachLastStmt(body.List, func(last ast.Stmt) {
+ var stmts []ast.Stmt
+ switch s := last.(type) {
+ case *ast.GoStmt:
+ stmts = litStmts(s.Call.Fun)
+ case *ast.DeferStmt:
+ stmts = litStmts(s.Call.Fun)
+ case *ast.ExprStmt: // check for errgroup.Group.Go
+ if call, ok := s.X.(*ast.CallExpr); ok {
+ stmts = litStmts(goInvoke(pass.TypesInfo, call))
+ }
}
- if pass.TypesInfo.Types[id].Type == nil {
- // Not referring to a variable (e.g. struct field name)
- return true
+ for _, stmt := range stmts {
+ reportCaptured(pass, vars, stmt)
}
- for _, v := range vars {
- if v.Obj == id.Obj {
- pass.ReportRangef(id, "loop variable %s captured by func literal",
- id.Name)
+ })
+
+ // Also check for testing.T.Run (with T.Parallel).
+ // We consider every t.Run statement in the loop body, because there is
+ // no commonly used mechanism for synchronizing parallel subtests.
+ // It is of course theoretically possible to synchronize parallel subtests,
+ // though such a pattern is likely to be exceedingly rare as it would be
+ // fighting against the test runner.
+ for _, s := range body.List {
+ switch s := s.(type) {
+ case *ast.ExprStmt:
+ if call, ok := s.X.(*ast.CallExpr); ok {
+ for _, stmt := range parallelSubtest(pass.TypesInfo, call) {
+ reportCaptured(pass, vars, stmt)
+ }
+
}
}
- return true
- })
+ }
})
return nil, nil
}
-// goInvokes returns a function expression that would be called asynchronously
+// reportCaptured reports a diagnostic stating a loop variable
+// has been captured by a func literal if checkStmt has escaping
+// references to vars. vars is expected to be variables updated by a loop statement,
+// and checkStmt is expected to be a statements from the body of a func literal in the loop.
+func reportCaptured(pass *analysis.Pass, vars []types.Object, checkStmt ast.Stmt) {
+ ast.Inspect(checkStmt, func(n ast.Node) bool {
+ id, ok := n.(*ast.Ident)
+ if !ok {
+ return true
+ }
+ obj := pass.TypesInfo.Uses[id]
+ if obj == nil {
+ return true
+ }
+ for _, v := range vars {
+ if v == obj {
+ pass.ReportRangef(id, "loop variable %s captured by func literal", id.Name)
+ }
+ }
+ return true
+ })
+}
+
+// forEachLastStmt calls onLast on each "last" statement in a list of statements.
+// "Last" is defined recursively so, for example, if the last statement is
+// a switch statement, then each switch case is also visited to examine
+// its last statements.
+func forEachLastStmt(stmts []ast.Stmt, onLast func(last ast.Stmt)) {
+ if len(stmts) == 0 {
+ return
+ }
+
+ s := stmts[len(stmts)-1]
+ switch s := s.(type) {
+ case *ast.IfStmt:
+ loop:
+ for {
+ forEachLastStmt(s.Body.List, onLast)
+ switch e := s.Else.(type) {
+ case *ast.BlockStmt:
+ forEachLastStmt(e.List, onLast)
+ break loop
+ case *ast.IfStmt:
+ s = e
+ case nil:
+ break loop
+ }
+ }
+ case *ast.ForStmt:
+ forEachLastStmt(s.Body.List, onLast)
+ case *ast.RangeStmt:
+ forEachLastStmt(s.Body.List, onLast)
+ case *ast.SwitchStmt:
+ for _, c := range s.Body.List {
+ cc := c.(*ast.CaseClause)
+ forEachLastStmt(cc.Body, onLast)
+ }
+ case *ast.TypeSwitchStmt:
+ for _, c := range s.Body.List {
+ cc := c.(*ast.CaseClause)
+ forEachLastStmt(cc.Body, onLast)
+ }
+ case *ast.SelectStmt:
+ for _, c := range s.Body.List {
+ cc := c.(*ast.CommClause)
+ forEachLastStmt(cc.Body, onLast)
+ }
+ default:
+ onLast(s)
+ }
+}
+
+// litStmts returns all statements from the function body of a function
+// literal.
+//
+// If fun is not a function literal, it returns nil.
+func litStmts(fun ast.Expr) []ast.Stmt {
+ lit, _ := fun.(*ast.FuncLit)
+ if lit == nil {
+ return nil
+ }
+ return lit.Body.List
+}
+
+// goInvoke returns a function expression that would be called asynchronously
// (but not awaited) in another goroutine as a consequence of the call.
// For example, given the g.Go call below, it returns the function literal expression.
//
-// import "sync/errgroup"
-// var g errgroup.Group
-// g.Go(func() error { ... })
+// import "sync/errgroup"
+// var g errgroup.Group
+// g.Go(func() error { ... })
//
// Currently only "golang.org/x/sync/errgroup.Group()" is considered.
-func goInvokes(info *types.Info, call *ast.CallExpr) ast.Expr {
- f := typeutil.StaticCallee(info, call)
- // Note: Currently only supports: golang.org/x/sync/errgroup.Go.
- if f == nil || f.Name() != "Go" {
+func goInvoke(info *types.Info, call *ast.CallExpr) ast.Expr {
+ if !isMethodCall(info, call, "golang.org/x/sync/errgroup", "Group", "Go") {
return nil
}
- recv := f.Type().(*types.Signature).Recv()
- if recv == nil {
+ return call.Args[0]
+}
+
+// parallelSubtest returns statements that can be easily proven to execute
+// concurrently via the go test runner, as t.Run has been invoked with a
+// function literal that calls t.Parallel.
+//
+// In practice, users rely on the fact that statements before the call to
+// t.Parallel are synchronous. For example by declaring test := test inside the
+// function literal, but before the call to t.Parallel.
+//
+// Therefore, we only flag references in statements that are obviously
+// dominated by a call to t.Parallel. As a simple heuristic, we only consider
+// statements following the final labeled statement in the function body, to
+// avoid scenarios where a jump would cause either the call to t.Parallel or
+// the problematic reference to be skipped.
+//
+// import "testing"
+//
+// func TestFoo(t *testing.T) {
+// tests := []int{0, 1, 2}
+// for i, test := range tests {
+// t.Run("subtest", func(t *testing.T) {
+// println(i, test) // OK
+// t.Parallel()
+// println(i, test) // Not OK
+// })
+// }
+// }
+func parallelSubtest(info *types.Info, call *ast.CallExpr) []ast.Stmt {
+ if !isMethodCall(info, call, "testing", "T", "Run") {
return nil
}
- rtype, ok := recv.Type().(*types.Pointer)
- if !ok {
+
+ if len(call.Args) != 2 {
+ // Ignore calls such as t.Run(fn()).
return nil
}
- named, ok := rtype.Elem().(*types.Named)
- if !ok {
+
+ lit, _ := call.Args[1].(*ast.FuncLit)
+ if lit == nil {
return nil
}
- if named.Obj().Name() != "Group" {
+
+ // Capture the *testing.T object for the first argument to the function
+ // literal.
+ if len(lit.Type.Params.List[0].Names) == 0 {
+ return nil
+ }
+
+ tObj := info.Defs[lit.Type.Params.List[0].Names[0]]
+ if tObj == nil {
return nil
}
+
+ // Match statements that occur after a call to t.Parallel following the final
+ // labeled statement in the function body.
+ //
+ // We iterate over lit.Body.List to have a simple, fast and "frequent enough"
+ // dominance relationship for t.Parallel(): lit.Body.List[i] dominates
+ // lit.Body.List[j] for i < j unless there is a jump.
+ var stmts []ast.Stmt
+ afterParallel := false
+ for _, stmt := range lit.Body.List {
+ stmt, labeled := unlabel(stmt)
+ if labeled {
+ // Reset: naively we don't know if a jump could have caused the
+ // previously considered statements to be skipped.
+ stmts = nil
+ afterParallel = false
+ }
+
+ if afterParallel {
+ stmts = append(stmts, stmt)
+ continue
+ }
+
+ // Check if stmt is a call to t.Parallel(), for the correct t.
+ exprStmt, ok := stmt.(*ast.ExprStmt)
+ if !ok {
+ continue
+ }
+ expr := exprStmt.X
+ if isMethodCall(info, expr, "testing", "T", "Parallel") {
+ call, _ := expr.(*ast.CallExpr)
+ if call == nil {
+ continue
+ }
+ x, _ := call.Fun.(*ast.SelectorExpr)
+ if x == nil {
+ continue
+ }
+ id, _ := x.X.(*ast.Ident)
+ if id == nil {
+ continue
+ }
+ if info.Uses[id] == tObj {
+ afterParallel = true
+ }
+ }
+ }
+
+ return stmts
+}
+
+// unlabel returns the inner statement for the possibly labeled statement stmt,
+// stripping any (possibly nested) *ast.LabeledStmt wrapper.
+//
+// The second result reports whether stmt was an *ast.LabeledStmt.
+func unlabel(stmt ast.Stmt) (ast.Stmt, bool) {
+ labeled := false
+ for {
+ labelStmt, ok := stmt.(*ast.LabeledStmt)
+ if !ok {
+ return stmt, labeled
+ }
+ labeled = true
+ stmt = labelStmt.Stmt
+ }
+}
+
+// isMethodCall reports whether expr is a method call of
+// <pkgPath>.<typeName>.<method>.
+func isMethodCall(info *types.Info, expr ast.Expr, pkgPath, typeName, method string) bool {
+ call, ok := expr.(*ast.CallExpr)
+ if !ok {
+ return false
+ }
+
+ // Check that we are calling a method <method>
+ f := typeutil.StaticCallee(info, call)
+ if f == nil || f.Name() != method {
+ return false
+ }
+ recv := f.Type().(*types.Signature).Recv()
+ if recv == nil {
+ return false
+ }
+
+ // Check that the receiver is a <pkgPath>.<typeName> or
+ // *<pkgPath>.<typeName>.
+ rtype := recv.Type()
+ if ptr, ok := recv.Type().(*types.Pointer); ok {
+ rtype = ptr.Elem()
+ }
+ named, ok := rtype.(*types.Named)
+ if !ok {
+ return false
+ }
+ if named.Obj().Name() != typeName {
+ return false
+ }
pkg := f.Pkg()
if pkg == nil {
- return nil
+ return false
}
- if pkg.Path() != "golang.org/x/sync/errgroup" {
- return nil
+ if pkg.Path() != pkgPath {
+ return false
}
- return call.Args[0]
+
+ return true
}
diff --git a/go/analysis/passes/loopclosure/loopclosure_test.go b/go/analysis/passes/loopclosure/loopclosure_test.go
index 1498838d7..55fb2a4a3 100644
--- a/go/analysis/passes/loopclosure/loopclosure_test.go
+++ b/go/analysis/passes/loopclosure/loopclosure_test.go
@@ -5,16 +5,16 @@
package loopclosure_test
import (
- "golang.org/x/tools/internal/typeparams"
"testing"
"golang.org/x/tools/go/analysis/analysistest"
"golang.org/x/tools/go/analysis/passes/loopclosure"
+ "golang.org/x/tools/internal/typeparams"
)
func Test(t *testing.T) {
testdata := analysistest.TestData()
- tests := []string{"a", "golang.org/..."}
+ tests := []string{"a", "golang.org/...", "subtests"}
if typeparams.Enabled {
tests = append(tests, "typeparams")
}
diff --git a/go/analysis/passes/loopclosure/testdata/src/a/a.go b/go/analysis/passes/loopclosure/testdata/src/a/a.go
index 2c8e2e6c4..7a7f05f66 100644
--- a/go/analysis/passes/loopclosure/testdata/src/a/a.go
+++ b/go/analysis/passes/loopclosure/testdata/src/a/a.go
@@ -6,7 +6,13 @@
package testdata
-import "golang.org/x/sync/errgroup"
+import (
+ "sync"
+
+ "golang.org/x/sync/errgroup"
+)
+
+var A int
func _() {
var s []int
@@ -49,6 +55,19 @@ func _() {
println(i, v)
}()
}
+
+ // iteration variable declared outside the loop
+ for A = range s {
+ go func() {
+ println(A) // want "loop variable A captured by func literal"
+ }()
+ }
+ // iteration variable declared in a different file
+ for B = range s {
+ go func() {
+ println(B) // want "loop variable B captured by func literal"
+ }()
+ }
// If the key of the range statement is not an identifier
// the code should not panic (it used to).
var x [2]int
@@ -91,9 +110,73 @@ func _() {
}
}
-// Group is used to test that loopclosure does not match on any type named "Group".
-// The checker only matches on methods "(*...errgroup.Group).Go".
-type Group struct{};
+// Cases that rely on recursively checking for last statements.
+func _() {
+
+ for i := range "outer" {
+ for j := range "inner" {
+ if j < 1 {
+ defer func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ } else if j < 2 {
+ go func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ } else {
+ go func() {
+ print(i)
+ }()
+ println("we don't catch the error above because of this statement")
+ }
+ }
+ }
+
+ for i := 0; i < 10; i++ {
+ for j := 0; j < 10; j++ {
+ if j < 1 {
+ switch j {
+ case 0:
+ defer func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ default:
+ go func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ }
+ } else if j < 2 {
+ var a interface{} = j
+ switch a.(type) {
+ case int:
+ defer func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ default:
+ go func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ }
+ } else {
+ ch := make(chan string)
+ select {
+ case <-ch:
+ defer func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ default:
+ go func() {
+ print(i) // want "loop variable i captured by func literal"
+ }()
+ }
+ }
+ }
+ }
+}
+
+// Group is used to test that loopclosure only matches Group.Go when Group is
+// from the golang.org/x/sync/errgroup package.
+type Group struct{}
func (g *Group) Go(func() error) {}
@@ -108,6 +191,21 @@ func _() {
return nil
})
}
+
+ for i, v := range s {
+ if i > 0 {
+ g.Go(func() error {
+ print(i) // want "loop variable i captured by func literal"
+ return nil
+ })
+ } else {
+ g.Go(func() error {
+ print(v) // want "loop variable v captured by func literal"
+ return nil
+ })
+ }
+ }
+
// Do not match other Group.Go cases
g1 := new(Group)
for i, v := range s {
@@ -118,3 +216,28 @@ func _() {
})
}
}
+
+// Real-world example from #16520, slightly simplified
+func _() {
+ var nodes []interface{}
+
+ critical := new(errgroup.Group)
+ others := sync.WaitGroup{}
+
+ isCritical := func(node interface{}) bool { return false }
+ run := func(node interface{}) error { return nil }
+
+ for _, node := range nodes {
+ if isCritical(node) {
+ critical.Go(func() error {
+ return run(node) // want "loop variable node captured by func literal"
+ })
+ } else {
+ others.Add(1)
+ go func() {
+ _ = run(node) // want "loop variable node captured by func literal"
+ others.Done()
+ }()
+ }
+ }
+}
diff --git a/go/analysis/passes/loopclosure/testdata/src/a/b.go b/go/analysis/passes/loopclosure/testdata/src/a/b.go
new file mode 100644
index 000000000..d4e5da418
--- /dev/null
+++ b/go/analysis/passes/loopclosure/testdata/src/a/b.go
@@ -0,0 +1,9 @@
+// Copyright 2022 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.
+
+package testdata
+
+// B is declared in a separate file to test that object resolution spans the
+// entire package.
+var B int
diff --git a/go/analysis/passes/loopclosure/testdata/src/subtests/subtest.go b/go/analysis/passes/loopclosure/testdata/src/subtests/subtest.go
new file mode 100644
index 000000000..50283ec61
--- /dev/null
+++ b/go/analysis/passes/loopclosure/testdata/src/subtests/subtest.go
@@ -0,0 +1,202 @@
+// Copyright 2022 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.
+
+// This file contains tests that the loopclosure analyzer detects leaked
+// references via parallel subtests.
+
+package subtests
+
+import (
+ "testing"
+)
+
+// T is used to test that loopclosure only matches T.Run when T is from the
+// testing package.
+type T struct{}
+
+// Run should not match testing.T.Run. Note that the second argument is
+// intentionally a *testing.T, not a *T, so that we can check both
+// testing.T.Parallel inside a T.Run, and a T.Parallel inside a testing.T.Run.
+func (t *T) Run(string, func(*testing.T)) {
+}
+
+func (t *T) Parallel() {}
+
+func _(t *testing.T) {
+ for i, test := range []int{1, 2, 3} {
+ // Check that parallel subtests are identified.
+ t.Run("", func(t *testing.T) {
+ t.Parallel()
+ println(i) // want "loop variable i captured by func literal"
+ println(test) // want "loop variable test captured by func literal"
+ })
+
+ // Check that serial tests are OK.
+ t.Run("", func(t *testing.T) {
+ println(i)
+ println(test)
+ })
+
+ // Check that the location of t.Parallel matters.
+ t.Run("", func(t *testing.T) {
+ println(i)
+ println(test)
+ t.Parallel()
+ println(i) // want "loop variable i captured by func literal"
+ println(test) // want "loop variable test captured by func literal"
+ })
+
+ // Check that *testing.T value matters.
+ t.Run("", func(t *testing.T) {
+ var x testing.T
+ x.Parallel()
+ println(i)
+ println(test)
+ })
+
+ // Check that shadowing the loop variables within the test literal is OK if
+ // it occurs before t.Parallel().
+ t.Run("", func(t *testing.T) {
+ i := i
+ test := test
+ t.Parallel()
+ println(i)
+ println(test)
+ })
+
+ // Check that shadowing the loop variables within the test literal is Not
+ // OK if it occurs after t.Parallel().
+ t.Run("", func(t *testing.T) {
+ t.Parallel()
+ i := i // want "loop variable i captured by func literal"
+ test := test // want "loop variable test captured by func literal"
+ println(i) // OK
+ println(test) // OK
+ })
+
+ // Check uses in nested blocks.
+ t.Run("", func(t *testing.T) {
+ t.Parallel()
+ {
+ println(i) // want "loop variable i captured by func literal"
+ println(test) // want "loop variable test captured by func literal"
+ }
+ })
+
+ // Check that we catch uses in nested subtests.
+ t.Run("", func(t *testing.T) {
+ t.Parallel()
+ t.Run("", func(t *testing.T) {
+ println(i) // want "loop variable i captured by func literal"
+ println(test) // want "loop variable test captured by func literal"
+ })
+ })
+
+ // Check that there is no diagnostic if t is not a *testing.T.
+ t.Run("", func(_ *testing.T) {
+ t := &T{}
+ t.Parallel()
+ println(i)
+ println(test)
+ })
+
+ // Check that there is no diagnostic when a jump to a label may have caused
+ // the call to t.Parallel to have been skipped.
+ t.Run("", func(t *testing.T) {
+ if true {
+ goto Test
+ }
+ t.Parallel()
+ Test:
+ println(i)
+ println(test)
+ })
+
+ // Check that there is no diagnostic when a jump to a label may have caused
+ // the loop variable reference to be skipped, but there is a diagnostic
+ // when both the call to t.Parallel and the loop variable reference occur
+ // after the final label in the block.
+ t.Run("", func(t *testing.T) {
+ if true {
+ goto Test
+ }
+ t.Parallel()
+ println(i) // maybe OK
+ Test:
+ t.Parallel()
+ println(test) // want "loop variable test captured by func literal"
+ })
+
+ // Check that multiple labels are handled.
+ t.Run("", func(t *testing.T) {
+ if true {
+ goto Test1
+ } else {
+ goto Test2
+ }
+ Test1:
+ Test2:
+ t.Parallel()
+ println(test) // want "loop variable test captured by func literal"
+ })
+
+ // Check that we do not have problems when t.Run has a single argument.
+ fn := func() (string, func(t *testing.T)) { return "", nil }
+ t.Run(fn())
+ }
+}
+
+// Check that there is no diagnostic when loop variables are shadowed within
+// the loop body.
+func _(t *testing.T) {
+ for i, test := range []int{1, 2, 3} {
+ i := i
+ test := test
+ t.Run("", func(t *testing.T) {
+ t.Parallel()
+ println(i)
+ println(test)
+ })
+ }
+}
+
+// Check that t.Run must be *testing.T.Run.
+func _(t *T) {
+ for i, test := range []int{1, 2, 3} {
+ t.Run("", func(t *testing.T) {
+ t.Parallel()
+ println(i)
+ println(test)
+ })
+ }
+}
+
+// Check that the top-level must be parallel in order to cause a diagnostic.
+//
+// From https://pkg.go.dev/testing:
+//
+// "Run does not return until parallel subtests have completed, providing a
+// way to clean up after a group of parallel tests"
+func _(t *testing.T) {
+ for _, test := range []int{1, 2, 3} {
+ // In this subtest, a/b must complete before the synchronous subtest "a"
+ // completes, so the reference to test does not escape the current loop
+ // iteration.
+ t.Run("a", func(s *testing.T) {
+ s.Run("b", func(u *testing.T) {
+ u.Parallel()
+ println(test)
+ })
+ })
+
+ // In this subtest, c executes concurrently, so the reference to test may
+ // escape the current loop iteration.
+ t.Run("c", func(s *testing.T) {
+ s.Parallel()
+ s.Run("d", func(u *testing.T) {
+ println(test) // want "loop variable test captured by func literal"
+ })
+ })
+ }
+}
diff --git a/go/analysis/passes/nilness/nilness.go b/go/analysis/passes/nilness/nilness.go
index 8fd8cd000..6849c33cc 100644
--- a/go/analysis/passes/nilness/nilness.go
+++ b/go/analysis/passes/nilness/nilness.go
@@ -15,6 +15,7 @@ import (
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/buildssa"
"golang.org/x/tools/go/ssa"
+ "golang.org/x/tools/internal/typeparams"
)
const Doc = `check for redundant or impossible nil comparisons
@@ -102,8 +103,11 @@ func runFunc(pass *analysis.Pass, fn *ssa.Function) {
for _, instr := range b.Instrs {
switch instr := instr.(type) {
case ssa.CallInstruction:
- notNil(stack, instr, instr.Common().Value,
- instr.Common().Description())
+ // A nil receiver may be okay for type params.
+ cc := instr.Common()
+ if !(cc.IsInvoke() && typeparams.IsTypeParam(cc.Value.Type())) {
+ notNil(stack, instr, cc.Value, cc.Description())
+ }
case *ssa.FieldAddr:
notNil(stack, instr, instr.X, "field selection")
case *ssa.IndexAddr:
@@ -250,7 +254,7 @@ func (n nilness) String() string { return nilnessStrings[n+1] }
// or unknown given the dominating stack of facts.
func nilnessOf(stack []fact, v ssa.Value) nilness {
switch v := v.(type) {
- // unwrap ChangeInterface values recursively, to detect if underlying
+ // unwrap ChangeInterface and Slice values recursively, to detect if underlying
// values have any facts recorded or are otherwise known with regard to nilness.
//
// This work must be in addition to expanding facts about
@@ -264,6 +268,10 @@ func nilnessOf(stack []fact, v ssa.Value) nilness {
if underlying := nilnessOf(stack, v.X); underlying != unknown {
return underlying
}
+ case *ssa.Slice:
+ if underlying := nilnessOf(stack, v.X); underlying != unknown {
+ return underlying
+ }
case *ssa.SliceToArrayPointer:
nn := nilnessOf(stack, v.X)
if slice2ArrayPtrLen(v) > 0 {
@@ -302,9 +310,9 @@ func nilnessOf(stack []fact, v ssa.Value) nilness {
return isnonnil
case *ssa.Const:
if v.IsNil() {
- return isnil
+ return isnil // nil or zero value of a pointer-like type
} else {
- return isnonnil
+ return unknown // non-pointer
}
}
diff --git a/go/analysis/passes/nilness/nilness_test.go b/go/analysis/passes/nilness/nilness_test.go
index b258c1efb..99c4dfbac 100644
--- a/go/analysis/passes/nilness/nilness_test.go
+++ b/go/analysis/passes/nilness/nilness_test.go
@@ -9,9 +9,26 @@ import (
"golang.org/x/tools/go/analysis/analysistest"
"golang.org/x/tools/go/analysis/passes/nilness"
+ "golang.org/x/tools/internal/typeparams"
)
func Test(t *testing.T) {
testdata := analysistest.TestData()
analysistest.Run(t, testdata, nilness.Analyzer, "a")
}
+
+func TestInstantiated(t *testing.T) {
+ if !typeparams.Enabled {
+ t.Skip("TestInstantiated requires type parameters")
+ }
+ testdata := analysistest.TestData()
+ analysistest.Run(t, testdata, nilness.Analyzer, "c")
+}
+
+func TestTypeSet(t *testing.T) {
+ if !typeparams.Enabled {
+ t.Skip("TestTypeSet requires type parameters")
+ }
+ testdata := analysistest.TestData()
+ analysistest.Run(t, testdata, nilness.Analyzer, "d")
+}
diff --git a/go/analysis/passes/nilness/testdata/src/a/a.go b/go/analysis/passes/nilness/testdata/src/a/a.go
index f4d8f455e..0629e08d8 100644
--- a/go/analysis/passes/nilness/testdata/src/a/a.go
+++ b/go/analysis/passes/nilness/testdata/src/a/a.go
@@ -130,7 +130,6 @@ func f9(x interface {
b()
c()
}) {
-
x.b() // we don't catch this panic because we don't have any facts yet
xx := interface {
a()
@@ -155,11 +154,27 @@ func f9(x interface {
}
}
+func f10() {
+ s0 := make([]string, 0)
+ if s0 == nil { // want "impossible condition: non-nil == nil"
+ print(0)
+ }
+
+ var s1 []string
+ if s1 == nil { // want "tautological condition: nil == nil"
+ print(0)
+ }
+ s2 := s1[:][:]
+ if s2 == nil { // want "tautological condition: nil == nil"
+ print(0)
+ }
+}
+
func unknown() bool {
return false
}
-func f10(a interface{}) {
+func f11(a interface{}) {
switch a.(type) {
case nil:
return
@@ -170,7 +185,7 @@ func f10(a interface{}) {
}
}
-func f11(a interface{}) {
+func f12(a interface{}) {
switch a {
case nil:
return
@@ -181,3 +196,23 @@ func f11(a interface{}) {
return
}
}
+
+type Y struct {
+ innerY
+}
+
+type innerY struct {
+ value int
+}
+
+func f13() {
+ var d *Y
+ print(d.value) // want "nil dereference in field selection"
+}
+
+func f14() {
+ var x struct{ f string }
+ if x == struct{ f string }{} { // we don't catch this tautology as we restrict to reference types
+ print(x)
+ }
+}
diff --git a/go/analysis/passes/nilness/testdata/src/c/c.go b/go/analysis/passes/nilness/testdata/src/c/c.go
new file mode 100644
index 000000000..c9a05a714
--- /dev/null
+++ b/go/analysis/passes/nilness/testdata/src/c/c.go
@@ -0,0 +1,14 @@
+package c
+
+func instantiated[X any](x *X) int {
+ if x == nil {
+ print(*x) // want "nil dereference in load"
+ }
+ return 1
+}
+
+var g int
+
+func init() {
+ g = instantiated[int](&g)
+}
diff --git a/go/analysis/passes/nilness/testdata/src/d/d.go b/go/analysis/passes/nilness/testdata/src/d/d.go
new file mode 100644
index 000000000..72bd1c872
--- /dev/null
+++ b/go/analysis/passes/nilness/testdata/src/d/d.go
@@ -0,0 +1,55 @@
+package d
+
+type message interface{ PR() }
+
+func noparam() {
+ var messageT message
+ messageT.PR() // want "nil dereference in dynamic method call"
+}
+
+func paramNonnil[T message]() {
+ var messageT T
+ messageT.PR() // cannot conclude messageT is nil.
+}
+
+func instance() {
+ // buildssa.BuilderMode does not include InstantiateGenerics.
+ paramNonnil[message]() // no warning is expected as param[message] id not built.
+}
+
+func param[T interface {
+ message
+ ~*int | ~chan int
+}]() {
+ var messageT T // messageT is nil.
+ messageT.PR() // nil receiver may be okay. See param[nilMsg].
+}
+
+type nilMsg chan int
+
+func (m nilMsg) PR() {
+ if m == nil {
+ print("not an error")
+ }
+}
+
+var G func() = param[nilMsg] // no warning
+
+func allNillable[T ~*int | ~chan int]() {
+ var x, y T // both are nillable and are nil.
+ if x != y { // want "impossible condition: nil != nil"
+ print("unreachable")
+ }
+}
+
+func notAll[T ~*int | ~chan int | ~int]() {
+ var x, y T // neither are nillable due to ~int
+ if x != y { // no warning
+ print("unreachable")
+ }
+}
+
+func noninvoke[T ~func()]() {
+ var x T
+ x() // want "nil dereference in dynamic function call"
+}
diff --git a/go/analysis/passes/pkgfact/pkgfact.go b/go/analysis/passes/pkgfact/pkgfact.go
index 2262fc4f1..f4f5616e5 100644
--- a/go/analysis/passes/pkgfact/pkgfact.go
+++ b/go/analysis/passes/pkgfact/pkgfact.go
@@ -10,14 +10,14 @@
// Each key/value pair comes from a top-level constant declaration
// whose name starts and ends with "_". For example:
//
-// package p
+// package p
//
-// const _greeting_ = "hello"
-// const _audience_ = "world"
+// const _greeting_ = "hello"
+// const _audience_ = "world"
//
// the pkgfact analysis output for package p would be:
//
-// {"greeting": "hello", "audience": "world"}.
+// {"greeting": "hello", "audience": "world"}.
//
// In addition, the analysis reports a diagnostic at each import
// showing which key/value pairs it contributes.
diff --git a/go/analysis/passes/printf/printf.go b/go/analysis/passes/printf/printf.go
index dee37d78a..daaf709a4 100644
--- a/go/analysis/passes/printf/printf.go
+++ b/go/analysis/passes/printf/printf.go
@@ -342,7 +342,6 @@ func checkPrintfFwd(pass *analysis.Pass, w *printfWrapper, call *ast.CallExpr, k
// not do so with gccgo, and nor do some other build systems.
// TODO(adonovan): eliminate the redundant facts once this restriction
// is lifted.
-//
var isPrint = stringSet{
"fmt.Errorf": true,
"fmt.Fprint": true,
@@ -584,7 +583,6 @@ func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, fn *types.F
argNum := firstArg
maxArgNum := firstArg
anyIndex := false
- anyW := false
for i, w := 0, 0; i < len(format); i += w {
w = 1
if format[i] != '%' {
@@ -607,11 +605,6 @@ func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, fn *types.F
pass.Reportf(call.Pos(), "%s does not support error-wrapping directive %%w", state.name)
return
}
- if anyW {
- pass.Reportf(call.Pos(), "%s call has more than one error-wrapping directive %%w", state.name)
- return
- }
- anyW = true
}
if len(state.argNums) > 0 {
// Continue with the next sequential argument.
@@ -673,12 +666,13 @@ func (s *formatState) parseIndex() bool {
s.scanNum()
ok := true
if s.nbytes == len(s.format) || s.nbytes == start || s.format[s.nbytes] != ']' {
- ok = false
- s.nbytes = strings.Index(s.format, "]")
+ ok = false // syntax error is either missing "]" or invalid index.
+ s.nbytes = strings.Index(s.format[start:], "]")
if s.nbytes < 0 {
s.pass.ReportRangef(s.call, "%s format %s is missing closing ]", s.name, s.format)
return false
}
+ s.nbytes = s.nbytes + start
}
arg32, err := strconv.ParseInt(s.format[start:s.nbytes], 10, 32)
if err != nil || !ok || arg32 <= 0 || arg32 > int64(len(s.call.Args)-s.firstArg) {
@@ -931,9 +925,9 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, state *formatState) (o
// recursiveStringer reports whether the argument e is a potential
// recursive call to stringer or is an error, such as t and &t in these examples:
//
-// func (t *T) String() string { printf("%s", t) }
-// func (t T) Error() string { printf("%s", t) }
-// func (t T) String() string { printf("%s", &t) }
+// func (t *T) String() string { printf("%s", t) }
+// func (t T) Error() string { printf("%s", t) }
+// func (t T) String() string { printf("%s", &t) }
func recursiveStringer(pass *analysis.Pass, e ast.Expr) (string, bool) {
typ := pass.TypesInfo.Types[e].Type
@@ -951,11 +945,16 @@ func recursiveStringer(pass *analysis.Pass, e ast.Expr) (string, bool) {
return "", false
}
+ // inScope returns true if e is in the scope of f.
+ inScope := func(e ast.Expr, f *types.Func) bool {
+ return f.Scope() != nil && f.Scope().Contains(e.Pos())
+ }
+
// Is the expression e within the body of that String or Error method?
var method *types.Func
- if strOk && strMethod.Pkg() == pass.Pkg && strMethod.Scope().Contains(e.Pos()) {
+ if strOk && strMethod.Pkg() == pass.Pkg && inScope(e, strMethod) {
method = strMethod
- } else if errOk && errMethod.Pkg() == pass.Pkg && errMethod.Scope().Contains(e.Pos()) {
+ } else if errOk && errMethod.Pkg() == pass.Pkg && inScope(e, errMethod) {
method = errMethod
} else {
return "", false
diff --git a/go/analysis/passes/printf/testdata/src/a/a.go b/go/analysis/passes/printf/testdata/src/a/a.go
index 5eca3172d..0c4d11bf0 100644
--- a/go/analysis/passes/printf/testdata/src/a/a.go
+++ b/go/analysis/passes/printf/testdata/src/a/a.go
@@ -217,6 +217,7 @@ func PrintfTests() {
Printf("%[2]*.[1]*[3]d x", 2, "hi", 4) // want `a.Printf format %\[2]\*\.\[1\]\*\[3\]d uses non-int \x22hi\x22 as argument of \*`
Printf("%[0]s x", "arg1") // want `a.Printf format has invalid argument index \[0\]`
Printf("%[0]d x", 1) // want `a.Printf format has invalid argument index \[0\]`
+ Printf("%[3]*.[2*[1]f", 1, 2, 3) // want `a.Printf format has invalid argument index \[2\*\[1\]`
// Something that satisfies the error interface.
var e error
fmt.Println(e.Error()) // ok
@@ -341,7 +342,7 @@ func PrintfTests() {
_ = fmt.Errorf("%[2]w %[1]s", "x", err) // OK
_ = fmt.Errorf("%[2]w %[1]s", e, "x") // want `fmt.Errorf format %\[2\]w has arg "x" of wrong type string`
_ = fmt.Errorf("%w", "x") // want `fmt.Errorf format %w has arg "x" of wrong type string`
- _ = fmt.Errorf("%w %w", err, err) // want `fmt.Errorf call has more than one error-wrapping directive %w`
+ _ = fmt.Errorf("%w %w", err, err) // OK
_ = fmt.Errorf("%w", interface{}(nil)) // want `fmt.Errorf format %w has arg interface{}\(nil\) of wrong type interface{}`
_ = fmt.Errorf("%w", errorTestOK(0)) // concrete value implements error
_ = fmt.Errorf("%w", errSubset) // interface value implements error
diff --git a/go/analysis/passes/printf/testdata/src/typeparams/diagnostics.go b/go/analysis/passes/printf/testdata/src/typeparams/diagnostics.go
index 76a9a205a..c4d7e530d 100644
--- a/go/analysis/passes/printf/testdata/src/typeparams/diagnostics.go
+++ b/go/analysis/passes/printf/testdata/src/typeparams/diagnostics.go
@@ -121,3 +121,25 @@ func TestTermReduction[T1 interface{ ~int | string }, T2 interface {
fmt.Printf("%d", t2)
fmt.Printf("%s", t2) // want "wrong type.*contains typeparams.myInt"
}
+
+type U[T any] struct{}
+
+func (u U[T]) String() string {
+ fmt.Println(u) // want `fmt.Println arg u causes recursive call to \(typeparams.U\[T\]\).String method`
+ return ""
+}
+
+type S[T comparable] struct {
+ t T
+}
+
+func (s S[T]) String() T {
+ fmt.Println(s) // Not flagged. We currently do not consider String() T to implement fmt.Stringer (see #55928).
+ return s.t
+}
+
+func TestInstanceStringer() {
+ // Tests String method with nil Scope (#55350)
+ fmt.Println(&S[string]{})
+ fmt.Println(&U[string]{})
+}
diff --git a/go/analysis/passes/printf/types.go b/go/analysis/passes/printf/types.go
index 270e917c8..7cbb0bdbf 100644
--- a/go/analysis/passes/printf/types.go
+++ b/go/analysis/passes/printf/types.go
@@ -299,13 +299,3 @@ func isConvertibleToString(typ types.Type) bool {
return false
}
-
-// hasBasicType reports whether x's type is a types.Basic with the given kind.
-func hasBasicType(pass *analysis.Pass, x ast.Expr, kind types.BasicKind) bool {
- t := pass.TypesInfo.Types[x].Type
- if t != nil {
- t = t.Underlying()
- }
- b, ok := t.(*types.Basic)
- return ok && b.Kind() == kind
-}
diff --git a/go/analysis/passes/shadow/shadow.go b/go/analysis/passes/shadow/shadow.go
index b160dcf5b..a19cecd14 100644
--- a/go/analysis/passes/shadow/shadow.go
+++ b/go/analysis/passes/shadow/shadow.go
@@ -120,7 +120,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
// the block, we should complain about it but don't.
// - A variable declared inside a function literal can falsely be identified
// as shadowing a variable in the outer function.
-//
type span struct {
min token.Pos
max token.Pos
diff --git a/go/analysis/passes/sigchanyzer/sigchanyzer.go b/go/analysis/passes/sigchanyzer/sigchanyzer.go
index 0d6c8ebf1..c490a84ea 100644
--- a/go/analysis/passes/sigchanyzer/sigchanyzer.go
+++ b/go/analysis/passes/sigchanyzer/sigchanyzer.go
@@ -50,7 +50,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
}
case *ast.CallExpr:
// Only signal.Notify(make(chan os.Signal), os.Interrupt) is safe,
- // conservatively treate others as not safe, see golang/go#45043
+ // conservatively treat others as not safe, see golang/go#45043
if isBuiltinMake(pass.TypesInfo, arg) {
return
}
diff --git a/go/analysis/passes/sortslice/analyzer.go b/go/analysis/passes/sortslice/analyzer.go
index 5eb957a18..f85837d66 100644
--- a/go/analysis/passes/sortslice/analyzer.go
+++ b/go/analysis/passes/sortslice/analyzer.go
@@ -52,11 +52,20 @@ func run(pass *analysis.Pass) (interface{}, error) {
arg := call.Args[0]
typ := pass.TypesInfo.Types[arg].Type
+
+ if tuple, ok := typ.(*types.Tuple); ok {
+ typ = tuple.At(0).Type() // special case for Slice(f(...))
+ }
+
switch typ.Underlying().(type) {
case *types.Slice, *types.Interface:
return
}
+ // Restore typ to the original type, we may unwrap the tuple above,
+ // typ might not be the type of arg.
+ typ = pass.TypesInfo.Types[arg].Type
+
var fixes []analysis.SuggestedFix
switch v := typ.Underlying().(type) {
case *types.Array:
diff --git a/go/analysis/passes/sortslice/testdata/src/a/a.go b/go/analysis/passes/sortslice/testdata/src/a/a.go
index bc6cc16e9..c6aca8df1 100644
--- a/go/analysis/passes/sortslice/testdata/src/a/a.go
+++ b/go/analysis/passes/sortslice/testdata/src/a/a.go
@@ -6,8 +6,8 @@ import "sort"
func IncorrectSort() {
i := 5
sortFn := func(i, j int) bool { return false }
- sort.Slice(i, sortFn) // want "sort.Slice's argument must be a slice; is called with int"
- sort.SliceStable(i, sortFn) // want "sort.SliceStable's argument must be a slice; is called with int"
+ sort.Slice(i, sortFn) // want "sort.Slice's argument must be a slice; is called with int"
+ sort.SliceStable(i, sortFn) // want "sort.SliceStable's argument must be a slice; is called with int"
sort.SliceIsSorted(i, sortFn) // want "sort.SliceIsSorted's argument must be a slice; is called with int"
}
@@ -62,3 +62,23 @@ func UnderlyingSlice() {
sort.SliceStable(s, sortFn)
sort.SliceIsSorted(s, sortFn)
}
+
+// FunctionResultsAsArguments passes a function which returns two values
+// that satisfy sort.Slice signature. It should not produce a diagnostic.
+func FunctionResultsAsArguments() {
+ s := []string{"a", "z", "ooo"}
+ sort.Slice(less(s))
+ sort.Slice(lessPtr(s)) // want `sort.Slice's argument must be a slice; is called with \(\*\[\]string,.*`
+}
+
+func less(s []string) ([]string, func(i, j int) bool) {
+ return s, func(i, j int) bool {
+ return s[i] < s[j]
+ }
+}
+
+func lessPtr(s []string) (*[]string, func(i, j int) bool) {
+ return &s, func(i, j int) bool {
+ return s[i] < s[j]
+ }
+}
diff --git a/go/analysis/passes/stdmethods/stdmethods.go b/go/analysis/passes/stdmethods/stdmethods.go
index cc9497179..41f455d10 100644
--- a/go/analysis/passes/stdmethods/stdmethods.go
+++ b/go/analysis/passes/stdmethods/stdmethods.go
@@ -134,6 +134,19 @@ func canonicalMethod(pass *analysis.Pass, id *ast.Ident) {
}
}
+ // Special case: Unwrap has two possible signatures.
+ // Check for Unwrap() []error here.
+ if id.Name == "Unwrap" {
+ if args.Len() == 0 && results.Len() == 1 {
+ t := typeString(results.At(0).Type())
+ if t == "error" || t == "[]error" {
+ return
+ }
+ }
+ pass.ReportRangef(id, "method Unwrap() should have signature Unwrap() error or Unwrap() []error")
+ return
+ }
+
// Do the =s (if any) all match?
if !matchParams(pass, expect.args, args, "=") || !matchParams(pass, expect.results, results, "=") {
return
diff --git a/go/analysis/passes/stdmethods/testdata/src/a/a.go b/go/analysis/passes/stdmethods/testdata/src/a/a.go
index c95cf5d2b..2b01f4693 100644
--- a/go/analysis/passes/stdmethods/testdata/src/a/a.go
+++ b/go/analysis/passes/stdmethods/testdata/src/a/a.go
@@ -49,7 +49,7 @@ func (E) Error() string { return "" } // E implements error.
func (E) As() {} // want `method As\(\) should have signature As\((any|interface\{\})\) bool`
func (E) Is() {} // want `method Is\(\) should have signature Is\(error\) bool`
-func (E) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error`
+func (E) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error or Unwrap\(\) \[\]error`
type F int
@@ -57,8 +57,18 @@ func (F) Error() string { return "" } // Both F and *F implement error.
func (*F) As() {} // want `method As\(\) should have signature As\((any|interface\{\})\) bool`
func (*F) Is() {} // want `method Is\(\) should have signature Is\(error\) bool`
-func (*F) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error`
+func (*F) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error or Unwrap\(\) \[\]error`
type G int
func (G) As(interface{}) bool // ok
+
+type W int
+
+func (W) Error() string { return "" }
+func (W) Unwrap() error { return nil } // ok
+
+type M int
+
+func (M) Error() string { return "" }
+func (M) Unwrap() []error { return nil } // ok
diff --git a/go/analysis/passes/stdmethods/testdata/src/typeparams/typeparams.go b/go/analysis/passes/stdmethods/testdata/src/typeparams/typeparams.go
index 72df30d49..3d4146e9b 100644
--- a/go/analysis/passes/stdmethods/testdata/src/typeparams/typeparams.go
+++ b/go/analysis/passes/stdmethods/testdata/src/typeparams/typeparams.go
@@ -30,7 +30,7 @@ func (E[_]) Error() string { return "" } // E implements error.
func (E[P]) As() {} // want `method As\(\) should have signature As\((any|interface\{\})\) bool`
func (E[_]) Is() {} // want `method Is\(\) should have signature Is\(error\) bool`
-func (E[_]) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error`
+func (E[_]) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error or Unwrap\(\) \[\]error`
type F[P any] int
@@ -38,4 +38,4 @@ func (F[_]) Error() string { return "" } // Both F and *F implement error.
func (*F[_]) As() {} // want `method As\(\) should have signature As\((any|interface\{\})\) bool`
func (*F[_]) Is() {} // want `method Is\(\) should have signature Is\(error\) bool`
-func (*F[_]) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error`
+func (*F[_]) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error or Unwrap\(\) \[\]error`
diff --git a/go/analysis/passes/tests/testdata/src/a/go118_test.go b/go/analysis/passes/tests/testdata/src/a/go118_test.go
index dc898daca..e2bc3f3a0 100644
--- a/go/analysis/passes/tests/testdata/src/a/go118_test.go
+++ b/go/analysis/passes/tests/testdata/src/a/go118_test.go
@@ -94,3 +94,8 @@ func FuzzObjectMethod(f *testing.F) {
}
f.Fuzz(obj.myVar) // ok
}
+
+// Test for golang/go#56505: checking fuzz arguments should not panic on *error.
+func FuzzIssue56505(f *testing.F) {
+ f.Fuzz(func(e *error) {}) // want "the first parameter of a fuzz target must be \\*testing.T"
+}
diff --git a/go/analysis/passes/tests/tests.go b/go/analysis/passes/tests/tests.go
index ffa5205dd..935aad00c 100644
--- a/go/analysis/passes/tests/tests.go
+++ b/go/analysis/passes/tests/tests.go
@@ -84,7 +84,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
return nil, nil
}
-// Checks the contents of a fuzz function.
+// checkFuzz checks the contents of a fuzz function.
func checkFuzz(pass *analysis.Pass, fn *ast.FuncDecl) {
params := checkFuzzCall(pass, fn)
if params != nil {
@@ -92,15 +92,17 @@ func checkFuzz(pass *analysis.Pass, fn *ast.FuncDecl) {
}
}
-// Check the arguments of f.Fuzz() calls :
-// 1. f.Fuzz() should call a function and it should be of type (*testing.F).Fuzz().
-// 2. The called function in f.Fuzz(func(){}) should not return result.
-// 3. First argument of func() should be of type *testing.T
-// 4. Second argument onwards should be of type []byte, string, bool, byte,
-// rune, float32, float64, int, int8, int16, int32, int64, uint, uint8, uint16,
-// uint32, uint64
-// 5. func() must not call any *F methods, e.g. (*F).Log, (*F).Error, (*F).Skip
-// The only *F methods that are allowed in the (*F).Fuzz function are (*F).Failed and (*F).Name.
+// checkFuzzCall checks the arguments of f.Fuzz() calls:
+//
+// 1. f.Fuzz() should call a function and it should be of type (*testing.F).Fuzz().
+// 2. The called function in f.Fuzz(func(){}) should not return result.
+// 3. First argument of func() should be of type *testing.T
+// 4. Second argument onwards should be of type []byte, string, bool, byte,
+// rune, float32, float64, int, int8, int16, int32, int64, uint, uint8, uint16,
+// uint32, uint64
+// 5. func() must not call any *F methods, e.g. (*F).Log, (*F).Error, (*F).Skip
+// The only *F methods that are allowed in the (*F).Fuzz function are (*F).Failed and (*F).Name.
+//
// Returns the list of parameters to the fuzz function, if they are valid fuzz parameters.
func checkFuzzCall(pass *analysis.Pass, fn *ast.FuncDecl) (params *types.Tuple) {
ast.Inspect(fn, func(n ast.Node) bool {
@@ -160,7 +162,7 @@ func checkFuzzCall(pass *analysis.Pass, fn *ast.FuncDecl) (params *types.Tuple)
return params
}
-// Check that the arguments of f.Add() calls have the same number and type of arguments as
+// checkAddCalls checks that the arguments of f.Add calls have the same number and type of arguments as
// the signature of the function passed to (*testing.F).Fuzz
func checkAddCalls(pass *analysis.Pass, fn *ast.FuncDecl, params *types.Tuple) {
ast.Inspect(fn, func(n ast.Node) bool {
@@ -267,7 +269,9 @@ func isTestingType(typ types.Type, testingType string) bool {
if !ok {
return false
}
- return named.Obj().Pkg().Path() == "testing" && named.Obj().Name() == testingType
+ obj := named.Obj()
+ // obj.Pkg is nil for the error type.
+ return obj != nil && obj.Pkg() != nil && obj.Pkg().Path() == "testing" && obj.Name() == testingType
}
// Validate that fuzz target function's arguments are of accepted types.
@@ -473,10 +477,12 @@ func checkTest(pass *analysis.Pass, fn *ast.FuncDecl, prefix string) {
if tparams := typeparams.ForFuncType(fn.Type); tparams != nil && len(tparams.List) > 0 {
// Note: cmd/go/internal/load also errors about TestXXX and BenchmarkXXX functions with type parameters.
// We have currently decided to also warn before compilation/package loading. This can help users in IDEs.
+ // TODO(adonovan): use ReportRangef(tparams).
pass.Reportf(fn.Pos(), "%s has type parameters: it will not be run by go test as a %sXXX function", fn.Name.Name, prefix)
}
if !isTestSuffix(fn.Name.Name[len(prefix):]) {
+ // TODO(adonovan): use ReportRangef(fn.Name).
pass.Reportf(fn.Pos(), "%s has malformed name: first letter after '%s' must not be lowercase", fn.Name.Name, prefix)
}
}
diff --git a/go/analysis/passes/timeformat/testdata/src/a/a.go b/go/analysis/passes/timeformat/testdata/src/a/a.go
new file mode 100644
index 000000000..98481446e
--- /dev/null
+++ b/go/analysis/passes/timeformat/testdata/src/a/a.go
@@ -0,0 +1,50 @@
+// Copyright 2022 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.
+
+// This file contains tests for the timeformat checker.
+
+package a
+
+import (
+ "time"
+
+ "b"
+)
+
+func hasError() {
+ a, _ := time.Parse("2006-02-01 15:04:05", "2021-01-01 00:00:00") // want `2006-02-01 should be 2006-01-02`
+ a.Format(`2006-02-01`) // want `2006-02-01 should be 2006-01-02`
+ a.Format("2006-02-01 15:04:05") // want `2006-02-01 should be 2006-01-02`
+
+ const c = "2006-02-01"
+ a.Format(c) // want `2006-02-01 should be 2006-01-02`
+}
+
+func notHasError() {
+ a, _ := time.Parse("2006-01-02 15:04:05", "2021-01-01 00:00:00")
+ a.Format("2006-01-02")
+
+ const c = "2006-01-02"
+ a.Format(c)
+
+ v := "2006-02-01"
+ a.Format(v) // Allowed though variables.
+
+ m := map[string]string{
+ "y": "2006-02-01",
+ }
+ a.Format(m["y"])
+
+ s := []string{"2006-02-01"}
+ a.Format(s[0])
+
+ a.Format(badFormat())
+
+ o := b.Parse("2006-02-01 15:04:05", "2021-01-01 00:00:00")
+ o.Format("2006-02-01")
+}
+
+func badFormat() string {
+ return "2006-02-01"
+}
diff --git a/go/analysis/passes/timeformat/testdata/src/a/a.go.golden b/go/analysis/passes/timeformat/testdata/src/a/a.go.golden
new file mode 100644
index 000000000..9eccded63
--- /dev/null
+++ b/go/analysis/passes/timeformat/testdata/src/a/a.go.golden
@@ -0,0 +1,50 @@
+// Copyright 2022 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.
+
+// This file contains tests for the timeformat checker.
+
+package a
+
+import (
+ "time"
+
+ "b"
+)
+
+func hasError() {
+ a, _ := time.Parse("2006-01-02 15:04:05", "2021-01-01 00:00:00") // want `2006-02-01 should be 2006-01-02`
+ a.Format(`2006-01-02`) // want `2006-02-01 should be 2006-01-02`
+ a.Format("2006-01-02 15:04:05") // want `2006-02-01 should be 2006-01-02`
+
+ const c = "2006-02-01"
+ a.Format(c) // want `2006-02-01 should be 2006-01-02`
+}
+
+func notHasError() {
+ a, _ := time.Parse("2006-01-02 15:04:05", "2021-01-01 00:00:00")
+ a.Format("2006-01-02")
+
+ const c = "2006-01-02"
+ a.Format(c)
+
+ v := "2006-02-01"
+ a.Format(v) // Allowed though variables.
+
+ m := map[string]string{
+ "y": "2006-02-01",
+ }
+ a.Format(m["y"])
+
+ s := []string{"2006-02-01"}
+ a.Format(s[0])
+
+ a.Format(badFormat())
+
+ o := b.Parse("2006-02-01 15:04:05", "2021-01-01 00:00:00")
+ o.Format("2006-02-01")
+}
+
+func badFormat() string {
+ return "2006-02-01"
+}
diff --git a/go/analysis/passes/timeformat/testdata/src/b/b.go b/go/analysis/passes/timeformat/testdata/src/b/b.go
new file mode 100644
index 000000000..de5690863
--- /dev/null
+++ b/go/analysis/passes/timeformat/testdata/src/b/b.go
@@ -0,0 +1,11 @@
+package b
+
+type B struct {
+}
+
+func Parse(string, string) B {
+ return B{}
+}
+
+func (b B) Format(string) {
+}
diff --git a/go/analysis/passes/timeformat/timeformat.go b/go/analysis/passes/timeformat/timeformat.go
new file mode 100644
index 000000000..acb198f95
--- /dev/null
+++ b/go/analysis/passes/timeformat/timeformat.go
@@ -0,0 +1,129 @@
+// Copyright 2022 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.
+
+// Package timeformat defines an Analyzer that checks for the use
+// of time.Format or time.Parse calls with a bad format.
+package timeformat
+
+import (
+ "go/ast"
+ "go/constant"
+ "go/token"
+ "go/types"
+ "strings"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/passes/inspect"
+ "golang.org/x/tools/go/ast/inspector"
+ "golang.org/x/tools/go/types/typeutil"
+)
+
+const badFormat = "2006-02-01"
+const goodFormat = "2006-01-02"
+
+const Doc = `check for calls of (time.Time).Format or time.Parse with 2006-02-01
+
+The timeformat checker looks for time formats with the 2006-02-01 (yyyy-dd-mm)
+format. Internationally, "yyyy-dd-mm" does not occur in common calendar date
+standards, and so it is more likely that 2006-01-02 (yyyy-mm-dd) was intended.
+`
+
+var Analyzer = &analysis.Analyzer{
+ Name: "timeformat",
+ Doc: Doc,
+ Requires: []*analysis.Analyzer{inspect.Analyzer},
+ Run: run,
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+ inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+
+ nodeFilter := []ast.Node{
+ (*ast.CallExpr)(nil),
+ }
+ inspect.Preorder(nodeFilter, func(n ast.Node) {
+ call := n.(*ast.CallExpr)
+ fn, ok := typeutil.Callee(pass.TypesInfo, call).(*types.Func)
+ if !ok {
+ return
+ }
+ if !isTimeDotFormat(fn) && !isTimeDotParse(fn) {
+ return
+ }
+ if len(call.Args) > 0 {
+ arg := call.Args[0]
+ badAt := badFormatAt(pass.TypesInfo, arg)
+
+ if badAt > -1 {
+ // Check if it's a literal string, otherwise we can't suggest a fix.
+ if _, ok := arg.(*ast.BasicLit); ok {
+ pos := int(arg.Pos()) + badAt + 1 // +1 to skip the " or `
+ end := pos + len(badFormat)
+
+ pass.Report(analysis.Diagnostic{
+ Pos: token.Pos(pos),
+ End: token.Pos(end),
+ Message: badFormat + " should be " + goodFormat,
+ SuggestedFixes: []analysis.SuggestedFix{{
+ Message: "Replace " + badFormat + " with " + goodFormat,
+ TextEdits: []analysis.TextEdit{{
+ Pos: token.Pos(pos),
+ End: token.Pos(end),
+ NewText: []byte(goodFormat),
+ }},
+ }},
+ })
+ } else {
+ pass.Reportf(arg.Pos(), badFormat+" should be "+goodFormat)
+ }
+ }
+ }
+ })
+ return nil, nil
+}
+
+func isTimeDotFormat(f *types.Func) bool {
+ if f.Name() != "Format" || f.Pkg().Path() != "time" {
+ return false
+ }
+ sig, ok := f.Type().(*types.Signature)
+ if !ok {
+ return false
+ }
+ // Verify that the receiver is time.Time.
+ recv := sig.Recv()
+ if recv == nil {
+ return false
+ }
+ named, ok := recv.Type().(*types.Named)
+ return ok && named.Obj().Name() == "Time"
+}
+
+func isTimeDotParse(f *types.Func) bool {
+ if f.Name() != "Parse" || f.Pkg().Path() != "time" {
+ return false
+ }
+ // Verify that there is no receiver.
+ sig, ok := f.Type().(*types.Signature)
+ return ok && sig.Recv() == nil
+}
+
+// badFormatAt return the start of a bad format in e or -1 if no bad format is found.
+func badFormatAt(info *types.Info, e ast.Expr) int {
+ tv, ok := info.Types[e]
+ if !ok { // no type info, assume good
+ return -1
+ }
+
+ t, ok := tv.Type.(*types.Basic)
+ if !ok || t.Info()&types.IsString == 0 {
+ return -1
+ }
+
+ if tv.Value == nil {
+ return -1
+ }
+
+ return strings.Index(constant.StringVal(tv.Value), badFormat)
+}
diff --git a/go/analysis/passes/timeformat/timeformat_test.go b/go/analysis/passes/timeformat/timeformat_test.go
new file mode 100644
index 000000000..86bbe1bb3
--- /dev/null
+++ b/go/analysis/passes/timeformat/timeformat_test.go
@@ -0,0 +1,17 @@
+// Copyright 2022 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.
+
+package timeformat_test
+
+import (
+ "testing"
+
+ "golang.org/x/tools/go/analysis/analysistest"
+ "golang.org/x/tools/go/analysis/passes/timeformat"
+)
+
+func Test(t *testing.T) {
+ testdata := analysistest.TestData()
+ analysistest.RunWithSuggestedFixes(t, testdata, timeformat.Analyzer, "a")
+}
diff --git a/go/analysis/passes/unusedwrite/unusedwrite.go b/go/analysis/passes/unusedwrite/unusedwrite.go
index 37a0e784b..9cc45e0a3 100644
--- a/go/analysis/passes/unusedwrite/unusedwrite.go
+++ b/go/analysis/passes/unusedwrite/unusedwrite.go
@@ -41,7 +41,7 @@ Another example is about non-pointer receiver:
`
// Analyzer reports instances of writes to struct fields and arrays
-//that are never read.
+// that are never read.
var Analyzer = &analysis.Analyzer{
Name: "unusedwrite",
Doc: Doc,
@@ -50,40 +50,49 @@ var Analyzer = &analysis.Analyzer{
}
func run(pass *analysis.Pass) (interface{}, error) {
- // Check the writes to struct and array objects.
- checkStore := func(store *ssa.Store) {
- // Consider field/index writes to an object whose elements are copied and not shared.
- // MapUpdate is excluded since only the reference of the map is copied.
- switch addr := store.Addr.(type) {
- case *ssa.FieldAddr:
- if isDeadStore(store, addr.X, addr) {
- // Report the bug.
+ ssainput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
+ for _, fn := range ssainput.SrcFuncs {
+ // TODO(taking): Iterate over fn._Instantiations() once exported. If so, have 1 report per Pos().
+ reports := checkStores(fn)
+ for _, store := range reports {
+ switch addr := store.Addr.(type) {
+ case *ssa.FieldAddr:
pass.Reportf(store.Pos(),
"unused write to field %s",
getFieldName(addr.X.Type(), addr.Field))
- }
- case *ssa.IndexAddr:
- if isDeadStore(store, addr.X, addr) {
- // Report the bug.
+ case *ssa.IndexAddr:
pass.Reportf(store.Pos(),
"unused write to array index %s", addr.Index)
}
}
}
+ return nil, nil
+}
- ssainput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
- for _, fn := range ssainput.SrcFuncs {
- // Visit each block. No need to visit fn.Recover.
- for _, blk := range fn.Blocks {
- for _, instr := range blk.Instrs {
- // Identify writes.
- if store, ok := instr.(*ssa.Store); ok {
- checkStore(store)
+// checkStores returns *Stores in fn whose address is written to but never used.
+func checkStores(fn *ssa.Function) []*ssa.Store {
+ var reports []*ssa.Store
+ // Visit each block. No need to visit fn.Recover.
+ for _, blk := range fn.Blocks {
+ for _, instr := range blk.Instrs {
+ // Identify writes.
+ if store, ok := instr.(*ssa.Store); ok {
+ // Consider field/index writes to an object whose elements are copied and not shared.
+ // MapUpdate is excluded since only the reference of the map is copied.
+ switch addr := store.Addr.(type) {
+ case *ssa.FieldAddr:
+ if isDeadStore(store, addr.X, addr) {
+ reports = append(reports, store)
+ }
+ case *ssa.IndexAddr:
+ if isDeadStore(store, addr.X, addr) {
+ reports = append(reports, store)
+ }
}
}
}
}
- return nil, nil
+ return reports
}
// isDeadStore determines whether a field/index write to an object is dead.