182 | 182 |
f.lintErrorStrings()
|
183 | 183 |
f.lintReceiverNames()
|
184 | 184 |
f.lintIncDec()
|
185 | |
f.lintMake()
|
186 | 185 |
f.lintErrorReturn()
|
187 | 186 |
f.lintUnexportedReturn()
|
188 | 187 |
f.lintTimeNames()
|
|
188 |
f.lintContextKeyTypes()
|
|
189 |
f.lintContextArgs()
|
189 | 190 |
}
|
190 | 191 |
|
191 | 192 |
type link string
|
|
235 | 236 |
|
236 | 237 |
var gcImporter = gcimporter.Import
|
237 | 238 |
|
238 | |
// importer implements go/types.Importer.
|
239 | |
// It also implements go/types.ImporterFrom, which was new in Go 1.6,
|
240 | |
// so vendoring will work.
|
|
239 |
// importer implements go/types.Importer{,From}.
|
241 | 240 |
type importer struct {
|
242 | 241 |
impFn func(packages map[string]*types.Package, path, srcDir string) (*types.Package, error)
|
243 | 242 |
packages map[string]*types.Package
|
|
247 | 246 |
return i.impFn(i.packages, path, "")
|
248 | 247 |
}
|
249 | 248 |
|
250 | |
// (importer).ImportFrom is in lint16.go.
|
|
249 |
func (i importer) ImportFrom(path, srcDir string, mode types.ImportMode) (*types.Package, error) {
|
|
250 |
return i.impFn(i.packages, path, srcDir)
|
|
251 |
}
|
251 | 252 |
|
252 | 253 |
func (p *pkg) typeCheck() error {
|
253 | 254 |
config := &types.Config{
|
|
727 | 728 |
// Only add entries that are highly unlikely to be non-initialisms.
|
728 | 729 |
// For instance, "ID" is fine (Freudian code is rare), but "AND" is not.
|
729 | 730 |
var commonInitialisms = map[string]bool{
|
|
731 |
"ACL": true,
|
730 | 732 |
"API": true,
|
731 | 733 |
"ASCII": true,
|
732 | 734 |
"CPU": true,
|
|
761 | 763 |
"UTF8": true,
|
762 | 764 |
"VM": true,
|
763 | 765 |
"XML": true,
|
|
766 |
"XMPP": true,
|
764 | 767 |
"XSRF": true,
|
765 | 768 |
"XSS": true,
|
766 | 769 |
}
|
|
933 | 936 |
"0i": true,
|
934 | 937 |
}
|
935 | 938 |
|
936 | |
// knownWeakerTypes is a set of types that are commonly used to weaken var declarations.
|
937 | |
// A common form of var declarations that legitimately mentions an explicit LHS type
|
938 | |
// is where the LHS type is "weaker" than the exact RHS type, where "weaker" means an
|
939 | |
// interface compared to a concrete type, or an interface compared to a superset interface.
|
940 | |
// A canonical example is `var out io.Writer = os.Stdout`.
|
941 | |
// This is only used when type checking fails to determine the exact types.
|
942 | |
var knownWeakerTypes = map[string]bool{
|
943 | |
"io.Reader": true,
|
944 | |
"io.Writer": true,
|
945 | |
"proto.Message": true,
|
946 | |
}
|
947 | |
|
948 | 939 |
// lintVarDecls examines variable declarations. It complains about declarations with
|
949 | 940 |
// redundant LHS types that can be inferred from the RHS.
|
950 | 941 |
func (f *file) lintVarDecls() {
|
|
984 | 975 |
}
|
985 | 976 |
lhsTyp := f.pkg.typeOf(v.Type)
|
986 | 977 |
rhsTyp := f.pkg.typeOf(rhs)
|
987 | |
if lhsTyp != nil && rhsTyp != nil && !types.Identical(lhsTyp, rhsTyp) {
|
|
978 |
|
|
979 |
if !validType(lhsTyp) || !validType(rhsTyp) {
|
|
980 |
// Type checking failed (often due to missing imports).
|
|
981 |
return false
|
|
982 |
}
|
|
983 |
|
|
984 |
if !types.Identical(lhsTyp, rhsTyp) {
|
988 | 985 |
// Assignment to a different type is not redundant.
|
989 | 986 |
return false
|
990 | 987 |
}
|
|
1003 | 1000 |
if defType, ok := f.isUntypedConst(rhs); ok && !isIdent(v.Type, defType) {
|
1004 | 1001 |
return false
|
1005 | 1002 |
}
|
1006 | |
// If the LHS is a known weaker type, and we couldn't type check both sides,
|
1007 | |
// don't warn.
|
1008 | |
if lhsTyp == nil || rhsTyp == nil {
|
1009 | |
if knownWeakerTypes[f.render(v.Type)] {
|
1010 | |
return false
|
1011 | |
}
|
1012 | |
}
|
1013 | 1003 |
|
1014 | 1004 |
f.errorf(v.Type, 0.8, category("type-inference"), "should omit type %s from declaration of var %s; it will be inferred from the right-hand side", f.render(v.Type), v.Names[0])
|
1015 | 1005 |
return false
|
1016 | 1006 |
}
|
1017 | 1007 |
return true
|
1018 | 1008 |
})
|
|
1009 |
}
|
|
1010 |
|
|
1011 |
func validType(T types.Type) bool {
|
|
1012 |
return T != nil &&
|
|
1013 |
T != types.Typ[types.Invalid] &&
|
|
1014 |
!strings.Contains(T.String(), "invalid type") // good but not foolproof
|
1019 | 1015 |
}
|
1020 | 1016 |
|
1021 | 1017 |
// lintElses examines else blocks. It complains about any else block whose if block ends in a return.
|
|
1216 | 1212 |
})
|
1217 | 1213 |
}
|
1218 | 1214 |
|
1219 | |
var badReceiverNames = map[string]bool{
|
1220 | |
"me": true,
|
1221 | |
"this": true,
|
1222 | |
"self": true,
|
1223 | |
}
|
1224 | |
|
1225 | 1215 |
// lintReceiverNames examines receiver names. It complains about inconsistent
|
1226 | 1216 |
// names used for the same type and names such as "this".
|
1227 | 1217 |
func (f *file) lintReceiverNames() {
|
|
1241 | 1231 |
f.errorf(n, 1, link(ref), category("naming"), `receiver name should not be an underscore`)
|
1242 | 1232 |
return true
|
1243 | 1233 |
}
|
1244 | |
if badReceiverNames[name] {
|
1245 | |
f.errorf(n, 1, link(ref), category("naming"), `receiver name should be a reflection of its identity; don't use generic names such as "me", "this", or "self"`)
|
|
1234 |
if name == "this" || name == "self" {
|
|
1235 |
f.errorf(n, 1, link(ref), category("naming"), `receiver name should be a reflection of its identity; don't use generic names such as "this" or "self"`)
|
1246 | 1236 |
return true
|
1247 | 1237 |
}
|
1248 | 1238 |
recv := receiverType(fn)
|
|
1279 | 1269 |
return true
|
1280 | 1270 |
}
|
1281 | 1271 |
f.errorf(as, 0.8, category("unary-op"), "should replace %s with %s%s", f.render(as), f.render(as.Lhs[0]), suffix)
|
1282 | |
return true
|
1283 | |
})
|
1284 | |
}
|
1285 | |
|
1286 | |
// lintMake examines statements that declare and initialize a variable with make.
|
1287 | |
// It complains if they are constructing a zero element slice.
|
1288 | |
func (f *file) lintMake() {
|
1289 | |
f.walk(func(n ast.Node) bool {
|
1290 | |
as, ok := n.(*ast.AssignStmt)
|
1291 | |
if !ok {
|
1292 | |
return true
|
1293 | |
}
|
1294 | |
// Only want single var := assignment statements.
|
1295 | |
if len(as.Lhs) != 1 || as.Tok != token.DEFINE {
|
1296 | |
return true
|
1297 | |
}
|
1298 | |
ce, ok := as.Rhs[0].(*ast.CallExpr)
|
1299 | |
if !ok {
|
1300 | |
return true
|
1301 | |
}
|
1302 | |
// Check if ce is make([]T, 0).
|
1303 | |
if !isIdent(ce.Fun, "make") || len(ce.Args) != 2 || !isZero(ce.Args[1]) {
|
1304 | |
return true
|
1305 | |
}
|
1306 | |
at, ok := ce.Args[0].(*ast.ArrayType)
|
1307 | |
if !ok || at.Len != nil {
|
1308 | |
return true
|
1309 | |
}
|
1310 | |
f.errorf(as, 0.8, category("slice"), `can probably use "var %s %s" instead`, f.render(as.Lhs[0]), f.render(at))
|
1311 | 1272 |
return true
|
1312 | 1273 |
})
|
1313 | 1274 |
}
|
|
1435 | 1396 |
})
|
1436 | 1397 |
}
|
1437 | 1398 |
|
|
1399 |
// lintContextKeyTypes checks for call expressions to context.WithValue with
|
|
1400 |
// basic types used for the key argument.
|
|
1401 |
// See: https://golang.org/issue/17293
|
|
1402 |
func (f *file) lintContextKeyTypes() {
|
|
1403 |
f.walk(func(node ast.Node) bool {
|
|
1404 |
switch node := node.(type) {
|
|
1405 |
case *ast.CallExpr:
|
|
1406 |
f.checkContextKeyType(node)
|
|
1407 |
}
|
|
1408 |
|
|
1409 |
return true
|
|
1410 |
})
|
|
1411 |
}
|
|
1412 |
|
|
1413 |
// checkContextKeyType reports an error if the call expression calls
|
|
1414 |
// context.WithValue with a key argument of basic type.
|
|
1415 |
func (f *file) checkContextKeyType(x *ast.CallExpr) {
|
|
1416 |
sel, ok := x.Fun.(*ast.SelectorExpr)
|
|
1417 |
if !ok {
|
|
1418 |
return
|
|
1419 |
}
|
|
1420 |
pkg, ok := sel.X.(*ast.Ident)
|
|
1421 |
if !ok || pkg.Name != "context" {
|
|
1422 |
return
|
|
1423 |
}
|
|
1424 |
if sel.Sel.Name != "WithValue" {
|
|
1425 |
return
|
|
1426 |
}
|
|
1427 |
|
|
1428 |
// key is second argument to context.WithValue
|
|
1429 |
if len(x.Args) != 3 {
|
|
1430 |
return
|
|
1431 |
}
|
|
1432 |
key := f.pkg.typesInfo.Types[x.Args[1]]
|
|
1433 |
|
|
1434 |
if _, ok := key.Type.(*types.Basic); ok {
|
|
1435 |
f.errorf(x, 1.0, category("context"), fmt.Sprintf("should not use basic type %s as key in context.WithValue", key.Type))
|
|
1436 |
}
|
|
1437 |
}
|
|
1438 |
|
|
1439 |
// lintContextArgs examines function declarations that contain an
|
|
1440 |
// argument with a type of context.Context
|
|
1441 |
// It complains if that argument isn't the first parameter.
|
|
1442 |
func (f *file) lintContextArgs() {
|
|
1443 |
f.walk(func(n ast.Node) bool {
|
|
1444 |
fn, ok := n.(*ast.FuncDecl)
|
|
1445 |
if !ok || len(fn.Type.Params.List) <= 1 {
|
|
1446 |
return true
|
|
1447 |
}
|
|
1448 |
// A context.Context should be the first parameter of a function.
|
|
1449 |
// Flag any that show up after the first.
|
|
1450 |
for _, arg := range fn.Type.Params.List[1:] {
|
|
1451 |
if isPkgDot(arg.Type, "context", "Context") {
|
|
1452 |
f.errorf(fn, 0.9, link("https://golang.org/pkg/context/"), category("arg-order"), "context.Context should be the first parameter of a function")
|
|
1453 |
break // only flag one
|
|
1454 |
}
|
|
1455 |
}
|
|
1456 |
return true
|
|
1457 |
})
|
|
1458 |
}
|
|
1459 |
|
|
1460 |
// receiverType returns the named type of the method receiver, sans "*",
|
|
1461 |
// or "invalid-type" if fn.Recv is ill formed.
|
1438 | 1462 |
func receiverType(fn *ast.FuncDecl) string {
|
1439 | 1463 |
switch e := fn.Recv.List[0].Type.(type) {
|
1440 | 1464 |
case *ast.Ident:
|
1441 | 1465 |
return e.Name
|
1442 | 1466 |
case *ast.StarExpr:
|
1443 | |
return e.X.(*ast.Ident).Name
|
1444 | |
}
|
1445 | |
panic(fmt.Sprintf("unknown method receiver AST node type %T", fn.Recv.List[0].Type))
|
|
1467 |
if id, ok := e.X.(*ast.Ident); ok {
|
|
1468 |
return id.Name
|
|
1469 |
}
|
|
1470 |
}
|
|
1471 |
// The parser accepts much more than just the legal forms.
|
|
1472 |
return "invalid-type"
|
1446 | 1473 |
}
|
1447 | 1474 |
|
1448 | 1475 |
func (f *file) walk(fn func(ast.Node) bool) {
|