From 0ed517e5e68813581de8f6a7e94211d82ff36dd2 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Wed, 6 Mar 2013 12:49:56 -0800 Subject: [PATCH] cmd/vet: isolate the type checking code into a separate file We can enable/disable type checking with a build tag. Should simplify cutting the go1.1 distribution free of go/types. R=golang-dev, rsc CC=golang-dev https://golang.org/cl/7482045 --- src/cmd/vet/Makefile | 3 +- src/cmd/vet/main.go | 19 +---- src/cmd/vet/print.go | 115 -------------------------- src/cmd/vet/taglit.go | 24 +----- src/cmd/vet/types.go | 178 ++++++++++++++++++++++++++++++++++++++++ src/cmd/vet/typestub.go | 45 ++++++++++ 6 files changed, 231 insertions(+), 153 deletions(-) create mode 100644 src/cmd/vet/types.go create mode 100644 src/cmd/vet/typestub.go diff --git a/src/cmd/vet/Makefile b/src/cmd/vet/Makefile index 307f4729cf..0241e3f058 100644 --- a/src/cmd/vet/Makefile +++ b/src/cmd/vet/Makefile @@ -2,7 +2,8 @@ # Use of this source code is governed by a BSD-style # license that can be found in the LICENSE file. +# Assumes go/types is installed test testshort: - go build -tags vet_test + go build -tags 'vet_test gotypes' ../../../test/errchk ./vet -compositewhitelist=false -printfuncs='Warn:1,Warnf:1' *.go diff --git a/src/cmd/vet/main.go b/src/cmd/vet/main.go index 887cc06424..952b80a95f 100644 --- a/src/cmd/vet/main.go +++ b/src/cmd/vet/main.go @@ -15,7 +15,6 @@ import ( "go/parser" "go/printer" "go/token" - "go/types" "io/ioutil" "os" "path/filepath" @@ -175,7 +174,7 @@ func doPackageDir(directory string) { } type Package struct { - types map[ast.Expr]types.Type + types map[ast.Expr]Type values map[ast.Expr]interface{} } @@ -207,22 +206,8 @@ func doPackage(names []string) { astFiles = append(astFiles, parsedFile) } pkg := new(Package) - pkg.types = make(map[ast.Expr]types.Type) - pkg.values = make(map[ast.Expr]interface{}) - exprFn := func(x ast.Expr, typ types.Type, val interface{}) { - pkg.types[x] = typ - if val != nil { - pkg.values[x] = val - } - } - // By providing the Context with our own error function, it will continue - // past the first error. There is no need for that function to do anything. - context := types.Context{ - Expr: exprFn, - Error: func(error) {}, - } // Type check the package. - _, err := context.Check(fs, astFiles) + err := pkg.check(fs, astFiles) if err != nil && *verbose { warnf("%s", err) } diff --git a/src/cmd/vet/print.go b/src/cmd/vet/print.go index 7bb99b0114..debfbf0bfb 100644 --- a/src/cmd/vet/print.go +++ b/src/cmd/vet/print.go @@ -10,7 +10,6 @@ import ( "flag" "go/ast" "go/token" - "go/types" "strconv" "strings" "unicode/utf8" @@ -302,59 +301,6 @@ func (f *File) checkPrintfArg(call *ast.CallExpr, verb rune, flags []byte, argNu f.Badf(call.Pos(), "unrecognized printf verb %q", verb) } -func (f *File) matchArgType(t printfArgType, arg ast.Expr) bool { - // TODO: for now, we can only test builtin types and untyped constants. - typ := f.pkg.types[arg] - if typ == nil { - return true - } - basic, ok := typ.(*types.Basic) - if !ok { - return true - } - switch basic.Kind { - case types.Bool: - return t&argBool != 0 - case types.Int, types.Int8, types.Int16, types.Int32, types.Int64: - fallthrough - case types.Uint, types.Uint8, types.Uint16, types.Uint32, types.Uint64, types.Uintptr: - return t&argInt != 0 - case types.Float32, types.Float64, types.Complex64, types.Complex128: - return t&argFloat != 0 - case types.String: - return t&argString != 0 - case types.UnsafePointer: - return t&(argPointer|argInt) != 0 - case types.UntypedBool: - return t&argBool != 0 - case types.UntypedComplex: - return t&argFloat != 0 - case types.UntypedFloat: - // If it's integral, we can use an int format. - switch f.pkg.values[arg].(type) { - case int, int8, int16, int32, int64: - return t&(argInt|argFloat) != 0 - case uint, uint8, uint16, uint32, uint64: - return t&(argInt|argFloat) != 0 - } - return t&argFloat != 0 - case types.UntypedInt: - return t&argInt != 0 - case types.UntypedRune: - return t&(argInt|argRune) != 0 - case types.UntypedString: - return t&argString != 0 - case types.UntypedNil: - return t&argPointer != 0 // TODO? - case types.Invalid: - if *verbose { - f.Warnf(arg.Pos(), "printf argument %v has invalid or unknown type", arg) - } - return true // Probably a type check problem. - } - return false -} - // checkPrint checks a call to an unformatted print routine such as Println. // call.Args[firstArg] is the first argument to be printed. func (f *File) checkPrint(call *ast.CallExpr, name string, firstArg int) { @@ -403,64 +349,3 @@ func (f *File) checkPrint(call *ast.CallExpr, name string, firstArg int) { } } } - -// numArgsInSignature tells how many formal arguments the function type -// being called has. -func (f *File) numArgsInSignature(call *ast.CallExpr) int { - // Check the type of the function or method declaration - typ := f.pkg.types[call.Fun] - if typ == nil { - return 0 - } - // The type must be a signature, but be sure for safety. - sig, ok := typ.(*types.Signature) - if !ok { - return 0 - } - return len(sig.Params) -} - -// isErrorMethodCall reports whether the call is of a method with signature -// func Error() string -// where "string" is the universe's string type. We know the method is called "Error". -func (f *File) isErrorMethodCall(call *ast.CallExpr) bool { - // Is it a selector expression? Otherwise it's a function call, not a method call. - sel, ok := call.Fun.(*ast.SelectorExpr) - if !ok { - return false - } - // The package is type-checked, so if there are no arguments, we're done. - if len(call.Args) > 0 { - return false - } - // Check the type of the method declaration - typ := f.pkg.types[sel] - if typ == nil { - return false - } - // The type must be a signature, but be sure for safety. - sig, ok := typ.(*types.Signature) - if !ok { - return false - } - // There must be a receiver for it to be a method call. Otherwise it is - // a function, not something that satisfies the error interface. - if sig.Recv == nil { - return false - } - // There must be no arguments. Already verified by type checking, but be thorough. - if len(sig.Params) > 0 { - return false - } - // Finally the real questions. - // There must be one result. - if len(sig.Results) != 1 { - return false - } - // It must have return type "string" from the universe. - result := sig.Results[0].Type - if types.IsIdentical(result, types.Typ[types.String]) { - return true - } - return false -} diff --git a/src/cmd/vet/taglit.go b/src/cmd/vet/taglit.go index 71bd7b71d0..a19187fea1 100644 --- a/src/cmd/vet/taglit.go +++ b/src/cmd/vet/taglit.go @@ -9,7 +9,6 @@ package main import ( "flag" "go/ast" - "go/types" "strings" ) @@ -22,19 +21,9 @@ func (f *File) checkUntaggedLiteral(c *ast.CompositeLit) { return } - // Check that the CompositeLit's type is a slice or array (which needs no tag), if possible. - typ := f.pkg.types[c] - if typ != nil { - // If it's a named type, pull out the underlying type. - if namedType, ok := typ.(*types.NamedType); ok { - typ = namedType.Underlying - } - switch typ.(type) { - case *types.Slice: - return - case *types.Array: - return - } + isStruct, typeString := f.pkg.isStruct(c) + if !isStruct { + return } // It's a struct, or we can't tell it's not a struct because we don't have types. @@ -72,11 +61,7 @@ func (f *File) checkUntaggedLiteral(c *ast.CompositeLit) { return } - pre := "" - if typ != nil { - pre = typ.String() + " " - } - f.Warn(c.Pos(), pre+"composite literal uses untagged fields") + f.Warn(c.Pos(), typeString+" composite literal uses untagged fields") } // pkgPath returns the import path "image/png" for the package name "png". @@ -124,7 +109,6 @@ var untaggedLiteralWhitelist = map[string]bool{ "encoding/xml.Comment": true, "encoding/xml.Directive": true, "exp/norm.Decomposition": true, - "exp/types.ObjList": true, "go/scanner.ErrorList": true, "image/color.Palette": true, "net.HardwareAddr": true, diff --git a/src/cmd/vet/types.go b/src/cmd/vet/types.go new file mode 100644 index 0000000000..32584e175e --- /dev/null +++ b/src/cmd/vet/types.go @@ -0,0 +1,178 @@ +// Copyright 2010 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. + +// +build gotypes + +// This file contains the pieces of the tool that require the go/types package. + +package main + +import ( + "go/ast" + "go/token" + "go/types" +) + +// Type is equivalent to go/types.Type. Repeating it here allows us to avoid +// depending on the go/types package. +type Type interface { + String() string +} + +func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) error { + pkg.types = make(map[ast.Expr]Type) + pkg.values = make(map[ast.Expr]interface{}) + exprFn := func(x ast.Expr, typ types.Type, val interface{}) { + pkg.types[x] = typ + if val != nil { + pkg.values[x] = val + } + } + // By providing the Context with our own error function, it will continue + // past the first error. There is no need for that function to do anything. + context := types.Context{ + Expr: exprFn, + Error: func(error) {}, + } + _, err := context.Check(fs, astFiles) + return err +} + +// isStruct reports whether the composite literal c is a struct. +// If it is not (probably a struct), it returns a printable form of the type. +func (pkg *Package) isStruct(c *ast.CompositeLit) (bool, string) { + // Check that the CompositeLit's type is a slice or array (which needs no tag), if possible. + typ := pkg.types[c] + if typ == nil { + return false, "" + } + // If it's a named type, pull out the underlying type. + if namedType, ok := typ.(*types.NamedType); ok { + typ = namedType.Underlying + } + switch typ.(type) { + case *types.Struct: + default: + return false, "" + } + typeString := "" + if typ != nil { + typeString = typ.String() + " " + } + return true, typeString +} + +func (f *File) matchArgType(t printfArgType, arg ast.Expr) bool { + // TODO: for now, we can only test builtin types and untyped constants. + typ := f.pkg.types[arg] + if typ == nil { + return true + } + basic, ok := typ.(*types.Basic) + if !ok { + return true + } + switch basic.Kind { + case types.Bool: + return t&argBool != 0 + case types.Int, types.Int8, types.Int16, types.Int32, types.Int64: + fallthrough + case types.Uint, types.Uint8, types.Uint16, types.Uint32, types.Uint64, types.Uintptr: + return t&argInt != 0 + case types.Float32, types.Float64, types.Complex64, types.Complex128: + return t&argFloat != 0 + case types.String: + return t&argString != 0 + case types.UnsafePointer: + return t&(argPointer|argInt) != 0 + case types.UntypedBool: + return t&argBool != 0 + case types.UntypedComplex: + return t&argFloat != 0 + case types.UntypedFloat: + // If it's integral, we can use an int format. + switch f.pkg.values[arg].(type) { + case int, int8, int16, int32, int64: + return t&(argInt|argFloat) != 0 + case uint, uint8, uint16, uint32, uint64: + return t&(argInt|argFloat) != 0 + } + return t&argFloat != 0 + case types.UntypedInt: + return t&argInt != 0 + case types.UntypedRune: + return t&(argInt|argRune) != 0 + case types.UntypedString: + return t&argString != 0 + case types.UntypedNil: + return t&argPointer != 0 // TODO? + case types.Invalid: + if *verbose { + f.Warnf(arg.Pos(), "printf argument %v has invalid or unknown type", arg) + } + return true // Probably a type check problem. + } + return false +} + +// numArgsInSignature tells how many formal arguments the function type +// being called has. +func (f *File) numArgsInSignature(call *ast.CallExpr) int { + // Check the type of the function or method declaration + typ := f.pkg.types[call.Fun] + if typ == nil { + return 0 + } + // The type must be a signature, but be sure for safety. + sig, ok := typ.(*types.Signature) + if !ok { + return 0 + } + return len(sig.Params) +} + +// isErrorMethodCall reports whether the call is of a method with signature +// func Error() string +// where "string" is the universe's string type. We know the method is called "Error". +func (f *File) isErrorMethodCall(call *ast.CallExpr) bool { + // Is it a selector expression? Otherwise it's a function call, not a method call. + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + // The package is type-checked, so if there are no arguments, we're done. + if len(call.Args) > 0 { + return false + } + // Check the type of the method declaration + typ := f.pkg.types[sel] + if typ == nil { + return false + } + // The type must be a signature, but be sure for safety. + sig, ok := typ.(*types.Signature) + if !ok { + return false + } + // There must be a receiver for it to be a method call. Otherwise it is + // a function, not something that satisfies the error interface. + if sig.Recv == nil { + return false + } + // There must be no arguments. Already verified by type checking, but be thorough. + if len(sig.Params) > 0 { + return false + } + // Finally the real questions. + // There must be one result. + if len(sig.Results) != 1 { + return false + } + // It must have return type "string" from the universe. + result := sig.Results[0].Type + if types.IsIdentical(result, types.Typ[types.String]) { + return true + } + return false +} diff --git a/src/cmd/vet/typestub.go b/src/cmd/vet/typestub.go new file mode 100644 index 0000000000..6ccaf8a808 --- /dev/null +++ b/src/cmd/vet/typestub.go @@ -0,0 +1,45 @@ +// Copyright 2010 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. + +// +build !gotypes + +// This file contains stubs for the pieces of the tool that require the go/types package, +// to be used if go/types is not available. + +package main + +import ( + "go/ast" + "go/token" +) + +// Type is equivalent to go/types.Type. Repeating it here allows us to avoid +// depending on the go/types package. +type Type interface { + String() string +} + +func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) error { + return nil +} + +func (pkg *Package) isStruct(c *ast.CompositeLit) (bool, string) { + return true, "struct" // Assume true, so we do the check. +} + +func (f *File) matchArgType(t printfArgType, arg ast.Expr) bool { + return true // We can't tell without types. +} + +func (f *File) numArgsInSignature(call *ast.CallExpr) int { + return 0 // We don't know. +} + +func (f *File) isErrorMethodCall(call *ast.CallExpr) bool { + // Is it a selector expression? Otherwise it's a function call, not a method call. + if _, ok := call.Fun.(*ast.SelectorExpr); !ok { + return false + } + return true // Best guess we can make without types. +}