1package stylecheck // import "honnef.co/go/tools/stylecheck" 2 3import ( 4 "fmt" 5 "go/ast" 6 "go/constant" 7 "go/token" 8 "go/types" 9 "strconv" 10 "strings" 11 "unicode" 12 "unicode/utf8" 13 14 "honnef.co/go/tools/config" 15 "honnef.co/go/tools/internal/passes/buildssa" 16 . "honnef.co/go/tools/lint/lintdsl" 17 "honnef.co/go/tools/ssa" 18 19 "golang.org/x/tools/go/analysis" 20 "golang.org/x/tools/go/analysis/passes/inspect" 21 "golang.org/x/tools/go/ast/inspector" 22 "golang.org/x/tools/go/types/typeutil" 23) 24 25func CheckPackageComment(pass *analysis.Pass) (interface{}, error) { 26 // - At least one file in a non-main package should have a package comment 27 // 28 // - The comment should be of the form 29 // "Package x ...". This has a slight potential for false 30 // positives, as multiple files can have package comments, in 31 // which case they get appended. But that doesn't happen a lot in 32 // the real world. 33 34 if pass.Pkg.Name() == "main" { 35 return nil, nil 36 } 37 hasDocs := false 38 for _, f := range pass.Files { 39 if IsInTest(pass, f) { 40 continue 41 } 42 if f.Doc != nil && len(f.Doc.List) > 0 { 43 hasDocs = true 44 prefix := "Package " + f.Name.Name + " " 45 if !strings.HasPrefix(strings.TrimSpace(f.Doc.Text()), prefix) { 46 ReportNodef(pass, f.Doc, `package comment should be of the form "%s..."`, prefix) 47 } 48 f.Doc.Text() 49 } 50 } 51 52 if !hasDocs { 53 for _, f := range pass.Files { 54 if IsInTest(pass, f) { 55 continue 56 } 57 ReportNodef(pass, f, "at least one file in a package should have a package comment") 58 } 59 } 60 return nil, nil 61} 62 63func CheckDotImports(pass *analysis.Pass) (interface{}, error) { 64 for _, f := range pass.Files { 65 imports: 66 for _, imp := range f.Imports { 67 path := imp.Path.Value 68 path = path[1 : len(path)-1] 69 for _, w := range config.For(pass).DotImportWhitelist { 70 if w == path { 71 continue imports 72 } 73 } 74 75 if imp.Name != nil && imp.Name.Name == "." && !IsInTest(pass, f) { 76 ReportNodefFG(pass, imp, "should not use dot imports") 77 } 78 } 79 } 80 return nil, nil 81} 82 83func CheckBlankImports(pass *analysis.Pass) (interface{}, error) { 84 fset := pass.Fset 85 for _, f := range pass.Files { 86 if IsInMain(pass, f) || IsInTest(pass, f) { 87 continue 88 } 89 90 // Collect imports of the form `import _ "foo"`, i.e. with no 91 // parentheses, as their comment will be associated with the 92 // (paren-free) GenDecl, not the import spec itself. 93 // 94 // We don't directly process the GenDecl so that we can 95 // correctly handle the following: 96 // 97 // import _ "foo" 98 // import _ "bar" 99 // 100 // where only the first import should get flagged. 101 skip := map[ast.Spec]bool{} 102 ast.Inspect(f, func(node ast.Node) bool { 103 switch node := node.(type) { 104 case *ast.File: 105 return true 106 case *ast.GenDecl: 107 if node.Tok != token.IMPORT { 108 return false 109 } 110 if node.Lparen == token.NoPos && node.Doc != nil { 111 skip[node.Specs[0]] = true 112 } 113 return false 114 } 115 return false 116 }) 117 for i, imp := range f.Imports { 118 pos := fset.Position(imp.Pos()) 119 120 if !IsBlank(imp.Name) { 121 continue 122 } 123 // Only flag the first blank import in a group of imports, 124 // or don't flag any of them, if the first one is 125 // commented 126 if i > 0 { 127 prev := f.Imports[i-1] 128 prevPos := fset.Position(prev.Pos()) 129 if pos.Line-1 == prevPos.Line && IsBlank(prev.Name) { 130 continue 131 } 132 } 133 134 if imp.Doc == nil && imp.Comment == nil && !skip[imp] { 135 ReportNodef(pass, imp, "a blank import should be only in a main or test package, or have a comment justifying it") 136 } 137 } 138 } 139 return nil, nil 140} 141 142func CheckIncDec(pass *analysis.Pass) (interface{}, error) { 143 // TODO(dh): this can be noisy for function bodies that look like this: 144 // x += 3 145 // ... 146 // x += 2 147 // ... 148 // x += 1 149 fn := func(node ast.Node) { 150 assign := node.(*ast.AssignStmt) 151 if assign.Tok != token.ADD_ASSIGN && assign.Tok != token.SUB_ASSIGN { 152 return 153 } 154 if (len(assign.Lhs) != 1 || len(assign.Rhs) != 1) || 155 !IsIntLiteral(assign.Rhs[0], "1") { 156 return 157 } 158 159 suffix := "" 160 switch assign.Tok { 161 case token.ADD_ASSIGN: 162 suffix = "++" 163 case token.SUB_ASSIGN: 164 suffix = "--" 165 } 166 167 ReportNodef(pass, assign, "should replace %s with %s%s", Render(pass, assign), Render(pass, assign.Lhs[0]), suffix) 168 } 169 pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Preorder([]ast.Node{(*ast.AssignStmt)(nil)}, fn) 170 return nil, nil 171} 172 173func CheckErrorReturn(pass *analysis.Pass) (interface{}, error) { 174fnLoop: 175 for _, fn := range pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs { 176 sig := fn.Type().(*types.Signature) 177 rets := sig.Results() 178 if rets == nil || rets.Len() < 2 { 179 continue 180 } 181 182 if rets.At(rets.Len()-1).Type() == types.Universe.Lookup("error").Type() { 183 // Last return type is error. If the function also returns 184 // errors in other positions, that's fine. 185 continue 186 } 187 for i := rets.Len() - 2; i >= 0; i-- { 188 if rets.At(i).Type() == types.Universe.Lookup("error").Type() { 189 pass.Reportf(rets.At(i).Pos(), "error should be returned as the last argument") 190 continue fnLoop 191 } 192 } 193 } 194 return nil, nil 195} 196 197// CheckUnexportedReturn checks that exported functions on exported 198// types do not return unexported types. 199func CheckUnexportedReturn(pass *analysis.Pass) (interface{}, error) { 200 for _, fn := range pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs { 201 if fn.Synthetic != "" || fn.Parent() != nil { 202 continue 203 } 204 if !ast.IsExported(fn.Name()) || IsInMain(pass, fn) || IsInTest(pass, fn) { 205 continue 206 } 207 sig := fn.Type().(*types.Signature) 208 if sig.Recv() != nil && !ast.IsExported(Dereference(sig.Recv().Type()).(*types.Named).Obj().Name()) { 209 continue 210 } 211 res := sig.Results() 212 for i := 0; i < res.Len(); i++ { 213 if named, ok := DereferenceR(res.At(i).Type()).(*types.Named); ok && 214 !ast.IsExported(named.Obj().Name()) && 215 named != types.Universe.Lookup("error").Type() { 216 pass.Reportf(fn.Pos(), "should not return unexported type") 217 } 218 } 219 } 220 return nil, nil 221} 222 223func CheckReceiverNames(pass *analysis.Pass) (interface{}, error) { 224 ssapkg := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).Pkg 225 for _, m := range ssapkg.Members { 226 if T, ok := m.Object().(*types.TypeName); ok && !T.IsAlias() { 227 ms := typeutil.IntuitiveMethodSet(T.Type(), nil) 228 for _, sel := range ms { 229 fn := sel.Obj().(*types.Func) 230 recv := fn.Type().(*types.Signature).Recv() 231 if Dereference(recv.Type()) != T.Type() { 232 // skip embedded methods 233 continue 234 } 235 if recv.Name() == "self" || recv.Name() == "this" { 236 ReportfFG(pass, recv.Pos(), `receiver name should be a reflection of its identity; don't use generic names such as "this" or "self"`) 237 } 238 if recv.Name() == "_" { 239 ReportfFG(pass, recv.Pos(), "receiver name should not be an underscore, omit the name if it is unused") 240 } 241 } 242 } 243 } 244 return nil, nil 245} 246 247func CheckReceiverNamesIdentical(pass *analysis.Pass) (interface{}, error) { 248 ssapkg := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).Pkg 249 for _, m := range ssapkg.Members { 250 names := map[string]int{} 251 252 var firstFn *types.Func 253 if T, ok := m.Object().(*types.TypeName); ok && !T.IsAlias() { 254 ms := typeutil.IntuitiveMethodSet(T.Type(), nil) 255 for _, sel := range ms { 256 fn := sel.Obj().(*types.Func) 257 recv := fn.Type().(*types.Signature).Recv() 258 if Dereference(recv.Type()) != T.Type() { 259 // skip embedded methods 260 continue 261 } 262 if firstFn == nil { 263 firstFn = fn 264 } 265 if recv.Name() != "" && recv.Name() != "_" { 266 names[recv.Name()]++ 267 } 268 } 269 } 270 271 if len(names) > 1 { 272 var seen []string 273 for name, count := range names { 274 seen = append(seen, fmt.Sprintf("%dx %q", count, name)) 275 } 276 277 pass.Reportf(firstFn.Pos(), "methods on the same type should have the same receiver name (seen %s)", strings.Join(seen, ", ")) 278 } 279 } 280 return nil, nil 281} 282 283func CheckContextFirstArg(pass *analysis.Pass) (interface{}, error) { 284 // TODO(dh): this check doesn't apply to test helpers. Example from the stdlib: 285 // func helperCommandContext(t *testing.T, ctx context.Context, s ...string) (cmd *exec.Cmd) { 286fnLoop: 287 for _, fn := range pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs { 288 if fn.Synthetic != "" || fn.Parent() != nil { 289 continue 290 } 291 params := fn.Signature.Params() 292 if params.Len() < 2 { 293 continue 294 } 295 if types.TypeString(params.At(0).Type(), nil) == "context.Context" { 296 continue 297 } 298 for i := 1; i < params.Len(); i++ { 299 param := params.At(i) 300 if types.TypeString(param.Type(), nil) == "context.Context" { 301 pass.Reportf(param.Pos(), "context.Context should be the first argument of a function") 302 continue fnLoop 303 } 304 } 305 } 306 return nil, nil 307} 308 309func CheckErrorStrings(pass *analysis.Pass) (interface{}, error) { 310 objNames := map[*ssa.Package]map[string]bool{} 311 ssapkg := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).Pkg 312 objNames[ssapkg] = map[string]bool{} 313 for _, m := range ssapkg.Members { 314 if typ, ok := m.(*ssa.Type); ok { 315 objNames[ssapkg][typ.Name()] = true 316 } 317 } 318 for _, fn := range pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs { 319 objNames[fn.Package()][fn.Name()] = true 320 } 321 322 for _, fn := range pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs { 323 if IsInTest(pass, fn) { 324 // We don't care about malformed error messages in tests; 325 // they're usually for direct human consumption, not part 326 // of an API 327 continue 328 } 329 for _, block := range fn.Blocks { 330 instrLoop: 331 for _, ins := range block.Instrs { 332 call, ok := ins.(*ssa.Call) 333 if !ok { 334 continue 335 } 336 if !IsCallTo(call.Common(), "errors.New") && !IsCallTo(call.Common(), "fmt.Errorf") { 337 continue 338 } 339 340 k, ok := call.Common().Args[0].(*ssa.Const) 341 if !ok { 342 continue 343 } 344 345 s := constant.StringVal(k.Value) 346 if len(s) == 0 { 347 continue 348 } 349 switch s[len(s)-1] { 350 case '.', ':', '!', '\n': 351 pass.Reportf(call.Pos(), "error strings should not end with punctuation or a newline") 352 } 353 idx := strings.IndexByte(s, ' ') 354 if idx == -1 { 355 // single word error message, probably not a real 356 // error but something used in tests or during 357 // debugging 358 continue 359 } 360 word := s[:idx] 361 first, n := utf8.DecodeRuneInString(word) 362 if !unicode.IsUpper(first) { 363 continue 364 } 365 for _, c := range word[n:] { 366 if unicode.IsUpper(c) { 367 // Word is probably an initialism or 368 // multi-word function name 369 continue instrLoop 370 } 371 } 372 373 word = strings.TrimRightFunc(word, func(r rune) bool { return unicode.IsPunct(r) }) 374 if objNames[fn.Package()][word] { 375 // Word is probably the name of a function or type in this package 376 continue 377 } 378 // First word in error starts with a capital 379 // letter, and the word doesn't contain any other 380 // capitals, making it unlikely to be an 381 // initialism or multi-word function name. 382 // 383 // It could still be a proper noun, though. 384 385 pass.Reportf(call.Pos(), "error strings should not be capitalized") 386 } 387 } 388 } 389 return nil, nil 390} 391 392func CheckTimeNames(pass *analysis.Pass) (interface{}, error) { 393 suffixes := []string{ 394 "Sec", "Secs", "Seconds", 395 "Msec", "Msecs", 396 "Milli", "Millis", "Milliseconds", 397 "Usec", "Usecs", "Microseconds", 398 "MS", "Ms", 399 } 400 fn := func(T types.Type, names []*ast.Ident) { 401 if !IsType(T, "time.Duration") && !IsType(T, "*time.Duration") { 402 return 403 } 404 for _, name := range names { 405 for _, suffix := range suffixes { 406 if strings.HasSuffix(name.Name, suffix) { 407 ReportNodef(pass, name, "var %s is of type %v; don't use unit-specific suffix %q", name.Name, T, suffix) 408 break 409 } 410 } 411 } 412 } 413 for _, f := range pass.Files { 414 ast.Inspect(f, func(node ast.Node) bool { 415 switch node := node.(type) { 416 case *ast.ValueSpec: 417 T := pass.TypesInfo.TypeOf(node.Type) 418 fn(T, node.Names) 419 case *ast.FieldList: 420 for _, field := range node.List { 421 T := pass.TypesInfo.TypeOf(field.Type) 422 fn(T, field.Names) 423 } 424 } 425 return true 426 }) 427 } 428 return nil, nil 429} 430 431func CheckErrorVarNames(pass *analysis.Pass) (interface{}, error) { 432 for _, f := range pass.Files { 433 for _, decl := range f.Decls { 434 gen, ok := decl.(*ast.GenDecl) 435 if !ok || gen.Tok != token.VAR { 436 continue 437 } 438 for _, spec := range gen.Specs { 439 spec := spec.(*ast.ValueSpec) 440 if len(spec.Names) != len(spec.Values) { 441 continue 442 } 443 444 for i, name := range spec.Names { 445 val := spec.Values[i] 446 if !IsCallToAST(pass, val, "errors.New") && !IsCallToAST(pass, val, "fmt.Errorf") { 447 continue 448 } 449 450 prefix := "err" 451 if name.IsExported() { 452 prefix = "Err" 453 } 454 if !strings.HasPrefix(name.Name, prefix) { 455 ReportNodef(pass, name, "error var %s should have name of the form %sFoo", name.Name, prefix) 456 } 457 } 458 } 459 } 460 } 461 return nil, nil 462} 463 464var httpStatusCodes = map[int]string{ 465 100: "StatusContinue", 466 101: "StatusSwitchingProtocols", 467 102: "StatusProcessing", 468 200: "StatusOK", 469 201: "StatusCreated", 470 202: "StatusAccepted", 471 203: "StatusNonAuthoritativeInfo", 472 204: "StatusNoContent", 473 205: "StatusResetContent", 474 206: "StatusPartialContent", 475 207: "StatusMultiStatus", 476 208: "StatusAlreadyReported", 477 226: "StatusIMUsed", 478 300: "StatusMultipleChoices", 479 301: "StatusMovedPermanently", 480 302: "StatusFound", 481 303: "StatusSeeOther", 482 304: "StatusNotModified", 483 305: "StatusUseProxy", 484 307: "StatusTemporaryRedirect", 485 308: "StatusPermanentRedirect", 486 400: "StatusBadRequest", 487 401: "StatusUnauthorized", 488 402: "StatusPaymentRequired", 489 403: "StatusForbidden", 490 404: "StatusNotFound", 491 405: "StatusMethodNotAllowed", 492 406: "StatusNotAcceptable", 493 407: "StatusProxyAuthRequired", 494 408: "StatusRequestTimeout", 495 409: "StatusConflict", 496 410: "StatusGone", 497 411: "StatusLengthRequired", 498 412: "StatusPreconditionFailed", 499 413: "StatusRequestEntityTooLarge", 500 414: "StatusRequestURITooLong", 501 415: "StatusUnsupportedMediaType", 502 416: "StatusRequestedRangeNotSatisfiable", 503 417: "StatusExpectationFailed", 504 418: "StatusTeapot", 505 422: "StatusUnprocessableEntity", 506 423: "StatusLocked", 507 424: "StatusFailedDependency", 508 426: "StatusUpgradeRequired", 509 428: "StatusPreconditionRequired", 510 429: "StatusTooManyRequests", 511 431: "StatusRequestHeaderFieldsTooLarge", 512 451: "StatusUnavailableForLegalReasons", 513 500: "StatusInternalServerError", 514 501: "StatusNotImplemented", 515 502: "StatusBadGateway", 516 503: "StatusServiceUnavailable", 517 504: "StatusGatewayTimeout", 518 505: "StatusHTTPVersionNotSupported", 519 506: "StatusVariantAlsoNegotiates", 520 507: "StatusInsufficientStorage", 521 508: "StatusLoopDetected", 522 510: "StatusNotExtended", 523 511: "StatusNetworkAuthenticationRequired", 524} 525 526func CheckHTTPStatusCodes(pass *analysis.Pass) (interface{}, error) { 527 whitelist := map[string]bool{} 528 for _, code := range config.For(pass).HTTPStatusCodeWhitelist { 529 whitelist[code] = true 530 } 531 fn := func(node ast.Node) bool { 532 if node == nil { 533 return true 534 } 535 call, ok := node.(*ast.CallExpr) 536 if !ok { 537 return true 538 } 539 540 var arg int 541 switch CallNameAST(pass, call) { 542 case "net/http.Error": 543 arg = 2 544 case "net/http.Redirect": 545 arg = 3 546 case "net/http.StatusText": 547 arg = 0 548 case "net/http.RedirectHandler": 549 arg = 1 550 default: 551 return true 552 } 553 lit, ok := call.Args[arg].(*ast.BasicLit) 554 if !ok { 555 return true 556 } 557 if whitelist[lit.Value] { 558 return true 559 } 560 561 n, err := strconv.Atoi(lit.Value) 562 if err != nil { 563 return true 564 } 565 s, ok := httpStatusCodes[n] 566 if !ok { 567 return true 568 } 569 ReportNodefFG(pass, lit, "should use constant http.%s instead of numeric literal %d", s, n) 570 return true 571 } 572 // OPT(dh): replace with inspector 573 for _, f := range pass.Files { 574 ast.Inspect(f, fn) 575 } 576 return nil, nil 577} 578 579func CheckDefaultCaseOrder(pass *analysis.Pass) (interface{}, error) { 580 fn := func(node ast.Node) { 581 stmt := node.(*ast.SwitchStmt) 582 list := stmt.Body.List 583 for i, c := range list { 584 if c.(*ast.CaseClause).List == nil && i != 0 && i != len(list)-1 { 585 ReportNodefFG(pass, c, "default case should be first or last in switch statement") 586 break 587 } 588 } 589 } 590 pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Preorder([]ast.Node{(*ast.SwitchStmt)(nil)}, fn) 591 return nil, nil 592} 593 594func CheckYodaConditions(pass *analysis.Pass) (interface{}, error) { 595 fn := func(node ast.Node) { 596 cond := node.(*ast.BinaryExpr) 597 if cond.Op != token.EQL && cond.Op != token.NEQ { 598 return 599 } 600 if _, ok := cond.X.(*ast.BasicLit); !ok { 601 return 602 } 603 if _, ok := cond.Y.(*ast.BasicLit); ok { 604 // Don't flag lit == lit conditions, just in case 605 return 606 } 607 ReportNodefFG(pass, cond, "don't use Yoda conditions") 608 } 609 pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Preorder([]ast.Node{(*ast.BinaryExpr)(nil)}, fn) 610 return nil, nil 611} 612 613func CheckInvisibleCharacters(pass *analysis.Pass) (interface{}, error) { 614 fn := func(node ast.Node) { 615 lit := node.(*ast.BasicLit) 616 if lit.Kind != token.STRING { 617 return 618 } 619 for _, r := range lit.Value { 620 if unicode.Is(unicode.Cf, r) { 621 ReportNodef(pass, lit, "string literal contains the Unicode format character %U, consider using the %q escape sequence", r, r) 622 } else if unicode.Is(unicode.Cc, r) && r != '\n' && r != '\t' && r != '\r' { 623 ReportNodef(pass, lit, "string literal contains the Unicode control character %U, consider using the %q escape sequence", r, r) 624 } 625 } 626 } 627 pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Preorder([]ast.Node{(*ast.BasicLit)(nil)}, fn) 628 return nil, nil 629} 630