Skip to content

Commit

Permalink
cmd/vet: warn on unkeyed struct pointer literals
Browse files Browse the repository at this point in the history
We did warn on them in some cases, but not others. In particular, if one
used a slice composite literal with struct pointer elements, and omitted
the type of an element's composite literal, it would not get any warning
even if it should get one.

The issue is that typ.Underlying() can be of type *types.Pointer. Skip
those levels of indirection before checking for a *types.Struct
underlying type.

isLocalType also needed a bit of tweaking to ignore dereferences.
Perhaps that can be rewritten now that we have type info, but let's
leave it for another time.

Fixes golang#23539.

Change-Id: I727a497284df1325b70d47a756519f5db1add25d
Reviewed-on: https://go-review.googlesource.com/89715
Run-TryBot: Daniel Martí <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
  • Loading branch information
mvdan committed Feb 21, 2018
1 parent 8fea862 commit 6ded116
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
14 changes: 13 additions & 1 deletion src/cmd/vet/composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,15 @@ func checkUnkeyedLiteral(f *File, node ast.Node) {
// skip whitelisted types
return
}
if _, ok := typ.Underlying().(*types.Struct); !ok {
under := typ.Underlying()
for {
ptr, ok := under.(*types.Pointer)
if !ok {
break
}
under = ptr.Elem().Underlying()
}
if _, ok := under.(*types.Struct); !ok {
// skip non-struct composite literals
return
}
Expand Down Expand Up @@ -69,6 +77,10 @@ func isLocalType(f *File, typeName string) bool {
return true
}

// make *foo.bar, **foo.bar, etc match with the "foo." prefix
// below
typeName = strings.TrimLeft(typeName, "*")

pkgname := f.pkg.path
if strings.HasPrefix(typeName, pkgname+".") {
return true
Expand Down
17 changes: 17 additions & 0 deletions src/cmd/vet/testdata/composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ var Okay6 = []MyStruct{
{"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.

Expand Down Expand Up @@ -101,3 +106,15 @@ var whitelistedPoint = image.Point{1, 2}
// Do not check type from unknown package.
// See issue 15408.
var unknownPkgVar = unknownpkg.Foobar{"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{
{1, 2}, // ERROR "unkeyed fields"
&unicode.CaseRange{1, 2}, // ERROR "unkeyed fields"
}

0 comments on commit 6ded116

Please sign in to comment.