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  }