blob: 11f6ed701f096217869124f90af07156c859d72b [file] [log] [blame]
// Copyright (c) 2013 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 or at
// https://developers.google.com/open-source/licenses/bsd.
// Package lint contains a linter for Go source code.
package lint
import (
"bytes"
"fmt"
"go/ast"
"go/parser"
"go/printer"
"go/token"
"regexp"
"sort"
"strconv"
"strings"
"unicode"
"unicode/utf8"
"golang.org/x/tools/go/exact"
"golang.org/x/tools/go/gcimporter"
"golang.org/x/tools/go/types"
)
const styleGuideBase = "https://golang.org/wiki/CodeReviewComments"
// A Linter lints Go source code.
type Linter struct {
}
// Problem represents a problem in some source code.
type Problem struct {
Position token.Position // position in source file
Text string // the prose that describes the problem
Link string // (optional) the link to the style guide for the problem
Confidence float64 // a value in (0,1] estimating the confidence in this problem's correctness
LineText string // the source line
Category string // a short name for the general category of the problem
// If the problem has a suggested fix (the minority case),
// ReplacementLine is a full replacement for the relevant line of the source file.
ReplacementLine string
}
func (p *Problem) String() string {
if p.Link != "" {
return p.Text + "\n\n" + p.Link
}
return p.Text
}
type byPosition []Problem
func (p byPosition) Len() int { return len(p) }
func (p byPosition) Swap(i, j int) { p[i], p[j] = p[j], p[i] }
func (p byPosition) Less(i, j int) bool {
pi, pj := p[i].Position, p[j].Position
if pi.Filename != pj.Filename {
return pi.Filename < pj.Filename
}
if pi.Line != pj.Line {
return pi.Line < pj.Line
}
if pi.Column != pj.Column {
return pi.Column < pj.Column
}
return p[i].Text < p[j].Text
}
// Lint lints src.
func (l *Linter) Lint(filename string, src []byte) ([]Problem, error) {
return l.LintFiles(map[string][]byte{filename: src})
}
// LintFiles lints a set of files of a single package.
// The argument is a map of filename to source.
func (l *Linter) LintFiles(files map[string][]byte) ([]Problem, error) {
if len(files) == 0 {
return nil, nil
}
pkg := &pkg{
fset: token.NewFileSet(),
files: make(map[string]*file),
}
var pkgName string
for filename, src := range files {
f, err := parser.ParseFile(pkg.fset, filename, src, parser.ParseComments)
if err != nil {
return nil, err
}
if pkgName == "" {
pkgName = f.Name.Name
} else if f.Name.Name != pkgName {
return nil, fmt.Errorf("%s is in package %s, not %s", filename, f.Name.Name, pkgName)
}
pkg.files[filename] = &file{
pkg: pkg,
f: f,
fset: pkg.fset,
src: src,
filename: filename,
}
}
return pkg.lint(), nil
}
// pkg represents a package being linted.
type pkg struct {
fset *token.FileSet
files map[string]*file
typesPkg *types.Package
typesInfo *types.Info
// sortable is the set of types in the package that implement sort.Interface.
sortable map[string]bool
// main is whether this is a "main" package.
main bool
problems []Problem
}
func (p *pkg) lint() []Problem {
if err := p.typeCheck(); err != nil {
/* TODO(dsymonds): Consider reporting these errors when golint operates on entire packages.
if e, ok := err.(types.Error); ok {
pos := p.fset.Position(e.Pos)
conf := 1.0
if strings.Contains(e.Msg, "can't find import: ") {
// Golint is probably being run in a context that doesn't support
// typechecking (e.g. package files aren't found), so don't warn about it.
conf = 0
}
if conf > 0 {
p.errorfAt(pos, conf, category("typechecking"), e.Msg)
}
// TODO(dsymonds): Abort if !e.Soft?
}
*/
}
p.scanSortable()
p.main = p.isMain()
for _, f := range p.files {
f.lint()
}
sort.Sort(byPosition(p.problems))
return p.problems
}
// file represents a file being linted.
type file struct {
pkg *pkg
f *ast.File
fset *token.FileSet
src []byte
filename string
}
func (f *file) isTest() bool { return strings.HasSuffix(f.filename, "_test.go") }
func (f *file) lint() {
f.lintPackageComment()
f.lintImports()
f.lintBlankImports()
f.lintExported()
f.lintNames()
f.lintVarDecls()
f.lintElses()
f.lintRanges()
f.lintErrorf()
f.lintErrors()
f.lintErrorStrings()
f.lintReceiverNames()
f.lintIncDec()
f.lintMake()
f.lintErrorReturn()
f.lintUnexportedReturn()
f.lintTimeNames()
f.lintDurationUnits()
}
type link string
type category string
// The variadic arguments may start with link and category types,
// and must end with a format string and any arguments.
// It returns the new Problem.
func (f *file) errorf(n ast.Node, confidence float64, args ...interface{}) *Problem {
pos := f.fset.Position(n.Pos())
if pos.Filename == "" {
pos.Filename = f.filename
}
return f.pkg.errorfAt(pos, confidence, args...)
}
func (p *pkg) errorfAt(pos token.Position, confidence float64, args ...interface{}) *Problem {
problem := Problem{
Position: pos,
Confidence: confidence,
}
if pos.Filename != "" {
// The file might not exist in our mapping if a //line directive was encountered.
if f, ok := p.files[pos.Filename]; ok {
problem.LineText = srcLine(f.src, pos)
}
}
argLoop:
for len(args) > 1 { // always leave at least the format string in args
switch v := args[0].(type) {
case link:
problem.Link = string(v)
case category:
problem.Category = string(v)
default:
break argLoop
}
args = args[1:]
}
problem.Text = fmt.Sprintf(args[0].(string), args[1:]...)
p.problems = append(p.problems, problem)
return &p.problems[len(p.problems)-1]
}
var gcImporter = gcimporter.Import
func (p *pkg) typeCheck() error {
config := &types.Config{
// By setting a no-op error reporter, the type checker does as much work as possible.
Error: func(error) {},
Import: gcImporter,
}
info := &types.Info{
Types: make(map[ast.Expr]types.TypeAndValue),
Defs: make(map[*ast.Ident]types.Object),
Uses: make(map[*ast.Ident]types.Object),
Scopes: make(map[ast.Node]*types.Scope),
}
var anyFile *file
var astFiles []*ast.File
for _, f := range p.files {
anyFile = f
astFiles = append(astFiles, f.f)
}
pkg, err := config.Check(anyFile.f.Name.Name, p.fset, astFiles, info)
// Remember the typechecking info, even if config.Check failed,
// since we will get partial information.
p.typesPkg = pkg
p.typesInfo = info
return err
}
func (p *pkg) typeOf(expr ast.Expr) types.Type {
if p.typesInfo == nil {
return nil
}
return p.typesInfo.TypeOf(expr)
}
func (p *pkg) isNamedType(typ types.Type, importPath, name string) bool {
n, ok := typ.(*types.Named)
if !ok {
return false
}
tn := n.Obj()
return tn != nil && tn.Pkg() != nil && tn.Pkg().Path() == importPath && tn.Name() == name
}
// scopeOf returns the tightest scope encompassing id.
func (p *pkg) scopeOf(id *ast.Ident) *types.Scope {
var scope *types.Scope
if obj := p.typesInfo.ObjectOf(id); obj != nil {
scope = obj.Parent()
}
if scope == p.typesPkg.Scope() {
// We were given a top-level identifier.
// Use the file-level scope instead of the package-level scope.
pos := id.Pos()
for _, f := range p.files {
if f.f.Pos() <= pos && pos < f.f.End() {
scope = p.typesInfo.Scopes[f.f]
break
}
}
}
return scope
}
func (p *pkg) scanSortable() {
p.sortable = make(map[string]bool)
// bitfield for which methods exist on each type.
const (
Len = 1 << iota
Less
Swap
)
nmap := map[string]int{"Len": Len, "Less": Less, "Swap": Swap}
has := make(map[string]int)
for _, f := range p.files {
f.walk(func(n ast.Node) bool {
fn, ok := n.(*ast.FuncDecl)
if !ok || fn.Recv == nil || len(fn.Recv.List) == 0 {
return true
}
// TODO(dsymonds): We could check the signature to be more precise.
recv := receiverType(fn)
if i, ok := nmap[fn.Name.Name]; ok {
has[recv] |= i
}
return false
})
}
for typ, ms := range has {
if ms == Len|Less|Swap {
p.sortable[typ] = true
}
}
}
func (p *pkg) isMain() bool {
for _, f := range p.files {
if f.isMain() {
return true
}
}
return false
}
func (f *file) isMain() bool {
if f.f.Name.Name == "main" {
return true
}
return false
}
// lintPackageComment checks package comments. It complains if
// there is no package comment, or if it is not of the right form.
// This has a notable false positive in that a package comment
// could rightfully appear in a different file of the same package,
// but that's not easy to fix since this linter is file-oriented.
func (f *file) lintPackageComment() {
if f.isTest() {
return
}
const ref = styleGuideBase + "#package-comments"
prefix := "Package " + f.f.Name.Name + " "
// Look for a detached package comment.
// First, scan for the last comment that occurs before the "package" keyword.
var lastCG *ast.CommentGroup
for _, cg := range f.f.Comments {
if cg.Pos() > f.f.Package {
// Gone past "package" keyword.
break
}
lastCG = cg
}
if lastCG != nil && strings.HasPrefix(lastCG.Text(), prefix) {
endPos := f.fset.Position(lastCG.End())
pkgPos := f.fset.Position(f.f.Package)
if endPos.Line+1 < pkgPos.Line {
// There isn't a great place to anchor this error;
// the start of the blank lines between the doc and the package statement
// is at least pointing at the location of the problem.
pos := token.Position{
Filename: endPos.Filename,
// Offset not set; it is non-trivial, and doesn't appear to be needed.
Line: endPos.Line + 1,
Column: 1,
}
f.pkg.errorfAt(pos, 0.9, link(ref), category("comments"), "package comment is detached; there should be no blank lines between it and the package statement")
return
}
}
if f.f.Doc == nil {
f.errorf(f.f, 0.2, link(ref), category("comments"), "should have a package comment, unless it's in another file for this package")
return
}
s := f.f.Doc.Text()
if ts := strings.TrimLeft(s, " \t"); ts != s {
f.errorf(f.f.Doc, 1, link(ref), category("comments"), "package comment should not have leading space")
s = ts
}
// Only non-main packages need to keep to this form.
if f.f.Name.Name != "main" && !strings.HasPrefix(s, prefix) {
f.errorf(f.f.Doc, 1, link(ref), category("comments"), `package comment should be of the form "%s..."`, prefix)
}
}
// lintBlankImports complains if a non-main package has blank imports that are
// not documented.
func (f *file) lintBlankImports() {
// In package main and in tests, we don't complain about blank imports.
if f.pkg.main || f.isTest() {
return
}
// The first element of each contiguous group of blank imports should have
// an explanatory comment of some kind.
for i, imp := range f.f.Imports {
pos := f.fset.Position(imp.Pos())
if !isBlank(imp.Name) {
continue // Ignore non-blank imports.
}
if i > 0 {
prev := f.f.Imports[i-1]
prevPos := f.fset.Position(prev.Pos())
if isBlank(prev.Name) && prevPos.Line+1 == pos.Line {
continue // A subsequent blank in a group.
}
}
// This is the first blank import of a group.
if imp.Doc == nil && imp.Comment == nil {
ref := ""
f.errorf(imp, 1, link(ref), category("imports"), "a blank import should be only in a main or test package, or have a comment justifying it")
}
}
}
// lintImports examines import blocks.
func (f *file) lintImports() {
for i, is := range f.f.Imports {
_ = i
if is.Name != nil && is.Name.Name == "." && !f.isTest() {
f.errorf(is, 1, link(styleGuideBase+"#import-dot"), category("imports"), "should not use dot imports")
}
}
}
const docCommentsLink = styleGuideBase + "#doc-comments"
// lintExported examines the exported names.
// It complains if any required doc comments are missing,
// or if they are not of the right form. The exact rules are in
// lintFuncDoc, lintTypeDoc and lintValueSpecDoc; this function
// also tracks the GenDecl structure being traversed to permit
// doc comments for constants to be on top of the const block.
// It also complains if the names stutter when combined with
// the package name.
func (f *file) lintExported() {
if f.isTest() {
return
}
var lastGen *ast.GenDecl // last GenDecl entered.
// Set of GenDecls that have already had missing comments flagged.
genDeclMissingComments := make(map[*ast.GenDecl]bool)
f.walk(func(node ast.Node) bool {
switch v := node.(type) {
case *ast.GenDecl:
if v.Tok == token.IMPORT {
return false
}
// token.CONST, token.TYPE or token.VAR
lastGen = v
return true
case *ast.FuncDecl:
f.lintFuncDoc(v)
if v.Recv == nil {
// Only check for stutter on functions, not methods.
// Method names are not used package-qualified.
f.checkStutter(v.Name, "func")
}
// Don't proceed inside funcs.
return false
case *ast.TypeSpec:
// inside a GenDecl, which usually has the doc
doc := v.Doc
if doc == nil {
doc = lastGen.Doc
}
f.lintTypeDoc(v, doc)
f.checkStutter(v.Name, "type")
// Don't proceed inside types.
return false
case *ast.ValueSpec:
f.lintValueSpecDoc(v, lastGen, genDeclMissingComments)
return false
}
return true
})
}
var allCapsRE = regexp.MustCompile(`^[A-Z0-9_]+$`)
// lintNames examines all names in the file.
// It complains if any use underscores or incorrect known initialisms.
func (f *file) lintNames() {
// Package names need slightly different handling than other names.
if strings.Contains(f.f.Name.Name, "_") && !strings.HasSuffix(f.f.Name.Name, "_test") {
f.errorf(f.f, 1, link("http://golang.org/doc/effective_go.html#package-names"), category("naming"), "don't use an underscore in package name")
}
check := func(id *ast.Ident, thing string) {
if id.Name == "_" {
return
}
// Handle two common styles from other languages that don't belong in Go.
if len(id.Name) >= 5 && allCapsRE.MatchString(id.Name) && strings.Contains(id.Name, "_") {
f.errorf(id, 0.8, link(styleGuideBase+"#mixed-caps"), category("naming"), "don't use ALL_CAPS in Go names; use CamelCase")
return
}
if len(id.Name) > 2 && id.Name[0] == 'k' && id.Name[1] >= 'A' && id.Name[1] <= 'Z' {
should := string(id.Name[1]+'a'-'A') + id.Name[2:]
f.errorf(id, 0.8, link(styleGuideBase+"#mixed-caps"), category("naming"), "don't use leading k in Go names; %s %s should be %s", thing, id.Name, should)
}
should := lintName(id.Name)
if id.Name == should {
return
}
if len(id.Name) > 2 && strings.Contains(id.Name[1:], "_") {
f.errorf(id, 0.9, link("http://golang.org/doc/effective_go.html#mixed-caps"), category("naming"), "don't use underscores in Go names; %s %s should be %s", thing, id.Name, should)
return
}
f.errorf(id, 0.8, link(styleGuideBase+"#initialisms"), category("naming"), "%s %s should be %s", thing, id.Name, should)
}
checkList := func(fl *ast.FieldList, thing string) {
if fl == nil {
return
}
for _, f := range fl.List {
for _, id := range f.Names {
check(id, thing)
}
}
}
f.walk(func(node ast.Node) bool {
switch v := node.(type) {
case *ast.AssignStmt:
if v.Tok == token.ASSIGN {
return true
}
for _, exp := range v.Lhs {
if id, ok := exp.(*ast.Ident); ok {
check(id, "var")
}
}
case *ast.FuncDecl:
if f.isTest() && (strings.HasPrefix(v.Name.Name, "Example") || strings.HasPrefix(v.Name.Name, "Test") || strings.HasPrefix(v.Name.Name, "Benchmark")) {
return true
}
check(v.Name, "func")
thing := "func"
if v.Recv != nil {
thing = "method"
}
checkList(v.Type.Params, thing+" parameter")
checkList(v.Type.Results, thing+" result")
case *ast.GenDecl:
if v.Tok == token.IMPORT {
return true
}
var thing string
switch v.Tok {
case token.CONST:
thing = "const"
case token.TYPE:
thing = "type"
case token.VAR:
thing = "var"
}
for _, spec := range v.Specs {
switch s := spec.(type) {
case *ast.TypeSpec:
check(s.Name, thing)
case *ast.ValueSpec:
for _, id := range s.Names {
check(id, thing)
}
}
}
case *ast.InterfaceType:
// Do not check interface method names.
// They are often constrainted by the method names of concrete types.
for _, x := range v.Methods.List {
ft, ok := x.Type.(*ast.FuncType)
if !ok { // might be an embedded interface name
continue
}
checkList(ft.Params, "interface method parameter")
checkList(ft.Results, "interface method result")
}
case *ast.RangeStmt:
if v.Tok == token.ASSIGN {
return true
}
if id, ok := v.Key.(*ast.Ident); ok {
check(id, "range var")
}
if id, ok := v.Value.(*ast.Ident); ok {
check(id, "range var")
}
case *ast.StructType:
for _, f := range v.Fields.List {
for _, id := range f.Names {
check(id, "struct field")
}
}
}
return true
})
}
// lintName returns a different name if it should be different.
func lintName(name string) (should string) {
// Fast path for simple cases: "_" and all lowercase.
if name == "_" {
return name
}
allLower := true
for _, r := range name {
if !unicode.IsLower(r) {
allLower = false
break
}
}
if allLower {
return name
}
// Split camelCase at any lower->upper transition, and split on underscores.
// Check each word for common initialisms.
runes := []rune(name)
w, i := 0, 0 // index of start of word, scan
for i+1 <= len(runes) {
eow := false // whether we hit the end of a word
if i+1 == len(runes) {
eow = true
} else if runes[i+1] == '_' {
// underscore; shift the remainder forward over any run of underscores
eow = true
n := 1
for i+n+1 < len(runes) && runes[i+n+1] == '_' {
n++
}
// Leave at most one underscore if the underscore is between two digits
if i+n+1 < len(runes) && unicode.IsDigit(runes[i]) && unicode.IsDigit(runes[i+n+1]) {
n--
}
copy(runes[i+1:], runes[i+n+1:])
runes = runes[:len(runes)-n]
} else if unicode.IsLower(runes[i]) && !unicode.IsLower(runes[i+1]) {
// lower->non-lower
eow = true
}
i++
if !eow {
continue
}
// [w,i) is a word.
word := string(runes[w:i])
if u := strings.ToUpper(word); commonInitialisms[u] {
// Keep consistent case, which is lowercase only at the start.
if w == 0 && unicode.IsLower(runes[w]) {
u = strings.ToLower(u)
}
// All the common initialisms are ASCII,
// so we can replace the bytes exactly.
copy(runes[w:], []rune(u))
} else if w > 0 && strings.ToLower(word) == word {
// already all lowercase, and not the first word, so uppercase the first character.
runes[w] = unicode.ToUpper(runes[w])
}
w = i
}
return string(runes)
}
// commonInitialisms is a set of common initialisms.
// Only add entries that are highly unlikely to be non-initialisms.
// For instance, "ID" is fine (Freudian code is rare), but "AND" is not.
var commonInitialisms = map[string]bool{
"API": true,
"ASCII": true,
"CPU": true,
"CSS": true,
"DNS": true,
"EOF": true,
"GUID": true,
"HTML": true,
"HTTP": true,
"HTTPS": true,
"ID": true,
"IP": true,
"JSON": true,
"LHS": true,
"QPS": true,
"RAM": true,
"RHS": true,
"RPC": true,
"SLA": true,
"SMTP": true,
"SQL": true,
"SSH": true,
"TCP": true,
"TLS": true,
"TTL": true,
"UDP": true,
"UI": true,
"UID": true,
"UUID": true,
"URI": true,
"URL": true,
"UTF8": true,
"VM": true,
"XML": true,
"XSRF": true,
"XSS": true,
}
// lintTypeDoc examines the doc comment on a type.
// It complains if they are missing from an exported type,
// or if they are not of the standard form.
func (f *file) lintTypeDoc(t *ast.TypeSpec, doc *ast.CommentGroup) {
if !ast.IsExported(t.Name.Name) {
return
}
if doc == nil {
f.errorf(t, 1, link(docCommentsLink), category("comments"), "exported type %v should have comment or be unexported", t.Name)
return
}
s := doc.Text()
articles := [...]string{"A", "An", "The"}
for _, a := range articles {
if strings.HasPrefix(s, a+" ") {
s = s[len(a)+1:]
break
}
}
if !strings.HasPrefix(s, t.Name.Name+" ") {
f.errorf(doc, 1, link(docCommentsLink), category("comments"), `comment on exported type %v should be of the form "%v ..." (with optional leading article)`, t.Name, t.Name)
}
}
var commonMethods = map[string]bool{
"Error": true,
"Read": true,
"ServeHTTP": true,
"String": true,
"Write": true,
}
// lintFuncDoc examines doc comments on functions and methods.
// It complains if they are missing, or not of the right form.
// It has specific exclusions for well-known methods (see commonMethods above).
func (f *file) lintFuncDoc(fn *ast.FuncDecl) {
if !ast.IsExported(fn.Name.Name) {
// func is unexported
return
}
kind := "function"
name := fn.Name.Name
if fn.Recv != nil && len(fn.Recv.List) > 0 {
// method
kind = "method"
recv := receiverType(fn)
if !ast.IsExported(recv) {
// receiver is unexported
return
}
if commonMethods[name] {
return
}
switch name {
case "Len", "Less", "Swap":
if f.pkg.sortable[recv] {
return
}
}
name = recv + "." + name
}
if fn.Doc == nil {
f.errorf(fn, 1, link(docCommentsLink), category("comments"), "exported %s %s should have comment or be unexported", kind, name)
return
}
s := fn.Doc.Text()
prefix := fn.Name.Name + " "
if !strings.HasPrefix(s, prefix) {
f.errorf(fn.Doc, 1, link(docCommentsLink), category("comments"), `comment on exported %s %s should be of the form "%s..."`, kind, name, prefix)
}
}
// lintValueSpecDoc examines package-global variables and constants.
// It complains if they are not individually declared,
// or if they are not suitably documented in the right form (unless they are in a block that is commented).
func (f *file) lintValueSpecDoc(vs *ast.ValueSpec, gd *ast.GenDecl, genDeclMissingComments map[*ast.GenDecl]bool) {
kind := "var"
if gd.Tok == token.CONST {
kind = "const"
}
if len(vs.Names) > 1 {
// Check that none are exported except for the first.
for _, n := range vs.Names[1:] {
if ast.IsExported(n.Name) {
f.errorf(vs, 1, category("comments"), "exported %s %s should have its own declaration", kind, n.Name)
return
}
}
}
// Only one name.
name := vs.Names[0].Name
if !ast.IsExported(name) {
return
}
if vs.Doc == nil {
if gd.Doc == nil && !genDeclMissingComments[gd] {
block := ""
if kind == "const" && gd.Lparen.IsValid() {
block = " (or a comment on this block)"
}
f.errorf(vs, 1, link(docCommentsLink), category("comments"), "exported %s %s should have comment%s or be unexported", kind, name, block)
genDeclMissingComments[gd] = true
}
return
}
prefix := name + " "
if !strings.HasPrefix(vs.Doc.Text(), prefix) {
f.errorf(vs.Doc, 1, link(docCommentsLink), category("comments"), `comment on exported %s %s should be of the form "%s..."`, kind, name, prefix)
}
}
func (f *file) checkStutter(id *ast.Ident, thing string) {
pkg, name := f.f.Name.Name, id.Name
if !ast.IsExported(name) {
// unexported name
return
}
// A name stutters if the package name is a strict prefix
// and the next character of the name starts a new word.
if len(name) <= len(pkg) {
// name is too short to stutter.
// This permits the name to be the same as the package name.
return
}
if !strings.EqualFold(pkg, name[:len(pkg)]) {
return
}
// We can assume the name is well-formed UTF-8.
// If the next rune after the package name is uppercase or an underscore
// the it's starting a new word and thus this name stutters.
rem := name[len(pkg):]
if next, _ := utf8.DecodeRuneInString(rem); next == '_' || unicode.IsUpper(next) {
f.errorf(id, 0.8, link(styleGuideBase+"#package-names"), category("naming"), "%s name will be used as %s.%s by other packages, and that stutters; consider calling this %s", thing, pkg, name, rem)
}
}
// zeroLiteral is a set of ast.BasicLit values that are zero values.
// It is not exhaustive.
var zeroLiteral = map[string]bool{
"false": true, // bool
// runes
`'\x00'`: true,
`'\000'`: true,
// strings
`""`: true,
"``": true,
// numerics
"0": true,
"0.": true,
"0.0": true,
"0i": true,
}
// knownWeakerTypes is a set of types that are commonly used to weaken var declarations.
// A common form of var declarations that legitimately mentions an explicit LHS type
// is where the LHS type is "weaker" than the exact RHS type, where "weaker" means an
// interface compared to a concrete type, or an interface compared to a superset interface.
// A canonical example is `var out io.Writer = os.Stdout`.
// This is only used when type checking fails to determine the exact types.
var knownWeakerTypes = map[string]bool{
"io.Reader": true,
"io.Writer": true,
"proto.Message": true,
}
// lintVarDecls examines variable declarations. It complains about declarations with
// redundant LHS types that can be inferred from the RHS.
func (f *file) lintVarDecls() {
var lastGen *ast.GenDecl // last GenDecl entered.
f.walk(func(node ast.Node) bool {
switch v := node.(type) {
case *ast.GenDecl:
if v.Tok != token.CONST && v.Tok != token.VAR {
return false
}
lastGen = v
return true
case *ast.ValueSpec:
if lastGen.Tok == token.CONST {
return false
}
if len(v.Names) > 1 || v.Type == nil || len(v.Values) == 0 {
return false
}
rhs := v.Values[0]
// An underscore var appears in a common idiom for compile-time interface satisfaction,
// as in "var _ Interface = (*Concrete)(nil)".
if isIdent(v.Names[0], "_") {
return false
}
// If the RHS is a zero value, suggest dropping it.
zero := false
if lit, ok := rhs.(*ast.BasicLit); ok {
zero = zeroLiteral[lit.Value]
} else if isIdent(rhs, "nil") {
zero = true
}
if zero {
f.errorf(rhs, 0.9, category("zero-value"), "should drop = %s from declaration of var %s; it is the zero value", f.render(rhs), v.Names[0])
return false
}
lhsTyp := f.pkg.typeOf(v.Type)
rhsTyp := f.pkg.typeOf(rhs)
if lhsTyp != nil && rhsTyp != nil && !types.Identical(lhsTyp, rhsTyp) {
// Assignment to a different type is not redundant.
return false
}
// The next three conditions are for suppressing the warning in situations
// where we were unable to typecheck.
// If the LHS type is an interface, don't warn, since it is probably a
// concrete type on the RHS. Note that our feeble lexical check here
// will only pick up interface{} and other literal interface types;
// that covers most of the cases we care to exclude right now.
if _, ok := v.Type.(*ast.InterfaceType); ok {
return false
}
// If the RHS is an untyped const, only warn if the LHS type is its default type.
if defType, ok := f.isUntypedConst(rhs); ok && !isIdent(v.Type, defType) {
return false
}
// If the LHS is a known weaker type, and we couldn't type check both sides,
// don't warn.
if lhsTyp == nil || rhsTyp == nil {
if knownWeakerTypes[f.render(v.Type)] {
return false
}
}
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])
return false
}
return true
})
}
// lintElses examines else blocks. It complains about any else block whose if block ends in a return.
func (f *file) lintElses() {
// We don't want to flag if { } else if { } else { } constructions.
// They will appear as an IfStmt whose Else field is also an IfStmt.
// Record such a node so we ignore it when we visit it.
ignore := make(map[*ast.IfStmt]bool)
f.walk(func(node ast.Node) bool {
ifStmt, ok := node.(*ast.IfStmt)
if !ok || ifStmt.Else == nil {
return true
}
if ignore[ifStmt] {
return true
}
if elseif, ok := ifStmt.Else.(*ast.IfStmt); ok {
ignore[elseif] = true
return true
}
if _, ok := ifStmt.Else.(*ast.BlockStmt); !ok {
// only care about elses without conditions
return true
}
if len(ifStmt.Body.List) == 0 {
return true
}
shortDecl := false // does the if statement have a ":=" initialization statement?
if ifStmt.Init != nil {
if as, ok := ifStmt.Init.(*ast.AssignStmt); ok && as.Tok == token.DEFINE {
shortDecl = true
}
}
lastStmt := ifStmt.Body.List[len(ifStmt.Body.List)-1]
if _, ok := lastStmt.(*ast.ReturnStmt); ok {
extra := ""
if shortDecl {
extra = " (move short variable declaration to its own line if necessary)"
}
f.errorf(ifStmt.Else, 1, link(styleGuideBase+"#indent-error-flow"), category("indent"), "if block ends with a return statement, so drop this else and outdent its block"+extra)
}
return true
})
}
// lintRanges examines range clauses. It complains about redundant constructions.
func (f *file) lintRanges() {
f.walk(func(node ast.Node) bool {
rs, ok := node.(*ast.RangeStmt)
if !ok {
return true
}
if rs.Value == nil {
// for x = range m { ... }
return true // single var form
}
if !isIdent(rs.Value, "_") {
// for ?, y = range m { ... }
return true
}
p := f.errorf(rs.Value, 1, category("range-loop"), "should omit 2nd value from range; this loop is equivalent to `for %s %s range ...`", f.render(rs.Key), rs.Tok)
newRS := *rs // shallow copy
newRS.Value = nil
p.ReplacementLine = f.firstLineOf(&newRS, rs)
return true
})
}
// lintErrorf examines errors.New and testing.Error calls. It complains if its only argument is an fmt.Sprintf invocation.
func (f *file) lintErrorf() {
f.walk(func(node ast.Node) bool {
ce, ok := node.(*ast.CallExpr)
if !ok || len(ce.Args) != 1 {
return true
}
isErrorsNew := isPkgDot(ce.Fun, "errors", "New")
var isTestingError bool
se, ok := ce.Fun.(*ast.SelectorExpr)
if ok && se.Sel.Name == "Error" {
if typ := f.pkg.typeOf(se.X); typ != nil {
isTestingError = typ.String() == "*testing.T"
}
}
if !isErrorsNew && !isTestingError {
return true
}
arg := ce.Args[0]
ce, ok = arg.(*ast.CallExpr)
if !ok || !isPkgDot(ce.Fun, "fmt", "Sprintf") {
return true
}
errorfPrefix := "fmt"
if isTestingError {
errorfPrefix = f.render(se.X)
}
p := f.errorf(node, 1, category("errors"), "should replace %s(fmt.Sprintf(...)) with %s.Errorf(...)", f.render(se), errorfPrefix)
m := f.srcLineWithMatch(ce, `^(.*)`+f.render(se)+`\(fmt\.Sprintf\((.*)\)\)(.*)$`)
if m != nil {
p.ReplacementLine = m[1] + errorfPrefix + ".Errorf(" + m[2] + ")" + m[3]
}
return true
})
}
// lintErrors examines global error vars. It complains if they aren't named in the standard way.
func (f *file) lintErrors() {
for _, decl := range f.f.Decls {
gd, ok := decl.(*ast.GenDecl)
if !ok || gd.Tok != token.VAR {
continue
}
for _, spec := range gd.Specs {
spec := spec.(*ast.ValueSpec)
if len(spec.Names) != 1 || len(spec.Values) != 1 {
continue
}
ce, ok := spec.Values[0].(*ast.CallExpr)
if !ok {
continue
}
if !isPkgDot(ce.Fun, "errors", "New") && !isPkgDot(ce.Fun, "fmt", "Errorf") {
continue
}
id := spec.Names[0]
prefix := "err"
if id.IsExported() {
prefix = "Err"
}
if !strings.HasPrefix(id.Name, prefix) {
f.errorf(id, 0.9, category("naming"), "error var %s should have name of the form %sFoo", id.Name, prefix)
}
}
}
}
func lintCapAndPunct(s string) (isCap, isPunct bool) {
first, firstN := utf8.DecodeRuneInString(s)
last, _ := utf8.DecodeLastRuneInString(s)
isPunct = last == '.' || last == ':' || last == '!'
isCap = unicode.IsUpper(first)
if isCap && len(s) > firstN {
// Don't flag strings starting with something that looks like an initialism.
if second, _ := utf8.DecodeRuneInString(s[firstN:]); unicode.IsUpper(second) {
isCap = false
}
}
return
}
// lintErrorStrings examines error strings. It complains if they are capitalized or end in punctuation.
func (f *file) lintErrorStrings() {
f.walk(func(node ast.Node) bool {
ce, ok := node.(*ast.CallExpr)
if !ok {
return true
}
if !isPkgDot(ce.Fun, "errors", "New") && !isPkgDot(ce.Fun, "fmt", "Errorf") {
return true
}
if len(ce.Args) < 1 {
return true
}
str, ok := ce.Args[0].(*ast.BasicLit)
if !ok || str.Kind != token.STRING {
return true
}
s, _ := strconv.Unquote(str.Value) // can assume well-formed Go
if s == "" {
return true
}
isCap, isPunct := lintCapAndPunct(s)
var msg string
switch {
case isCap && isPunct:
msg = "error strings should not be capitalized and should not end with punctuation"
case isCap:
msg = "error strings should not be capitalized"
case isPunct:
msg = "error strings should not end with punctuation"
default:
return true
}
// People use proper nouns and exported Go identifiers in error strings,
// so decrease the confidence of warnings for capitalization.
conf := 0.8
if isCap {
conf = 0.6
}
f.errorf(str, conf, link(styleGuideBase+"#error-strings"), category("errors"), msg)
return true
})
}
var badReceiverNames = map[string]bool{
"me": true,
"this": true,
"self": true,
}
// lintReceiverNames examines receiver names. It complains about inconsistent
// names used for the same type and names such as "this".
func (f *file) lintReceiverNames() {
typeReceiver := map[string]string{}
f.walk(func(n ast.Node) bool {
fn, ok := n.(*ast.FuncDecl)
if !ok || fn.Recv == nil || len(fn.Recv.List) == 0 {
return true
}
names := fn.Recv.List[0].Names
if len(names) < 1 {
return true
}
name := names[0].Name
const ref = styleGuideBase + "#receiver-names"
if name == "_" {
f.errorf(n, 1, link(ref), category("naming"), `receiver name should not be an underscore`)
return true
}
if badReceiverNames[name] {
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"`)
return true
}
recv := receiverType(fn)
if prev, ok := typeReceiver[recv]; ok && prev != name {
f.errorf(n, 1, link(ref), category("naming"), "receiver name %s should be consistent with previous receiver name %s for %s", name, prev, recv)
return true
}
typeReceiver[recv] = name
return true
})
}
// lintIncDec examines statements that increment or decrement a variable.
// It complains if they don't use x++ or x--.
func (f *file) lintIncDec() {
f.walk(func(n ast.Node) bool {
as, ok := n.(*ast.AssignStmt)
if !ok {
return true
}
if len(as.Lhs) != 1 {
return true
}
if !isOne(as.Rhs[0]) {
return true
}
var suffix string
switch as.Tok {
case token.ADD_ASSIGN:
suffix = "++"
case token.SUB_ASSIGN:
suffix = "--"
default:
return true
}
f.errorf(as, 0.8, category("unary-op"), "should replace %s with %s%s", f.render(as), f.render(as.Lhs[0]), suffix)
return true
})
}
// lineMake examines statements that declare and initialize a variable with make.
// It complains if they are constructing a zero element slice.
func (f *file) lintMake() {
f.walk(func(n ast.Node) bool {
as, ok := n.(*ast.AssignStmt)
if !ok {
return true
}
// Only want single var := assignment statements.
if len(as.Lhs) != 1 || as.Tok != token.DEFINE {
return true
}
ce, ok := as.Rhs[0].(*ast.CallExpr)
if !ok {
return true
}
// Check if ce is make([]T, 0).
if !isIdent(ce.Fun, "make") || len(ce.Args) != 2 || !isZero(ce.Args[1]) {
return true
}
at, ok := ce.Args[0].(*ast.ArrayType)
if !ok || at.Len != nil {
return true
}
f.errorf(as, 0.8, category("slice"), `can probably use "var %s %s" instead`, f.render(as.Lhs[0]), f.render(at))
return true
})
}
// lintErrorReturn examines function declarations that return an error.
// It complains if the error isn't the last parameter.
func (f *file) lintErrorReturn() {
f.walk(func(n ast.Node) bool {
fn, ok := n.(*ast.FuncDecl)
if !ok || fn.Type.Results == nil {
return true
}
ret := fn.Type.Results.List
if len(ret) <= 1 {
return true
}
// An error return parameter should be the last parameter.
// Flag any error parameters found before the last.
for _, r := range ret[:len(ret)-1] {
if isIdent(r.Type, "error") {
f.errorf(fn, 0.9, category("arg-order"), "error should be the last type when returning multiple items")
break // only flag one
}
}
return true
})
}
// lintUnexportedReturn examines exported function declarations.
// It complains if any return an unexported type.
func (f *file) lintUnexportedReturn() {
f.walk(func(n ast.Node) bool {
fn, ok := n.(*ast.FuncDecl)
if !ok {
return true
}
if fn.Type.Results == nil {
return false
}
if !fn.Name.IsExported() {
return false
}
thing := "func"
if fn.Recv != nil && len(fn.Recv.List) > 0 {
thing = "method"
if !ast.IsExported(receiverType(fn)) {
// Don't report exported methods of unexported types,
// such as private implementations of sort.Interface.
return false
}
}
for _, ret := range fn.Type.Results.List {
typ := f.pkg.typeOf(ret.Type)
if exportedType(typ) {
continue
}
f.errorf(ret.Type, 0.8, category("unexported-type-in-api"),
"exported %s %s returns unexported type %s, which can be annoying to use",
thing, fn.Name.Name, typ)
break // only flag one
}
return false
})
}
// exportedType reports whether typ is an exported type.
// It is imprecise, and will err on the side of returning true,
// such as for composite types.
func exportedType(typ types.Type) bool {
switch T := typ.(type) {
case *types.Named:
// Builtin types have no package.
return T.Obj().Pkg() == nil || T.Obj().Exported()
case *types.Map:
return exportedType(T.Key()) && exportedType(T.Elem())
case interface {
Elem() types.Type
}: // array, slice, pointer, chan
return exportedType(T.Elem())
}
// Be conservative about other types, such as struct, interface, etc.
return true
}
// timeSuffixes is a list of name suffixes that imply a time unit.
// This is not an exhaustive list.
var timeSuffixes = []string{
"Sec", "Secs", "Seconds",
"Msec", "Msecs",
"Milli", "Millis", "Milliseconds",
"Usec", "Usecs", "Microseconds",
"MS", "Ms",
}
func (f *file) lintTimeNames() {
f.walk(func(node ast.Node) bool {
v, ok := node.(*ast.ValueSpec)
if !ok {
return true
}
for _, name := range v.Names {
origTyp := f.pkg.typeOf(name)
// Look for time.Duration or *time.Duration;
// the latter is common when using flag.Duration.
typ := origTyp
if pt, ok := typ.(*types.Pointer); ok {
typ = pt.Elem()
}
if !f.pkg.isNamedType(typ, "time", "Duration") {
continue
}
suffix := ""
for _, suf := range timeSuffixes {
if strings.HasSuffix(name.Name, suf) {
suffix = suf
break
}
}
if suffix == "" {
continue
}
f.errorf(v, 0.9, category("time"), "var %s is of type %v; don't use unit-specific suffix %q", name.Name, origTyp, suffix)
}
return true
})
}
func (f *file) lintDurationUnits() {
f.walk(func(node ast.Node) bool {
e, ok := node.(ast.Expr)
if !ok {
return true
}
tv := f.pkg.typesInfo.Types[e]
if !f.pkg.isNamedType(tv.Type, "time", "Duration") {
return true
}
if tv.Value == nil {
return true
}
// OK for 0 to be unitless; non-constants are OK.
if x, ok := exact.Int64Val(tv.Value); !ok || x == 0 {
return true
}
// e has a constant value. What matters now is how it was defined.
// Its definition might be spread across many expressions:
//
// const (
// a = iota
// b
// c = b * time.Second
// e = c * 10
// )
//
// Rather than attempt to reconstruct e's origins,
// just handle the common cases.
switch e.(type) {
default:
// Assume that any non-literal, non-call expression is ok.
// In particular, allow 3 * time.Second, which is an *ast.BinExpr.
// This means that we fail to flag cases like
// time.Duration(15) + time.Duration(30) and
// time.Sleep(x) where 'const x = 10'.
return false
case *ast.CallExpr:
// Descend into call expressions in order to catch time.Duration(15).
return true
case *ast.BasicLit:
// A basic literal cannot possibly use time package constants.
}
f.errorf(e, 0.8, category("time"), `time.Duration expression "%v" should use time package constants`, f.render(e))
// TODO: Generate a ReplacementLine with an appropriately sized unit.
// For example, 3e9 should become 3 * time.Second. The unit part is easy.
// The challenge is figuring out exactly what else is on the line
// and handling the case in which there are multiple expressions
// to replace on the same line.
// The latter opens up API questions. If there are multiple
// problems on the same line, should they all generate identical
// replacements, or one replacement per problem?
// The former would mean that this linter has to maintain state across
// expressions.
// Also, the test framework does not yet have a means to represent
// multiple matches or multiple replacements on a single line.
return true
})
}
func receiverType(fn *ast.FuncDecl) string {
switch e := fn.Recv.List[0].Type.(type) {
case *ast.Ident:
return e.Name
case *ast.StarExpr:
return e.X.(*ast.Ident).Name
}
panic(fmt.Sprintf("unknown method receiver AST node type %T", fn.Recv.List[0].Type))
}
func (f *file) walk(fn func(ast.Node) bool) {
ast.Walk(walker(fn), f.f)
}
func (f *file) render(x interface{}) string {
var buf bytes.Buffer
if err := printer.Fprint(&buf, f.fset, x); err != nil {
panic(err)
}
return buf.String()
}
func (f *file) debugRender(x interface{}) string {
var buf bytes.Buffer
if err := ast.Fprint(&buf, f.fset, x, nil); err != nil {
panic(err)
}
return buf.String()
}
// walker adapts a function to satisfy the ast.Visitor interface.
// The function return whether the walk should proceed into the node's children.
type walker func(ast.Node) bool
func (w walker) Visit(node ast.Node) ast.Visitor {
if w(node) {
return w
}
return nil
}
func isIdent(expr ast.Expr, ident string) bool {
id, ok := expr.(*ast.Ident)
return ok && id.Name == ident
}
// isBlank returns whether id is the blank identifier "_".
// If id == nil, the answer is false.
func isBlank(id *ast.Ident) bool { return id != nil && id.Name == "_" }
func isPkgDot(expr ast.Expr, pkg, name string) bool {
sel, ok := expr.(*ast.SelectorExpr)
return ok && isIdent(sel.X, pkg) && isIdent(sel.Sel, name)
}
func isZero(expr ast.Expr) bool {
lit, ok := expr.(*ast.BasicLit)
return ok && lit.Kind == token.INT && lit.Value == "0"
}
func isOne(expr ast.Expr) bool {
lit, ok := expr.(*ast.BasicLit)
return ok && lit.Kind == token.INT && lit.Value == "1"
}
var basicTypeKinds = map[types.BasicKind]string{
types.UntypedBool: "bool",
types.UntypedInt: "int",
types.UntypedRune: "rune",
types.UntypedFloat: "float64",
types.UntypedComplex: "complex128",
types.UntypedString: "string",
}
// isUntypedConst reports whether expr is an untyped constant,
// and indicates what its default type is.
// scope may be nil.
func (f *file) isUntypedConst(expr ast.Expr) (defType string, ok bool) {
// Re-evaluate expr outside of its context to see if it's untyped.
// (An expr evaluated within, for example, an assignment context will get the type of the LHS.)
exprStr := f.render(expr)
tv, err := types.Eval(f.fset, f.pkg.typesPkg, expr.Pos(), exprStr)
if err != nil {
return "", false
}
if b, ok := tv.Type.(*types.Basic); ok {
if dt, ok := basicTypeKinds[b.Kind()]; ok {
return dt, true
}
}
return "", false
}
// firstLineOf renders the given node and returns its first line.
// It will also match the indentation of another node.
func (f *file) firstLineOf(node, match ast.Node) string {
line := f.render(node)
if i := strings.Index(line, "\n"); i >= 0 {
line = line[:i]
}
return f.indentOf(match) + line
}
func (f *file) indentOf(node ast.Node) string {
line := srcLine(f.src, f.fset.Position(node.Pos()))
for i, r := range line {
switch r {
case ' ', '\t':
default:
return line[:i]
}
}
return line // unusual or empty line
}
func (f *file) srcLineWithMatch(node ast.Node, pattern string) (m []string) {
line := srcLine(f.src, f.fset.Position(node.Pos()))
line = strings.TrimSuffix(line, "\n")
rx := regexp.MustCompile(pattern)
return rx.FindStringSubmatch(line)
}
// srcLine returns the complete line at p, including the terminating newline.
func srcLine(src []byte, p token.Position) string {
// Run to end of line in both directions if not at line start/end.
lo, hi := p.Offset, p.Offset+1
for lo > 0 && src[lo-1] != '\n' {
lo--
}
for hi < len(src) && src[hi-1] != '\n' {
hi++
}
return string(src[lo:hi])
}