staticcheck (#313)

* CI: use staticcheck for linting

This commit switches the linter for Go code from golint to staticcheck.
Golint has been deprecated since last year and staticcheck is a
recommended replacement.

Signed-off-by: Lucas Servén Marín <lserven@gmail.com>

* revendor

Signed-off-by: Lucas Servén Marín <lserven@gmail.com>

* cmd,pkg: fix lint warnings

Signed-off-by: Lucas Servén Marín <lserven@gmail.com>
This commit is contained in:
Lucas Servén Marín
2022-05-19 19:45:43 +02:00
committed by GitHub
parent 93f46e03ea
commit 50fbc2eec2
227 changed files with 55458 additions and 2689 deletions

View File

@@ -0,0 +1,83 @@
package stylecheck
import (
"honnef.co/go/tools/analysis/facts"
"honnef.co/go/tools/analysis/lint"
"honnef.co/go/tools/config"
"honnef.co/go/tools/internal/passes/buildir"
"honnef.co/go/tools/internal/sharedcheck"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
)
var Analyzers = lint.InitializeAnalyzers(Docs, map[string]*analysis.Analyzer{
"ST1000": {
Run: CheckPackageComment,
},
"ST1001": {
Run: CheckDotImports,
Requires: []*analysis.Analyzer{facts.Generated, config.Analyzer},
},
"ST1003": {
Run: CheckNames,
Requires: []*analysis.Analyzer{inspect.Analyzer, facts.Generated, config.Analyzer},
},
"ST1005": {
Run: CheckErrorStrings,
Requires: []*analysis.Analyzer{buildir.Analyzer},
},
"ST1006": {
Run: CheckReceiverNames,
Requires: []*analysis.Analyzer{buildir.Analyzer, facts.Generated},
},
"ST1008": {
Run: CheckErrorReturn,
Requires: []*analysis.Analyzer{buildir.Analyzer},
},
"ST1011": {
Run: CheckTimeNames,
Requires: []*analysis.Analyzer{inspect.Analyzer},
},
"ST1012": {
Run: CheckErrorVarNames,
},
"ST1013": {
Run: CheckHTTPStatusCodes,
// TODO(dh): why does this depend on facts.TokenFile?
Requires: []*analysis.Analyzer{facts.Generated, facts.TokenFile, config.Analyzer, inspect.Analyzer},
},
"ST1015": {
Run: CheckDefaultCaseOrder,
Requires: []*analysis.Analyzer{inspect.Analyzer, facts.Generated, facts.TokenFile},
},
"ST1016": {
Run: CheckReceiverNamesIdentical,
Requires: []*analysis.Analyzer{buildir.Analyzer, facts.Generated},
},
"ST1017": {
Run: CheckYodaConditions,
Requires: []*analysis.Analyzer{inspect.Analyzer, facts.Generated, facts.TokenFile},
},
"ST1018": {
Run: CheckInvisibleCharacters,
Requires: []*analysis.Analyzer{inspect.Analyzer},
},
"ST1019": {
Run: CheckDuplicatedImports,
Requires: []*analysis.Analyzer{facts.Generated},
},
"ST1020": {
Run: CheckExportedFunctionDocs,
Requires: []*analysis.Analyzer{facts.Generated, inspect.Analyzer},
},
"ST1021": {
Run: CheckExportedTypeDocs,
Requires: []*analysis.Analyzer{facts.Generated, inspect.Analyzer},
},
"ST1022": {
Run: CheckExportedVarDocs,
Requires: []*analysis.Analyzer{facts.Generated, inspect.Analyzer},
},
"ST1023": sharedcheck.RedundantTypeInDeclarationChecker("should", false),
})

View File

@@ -0,0 +1,262 @@
// Package stylecheck contains analyzes that enforce style rules.
// Most of the recommendations made are universally agreed upon by the wider Go community.
// Some analyzes, however, implement stricter rules that not everyone will agree with.
// In the context of Staticcheck, these analyzes are not enabled by default.
//
// For the most part it is recommended to follow the advice given by the analyzers that are enabled by default,
// but you may want to disable additional analyzes on a case by case basis.
package stylecheck
import "honnef.co/go/tools/analysis/lint"
var Docs = lint.Markdownify(map[string]*lint.RawDocumentation{
"ST1000": {
Title: `Incorrect or missing package comment`,
Text: `Packages must have a package comment that is formatted according to
the guidelines laid out in
https://github.com/golang/go/wiki/CodeReviewComments#package-comments.`,
Since: "2019.1",
NonDefault: true,
MergeIf: lint.MergeIfAny,
},
"ST1001": {
Title: `Dot imports are discouraged`,
Text: `Dot imports that aren't in external test packages are discouraged.
The \'dot_import_whitelist\' option can be used to whitelist certain
imports.
Quoting Go Code Review Comments:
> The \'import .\' form can be useful in tests that, due to circular
> dependencies, cannot be made part of the package being tested:
>
> package foo_test
>
> import (
> "bar/testutil" // also imports "foo"
> . "foo"
> )
>
> In this case, the test file cannot be in package foo because it
> uses \'bar/testutil\', which imports \'foo\'. So we use the \'import .\'
> form to let the file pretend to be part of package foo even though
> it is not. Except for this one case, do not use \'import .\' in your
> programs. It makes the programs much harder to read because it is
> unclear whether a name like \'Quux\' is a top-level identifier in the
> current package or in an imported package.`,
Since: "2019.1",
Options: []string{"dot_import_whitelist"},
MergeIf: lint.MergeIfAny,
},
"ST1003": {
Title: `Poorly chosen identifier`,
Text: `Identifiers, such as variable and package names, follow certain rules.
See the following links for details:
- https://golang.org/doc/effective_go.html#package-names
- https://golang.org/doc/effective_go.html#mixed-caps
- https://github.com/golang/go/wiki/CodeReviewComments#initialisms
- https://github.com/golang/go/wiki/CodeReviewComments#variable-names`,
Since: "2019.1",
NonDefault: true,
Options: []string{"initialisms"},
MergeIf: lint.MergeIfAny,
},
"ST1005": {
Title: `Incorrectly formatted error string`,
Text: `Error strings follow a set of guidelines to ensure uniformity and good
composability.
Quoting Go Code Review Comments:
> Error strings should not be capitalized (unless beginning with
> proper nouns or acronyms) or end with punctuation, since they are
> usually printed following other context. That is, use
> \'fmt.Errorf("something bad")\' not \'fmt.Errorf("Something bad")\', so
> that \'log.Printf("Reading %s: %v", filename, err)\' formats without a
> spurious capital letter mid-message.`,
Since: "2019.1",
MergeIf: lint.MergeIfAny,
},
"ST1006": {
Title: `Poorly chosen receiver name`,
Text: `Quoting Go Code Review Comments:
> The name of a method's receiver should be a reflection of its
> identity; often a one or two letter abbreviation of its type
> suffices (such as "c" or "cl" for "Client"). Don't use generic
> names such as "me", "this" or "self", identifiers typical of
> object-oriented languages that place more emphasis on methods as
> opposed to functions. The name need not be as descriptive as that
> of a method argument, as its role is obvious and serves no
> documentary purpose. It can be very short as it will appear on
> almost every line of every method of the type; familiarity admits
> brevity. Be consistent, too: if you call the receiver "c" in one
> method, don't call it "cl" in another.`,
Since: "2019.1",
MergeIf: lint.MergeIfAny,
},
"ST1008": {
Title: `A function's error value should be its last return value`,
Text: `A function's error value should be its last return value.`,
Since: `2019.1`,
MergeIf: lint.MergeIfAny,
},
"ST1011": {
Title: `Poorly chosen name for variable of type \'time.Duration\'`,
Text: `\'time.Duration\' values represent an amount of time, which is represented
as a count of nanoseconds. An expression like \'5 * time.Microsecond\'
yields the value \'5000\'. It is therefore not appropriate to suffix a
variable of type \'time.Duration\' with any time unit, such as \'Msec\' or
\'Milli\'.`,
Since: `2019.1`,
MergeIf: lint.MergeIfAny,
},
"ST1012": {
Title: `Poorly chosen name for error variable`,
Text: `Error variables that are part of an API should be called \'errFoo\' or
\'ErrFoo\'.`,
Since: "2019.1",
MergeIf: lint.MergeIfAny,
},
"ST1013": {
Title: `Should use constants for HTTP error codes, not magic numbers`,
Text: `HTTP has a tremendous number of status codes. While some of those are
well known (200, 400, 404, 500), most of them are not. The \'net/http\'
package provides constants for all status codes that are part of the
various specifications. It is recommended to use these constants
instead of hard-coding magic numbers, to vastly improve the
readability of your code.`,
Since: "2019.1",
Options: []string{"http_status_code_whitelist"},
MergeIf: lint.MergeIfAny,
},
"ST1015": {
Title: `A switch's default case should be the first or last case`,
Since: "2019.1",
MergeIf: lint.MergeIfAny,
},
"ST1016": {
Title: `Use consistent method receiver names`,
Since: "2019.1",
NonDefault: true,
MergeIf: lint.MergeIfAny,
},
"ST1017": {
Title: `Don't use Yoda conditions`,
Text: `Yoda conditions are conditions of the kind \"if 42 == x\", where the
literal is on the left side of the comparison. These are a common
idiom in languages in which assignment is an expression, to avoid bugs
of the kind \"if (x = 42)\". In Go, which doesn't allow for this kind of
bug, we prefer the more idiomatic \"if x == 42\".`,
Since: "2019.2",
MergeIf: lint.MergeIfAny,
},
"ST1018": {
Title: `Avoid zero-width and control characters in string literals`,
Since: "2019.2",
MergeIf: lint.MergeIfAny,
},
"ST1019": {
Title: `Importing the same package multiple times`,
Text: `Go allows importing the same package multiple times, as long as
different import aliases are being used. That is, the following
bit of code is valid:
import (
"fmt"
fumpt "fmt"
format "fmt"
_ "fmt"
)
However, this is very rarely done on purpose. Usually, it is a
sign of code that got refactored, accidentally adding duplicate
import statements. It is also a rarely known feature, which may
contribute to confusion.
Do note that sometimes, this feature may be used
intentionally (see for example
https://github.com/golang/go/commit/3409ce39bfd7584523b7a8c150a310cea92d879d)
if you want to allow this pattern in your code base, you're
advised to disable this check.`,
Since: "2020.1",
MergeIf: lint.MergeIfAny,
},
"ST1020": {
Title: "The documentation of an exported function should start with the function's name",
Text: `Doc comments work best as complete sentences, which
allow a wide variety of automated presentations. The first sentence
should be a one-sentence summary that starts with the name being
declared.
If every doc comment begins with the name of the item it describes,
you can use the \'doc\' subcommand of the \'go\' tool and run the output
through grep.
See https://golang.org/doc/effective_go.html#commentary for more
information on how to write good documentation.`,
Since: "2020.1",
NonDefault: true,
MergeIf: lint.MergeIfAny,
},
"ST1021": {
Title: "The documentation of an exported type should start with type's name",
Text: `Doc comments work best as complete sentences, which
allow a wide variety of automated presentations. The first sentence
should be a one-sentence summary that starts with the name being
declared.
If every doc comment begins with the name of the item it describes,
you can use the \'doc\' subcommand of the \'go\' tool and run the output
through grep.
See https://golang.org/doc/effective_go.html#commentary for more
information on how to write good documentation.`,
Since: "2020.1",
NonDefault: true,
MergeIf: lint.MergeIfAny,
},
"ST1022": {
Title: "The documentation of an exported variable or constant should start with variable's name",
Text: `Doc comments work best as complete sentences, which
allow a wide variety of automated presentations. The first sentence
should be a one-sentence summary that starts with the name being
declared.
If every doc comment begins with the name of the item it describes,
you can use the \'doc\' subcommand of the \'go\' tool and run the output
through grep.
See https://golang.org/doc/effective_go.html#commentary for more
information on how to write good documentation.`,
Since: "2020.1",
NonDefault: true,
MergeIf: lint.MergeIfAny,
},
"ST1023": {
Title: "Redundant type in variable declaration",
Since: "2021.1",
NonDefault: true,
MergeIf: lint.MergeIfAll,
},
})

View File

@@ -0,0 +1,958 @@
package stylecheck
import (
"fmt"
"go/ast"
"go/constant"
"go/token"
"go/types"
"sort"
"strconv"
"strings"
"unicode"
"unicode/utf8"
"honnef.co/go/tools/analysis/code"
"honnef.co/go/tools/analysis/edit"
"honnef.co/go/tools/analysis/lint"
"honnef.co/go/tools/analysis/report"
"honnef.co/go/tools/config"
"honnef.co/go/tools/go/ast/astutil"
"honnef.co/go/tools/go/ir"
"honnef.co/go/tools/go/ir/irutil"
"honnef.co/go/tools/go/types/typeutil"
"honnef.co/go/tools/internal/passes/buildir"
"honnef.co/go/tools/pattern"
"golang.org/x/exp/typeparams"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/ast/inspector"
)
func docText(doc *ast.CommentGroup) (string, bool) {
if doc == nil {
return "", false
}
// We trim spaces primarily because of /**/ style comments, which often have leading space.
text := strings.TrimSpace(doc.Text())
return text, text != ""
}
func CheckPackageComment(pass *analysis.Pass) (interface{}, error) {
// - At least one file in a non-main package should have a package comment
//
// - The comment should be of the form
// "Package x ...". This has a slight potential for false
// positives, as multiple files can have package comments, in
// which case they get appended. But that doesn't happen a lot in
// the real world.
if pass.Pkg.Name() == "main" {
return nil, nil
}
hasDocs := false
for _, f := range pass.Files {
if code.IsInTest(pass, f) {
continue
}
text, ok := docText(f.Doc)
if ok {
hasDocs = true
prefix := "Package " + f.Name.Name + " "
if !strings.HasPrefix(text, prefix) {
report.Report(pass, f.Doc, fmt.Sprintf(`package comment should be of the form "%s..."`, prefix))
}
}
}
if !hasDocs {
for _, f := range pass.Files {
if code.IsInTest(pass, f) {
continue
}
report.Report(pass, f, "at least one file in a package should have a package comment", report.ShortRange())
}
}
return nil, nil
}
func CheckDotImports(pass *analysis.Pass) (interface{}, error) {
for _, f := range pass.Files {
imports:
for _, imp := range f.Imports {
path := imp.Path.Value
path = path[1 : len(path)-1]
for _, w := range config.For(pass).DotImportWhitelist {
if w == path {
continue imports
}
}
if imp.Name != nil && imp.Name.Name == "." && !code.IsInTest(pass, f) {
report.Report(pass, imp, "should not use dot imports", report.FilterGenerated())
}
}
}
return nil, nil
}
func CheckDuplicatedImports(pass *analysis.Pass) (interface{}, error) {
for _, f := range pass.Files {
// Collect all imports by their import path
imports := make(map[string][]*ast.ImportSpec, len(f.Imports))
for _, imp := range f.Imports {
imports[imp.Path.Value] = append(imports[imp.Path.Value], imp)
}
for path, value := range imports {
if path[1:len(path)-1] == "unsafe" {
// Don't flag unsafe. Cgo generated code imports
// unsafe using the blank identifier, and most
// user-written cgo code also imports unsafe
// explicitly.
continue
}
// If there's more than one import per path, we flag that
if len(value) > 1 {
s := fmt.Sprintf("package %s is being imported more than once", path)
opts := []report.Option{report.FilterGenerated()}
for _, imp := range value[1:] {
opts = append(opts, report.Related(imp, fmt.Sprintf("other import of %s", path)))
}
report.Report(pass, value[0], s, opts...)
}
}
}
return nil, nil
}
func CheckBlankImports(pass *analysis.Pass) (interface{}, error) {
fset := pass.Fset
for _, f := range pass.Files {
if code.IsMainLike(pass) || code.IsInTest(pass, f) {
continue
}
// Collect imports of the form `import _ "foo"`, i.e. with no
// parentheses, as their comment will be associated with the
// (paren-free) GenDecl, not the import spec itself.
//
// We don't directly process the GenDecl so that we can
// correctly handle the following:
//
// import _ "foo"
// import _ "bar"
//
// where only the first import should get flagged.
skip := map[ast.Spec]bool{}
ast.Inspect(f, func(node ast.Node) bool {
switch node := node.(type) {
case *ast.File:
return true
case *ast.GenDecl:
if node.Tok != token.IMPORT {
return false
}
if node.Lparen == token.NoPos && node.Doc != nil {
skip[node.Specs[0]] = true
}
return false
}
return false
})
for i, imp := range f.Imports {
pos := fset.Position(imp.Pos())
if !astutil.IsBlank(imp.Name) {
continue
}
// Only flag the first blank import in a group of imports,
// or don't flag any of them, if the first one is
// commented
if i > 0 {
prev := f.Imports[i-1]
prevPos := fset.Position(prev.Pos())
if pos.Line-1 == prevPos.Line && astutil.IsBlank(prev.Name) {
continue
}
}
if imp.Doc == nil && imp.Comment == nil && !skip[imp] {
report.Report(pass, imp, "a blank import should be only in a main or test package, or have a comment justifying it")
}
}
}
return nil, nil
}
var checkIncDecQ = pattern.MustParse(`(AssignStmt x tok@(Or "+=" "-=") (BasicLit "INT" "1"))`)
func CheckIncDec(pass *analysis.Pass) (interface{}, error) {
// TODO(dh): this can be noisy for function bodies that look like this:
// x += 3
// ...
// x += 2
// ...
// x += 1
fn := func(node ast.Node) {
m, ok := code.Match(pass, checkIncDecQ, node)
if !ok {
return
}
suffix := ""
switch m.State["tok"].(token.Token) {
case token.ADD_ASSIGN:
suffix = "++"
case token.SUB_ASSIGN:
suffix = "--"
}
report.Report(pass, node, fmt.Sprintf("should replace %s with %s%s", report.Render(pass, node), report.Render(pass, m.State["x"].(ast.Node)), suffix))
}
code.Preorder(pass, fn, (*ast.AssignStmt)(nil))
return nil, nil
}
func CheckErrorReturn(pass *analysis.Pass) (interface{}, error) {
fnLoop:
for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
sig := fn.Type().(*types.Signature)
rets := sig.Results()
if rets == nil || rets.Len() < 2 {
continue
}
if rets.At(rets.Len()-1).Type() == types.Universe.Lookup("error").Type() {
// Last return type is error. If the function also returns
// errors in other positions, that's fine.
continue
}
if rets.Len() >= 2 && rets.At(rets.Len()-1).Type() == types.Universe.Lookup("bool").Type() && rets.At(rets.Len()-2).Type() == types.Universe.Lookup("error").Type() {
// Accept (..., error, bool) and assume it's a comma-ok function. It's not clear whether the bool should come last or not for these kinds of functions.
continue
}
for i := rets.Len() - 2; i >= 0; i-- {
if rets.At(i).Type() == types.Universe.Lookup("error").Type() {
report.Report(pass, rets.At(i), "error should be returned as the last argument", report.ShortRange())
continue fnLoop
}
}
}
return nil, nil
}
// CheckUnexportedReturn checks that exported functions on exported
// types do not return unexported types.
func CheckUnexportedReturn(pass *analysis.Pass) (interface{}, error) {
for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
if fn.Synthetic != 0 || fn.Parent() != nil {
continue
}
if !ast.IsExported(fn.Name()) || code.IsMain(pass) || code.IsInTest(pass, fn) {
continue
}
sig := fn.Type().(*types.Signature)
if sig.Recv() != nil && !ast.IsExported(typeutil.Dereference(sig.Recv().Type()).(*types.Named).Obj().Name()) {
continue
}
res := sig.Results()
for i := 0; i < res.Len(); i++ {
if named, ok := typeutil.DereferenceR(res.At(i).Type()).(*types.Named); ok &&
!ast.IsExported(named.Obj().Name()) &&
named != types.Universe.Lookup("error").Type() {
report.Report(pass, fn, "should not return unexported type")
}
}
}
return nil, nil
}
func CheckReceiverNames(pass *analysis.Pass) (interface{}, error) {
irpkg := pass.ResultOf[buildir.Analyzer].(*buildir.IR).Pkg
for _, m := range irpkg.Members {
if T, ok := m.Object().(*types.TypeName); ok && !T.IsAlias() {
ms := typeutil.IntuitiveMethodSet(T.Type(), nil)
for _, sel := range ms {
fn := sel.Obj().(*types.Func)
recv := fn.Type().(*types.Signature).Recv()
if typeutil.Dereference(recv.Type()) != T.Type() {
// skip embedded methods
continue
}
if recv.Name() == "self" || recv.Name() == "this" {
report.Report(pass, recv, `receiver name should be a reflection of its identity; don't use generic names such as "this" or "self"`, report.FilterGenerated())
}
if recv.Name() == "_" {
report.Report(pass, recv, "receiver name should not be an underscore, omit the name if it is unused", report.FilterGenerated())
}
}
}
}
return nil, nil
}
func CheckReceiverNamesIdentical(pass *analysis.Pass) (interface{}, error) {
irpkg := pass.ResultOf[buildir.Analyzer].(*buildir.IR).Pkg
for _, m := range irpkg.Members {
names := map[string]int{}
var firstFn *types.Func
if T, ok := m.Object().(*types.TypeName); ok && !T.IsAlias() {
ms := typeutil.IntuitiveMethodSet(T.Type(), nil)
for _, sel := range ms {
fn := sel.Obj().(*types.Func)
recv := fn.Type().(*types.Signature).Recv()
if code.IsGenerated(pass, recv.Pos()) {
// Don't concern ourselves with methods in generated code
continue
}
if typeutil.Dereference(recv.Type()) != T.Type() {
// skip embedded methods
continue
}
if firstFn == nil {
firstFn = fn
}
if recv.Name() != "" && recv.Name() != "_" {
names[recv.Name()]++
}
}
}
if len(names) > 1 {
var seen []string
for name, count := range names {
seen = append(seen, fmt.Sprintf("%dx %q", count, name))
}
sort.Strings(seen)
report.Report(pass, firstFn, fmt.Sprintf("methods on the same type should have the same receiver name (seen %s)", strings.Join(seen, ", ")))
}
}
return nil, nil
}
func CheckContextFirstArg(pass *analysis.Pass) (interface{}, error) {
// TODO(dh): this check doesn't apply to test helpers. Example from the stdlib:
// func helperCommandContext(t *testing.T, ctx context.Context, s ...string) (cmd *exec.Cmd) {
fnLoop:
for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
if fn.Synthetic != 0 || fn.Parent() != nil {
continue
}
params := fn.Signature.Params()
if params.Len() < 2 {
continue
}
if types.TypeString(params.At(0).Type(), nil) == "context.Context" {
continue
}
for i := 1; i < params.Len(); i++ {
param := params.At(i)
if types.TypeString(param.Type(), nil) == "context.Context" {
report.Report(pass, param, "context.Context should be the first argument of a function", report.ShortRange())
continue fnLoop
}
}
}
return nil, nil
}
func CheckErrorStrings(pass *analysis.Pass) (interface{}, error) {
objNames := map[*ir.Package]map[string]bool{}
irpkg := pass.ResultOf[buildir.Analyzer].(*buildir.IR).Pkg
objNames[irpkg] = map[string]bool{}
for _, m := range irpkg.Members {
if typ, ok := m.(*ir.Type); ok {
objNames[irpkg][typ.Name()] = true
}
}
for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
objNames[fn.Package()][fn.Name()] = true
}
for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
if code.IsInTest(pass, fn) {
// We don't care about malformed error messages in tests;
// they're usually for direct human consumption, not part
// of an API
continue
}
for _, block := range fn.Blocks {
instrLoop:
for _, ins := range block.Instrs {
call, ok := ins.(*ir.Call)
if !ok {
continue
}
if !irutil.IsCallToAny(call.Common(), "errors.New", "fmt.Errorf") {
continue
}
k, ok := call.Common().Args[0].(*ir.Const)
if !ok {
continue
}
s := constant.StringVal(k.Value)
if len(s) == 0 {
continue
}
switch s[len(s)-1] {
case '.', ':', '!', '\n':
report.Report(pass, call, "error strings should not end with punctuation or newlines")
}
idx := strings.IndexByte(s, ' ')
if idx == -1 {
// single word error message, probably not a real
// error but something used in tests or during
// debugging
continue
}
word := s[:idx]
first, n := utf8.DecodeRuneInString(word)
if !unicode.IsUpper(first) {
continue
}
for _, c := range word[n:] {
if unicode.IsUpper(c) {
// Word is probably an initialism or
// multi-word function name
continue instrLoop
}
}
if strings.ContainsRune(word, '(') {
// Might be a function call
continue instrLoop
}
word = strings.TrimRightFunc(word, func(r rune) bool { return unicode.IsPunct(r) })
if objNames[fn.Package()][word] {
// Word is probably the name of a function or type in this package
continue
}
// First word in error starts with a capital
// letter, and the word doesn't contain any other
// capitals, making it unlikely to be an
// initialism or multi-word function name.
//
// It could still be a proper noun, though.
report.Report(pass, call, "error strings should not be capitalized")
}
}
}
return nil, nil
}
func CheckTimeNames(pass *analysis.Pass) (interface{}, error) {
suffixes := []string{
"Sec", "Secs", "Seconds",
"Msec", "Msecs",
"Milli", "Millis", "Milliseconds",
"Usec", "Usecs", "Microseconds",
"MS", "Ms",
}
fn := func(names []*ast.Ident) {
for _, name := range names {
if _, ok := pass.TypesInfo.Defs[name]; !ok {
continue
}
T := pass.TypesInfo.TypeOf(name)
if !typeutil.IsType(T, "time.Duration") && !typeutil.IsType(T, "*time.Duration") {
continue
}
for _, suffix := range suffixes {
if strings.HasSuffix(name.Name, suffix) {
report.Report(pass, name, fmt.Sprintf("var %s is of type %v; don't use unit-specific suffix %q", name.Name, T, suffix))
break
}
}
}
}
fn2 := func(node ast.Node) {
switch node := node.(type) {
case *ast.ValueSpec:
fn(node.Names)
case *ast.FieldList:
for _, field := range node.List {
fn(field.Names)
}
case *ast.AssignStmt:
if node.Tok != token.DEFINE {
break
}
var names []*ast.Ident
for _, lhs := range node.Lhs {
if lhs, ok := lhs.(*ast.Ident); ok {
names = append(names, lhs)
}
}
fn(names)
}
}
code.Preorder(pass, fn2, (*ast.ValueSpec)(nil), (*ast.FieldList)(nil), (*ast.AssignStmt)(nil))
return nil, nil
}
func CheckErrorVarNames(pass *analysis.Pass) (interface{}, error) {
for _, f := range pass.Files {
for _, decl := range f.Decls {
gen, ok := decl.(*ast.GenDecl)
if !ok || gen.Tok != token.VAR {
continue
}
for _, spec := range gen.Specs {
spec := spec.(*ast.ValueSpec)
if len(spec.Names) != len(spec.Values) {
continue
}
for i, name := range spec.Names {
val := spec.Values[i]
if !code.IsCallToAny(pass, val, "errors.New", "fmt.Errorf") {
continue
}
if pass.Pkg.Path() == "net/http" && strings.HasPrefix(name.Name, "http2err") {
// special case for internal variable names of
// bundled HTTP 2 code in net/http
continue
}
prefix := "err"
if name.IsExported() {
prefix = "Err"
}
if !strings.HasPrefix(name.Name, prefix) {
report.Report(pass, name, fmt.Sprintf("error var %s should have name of the form %sFoo", name.Name, prefix))
}
}
}
}
}
return nil, nil
}
var httpStatusCodes = map[int64]string{
100: "StatusContinue",
101: "StatusSwitchingProtocols",
102: "StatusProcessing",
200: "StatusOK",
201: "StatusCreated",
202: "StatusAccepted",
203: "StatusNonAuthoritativeInfo",
204: "StatusNoContent",
205: "StatusResetContent",
206: "StatusPartialContent",
207: "StatusMultiStatus",
208: "StatusAlreadyReported",
226: "StatusIMUsed",
300: "StatusMultipleChoices",
301: "StatusMovedPermanently",
302: "StatusFound",
303: "StatusSeeOther",
304: "StatusNotModified",
305: "StatusUseProxy",
307: "StatusTemporaryRedirect",
308: "StatusPermanentRedirect",
400: "StatusBadRequest",
401: "StatusUnauthorized",
402: "StatusPaymentRequired",
403: "StatusForbidden",
404: "StatusNotFound",
405: "StatusMethodNotAllowed",
406: "StatusNotAcceptable",
407: "StatusProxyAuthRequired",
408: "StatusRequestTimeout",
409: "StatusConflict",
410: "StatusGone",
411: "StatusLengthRequired",
412: "StatusPreconditionFailed",
413: "StatusRequestEntityTooLarge",
414: "StatusRequestURITooLong",
415: "StatusUnsupportedMediaType",
416: "StatusRequestedRangeNotSatisfiable",
417: "StatusExpectationFailed",
418: "StatusTeapot",
422: "StatusUnprocessableEntity",
423: "StatusLocked",
424: "StatusFailedDependency",
426: "StatusUpgradeRequired",
428: "StatusPreconditionRequired",
429: "StatusTooManyRequests",
431: "StatusRequestHeaderFieldsTooLarge",
451: "StatusUnavailableForLegalReasons",
500: "StatusInternalServerError",
501: "StatusNotImplemented",
502: "StatusBadGateway",
503: "StatusServiceUnavailable",
504: "StatusGatewayTimeout",
505: "StatusHTTPVersionNotSupported",
506: "StatusVariantAlsoNegotiates",
507: "StatusInsufficientStorage",
508: "StatusLoopDetected",
510: "StatusNotExtended",
511: "StatusNetworkAuthenticationRequired",
}
func CheckHTTPStatusCodes(pass *analysis.Pass) (interface{}, error) {
whitelist := map[string]bool{}
for _, code := range config.For(pass).HTTPStatusCodeWhitelist {
whitelist[code] = true
}
fn := func(node ast.Node) {
call := node.(*ast.CallExpr)
var arg int
switch code.CallName(pass, call) {
case "net/http.Error":
arg = 2
case "net/http.Redirect":
arg = 3
case "net/http.StatusText":
arg = 0
case "net/http.RedirectHandler":
arg = 1
default:
return
}
if arg >= len(call.Args) {
return
}
tv, ok := code.IntegerLiteral(pass, call.Args[arg])
if !ok {
return
}
n, ok := constant.Int64Val(tv.Value)
if !ok {
return
}
if whitelist[strconv.FormatInt(n, 10)] {
return
}
s, ok := httpStatusCodes[n]
if !ok {
return
}
lit := call.Args[arg]
report.Report(pass, lit, fmt.Sprintf("should use constant http.%s instead of numeric literal %d", s, n),
report.FilterGenerated(),
report.Fixes(edit.Fix(fmt.Sprintf("use http.%s instead of %d", s, n), edit.ReplaceWithString(lit, "http."+s))))
}
code.Preorder(pass, fn, (*ast.CallExpr)(nil))
return nil, nil
}
func CheckDefaultCaseOrder(pass *analysis.Pass) (interface{}, error) {
fn := func(node ast.Node) {
stmt := node.(*ast.SwitchStmt)
list := stmt.Body.List
for i, c := range list {
if c.(*ast.CaseClause).List == nil && i != 0 && i != len(list)-1 {
report.Report(pass, c, "default case should be first or last in switch statement", report.FilterGenerated())
break
}
}
}
code.Preorder(pass, fn, (*ast.SwitchStmt)(nil))
return nil, nil
}
var (
checkYodaConditionsQ = pattern.MustParse(`(BinaryExpr left@(TrulyConstantExpression _) tok@(Or "==" "!=") right@(Not (TrulyConstantExpression _)))`)
checkYodaConditionsR = pattern.MustParse(`(BinaryExpr right tok left)`)
)
func CheckYodaConditions(pass *analysis.Pass) (interface{}, error) {
fn := func(node ast.Node) {
if _, edits, ok := code.MatchAndEdit(pass, checkYodaConditionsQ, checkYodaConditionsR, node); ok {
report.Report(pass, node, "don't use Yoda conditions",
report.FilterGenerated(),
report.Fixes(edit.Fix("un-Yoda-fy", edits...)))
}
}
code.Preorder(pass, fn, (*ast.BinaryExpr)(nil))
return nil, nil
}
func CheckInvisibleCharacters(pass *analysis.Pass) (interface{}, error) {
fn := func(node ast.Node) {
lit := node.(*ast.BasicLit)
if lit.Kind != token.STRING {
return
}
type invalid struct {
r rune
off int
}
var invalids []invalid
hasFormat := false
hasControl := false
prev := rune(-1)
const zwj = '\u200d'
for off, r := range lit.Value {
if unicode.Is(unicode.Cf, r) {
// Don't flag joined emojis. These are multiple emojis joined with ZWJ, which some platform render as single composite emojis.
// For the purpose of this check, we consider all symbols, including all symbol modifiers, emoji.
if r != zwj || (r == zwj && !unicode.Is(unicode.S, prev)) {
invalids = append(invalids, invalid{r, off})
hasFormat = true
}
} else if unicode.Is(unicode.Cc, r) && r != '\n' && r != '\t' && r != '\r' {
invalids = append(invalids, invalid{r, off})
hasControl = true
}
prev = r
}
switch len(invalids) {
case 0:
return
case 1:
var kind string
if hasFormat {
kind = "format"
} else if hasControl {
kind = "control"
} else {
panic("unreachable")
}
r := invalids[0]
msg := fmt.Sprintf("string literal contains the Unicode %s character %U, consider using the %q escape sequence instead", kind, r.r, r.r)
replacement := strconv.QuoteRune(r.r)
replacement = replacement[1 : len(replacement)-1]
edit := analysis.SuggestedFix{
Message: fmt.Sprintf("replace %s character %U with %q", kind, r.r, r.r),
TextEdits: []analysis.TextEdit{{
Pos: lit.Pos() + token.Pos(r.off),
End: lit.Pos() + token.Pos(r.off) + token.Pos(utf8.RuneLen(r.r)),
NewText: []byte(replacement),
}},
}
delete := analysis.SuggestedFix{
Message: fmt.Sprintf("delete %s character %U", kind, r.r),
TextEdits: []analysis.TextEdit{{
Pos: lit.Pos() + token.Pos(r.off),
End: lit.Pos() + token.Pos(r.off) + token.Pos(utf8.RuneLen(r.r)),
}},
}
report.Report(pass, lit, msg, report.Fixes(edit, delete))
default:
var kind string
if hasFormat && hasControl {
kind = "format and control"
} else if hasFormat {
kind = "format"
} else if hasControl {
kind = "control"
} else {
panic("unreachable")
}
msg := fmt.Sprintf("string literal contains Unicode %s characters, consider using escape sequences instead", kind)
var edits []analysis.TextEdit
var deletions []analysis.TextEdit
for _, r := range invalids {
replacement := strconv.QuoteRune(r.r)
replacement = replacement[1 : len(replacement)-1]
edits = append(edits, analysis.TextEdit{
Pos: lit.Pos() + token.Pos(r.off),
End: lit.Pos() + token.Pos(r.off) + token.Pos(utf8.RuneLen(r.r)),
NewText: []byte(replacement),
})
deletions = append(deletions, analysis.TextEdit{
Pos: lit.Pos() + token.Pos(r.off),
End: lit.Pos() + token.Pos(r.off) + token.Pos(utf8.RuneLen(r.r)),
})
}
edit := analysis.SuggestedFix{
Message: fmt.Sprintf("replace all %s characters with escape sequences", kind),
TextEdits: edits,
}
delete := analysis.SuggestedFix{
Message: fmt.Sprintf("delete all %s characters", kind),
TextEdits: deletions,
}
report.Report(pass, lit, msg, report.Fixes(edit, delete))
}
}
code.Preorder(pass, fn, (*ast.BasicLit)(nil))
return nil, nil
}
func CheckExportedFunctionDocs(pass *analysis.Pass) (interface{}, error) {
fn := func(node ast.Node) {
if code.IsInTest(pass, node) {
return
}
decl := node.(*ast.FuncDecl)
text, ok := docText(decl.Doc)
if !ok {
return
}
if !ast.IsExported(decl.Name.Name) {
return
}
kind := "function"
if decl.Recv != nil {
kind = "method"
var ident *ast.Ident
T := decl.Recv.List[0].Type
if T_, ok := T.(*ast.StarExpr); ok {
T = T_.X
}
switch T := T.(type) {
case *ast.IndexExpr:
ident = T.X.(*ast.Ident)
case *typeparams.IndexListExpr:
ident = T.X.(*ast.Ident)
case *ast.Ident:
ident = T
default:
lint.ExhaustiveTypeSwitch(T)
}
if !ast.IsExported(ident.Name) {
return
}
}
prefix := decl.Name.Name + " "
if !strings.HasPrefix(text, prefix) {
report.Report(pass, decl.Doc, fmt.Sprintf(`comment on exported %s %s should be of the form "%s..."`, kind, decl.Name.Name, prefix), report.FilterGenerated())
}
}
code.Preorder(pass, fn, (*ast.FuncDecl)(nil))
return nil, nil
}
func CheckExportedTypeDocs(pass *analysis.Pass) (interface{}, error) {
var genDecl *ast.GenDecl
fn := func(node ast.Node, push bool) bool {
if !push {
genDecl = nil
return false
}
if code.IsInTest(pass, node) {
return false
}
switch node := node.(type) {
case *ast.GenDecl:
if node.Tok == token.IMPORT {
return false
}
genDecl = node
return true
case *ast.TypeSpec:
if !ast.IsExported(node.Name.Name) {
return false
}
doc := node.Doc
text, ok := docText(doc)
if !ok {
if len(genDecl.Specs) != 1 {
// more than one spec in the GenDecl, don't validate the
// docstring
return false
}
if genDecl.Lparen.IsValid() {
// 'type ( T )' is weird, don't guess the user's intention
return false
}
doc = genDecl.Doc
text, ok = docText(doc)
if !ok {
return false
}
}
// Check comment before we strip articles in case the type's name is an article.
if strings.HasPrefix(text, node.Name.Name+" ") {
return false
}
s := 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, node.Name.Name+" ") {
report.Report(pass, doc, fmt.Sprintf(`comment on exported type %s should be of the form "%s ..." (with optional leading article)`, node.Name.Name, node.Name.Name), report.FilterGenerated())
}
return false
case *ast.FuncLit, *ast.FuncDecl:
return false
default:
lint.ExhaustiveTypeSwitch(node)
return false
}
}
pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Nodes([]ast.Node{(*ast.GenDecl)(nil), (*ast.TypeSpec)(nil), (*ast.FuncLit)(nil), (*ast.FuncDecl)(nil)}, fn)
return nil, nil
}
func CheckExportedVarDocs(pass *analysis.Pass) (interface{}, error) {
var genDecl *ast.GenDecl
fn := func(node ast.Node, push bool) bool {
if !push {
genDecl = nil
return false
}
if code.IsInTest(pass, node) {
return false
}
switch node := node.(type) {
case *ast.GenDecl:
if node.Tok == token.IMPORT {
return false
}
genDecl = node
return true
case *ast.ValueSpec:
if genDecl.Lparen.IsValid() || len(node.Names) > 1 {
// Don't try to guess the user's intention
return false
}
name := node.Names[0].Name
if !ast.IsExported(name) {
return false
}
text, ok := docText(genDecl.Doc)
if !ok {
return false
}
prefix := name + " "
if !strings.HasPrefix(text, prefix) {
kind := "var"
if genDecl.Tok == token.CONST {
kind = "const"
}
report.Report(pass, genDecl.Doc, fmt.Sprintf(`comment on exported %s %s should be of the form "%s..."`, kind, name, prefix), report.FilterGenerated())
}
return false
case *ast.FuncLit, *ast.FuncDecl:
return false
default:
lint.ExhaustiveTypeSwitch(node)
return false
}
}
pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Nodes([]ast.Node{(*ast.GenDecl)(nil), (*ast.ValueSpec)(nil), (*ast.FuncLit)(nil), (*ast.FuncDecl)(nil)}, fn)
return nil, nil
}

View File

@@ -0,0 +1,281 @@
// Copyright (c) 2013 The Go Authors. All rights reserved.
// Copyright (c) 2018 Dominik Honnef. All rights reserved.
package stylecheck
import (
"fmt"
"go/ast"
"go/token"
"strings"
"unicode"
"honnef.co/go/tools/analysis/code"
"honnef.co/go/tools/analysis/report"
"honnef.co/go/tools/config"
"golang.org/x/tools/go/analysis"
)
// knownNameExceptions is a set of names that are known to be exempt from naming checks.
// This is usually because they are constrained by having to match names in the
// standard library.
var knownNameExceptions = map[string]bool{
"LastInsertId": true, // must match database/sql
"kWh": true,
}
func CheckNames(pass *analysis.Pass) (interface{}, error) {
// A large part of this function is copied from
// github.com/golang/lint, Copyright (c) 2013 The Go Authors,
// licensed under the BSD 3-clause license.
allCaps := func(s string) bool {
hasUppercaseLetters := false
for _, r := range s {
if !hasUppercaseLetters && r >= 'A' && r <= 'Z' {
hasUppercaseLetters = true
}
if !((r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_') {
return false
}
}
return hasUppercaseLetters
}
check := func(id *ast.Ident, thing string, initialisms map[string]bool) {
if id.Name == "_" {
return
}
if knownNameExceptions[id.Name] {
return
}
// Handle two common styles from other languages that don't belong in Go.
if len(id.Name) >= 5 && allCaps(id.Name) && strings.Contains(id.Name, "_") {
report.Report(pass, id, "should not use ALL_CAPS in Go names; use CamelCase instead", report.FilterGenerated())
return
}
should := lintName(id.Name, initialisms)
if id.Name == should {
return
}
if len(id.Name) > 2 && strings.Contains(id.Name[1:len(id.Name)-1], "_") {
report.Report(pass, id, fmt.Sprintf("should not use underscores in Go names; %s %s should be %s", thing, id.Name, should), report.FilterGenerated())
return
}
report.Report(pass, id, fmt.Sprintf("%s %s should be %s", thing, id.Name, should), report.FilterGenerated())
}
checkList := func(fl *ast.FieldList, thing string, initialisms map[string]bool) {
if fl == nil {
return
}
for _, f := range fl.List {
for _, id := range f.Names {
check(id, thing, initialisms)
}
}
}
il := config.For(pass).Initialisms
initialisms := make(map[string]bool, len(il))
for _, word := range il {
initialisms[word] = true
}
for _, f := range pass.Files {
// Package names need slightly different handling than other names.
if !strings.HasSuffix(f.Name.Name, "_test") && strings.Contains(f.Name.Name, "_") {
report.Report(pass, f, "should not use underscores in package names", report.FilterGenerated())
}
if strings.IndexFunc(f.Name.Name, unicode.IsUpper) != -1 {
report.Report(pass, f, fmt.Sprintf("should not use MixedCaps in package name; %s should be %s", f.Name.Name, strings.ToLower(f.Name.Name)), report.FilterGenerated())
}
}
fn := func(node ast.Node) {
switch v := node.(type) {
case *ast.AssignStmt:
if v.Tok != token.DEFINE {
return
}
for _, exp := range v.Lhs {
if id, ok := exp.(*ast.Ident); ok {
check(id, "var", initialisms)
}
}
case *ast.FuncDecl:
// Functions with no body are defined elsewhere (in
// assembly, or via go:linkname). These are likely to
// be something very low level (such as the runtime),
// where our rules don't apply.
if v.Body == nil {
return
}
if code.IsInTest(pass, v) && (strings.HasPrefix(v.Name.Name, "Example") || strings.HasPrefix(v.Name.Name, "Test") || strings.HasPrefix(v.Name.Name, "Benchmark")) {
return
}
thing := "func"
if v.Recv != nil {
thing = "method"
}
if !isTechnicallyExported(v) {
check(v.Name, thing, initialisms)
}
checkList(v.Type.Params, thing+" parameter", initialisms)
checkList(v.Type.Results, thing+" result", initialisms)
case *ast.GenDecl:
if v.Tok == token.IMPORT {
return
}
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, initialisms)
case *ast.ValueSpec:
for _, id := range s.Names {
check(id, thing, initialisms)
}
}
}
case *ast.InterfaceType:
// Do not check interface method names.
// They are often constrained 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", initialisms)
checkList(ft.Results, "interface method result", initialisms)
}
case *ast.RangeStmt:
if v.Tok == token.ASSIGN {
return
}
if id, ok := v.Key.(*ast.Ident); ok {
check(id, "range var", initialisms)
}
if id, ok := v.Value.(*ast.Ident); ok {
check(id, "range var", initialisms)
}
case *ast.StructType:
for _, f := range v.Fields.List {
for _, id := range f.Names {
check(id, "struct field", initialisms)
}
}
}
}
needle := []ast.Node{
(*ast.AssignStmt)(nil),
(*ast.FuncDecl)(nil),
(*ast.GenDecl)(nil),
(*ast.InterfaceType)(nil),
(*ast.RangeStmt)(nil),
(*ast.StructType)(nil),
}
code.Preorder(pass, fn, needle...)
return nil, nil
}
// lintName returns a different name if it should be different.
func lintName(name string, initialisms map[string]bool) (should string) {
// A large part of this function is copied from
// github.com/golang/lint, Copyright (c) 2013 The Go Authors,
// licensed under the BSD 3-clause license.
// Fast path for simple cases: "_" and all lowercase.
if name == "_" {
return name
}
if strings.IndexFunc(name, func(r rune) bool { return !unicode.IsLower(r) }) == -1 {
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] == '_' && i+1 != len(runes)-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); initialisms[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.
// TODO(dh): this won't be true once we allow custom initialisms
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)
}
func isTechnicallyExported(f *ast.FuncDecl) bool {
if f.Recv != nil || f.Doc == nil {
return false
}
const export = "//export "
const linkname = "//go:linkname "
for _, c := range f.Doc.List {
if strings.HasPrefix(c.Text, export) && len(c.Text) == len(export)+len(f.Name.Name) && c.Text[len(export):] == f.Name.Name {
return true
}
if strings.HasPrefix(c.Text, linkname) {
return true
}
}
return false
}