diff --git a/go/analysis/passes/composite/composite.go b/go/analysis/passes/composite/composite.go index d3670aca97..64e184d343 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 952de8bfda..7afaaa7ffd 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 3a5bc203b0..cd69d39517 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 0000000000..fe73a2e0a1 --- /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 0000000000..20b652e88d --- /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 dd5d57efed..f9a5e1fb10 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 0000000000..66cd9158cb --- /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} +}