Skip to content

Commit 8314b7a

Browse files
suzmuesuzmue
committed
go/analysis: add suggested fix for unkeyed composite literals
Include a suggested fix with the diagnostic for unkeyed composite literals. This suggested fix will add the name of each of the fields. For golang/go#53062 Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224 Reviewed-on: https://go-review.googlesource.com/c/tools/+/414674 gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Alan Donovan <[email protected]> Run-TryBot: Suzy Mueller <[email protected]>
1 parent 8865782 commit 8314b7a

File tree

7 files changed

+245
-12
lines changed

7 files changed

+245
-12
lines changed

‎go/analysis/passes/composite/composite.go

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
package composite
88

99
import (
10+
"fmt"
1011
"go/ast"
1112
"go/types"
1213
"strings"
@@ -83,7 +84,8 @@ func run(pass *analysis.Pass) (interface{}, error) {
8384
}
8485
for _, typ := range structuralTypes {
8586
under := deref(typ.Underlying())
86-
if _, ok := under.(*types.Struct); !ok {
87+
strct, ok := under.(*types.Struct)
88+
if !ok {
8789
// skip non-struct composite literals
8890
continue
8991
}
@@ -92,20 +94,47 @@ func run(pass *analysis.Pass) (interface{}, error) {
9294
continue
9395
}
9496

95-
// check if the CompositeLit contains an unkeyed field
97+
// check if the struct contains an unkeyed field
9698
allKeyValue := true
97-
for _, e := range cl.Elts {
99+
var suggestedFixAvailable = len(cl.Elts) == strct.NumFields()
100+
var missingKeys []analysis.TextEdit
101+
for i, e := range cl.Elts {
98102
if _, ok := e.(*ast.KeyValueExpr); !ok {
99103
allKeyValue = false
100-
break
104+
if i >= strct.NumFields() {
105+
break
106+
}
107+
field := strct.Field(i)
108+
if !field.Exported() {
109+
// Adding unexported field names for structs not defined
110+
// locally will not work.
111+
suggestedFixAvailable = false
112+
break
113+
}
114+
missingKeys = append(missingKeys, analysis.TextEdit{
115+
Pos: e.Pos(),
116+
End: e.Pos(),
117+
NewText: []byte(fmt.Sprintf("%s: ", field.Name())),
118+
})
101119
}
102120
}
103121
if allKeyValue {
104-
// all the composite literal fields are keyed
122+
// all the struct fields are keyed
105123
continue
106124
}
107125

108-
pass.ReportRangef(cl, "%s composite literal uses unkeyed fields", typeName)
126+
diag := analysis.Diagnostic{
127+
Pos: cl.Pos(),
128+
End: cl.End(),
129+
Message: fmt.Sprintf("%s struct literal uses unkeyed fields", typeName),
130+
}
131+
if suggestedFixAvailable {
132+
diag.SuggestedFixes = []analysis.SuggestedFix{{
133+
Message: "Add field names to struct literal",
134+
TextEdits: missingKeys,
135+
}}
136+
}
137+
pass.Report(diag)
109138
return
110139
}
111140
})

‎go/analysis/passes/composite/composite_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,5 @@ func Test(t *testing.T) {
1818
if typeparams.Enabled {
1919
pkgs = append(pkgs, "typeparams")
2020
}
21-
analysistest.Run(t, testdata, composite.Analyzer, pkgs...)
21+
analysistest.RunWithSuggestedFixes(t, testdata, composite.Analyzer, pkgs...)
2222
}

‎go/analysis/passes/composite/testdata/src/a/a.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"go/scanner"
1212
"go/token"
1313
"image"
14+
"sync"
1415
"unicode"
1516
)
1617

@@ -79,6 +80,18 @@ var badStructLiteral = flag.Flag{ // want "unkeyed fields"
7980
nil, // Value
8081
"DefValue",
8182
}
83+
var tooManyFieldsStructLiteral = flag.Flag{ // want "unkeyed fields"
84+
"Name",
85+
"Usage",
86+
nil, // Value
87+
"DefValue",
88+
"Extra Field",
89+
}
90+
var tooFewFieldsStructLiteral = flag.Flag{ // want "unkeyed fields"
91+
"Name",
92+
"Usage",
93+
nil, // Value
94+
}
8295

8396
var delta [3]rune
8497

@@ -100,6 +113,10 @@ var badScannerErrorList = scanner.ErrorList{
100113
&scanner.Error{token.Position{}, "foobar"}, // want "unkeyed fields"
101114
}
102115

116+
// sync.Mutex has unexported fields. We expect a diagnostic but no
117+
// suggested fix.
118+
var mu = sync.Mutex{0, 0} // want "unkeyed fields"
119+
103120
// Check whitelisted structs: if vet is run with --compositewhitelist=false,
104121
// this line triggers an error.
105122
var whitelistedPoint = image.Point{1, 2}

‎go/analysis/passes/composite/testdata/src/a/a.go.golden

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
// Copyright 2012 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// This file contains the test for untagged struct literals.
6+
7+
package a
8+
9+
import (
10+
"flag"
11+
"go/scanner"
12+
"go/token"
13+
"image"
14+
"sync"
15+
"unicode"
16+
)
17+
18+
var Okay1 = []string{
19+
"Name",
20+
"Usage",
21+
"DefValue",
22+
}
23+
24+
var Okay2 = map[string]bool{
25+
"Name": true,
26+
"Usage": true,
27+
"DefValue": true,
28+
}
29+
30+
var Okay3 = struct {
31+
X string
32+
Y string
33+
Z string
34+
}{
35+
"Name",
36+
"Usage",
37+
"DefValue",
38+
}
39+
40+
var Okay4 = []struct {
41+
A int
42+
B int
43+
}{
44+
{1, 2},
45+
{3, 4},
46+
}
47+
48+
type MyStruct struct {
49+
X string
50+
Y string
51+
Z string
52+
}
53+
54+
var Okay5 = &MyStruct{
55+
"Name",
56+
"Usage",
57+
"DefValue",
58+
}
59+
60+
var Okay6 = []MyStruct{
61+
{"foo", "bar", "baz"},
62+
{"aa", "bb", "cc"},
63+
}
64+
65+
var Okay7 = []*MyStruct{
66+
{"foo", "bar", "baz"},
67+
{"aa", "bb", "cc"},
68+
}
69+
70+
// Testing is awkward because we need to reference things from a separate package
71+
// to trigger the warnings.
72+
73+
var goodStructLiteral = flag.Flag{
74+
Name: "Name",
75+
Usage: "Usage",
76+
}
77+
var badStructLiteral = flag.Flag{ // want "unkeyed fields"
78+
Name: "Name",
79+
Usage: "Usage",
80+
Value: nil, // Value
81+
DefValue: "DefValue",
82+
}
83+
var tooManyFieldsStructLiteral = flag.Flag{ // want "unkeyed fields"
84+
"Name",
85+
"Usage",
86+
nil, // Value
87+
"DefValue",
88+
"Extra Field",
89+
}
90+
var tooFewFieldsStructLiteral = flag.Flag{ // want "unkeyed fields"
91+
"Name",
92+
"Usage",
93+
nil, // Value
94+
}
95+
96+
var delta [3]rune
97+
98+
// SpecialCase is a named slice of CaseRange to test issue 9171.
99+
var goodNamedSliceLiteral = unicode.SpecialCase{
100+
{Lo: 1, Hi: 2, Delta: delta},
101+
unicode.CaseRange{Lo: 1, Hi: 2, Delta: delta},
102+
}
103+
var badNamedSliceLiteral = unicode.SpecialCase{
104+
{Lo: 1, Hi: 2, Delta: delta}, // want "unkeyed fields"
105+
unicode.CaseRange{Lo: 1, Hi: 2, Delta: delta}, // want "unkeyed fields"
106+
}
107+
108+
// ErrorList is a named slice, so no warnings should be emitted.
109+
var goodScannerErrorList = scanner.ErrorList{
110+
&scanner.Error{Msg: "foobar"},
111+
}
112+
var badScannerErrorList = scanner.ErrorList{
113+
&scanner.Error{Pos: token.Position{}, Msg: "foobar"}, // want "unkeyed fields"
114+
}
115+
116+
// sync.Mutex has unexported fields. We expect a diagnostic but no
117+
// suggested fix.
118+
var mu = sync.Mutex{0, 0} // want "unkeyed fields"
119+
120+
// Check whitelisted structs: if vet is run with --compositewhitelist=false,
121+
// this line triggers an error.
122+
var whitelistedPoint = image.Point{1, 2}
123+
124+
// Do not check type from unknown package.
125+
// See issue 15408.
126+
var unknownPkgVar = unicode.NoSuchType{"foo", "bar"}
127+
128+
// A named pointer slice of CaseRange to test issue 23539. In
129+
// particular, we're interested in how some slice elements omit their
130+
// type.
131+
var goodNamedPointerSliceLiteral = []*unicode.CaseRange{
132+
{Lo: 1, Hi: 2},
133+
&unicode.CaseRange{Lo: 1, Hi: 2},
134+
}
135+
var badNamedPointerSliceLiteral = []*unicode.CaseRange{
136+
{Lo: 1, Hi: 2, Delta: delta}, // want "unkeyed fields"
137+
&unicode.CaseRange{Lo: 1, Hi: 2, Delta: delta}, // want "unkeyed fields"
138+
}
139+
140+
// unicode.Range16 is whitelisted, so there'll be no vet error
141+
var range16 = unicode.Range16{0xfdd0, 0xfdef, 1}
142+
143+
// unicode.Range32 is whitelisted, so there'll be no vet error
144+
var range32 = unicode.Range32{0x1fffe, 0x1ffff, 1}

‎go/analysis/passes/composite/testdata/src/a/a_fuzz_test.go.golden

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2022 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
//go:build go1.18
6+
// +build go1.18
7+
8+
package a
9+
10+
import "testing"
11+
12+
var fuzzTargets = []testing.InternalFuzzTarget{
13+
{"Fuzz", Fuzz},
14+
}
15+
16+
func Fuzz(f *testing.F) {}

‎go/analysis/passes/composite/testdata/src/typeparams/typeparams.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ package typeparams
66

77
import "typeparams/lib"
88

9-
type localStruct struct { F int }
9+
type localStruct struct{ F int }
1010

1111
func F[
1212
T1 ~struct{ f int },
@@ -20,8 +20,8 @@ func F[
2020
_ = T1{2}
2121
_ = T2a{2}
2222
_ = T2b{2} // want "unkeyed fields"
23-
_ = T3{1,2}
24-
_ = T4{1,2}
25-
_ = T5{1:2}
26-
_ = T6{1:2}
23+
_ = T3{1, 2}
24+
_ = T4{1, 2}
25+
_ = T5{1: 2}
26+
_ = T6{1: 2}
2727
}

‎go/analysis/passes/composite/testdata/src/typeparams/typeparams.go.golden

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright 2021 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package typeparams
6+
7+
import "typeparams/lib"
8+
9+
type localStruct struct{ F int }
10+
11+
func F[
12+
T1 ~struct{ f int },
13+
T2a localStruct,
14+
T2b lib.Struct,
15+
T3 ~[]int,
16+
T4 lib.Slice,
17+
T5 ~map[int]int,
18+
T6 lib.Map,
19+
]() {
20+
_ = T1{2}
21+
_ = T2a{2}
22+
_ = T2b{F: 2} // want "unkeyed fields"
23+
_ = T3{1, 2}
24+
_ = T4{1, 2}
25+
_ = T5{1: 2}
26+
_ = T6{1: 2}
27+
}

0 commit comments

Comments
 (0)