go/analysis/passes/composite: split out of vet

Change-Id: Ie6e05bea1c8c607407479f5f45dea6fcec62f60c
Reviewed-on: https://go-review.googlesource.com/c/140739
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
This commit is contained in:
Alan Donovan 2018-10-08 20:37:15 -04:00
parent 34804b1db3
commit f78a1e9345
5 changed files with 133 additions and 104 deletions

View File

@ -0,0 +1,104 @@
// 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.
package composite
import (
"go/ast"
"go/types"
"strings"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/ast/inspector"
)
var Analyzer = &analysis.Analyzer{
Name: "composites",
Doc: `checked for unkeyed composite literals
This analyzer reports a diagnostic for composite literals of struct
types imported from another package that do not use the field-keyed
syntax. Such literals are fragile because the addition of a new field
(even if unexported) to the struct will cause compilation to fail.`,
Requires: []*analysis.Analyzer{inspect.Analyzer},
RunDespiteErrors: true,
Run: run,
}
var whitelist = true
func init() {
Analyzer.Flags.BoolVar(&whitelist, "whitelist", whitelist, "use composite white list; for testing only")
}
// runUnkeyedLiteral checks if a composite literal is a struct literal with
// unkeyed fields.
func run(pass *analysis.Pass) (interface{}, error) {
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
nodeFilter := []ast.Node{
(*ast.CompositeLit)(nil),
}
inspect.Preorder(nodeFilter, func(n ast.Node) {
cl := n.(*ast.CompositeLit)
typ := pass.TypesInfo.Types[cl].Type
if typ == nil {
// cannot determine composite literals' type, skip it
return
}
typeName := typ.String()
if whitelist && unkeyedLiteral[typeName] {
// skip whitelisted types
return
}
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
}
if isLocalType(pass, typ) {
// allow unkeyed locally defined composite literal
return
}
// check if the CompositeLit contains an unkeyed field
allKeyValue := true
for _, e := range cl.Elts {
if _, ok := e.(*ast.KeyValueExpr); !ok {
allKeyValue = false
break
}
}
if allKeyValue {
// all the composite literal fields are keyed
return
}
pass.Reportf(cl.Pos(), "%s composite literal uses unkeyed fields", typeName)
})
return nil, nil
}
func isLocalType(pass *analysis.Pass, typ types.Type) bool {
switch x := typ.(type) {
case *types.Struct:
// struct literals are local types
return true
case *types.Pointer:
return isLocalType(pass, x.Elem())
case *types.Named:
// names in package foo are local to foo_test too
return strings.TrimSuffix(x.Obj().Pkg().Path(), "_test") == strings.TrimSuffix(pass.Pkg.Path(), "_test")
}
return false
}

View File

@ -0,0 +1,13 @@
package composite_test
import (
"testing"
"golang.org/x/tools/go/analysis/analysistest"
"golang.org/x/tools/go/analysis/passes/composite"
)
func TestFromFileSystem(t *testing.T) {
testdata := analysistest.TestData()
analysistest.Run(t, testdata, composite.Analyzer, "a")
}

View File

@ -4,15 +4,14 @@
// This file contains the test for untagged struct literals. // This file contains the test for untagged struct literals.
package testdata package a
import ( import (
"flag" "flag"
"go/scanner" "go/scanner"
"go/token"
"image" "image"
"unicode" "unicode"
"path/to/unknownpkg"
) )
var Okay1 = []string{ var Okay1 = []string{
@ -74,21 +73,23 @@ var goodStructLiteral = flag.Flag{
Name: "Name", Name: "Name",
Usage: "Usage", Usage: "Usage",
} }
var badStructLiteral = flag.Flag{ // ERROR "unkeyed fields" var badStructLiteral = flag.Flag{ // want "unkeyed fields"
"Name", "Name",
"Usage", "Usage",
nil, // Value nil, // Value
"DefValue", "DefValue",
} }
var delta [3]rune
// SpecialCase is a named slice of CaseRange to test issue 9171. // SpecialCase is a named slice of CaseRange to test issue 9171.
var goodNamedSliceLiteral = unicode.SpecialCase{ var goodNamedSliceLiteral = unicode.SpecialCase{
{Lo: 1, Hi: 2}, {Lo: 1, Hi: 2, Delta: delta},
unicode.CaseRange{Lo: 1, Hi: 2}, unicode.CaseRange{Lo: 1, Hi: 2, Delta: delta},
} }
var badNamedSliceLiteral = unicode.SpecialCase{ var badNamedSliceLiteral = unicode.SpecialCase{
{1, 2}, // ERROR "unkeyed fields" {1, 2, delta}, // want "unkeyed fields"
unicode.CaseRange{1, 2}, // ERROR "unkeyed fields" unicode.CaseRange{1, 2, delta}, // want "unkeyed fields"
} }
// ErrorList is a named slice, so no warnings should be emitted. // ErrorList is a named slice, so no warnings should be emitted.
@ -96,7 +97,7 @@ var goodScannerErrorList = scanner.ErrorList{
&scanner.Error{Msg: "foobar"}, &scanner.Error{Msg: "foobar"},
} }
var badScannerErrorList = scanner.ErrorList{ var badScannerErrorList = scanner.ErrorList{
&scanner.Error{"foobar"}, // ERROR "unkeyed fields" &scanner.Error{token.Position{}, "foobar"}, // want "unkeyed fields"
} }
// Check whitelisted structs: if vet is run with --compositewhitelist=false, // Check whitelisted structs: if vet is run with --compositewhitelist=false,
@ -105,7 +106,7 @@ var whitelistedPoint = image.Point{1, 2}
// Do not check type from unknown package. // Do not check type from unknown package.
// See issue 15408. // See issue 15408.
var unknownPkgVar = unknownpkg.Foobar{"foo", "bar"} var unknownPkgVar = unicode.NoSuchType{"foo", "bar"}
// A named pointer slice of CaseRange to test issue 23539. In // A named pointer slice of CaseRange to test issue 23539. In
// particular, we're interested in how some slice elements omit their // particular, we're interested in how some slice elements omit their
@ -115,6 +116,6 @@ var goodNamedPointerSliceLiteral = []*unicode.CaseRange{
&unicode.CaseRange{Lo: 1, Hi: 2}, &unicode.CaseRange{Lo: 1, Hi: 2},
} }
var badNamedPointerSliceLiteral = []*unicode.CaseRange{ var badNamedPointerSliceLiteral = []*unicode.CaseRange{
{1, 2}, // ERROR "unkeyed fields" {1, 2, delta}, // want "unkeyed fields"
&unicode.CaseRange{1, 2}, // ERROR "unkeyed fields" &unicode.CaseRange{1, 2, delta}, // want "unkeyed fields"
} }

View File

@ -2,12 +2,11 @@
// Use of this source code is governed by a BSD-style // Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file. // license that can be found in the LICENSE file.
// Package whitelist defines exceptions for the vet tool. package composite
package whitelist
// UnkeyedLiteral is a white list of types in the standard packages // unkeyedLiteral is a white list of types in the standard packages
// that are used with unkeyed literals we deem to be acceptable. // that are used with unkeyed literals we deem to be acceptable.
var UnkeyedLiteral = map[string]bool{ var unkeyedLiteral = map[string]bool{
// These image and image/color struct types are frozen. We will never add fields to them. // These image and image/color struct types are frozen. We will never add fields to them.
"image/color.Alpha16": true, "image/color.Alpha16": true,
"image/color.Alpha": true, "image/color.Alpha": true,

View File

@ -1,88 +0,0 @@
// +build ignore
// 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 unkeyed struct literals.
package main
import (
"cmd/vet/internal/whitelist"
"flag"
"go/ast"
"go/types"
"strings"
)
var compositeWhiteList = flag.Bool("compositewhitelist", true, "use composite white list; for testing only")
func init() {
register("composites",
"check that composite literals of types from imported packages use field-keyed elements",
checkUnkeyedLiteral,
compositeLit)
}
// checkUnkeyedLiteral checks if a composite literal is a struct literal with
// unkeyed fields.
func checkUnkeyedLiteral(f *File, node ast.Node) {
cl := node.(*ast.CompositeLit)
typ := f.pkg.types[cl].Type
if typ == nil {
// cannot determine composite literals' type, skip it
return
}
typeName := typ.String()
if *compositeWhiteList && whitelist.UnkeyedLiteral[typeName] {
// skip whitelisted types
return
}
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
}
if isLocalType(f, typ) {
// allow unkeyed locally defined composite literal
return
}
// check if the CompositeLit contains an unkeyed field
allKeyValue := true
for _, e := range cl.Elts {
if _, ok := e.(*ast.KeyValueExpr); !ok {
allKeyValue = false
break
}
}
if allKeyValue {
// all the composite literal fields are keyed
return
}
f.Badf(cl.Pos(), "%s composite literal uses unkeyed fields", typeName)
}
func isLocalType(f *File, typ types.Type) bool {
switch x := typ.(type) {
case *types.Struct:
// struct literals are local types
return true
case *types.Pointer:
return isLocalType(f, x.Elem())
case *types.Named:
// names in package foo are local to foo_test too
return strings.TrimSuffix(x.Obj().Pkg().Path(), "_test") == strings.TrimSuffix(f.pkg.path, "_test")
}
return false
}