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