[tools] go/analysis: add suggested fix for unkeyed composite literals

130 views
Skip to first unread message

Suzy Mueller (Gerrit)

unread,
Jun 27, 2022, 6:16:28 PM6/27/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Suzy Mueller has uploaded this change for review.

View Change

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
---
M go/analysis/passes/composite/composite.go
M go/analysis/passes/composite/composite_test.go
A go/analysis/passes/composite/testdata/src/a/a.go.golden
A go/analysis/passes/composite/testdata/src/a/a_fuzz_test.go.golden
M go/analysis/passes/composite/testdata/src/typeparams/typeparams.go
A go/analysis/passes/composite/testdata/src/typeparams/typeparams.go.golden
6 files changed, 230 insertions(+), 10 deletions(-)

diff --git a/go/analysis/passes/composite/composite.go b/go/analysis/passes/composite/composite.go
index d3670ac..53a4b04 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 @@
}
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
}
@@ -94,10 +96,24 @@

// check if the CompositeLit contains an unkeyed field
allKeyValue := true
- for _, e := range cl.Elts {
+ var suggestedFixAvailable = true
+ var missingKeys []analysis.TextEdit
+ for i, e := range cl.Elts {
if _, ok := e.(*ast.KeyValueExpr); !ok {
allKeyValue = false
- break
+ field := strct.Field(i)
+ if field == nil {
+ // If there is something wrong with the field we want to add,
+ // still send the diagnostic but don't include the suggested
+ // fix.
+ suggestedFixAvailable = false
+ break
+ }
+ missingKeys = append(missingKeys, analysis.TextEdit{
+ Pos: e.Pos(),
+ End: e.Pos(),
+ NewText: []byte(fmt.Sprintf("%s: ", field.Name())),
+ })
}
}
if allKeyValue {
@@ -105,7 +121,26 @@
continue
}

- pass.ReportRangef(cl, "%s composite literal uses unkeyed fields", typeName)
+ diag := analysis.Diagnostic{
+ Pos: cl.Pos(),
+ End: cl.End(),
+ Message: fmt.Sprintf("%s composite literal uses unkeyed fields", typeName),
+ }
+ if suggestedFixAvailable {
+ diag.SuggestedFixes = []analysis.SuggestedFix{{
+ Message: "Add composite literal keys",
+ TextEdits: missingKeys,
+ }}
+ }
+ pass.Report(analysis.Diagnostic{
+ Pos: cl.Pos(),
+ End: cl.End(),
+ Message: fmt.Sprintf("%s composite literal uses unkeyed fields", typeName),
+ SuggestedFixes: []analysis.SuggestedFix{{
+ Message: "Add composite literal keys",
+ TextEdits: missingKeys,
+ }},
+ })
return
}
})
diff --git a/go/analysis/passes/composite/composite_test.go b/go/analysis/passes/composite/composite_test.go
index 952de8b..7afaaa7 100644
--- a/go/analysis/passes/composite/composite_test.go
+++ b/go/analysis/passes/composite/composite_test.go
@@ -18,5 +18,5 @@
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.golden b/go/analysis/passes/composite/testdata/src/a/a.go.golden
new file mode 100644
index 0000000..cc13ab4
--- /dev/null
+++ b/go/analysis/passes/composite/testdata/src/a/a.go.golden
@@ -0,0 +1,127 @@
+// 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"
+ "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 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"
+}
+
+// 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 0000000..20b652e
--- /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 dd5d57e..f9a5e1f 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 @@

import "typeparams/lib"

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

func F[
T1 ~struct{ f int },
@@ -20,8 +20,8 @@
_ = 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 0000000..66cd915
--- /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}
+}

To view, visit change 414674. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: tools
Gerrit-Branch: master
Gerrit-Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224
Gerrit-Change-Number: 414674
Gerrit-PatchSet: 1
Gerrit-Owner: Suzy Mueller <suz...@golang.org>
Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
Gerrit-MessageType: newchange

kokoro (Gerrit)

unread,
Jun 27, 2022, 6:22:35 PM6/27/22
to Suzy Mueller, goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com

Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/33756279-3b10-4131-a7ec-457b42ebdc3c

Patch set 1:gopls-CI +1

View Change

    To view, visit change 414674. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: tools
    Gerrit-Branch: master
    Gerrit-Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224
    Gerrit-Change-Number: 414674
    Gerrit-PatchSet: 1
    Gerrit-Owner: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: kokoro <noreply...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Comment-Date: Mon, 27 Jun 2022 22:22:31 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Alan Donovan (Gerrit)

    unread,
    Jun 28, 2022, 4:25:19 PM6/28/22
    to Suzy Mueller, goph...@pubsubhelper.golang.org, Dylan Le, Gopher Robot, kokoro, golang-co...@googlegroups.com

    Attention is currently required from: Suzy Mueller.

    View Change

    6 comments:

    • Patchset:

    • File go/analysis/passes/composite/composite.go:

      • Patch Set #1, Line 104: field := strct.Field(i)

        Analyzers with RunDespiteErrors=true must be very defensive. Here specifically, it's possible that the struct literal has more entries than the struct has fields. (This is of course a compile error.) So we need to check that i < strct.NumFields before this statement. And let's add a test case that would have caught it.


        (I do wonder whether RunDespiteErrors is worthwhile in most cases. It's much simpler to run analyzers only when the user has gotten their program to compile. I suppose it may be useful in an analyzer that points out a hard-to-spot mistake that would make it hard to get the program to compile.)

      • Patch Set #1, Line 105: if field == nil {

        When is this condition true? I would not expect it.

      • Patch Set #1, Line 115: NewText: []byte(fmt.Sprintf("%s: ", field.Name())),

        If field.Name() is nonexported then the fix won't compile---and nor would the original code, so perhaps the right thing to do in that case is nothing.

      • Patch Set #1, Line 131: composite

        "Add field names to struct literal"

        ditto below

      • Patch Set #1, Line 138: Message: fmt.Sprintf("%s composite literal uses unkeyed fields", typeName),

        s/composite/struct/

    To view, visit change 414674. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: tools
    Gerrit-Branch: master
    Gerrit-Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224
    Gerrit-Change-Number: 414674
    Gerrit-PatchSet: 1
    Gerrit-Owner: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Gopher Robot <go...@golang.org>
    Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: kokoro <noreply...@google.com>
    Gerrit-CC: Dylan Le <dungt...@google.com>
    Gerrit-Attention: Suzy Mueller <suz...@golang.org>
    Gerrit-Comment-Date: Tue, 28 Jun 2022 20:25:14 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Suzy Mueller (Gerrit)

    unread,
    Jun 29, 2022, 3:49:34 PM6/29/22
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

    Attention is currently required from: Alan Donovan, Suzy Mueller.

    Suzy Mueller uploaded patch set #2 to this change.

    View Change

    The following approvals got outdated and were removed: Run-TryBot+1 by Suzy Mueller, TryBot-Result+1 by Gopher Robot, gopls-CI+1 by kokoro

    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
    ---
    M go/analysis/passes/composite/composite.go
    M go/analysis/passes/composite/composite_test.go
    M go/analysis/passes/composite/testdata/src/a/a.go

    A go/analysis/passes/composite/testdata/src/a/a.go.golden
    A go/analysis/passes/composite/testdata/src/a/a_fuzz_test.go.golden
    M go/analysis/passes/composite/testdata/src/typeparams/typeparams.go
    A go/analysis/passes/composite/testdata/src/typeparams/typeparams.go.golden
    7 files changed, 250 insertions(+), 12 deletions(-)

    To view, visit change 414674. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: tools
    Gerrit-Branch: master
    Gerrit-Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224
    Gerrit-Change-Number: 414674
    Gerrit-PatchSet: 2
    Gerrit-Owner: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Gopher Robot <go...@golang.org>
    Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: kokoro <noreply...@google.com>
    Gerrit-CC: Dylan Le <dungt...@google.com>
    Gerrit-Attention: Alan Donovan <adon...@google.com>
    Gerrit-Attention: Suzy Mueller <suz...@golang.org>
    Gerrit-MessageType: newpatchset

    Suzy Mueller (Gerrit)

    unread,
    Jun 29, 2022, 3:49:35 PM6/29/22
    to goph...@pubsubhelper.golang.org, Alan Donovan, Dylan Le, Gopher Robot, kokoro, golang-co...@googlegroups.com

    Attention is currently required from: Alan Donovan, Suzy Mueller.

    View Change

    5 comments:

    • File go/analysis/passes/composite/composite.go:

      • Analyzers with RunDespiteErrors=true must be very defensive. […]

        Good catch. Adding a test case.

      • When I read the documentation for (*types.Struct).Field, I had interpreted it to mean that it would return nil for out of bounds i (which is clearly a very incorrect assumption). Now that we have dealt with out of bounds above I will remove this check.

      • If field. […]

        Will add a check for unexported as well. Not sure the best way to test this though.

      • "Add field names to struct literal" […]

        Done

      • Patch Set #1, Line 138: Message: fmt.Sprintf("%s composite literal uses unkeyed fields", typeName),

        s/composite/struct/

      • Done.

    To view, visit change 414674. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: tools
    Gerrit-Branch: master
    Gerrit-Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224
    Gerrit-Change-Number: 414674
    Gerrit-PatchSet: 2
    Gerrit-Owner: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Gopher Robot <go...@golang.org>
    Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: kokoro <noreply...@google.com>
    Gerrit-CC: Dylan Le <dungt...@google.com>
    Gerrit-Attention: Alan Donovan <adon...@google.com>
    Gerrit-Attention: Suzy Mueller <suz...@golang.org>
    Gerrit-Comment-Date: Wed, 29 Jun 2022 19:49:30 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Alan Donovan <adon...@google.com>
    Gerrit-MessageType: comment

    kokoro (Gerrit)

    unread,
    Jun 29, 2022, 3:55:37 PM6/29/22
    to Suzy Mueller, goph...@pubsubhelper.golang.org, Alan Donovan, Dylan Le, Gopher Robot, golang-co...@googlegroups.com

    Attention is currently required from: Alan Donovan, Suzy Mueller.

    Kokoro presubmit build finished with status: SUCCESS
    Logs at: https://source.cloud.google.com/results/invocations/1f764b9d-4efa-4ee3-b80a-d0230b8a6094

    Patch set 2:gopls-CI +1

    View Change

      To view, visit change 414674. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: tools
      Gerrit-Branch: master
      Gerrit-Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224
      Gerrit-Change-Number: 414674
      Gerrit-PatchSet: 2
      Gerrit-Owner: Suzy Mueller <suz...@golang.org>
      Gerrit-Reviewer: Alan Donovan <adon...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
      Gerrit-Reviewer: kokoro <noreply...@google.com>
      Gerrit-CC: Dylan Le <dungt...@google.com>
      Gerrit-Attention: Alan Donovan <adon...@google.com>
      Gerrit-Attention: Suzy Mueller <suz...@golang.org>
      Gerrit-Comment-Date: Wed, 29 Jun 2022 19:55:33 +0000

      Alan Donovan (Gerrit)

      unread,
      Jun 29, 2022, 4:47:17 PM6/29/22
      to Suzy Mueller, goph...@pubsubhelper.golang.org, Gopher Robot, kokoro, Dylan Le, golang-co...@googlegroups.com

      Attention is currently required from: Suzy Mueller.

      View Change

      2 comments:

      • Patchset:

      • File go/analysis/passes/composite/composite.go:

        • Patch Set #1, Line 115: NewText: []byte(fmt.Sprintf("%s: ", field.Name())),

          Will add a check for unexported as well. Not sure the best way to test this though.

          &sync.Mutex{0, 0}?

          The representation of Mutex is very unlikely to change.

      To view, visit change 414674. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: tools
      Gerrit-Branch: master
      Gerrit-Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224
      Gerrit-Change-Number: 414674
      Gerrit-PatchSet: 2
      Gerrit-Owner: Suzy Mueller <suz...@golang.org>
      Gerrit-Reviewer: Alan Donovan <adon...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
      Gerrit-Reviewer: kokoro <noreply...@google.com>
      Gerrit-CC: Dylan Le <dungt...@google.com>
      Gerrit-Attention: Suzy Mueller <suz...@golang.org>
      Gerrit-Comment-Date: Wed, 29 Jun 2022 20:47:12 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Alan Donovan <adon...@google.com>
      Comment-In-Reply-To: Suzy Mueller <suz...@golang.org>
      Gerrit-MessageType: comment

      Suzy Mueller (Gerrit)

      unread,
      Jun 30, 2022, 1:01:45 PM6/30/22
      to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

      Attention is currently required from: Alan Donovan, Suzy Mueller.

      Suzy Mueller uploaded patch set #3 to this change.

      View Change

      The following approvals got outdated and were removed: Run-TryBot+1 by Suzy Mueller, TryBot-Result+1 by Gopher Robot, gopls-CI+1 by kokoro

      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
      ---
      M go/analysis/passes/composite/composite.go
      M go/analysis/passes/composite/composite_test.go
      M go/analysis/passes/composite/testdata/src/a/a.go
      A go/analysis/passes/composite/testdata/src/a/a.go.golden
      A go/analysis/passes/composite/testdata/src/a/a_fuzz_test.go.golden
      M go/analysis/passes/composite/testdata/src/typeparams/typeparams.go
      A go/analysis/passes/composite/testdata/src/typeparams/typeparams.go.golden
      7 files changed, 260 insertions(+), 12 deletions(-)

      To view, visit change 414674. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: tools
      Gerrit-Branch: master
      Gerrit-Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224
      Gerrit-Change-Number: 414674
      Gerrit-PatchSet: 3
      Gerrit-Owner: Suzy Mueller <suz...@golang.org>
      Gerrit-Reviewer: Alan Donovan <adon...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
      Gerrit-Reviewer: kokoro <noreply...@google.com>
      Gerrit-CC: Dylan Le <dungt...@google.com>
      Gerrit-Attention: Alan Donovan <adon...@google.com>
      Gerrit-Attention: Suzy Mueller <suz...@golang.org>
      Gerrit-MessageType: newpatchset

      Suzy Mueller (Gerrit)

      unread,
      Jun 30, 2022, 1:01:46 PM6/30/22
      to goph...@pubsubhelper.golang.org, Gopher Robot, kokoro, Alan Donovan, Dylan Le, golang-co...@googlegroups.com

      Attention is currently required from: Alan Donovan, Suzy Mueller.

      View Change

      1 comment:

      • File go/analysis/passes/composite/composite.go:

        • &sync.Mutex{0, 0}? […]

          Nice! Thanks.

      To view, visit change 414674. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: tools
      Gerrit-Branch: master
      Gerrit-Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224
      Gerrit-Change-Number: 414674
      Gerrit-PatchSet: 3
      Gerrit-Owner: Suzy Mueller <suz...@golang.org>
      Gerrit-Reviewer: Alan Donovan <adon...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
      Gerrit-Reviewer: kokoro <noreply...@google.com>
      Gerrit-CC: Dylan Le <dungt...@google.com>
      Gerrit-Attention: Alan Donovan <adon...@google.com>
      Gerrit-Attention: Suzy Mueller <suz...@golang.org>
      Gerrit-Comment-Date: Thu, 30 Jun 2022 17:01:40 +0000

      Alan Donovan (Gerrit)

      unread,
      Jun 30, 2022, 1:05:59 PM6/30/22
      to Suzy Mueller, goph...@pubsubhelper.golang.org, Gopher Robot, kokoro, Dylan Le, golang-co...@googlegroups.com

      Attention is currently required from: Suzy Mueller.

      Patch set 3:Code-Review +2

      View Change

        To view, visit change 414674. To unsubscribe, or for help writing mail filters, visit settings.

        Gerrit-Project: tools
        Gerrit-Branch: master
        Gerrit-Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224
        Gerrit-Change-Number: 414674
        Gerrit-PatchSet: 3
        Gerrit-Owner: Suzy Mueller <suz...@golang.org>
        Gerrit-Reviewer: Alan Donovan <adon...@google.com>
        Gerrit-Reviewer: Gopher Robot <go...@golang.org>
        Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
        Gerrit-Reviewer: kokoro <noreply...@google.com>
        Gerrit-CC: Dylan Le <dungt...@google.com>
        Gerrit-Attention: Suzy Mueller <suz...@golang.org>
        Gerrit-Comment-Date: Thu, 30 Jun 2022 17:05:54 +0000

        kokoro (Gerrit)

        unread,
        Jun 30, 2022, 1:11:43 PM6/30/22
        to Suzy Mueller, goph...@pubsubhelper.golang.org, Alan Donovan, Gopher Robot, Dylan Le, golang-co...@googlegroups.com

        Attention is currently required from: Suzy Mueller.

        Kokoro presubmit build finished with status: SUCCESS
        Logs at: https://source.cloud.google.com/results/invocations/4793bbe8-fd69-4576-bb16-1f689f548b33

        Patch set 3:gopls-CI +1

        View Change

          To view, visit change 414674. To unsubscribe, or for help writing mail filters, visit settings.

          Gerrit-Project: tools
          Gerrit-Branch: master
          Gerrit-Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224
          Gerrit-Change-Number: 414674
          Gerrit-PatchSet: 3
          Gerrit-Owner: Suzy Mueller <suz...@golang.org>
          Gerrit-Reviewer: Alan Donovan <adon...@google.com>
          Gerrit-Reviewer: Gopher Robot <go...@golang.org>
          Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
          Gerrit-Reviewer: kokoro <noreply...@google.com>
          Gerrit-CC: Dylan Le <dungt...@google.com>
          Gerrit-Attention: Suzy Mueller <suz...@golang.org>
          Gerrit-Comment-Date: Thu, 30 Jun 2022 17:11:34 +0000

          Suzy Mueller (Gerrit)

          unread,
          Jun 30, 2022, 1:24:25 PM6/30/22
          to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Gopher Robot, kokoro, Alan Donovan, Dylan Le, golang-co...@googlegroups.com

          Suzy Mueller submitted this change.

          View Change


          Approvals: Alan Donovan: Looks good to me, approved Gopher Robot: TryBots succeeded Suzy Mueller: Run TryBots kokoro: gopls CI succeeded
          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 <noreply...@google.com>
          TryBot-Result: Gopher Robot <go...@golang.org>
          Reviewed-by: Alan Donovan <adon...@google.com>
          Run-TryBot: Suzy Mueller <suz...@golang.org>

          ---
          M go/analysis/passes/composite/composite.go
          M go/analysis/passes/composite/composite_test.go
          M go/analysis/passes/composite/testdata/src/a/a.go
          A go/analysis/passes/composite/testdata/src/a/a.go.golden
          A go/analysis/passes/composite/testdata/src/a/a_fuzz_test.go.golden
          M go/analysis/passes/composite/testdata/src/typeparams/typeparams.go
          A go/analysis/passes/composite/testdata/src/typeparams/typeparams.go.golden
          7 files changed, 265 insertions(+), 12 deletions(-)

          diff --git a/go/analysis/passes/composite/composite.go b/go/analysis/passes/composite/composite.go
          index d3670ac..64e184d 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 @@
          }
          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 @@
          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 952de8b..7afaaa7 100644
          --- a/go/analysis/passes/composite/composite_test.go
          +++ b/go/analysis/passes/composite/composite_test.go
          @@ -18,5 +18,5 @@
          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 3a5bc20..cd69d39 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 @@
          "go/scanner"
          "go/token"
          "image"
          + "sync"
          "unicode"
          )

          @@ -79,6 +80,18 @@
          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 @@
          &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 0000000..fe73a2e
          --- /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"
          +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"
          +

          To view, visit change 414674. To unsubscribe, or for help writing mail filters, visit settings.

          Gerrit-Project: tools
          Gerrit-Branch: master
          Gerrit-Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224
          Gerrit-Change-Number: 414674
          Gerrit-PatchSet: 4
          Gerrit-Owner: Suzy Mueller <suz...@golang.org>
          Gerrit-Reviewer: Alan Donovan <adon...@google.com>
          Gerrit-Reviewer: Gopher Robot <go...@golang.org>
          Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
          Gerrit-Reviewer: kokoro <noreply...@google.com>
          Gerrit-CC: Dylan Le <dungt...@google.com>
          Gerrit-MessageType: merged
          Reply all
          Reply to author
          Forward
          0 new messages