1
0

Update and fix staticcheck

This commit is contained in:
kolaente
2020-05-29 22:15:21 +02:00
parent aae1bc3cab
commit a525787ab7
100 changed files with 12353 additions and 7912 deletions

View File

@ -1,111 +1,81 @@
package stylecheck
import (
"flag"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"honnef.co/go/tools/config"
"honnef.co/go/tools/facts"
"honnef.co/go/tools/internal/passes/buildssa"
"honnef.co/go/tools/internal/passes/buildir"
"honnef.co/go/tools/lint/lintutil"
)
func newFlagSet() flag.FlagSet {
fs := flag.NewFlagSet("", flag.PanicOnError)
fs.Var(lintutil.NewVersionFlag(), "go", "Target Go version")
return *fs
}
var Analyzers = map[string]*analysis.Analyzer{
var Analyzers = lintutil.InitializeAnalyzers(Docs, map[string]*analysis.Analyzer{
"ST1000": {
Name: "ST1000",
Run: CheckPackageComment,
Doc: Docs["ST1000"].String(),
Requires: []*analysis.Analyzer{},
Flags: newFlagSet(),
Run: CheckPackageComment,
},
"ST1001": {
Name: "ST1001",
Run: CheckDotImports,
Doc: Docs["ST1001"].String(),
Requires: []*analysis.Analyzer{facts.Generated, config.Analyzer},
Flags: newFlagSet(),
},
"ST1003": {
Name: "ST1003",
Run: CheckNames,
Doc: Docs["ST1003"].String(),
Requires: []*analysis.Analyzer{facts.Generated, config.Analyzer},
Flags: newFlagSet(),
Requires: []*analysis.Analyzer{inspect.Analyzer, facts.Generated, config.Analyzer},
},
"ST1005": {
Name: "ST1005",
Run: CheckErrorStrings,
Doc: Docs["ST1005"].String(),
Requires: []*analysis.Analyzer{buildssa.Analyzer},
Flags: newFlagSet(),
Requires: []*analysis.Analyzer{buildir.Analyzer},
},
"ST1006": {
Name: "ST1006",
Run: CheckReceiverNames,
Doc: Docs["ST1006"].String(),
Requires: []*analysis.Analyzer{buildssa.Analyzer, facts.Generated},
Flags: newFlagSet(),
Requires: []*analysis.Analyzer{buildir.Analyzer, facts.Generated},
},
"ST1008": {
Name: "ST1008",
Run: CheckErrorReturn,
Doc: Docs["ST1008"].String(),
Requires: []*analysis.Analyzer{buildssa.Analyzer},
Flags: newFlagSet(),
Requires: []*analysis.Analyzer{buildir.Analyzer},
},
"ST1011": {
Name: "ST1011",
Run: CheckTimeNames,
Doc: Docs["ST1011"].String(),
Flags: newFlagSet(),
Run: CheckTimeNames,
Requires: []*analysis.Analyzer{inspect.Analyzer},
},
"ST1012": {
Name: "ST1012",
Run: CheckErrorVarNames,
Doc: Docs["ST1012"].String(),
Requires: []*analysis.Analyzer{config.Analyzer},
Flags: newFlagSet(),
},
"ST1013": {
Name: "ST1013",
Run: CheckHTTPStatusCodes,
Doc: Docs["ST1013"].String(),
Requires: []*analysis.Analyzer{facts.Generated, facts.TokenFile, config.Analyzer},
Flags: newFlagSet(),
Run: CheckHTTPStatusCodes,
// TODO(dh): why does this depend on facts.TokenFile?
Requires: []*analysis.Analyzer{facts.Generated, facts.TokenFile, config.Analyzer, inspect.Analyzer},
},
"ST1015": {
Name: "ST1015",
Run: CheckDefaultCaseOrder,
Doc: Docs["ST1015"].String(),
Requires: []*analysis.Analyzer{inspect.Analyzer, facts.Generated, facts.TokenFile},
Flags: newFlagSet(),
},
"ST1016": {
Name: "ST1016",
Run: CheckReceiverNamesIdentical,
Doc: Docs["ST1016"].String(),
Requires: []*analysis.Analyzer{buildssa.Analyzer},
Flags: newFlagSet(),
Requires: []*analysis.Analyzer{buildir.Analyzer, facts.Generated},
},
"ST1017": {
Name: "ST1017",
Run: CheckYodaConditions,
Doc: Docs["ST1017"].String(),
Requires: []*analysis.Analyzer{inspect.Analyzer, facts.Generated, facts.TokenFile},
Flags: newFlagSet(),
},
"ST1018": {
Name: "ST1018",
Run: CheckInvisibleCharacters,
Doc: Docs["ST1018"].String(),
Requires: []*analysis.Analyzer{inspect.Analyzer},
Flags: newFlagSet(),
},
}
"ST1019": {
Run: CheckDuplicatedImports,
Requires: []*analysis.Analyzer{facts.Generated, config.Analyzer},
},
"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},
},
})

View File

@ -3,7 +3,7 @@ package stylecheck
import "honnef.co/go/tools/lint"
var Docs = map[string]*lint.Documentation{
"ST1000": &lint.Documentation{
"ST1000": {
Title: `Incorrect or missing package comment`,
Text: `Packages must have a package comment that is formatted according to
the guidelines laid out in
@ -12,7 +12,7 @@ https://github.com/golang/go/wiki/CodeReviewComments#package-comments.`,
NonDefault: true,
},
"ST1001": &lint.Documentation{
"ST1001": {
Title: `Dot imports are discouraged`,
Text: `Dot imports that aren't in external test packages are discouraged.
@ -42,7 +42,7 @@ Quoting Go Code Review Comments:
Options: []string{"dot_import_whitelist"},
},
"ST1003": &lint.Documentation{
"ST1003": {
Title: `Poorly chosen identifier`,
Text: `Identifiers, such as variable and package names, follow certain rules.
@ -57,7 +57,7 @@ See the following links for details:
Options: []string{"initialisms"},
},
"ST1005": &lint.Documentation{
"ST1005": {
Title: `Incorrectly formatted error string`,
Text: `Error strings follow a set of guidelines to ensure uniformity and good
composability.
@ -73,7 +73,7 @@ Quoting Go Code Review Comments:
Since: "2019.1",
},
"ST1006": &lint.Documentation{
"ST1006": {
Title: `Poorly chosen receiver name`,
Text: `Quoting Go Code Review Comments:
@ -91,13 +91,13 @@ Quoting Go Code Review Comments:
Since: "2019.1",
},
"ST1008": &lint.Documentation{
"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`,
},
"ST1011": &lint.Documentation{
"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
@ -107,14 +107,14 @@ Milli.`,
Since: `2019.1`,
},
"ST1012": &lint.Documentation{
"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",
},
"ST1013": &lint.Documentation{
"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
@ -126,18 +126,18 @@ readability of your code.`,
Options: []string{"http_status_code_whitelist"},
},
"ST1015": &lint.Documentation{
"ST1015": {
Title: `A switch's default case should be the first or last case`,
Since: "2019.1",
},
"ST1016": &lint.Documentation{
"ST1016": {
Title: `Use consistent method receiver names`,
Since: "2019.1",
NonDefault: true,
},
"ST1017": &lint.Documentation{
"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
@ -147,8 +147,85 @@ bug, we prefer the more idiomatic 'if x == 42'.`,
Since: "2019.2",
},
"ST1018": &lint.Documentation{
"ST1018": {
Title: `Avoid zero-width and control characters in string literals`,
Since: "2019.2",
},
"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",
},
"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,
},
"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,
},
"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,
},
}

View File

@ -6,15 +6,20 @@ import (
"go/constant"
"go/token"
"go/types"
"sort"
"strconv"
"strings"
"unicode"
"unicode/utf8"
"honnef.co/go/tools/code"
"honnef.co/go/tools/config"
"honnef.co/go/tools/internal/passes/buildssa"
"honnef.co/go/tools/edit"
"honnef.co/go/tools/internal/passes/buildir"
"honnef.co/go/tools/ir"
. "honnef.co/go/tools/lint/lintdsl"
"honnef.co/go/tools/ssa"
"honnef.co/go/tools/pattern"
"honnef.co/go/tools/report"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
@ -36,14 +41,14 @@ func CheckPackageComment(pass *analysis.Pass) (interface{}, error) {
}
hasDocs := false
for _, f := range pass.Files {
if IsInTest(pass, f) {
if code.IsInTest(pass, f) {
continue
}
if f.Doc != nil && len(f.Doc.List) > 0 {
hasDocs = true
prefix := "Package " + f.Name.Name + " "
if !strings.HasPrefix(strings.TrimSpace(f.Doc.Text()), prefix) {
ReportNodef(pass, f.Doc, `package comment should be of the form "%s..."`, prefix)
report.Report(pass, f.Doc, fmt.Sprintf(`package comment should be of the form "%s..."`, prefix))
}
f.Doc.Text()
}
@ -51,10 +56,10 @@ func CheckPackageComment(pass *analysis.Pass) (interface{}, error) {
if !hasDocs {
for _, f := range pass.Files {
if IsInTest(pass, f) {
if code.IsInTest(pass, f) {
continue
}
ReportNodef(pass, f, "at least one file in a package should have a package comment")
report.Report(pass, f, "at least one file in a package should have a package comment", report.ShortRange())
}
}
return nil, nil
@ -72,8 +77,38 @@ func CheckDotImports(pass *analysis.Pass) (interface{}, error) {
}
}
if imp.Name != nil && imp.Name.Name == "." && !IsInTest(pass, f) {
ReportNodefFG(pass, imp, "should not use dot 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...)
}
}
}
@ -83,7 +118,7 @@ func CheckDotImports(pass *analysis.Pass) (interface{}, error) {
func CheckBlankImports(pass *analysis.Pass) (interface{}, error) {
fset := pass.Fset
for _, f := range pass.Files {
if IsInMain(pass, f) || IsInTest(pass, f) {
if code.IsMainLike(pass) || code.IsInTest(pass, f) {
continue
}
@ -117,7 +152,7 @@ func CheckBlankImports(pass *analysis.Pass) (interface{}, error) {
for i, imp := range f.Imports {
pos := fset.Position(imp.Pos())
if !IsBlank(imp.Name) {
if !code.IsBlank(imp.Name) {
continue
}
// Only flag the first blank import in a group of imports,
@ -126,13 +161,13 @@ func CheckBlankImports(pass *analysis.Pass) (interface{}, error) {
if i > 0 {
prev := f.Imports[i-1]
prevPos := fset.Position(prev.Pos())
if pos.Line-1 == prevPos.Line && IsBlank(prev.Name) {
if pos.Line-1 == prevPos.Line && code.IsBlank(prev.Name) {
continue
}
}
if imp.Doc == nil && imp.Comment == nil && !skip[imp] {
ReportNodef(pass, imp, "a blank import should be only in a main or test package, or have a comment justifying it")
report.Report(pass, imp, "a blank import should be only in a main or test package, or have a comment justifying it")
}
}
}
@ -152,7 +187,7 @@ func CheckIncDec(pass *analysis.Pass) (interface{}, error) {
return
}
if (len(assign.Lhs) != 1 || len(assign.Rhs) != 1) ||
!IsIntLiteral(assign.Rhs[0], "1") {
!code.IsIntLiteral(assign.Rhs[0], "1") {
return
}
@ -164,15 +199,15 @@ func CheckIncDec(pass *analysis.Pass) (interface{}, error) {
suffix = "--"
}
ReportNodef(pass, assign, "should replace %s with %s%s", Render(pass, assign), Render(pass, assign.Lhs[0]), suffix)
report.Report(pass, assign, fmt.Sprintf("should replace %s with %s%s", report.Render(pass, assign), report.Render(pass, assign.Lhs[0]), suffix))
}
pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Preorder([]ast.Node{(*ast.AssignStmt)(nil)}, fn)
code.Preorder(pass, fn, (*ast.AssignStmt)(nil))
return nil, nil
}
func CheckErrorReturn(pass *analysis.Pass) (interface{}, error) {
fnLoop:
for _, fn := range pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs {
for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
sig := fn.Type().(*types.Signature)
rets := sig.Results()
if rets == nil || rets.Len() < 2 {
@ -186,7 +221,7 @@ fnLoop:
}
for i := rets.Len() - 2; i >= 0; i-- {
if rets.At(i).Type() == types.Universe.Lookup("error").Type() {
pass.Reportf(rets.At(i).Pos(), "error should be returned as the last argument")
report.Report(pass, rets.At(i), "error should be returned as the last argument", report.ShortRange())
continue fnLoop
}
}
@ -197,23 +232,23 @@ fnLoop:
// 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[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs {
for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
if fn.Synthetic != "" || fn.Parent() != nil {
continue
}
if !ast.IsExported(fn.Name()) || IsInMain(pass, fn) || IsInTest(pass, fn) {
if !ast.IsExported(fn.Name()) || code.IsMain(pass) || code.IsInTest(pass, fn) {
continue
}
sig := fn.Type().(*types.Signature)
if sig.Recv() != nil && !ast.IsExported(Dereference(sig.Recv().Type()).(*types.Named).Obj().Name()) {
if sig.Recv() != nil && !ast.IsExported(code.Dereference(sig.Recv().Type()).(*types.Named).Obj().Name()) {
continue
}
res := sig.Results()
for i := 0; i < res.Len(); i++ {
if named, ok := DereferenceR(res.At(i).Type()).(*types.Named); ok &&
if named, ok := code.DereferenceR(res.At(i).Type()).(*types.Named); ok &&
!ast.IsExported(named.Obj().Name()) &&
named != types.Universe.Lookup("error").Type() {
pass.Reportf(fn.Pos(), "should not return unexported type")
report.Report(pass, fn, "should not return unexported type")
}
}
}
@ -221,22 +256,22 @@ func CheckUnexportedReturn(pass *analysis.Pass) (interface{}, error) {
}
func CheckReceiverNames(pass *analysis.Pass) (interface{}, error) {
ssapkg := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).Pkg
for _, m := range ssapkg.Members {
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 Dereference(recv.Type()) != T.Type() {
if code.Dereference(recv.Type()) != T.Type() {
// skip embedded methods
continue
}
if recv.Name() == "self" || recv.Name() == "this" {
ReportfFG(pass, recv.Pos(), `receiver name should be a reflection of its identity; don't use generic names such as "this" or "self"`)
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() == "_" {
ReportfFG(pass, recv.Pos(), "receiver name should not be an underscore, omit the name if it is unused")
report.Report(pass, recv, "receiver name should not be an underscore, omit the name if it is unused", report.FilterGenerated())
}
}
}
@ -245,8 +280,8 @@ func CheckReceiverNames(pass *analysis.Pass) (interface{}, error) {
}
func CheckReceiverNamesIdentical(pass *analysis.Pass) (interface{}, error) {
ssapkg := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).Pkg
for _, m := range ssapkg.Members {
irpkg := pass.ResultOf[buildir.Analyzer].(*buildir.IR).Pkg
for _, m := range irpkg.Members {
names := map[string]int{}
var firstFn *types.Func
@ -255,7 +290,11 @@ func CheckReceiverNamesIdentical(pass *analysis.Pass) (interface{}, error) {
for _, sel := range ms {
fn := sel.Obj().(*types.Func)
recv := fn.Type().(*types.Signature).Recv()
if Dereference(recv.Type()) != T.Type() {
if code.IsGenerated(pass, recv.Pos()) {
// Don't concern ourselves with methods in generated code
continue
}
if code.Dereference(recv.Type()) != T.Type() {
// skip embedded methods
continue
}
@ -273,8 +312,9 @@ func CheckReceiverNamesIdentical(pass *analysis.Pass) (interface{}, error) {
for name, count := range names {
seen = append(seen, fmt.Sprintf("%dx %q", count, name))
}
sort.Strings(seen)
pass.Reportf(firstFn.Pos(), "methods on the same type should have the same receiver name (seen %s)", strings.Join(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
@ -284,7 +324,7 @@ 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[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs {
for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
if fn.Synthetic != "" || fn.Parent() != nil {
continue
}
@ -298,7 +338,7 @@ fnLoop:
for i := 1; i < params.Len(); i++ {
param := params.At(i)
if types.TypeString(param.Type(), nil) == "context.Context" {
pass.Reportf(param.Pos(), "context.Context should be the first argument of a function")
report.Report(pass, param, "context.Context should be the first argument of a function", report.ShortRange())
continue fnLoop
}
}
@ -307,20 +347,20 @@ fnLoop:
}
func CheckErrorStrings(pass *analysis.Pass) (interface{}, error) {
objNames := map[*ssa.Package]map[string]bool{}
ssapkg := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).Pkg
objNames[ssapkg] = map[string]bool{}
for _, m := range ssapkg.Members {
if typ, ok := m.(*ssa.Type); ok {
objNames[ssapkg][typ.Name()] = true
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[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs {
for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
objNames[fn.Package()][fn.Name()] = true
}
for _, fn := range pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs {
if IsInTest(pass, fn) {
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
@ -329,15 +369,15 @@ func CheckErrorStrings(pass *analysis.Pass) (interface{}, error) {
for _, block := range fn.Blocks {
instrLoop:
for _, ins := range block.Instrs {
call, ok := ins.(*ssa.Call)
call, ok := ins.(*ir.Call)
if !ok {
continue
}
if !IsCallTo(call.Common(), "errors.New") && !IsCallTo(call.Common(), "fmt.Errorf") {
if !code.IsCallToAny(call.Common(), "errors.New", "fmt.Errorf") {
continue
}
k, ok := call.Common().Args[0].(*ssa.Const)
k, ok := call.Common().Args[0].(*ir.Const)
if !ok {
continue
}
@ -348,7 +388,7 @@ func CheckErrorStrings(pass *analysis.Pass) (interface{}, error) {
}
switch s[len(s)-1] {
case '.', ':', '!', '\n':
pass.Reportf(call.Pos(), "error strings should not end with punctuation or a newline")
report.Report(pass, call, "error strings should not end with punctuation or a newline")
}
idx := strings.IndexByte(s, ' ')
if idx == -1 {
@ -382,7 +422,7 @@ func CheckErrorStrings(pass *analysis.Pass) (interface{}, error) {
//
// It could still be a proper noun, though.
pass.Reportf(call.Pos(), "error strings should not be capitalized")
report.Report(pass, call, "error strings should not be capitalized")
}
}
}
@ -397,34 +437,47 @@ func CheckTimeNames(pass *analysis.Pass) (interface{}, error) {
"Usec", "Usecs", "Microseconds",
"MS", "Ms",
}
fn := func(T types.Type, names []*ast.Ident) {
if !IsType(T, "time.Duration") && !IsType(T, "*time.Duration") {
return
}
fn := func(names []*ast.Ident) {
for _, name := range names {
if _, ok := pass.TypesInfo.Defs[name]; !ok {
continue
}
T := pass.TypesInfo.TypeOf(name)
if !code.IsType(T, "time.Duration") && !code.IsType(T, "*time.Duration") {
continue
}
for _, suffix := range suffixes {
if strings.HasSuffix(name.Name, suffix) {
ReportNodef(pass, name, "var %s is of type %v; don't use unit-specific suffix %q", name.Name, T, 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
}
}
}
}
for _, f := range pass.Files {
ast.Inspect(f, func(node ast.Node) bool {
switch node := node.(type) {
case *ast.ValueSpec:
T := pass.TypesInfo.TypeOf(node.Type)
fn(T, node.Names)
case *ast.FieldList:
for _, field := range node.List {
T := pass.TypesInfo.TypeOf(field.Type)
fn(T, field.Names)
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)
}
}
return true
})
fn(names)
}
}
code.Preorder(pass, fn2, (*ast.ValueSpec)(nil), (*ast.FieldList)(nil), (*ast.AssignStmt)(nil))
return nil, nil
}
@ -443,16 +496,21 @@ func CheckErrorVarNames(pass *analysis.Pass) (interface{}, error) {
for i, name := range spec.Names {
val := spec.Values[i]
if !IsCallToAST(pass, val, "errors.New") && !IsCallToAST(pass, val, "fmt.Errorf") {
if !code.IsCallToAnyAST(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) {
ReportNodef(pass, name, "error var %s should have name of the form %sFoo", name.Name, prefix)
report.Report(pass, name, fmt.Sprintf("error var %s should have name of the form %sFoo", name.Name, prefix))
}
}
}
@ -528,17 +586,11 @@ func CheckHTTPStatusCodes(pass *analysis.Pass) (interface{}, error) {
for _, code := range config.For(pass).HTTPStatusCodeWhitelist {
whitelist[code] = true
}
fn := func(node ast.Node) bool {
if node == nil {
return true
}
call, ok := node.(*ast.CallExpr)
if !ok {
return true
}
fn := func(node ast.Node) {
call := node.(*ast.CallExpr)
var arg int
switch CallNameAST(pass, call) {
switch code.CallNameAST(pass, call) {
case "net/http.Error":
arg = 2
case "net/http.Redirect":
@ -548,31 +600,29 @@ func CheckHTTPStatusCodes(pass *analysis.Pass) (interface{}, error) {
case "net/http.RedirectHandler":
arg = 1
default:
return true
return
}
lit, ok := call.Args[arg].(*ast.BasicLit)
if !ok {
return true
return
}
if whitelist[lit.Value] {
return true
return
}
n, err := strconv.Atoi(lit.Value)
if err != nil {
return true
return
}
s, ok := httpStatusCodes[n]
if !ok {
return true
return
}
ReportNodefFG(pass, lit, "should use constant http.%s instead of numeric literal %d", s, n)
return true
}
// OPT(dh): replace with inspector
for _, f := range pass.Files {
ast.Inspect(f, fn)
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(pass.Fset, lit, "http."+s))))
}
code.Preorder(pass, fn, (*ast.CallExpr)(nil))
return nil, nil
}
@ -582,31 +632,29 @@ func CheckDefaultCaseOrder(pass *analysis.Pass) (interface{}, error) {
list := stmt.Body.List
for i, c := range list {
if c.(*ast.CaseClause).List == nil && i != 0 && i != len(list)-1 {
ReportNodefFG(pass, c, "default case should be first or last in switch statement")
report.Report(pass, c, "default case should be first or last in switch statement", report.FilterGenerated())
break
}
}
}
pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Preorder([]ast.Node{(*ast.SwitchStmt)(nil)}, fn)
code.Preorder(pass, fn, (*ast.SwitchStmt)(nil))
return nil, nil
}
var (
checkYodaConditionsQ = pattern.MustParse(`(BinaryExpr left@(BasicLit _ _) tok@(Or "==" "!=") right@(Not (BasicLit _ _)))`)
checkYodaConditionsR = pattern.MustParse(`(BinaryExpr right tok left)`)
)
func CheckYodaConditions(pass *analysis.Pass) (interface{}, error) {
fn := func(node ast.Node) {
cond := node.(*ast.BinaryExpr)
if cond.Op != token.EQL && cond.Op != token.NEQ {
return
if _, edits, ok := 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...)))
}
if _, ok := cond.X.(*ast.BasicLit); !ok {
return
}
if _, ok := cond.Y.(*ast.BasicLit); ok {
// Don't flag lit == lit conditions, just in case
return
}
ReportNodefFG(pass, cond, "don't use Yoda conditions")
}
pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Preorder([]ast.Node{(*ast.BinaryExpr)(nil)}, fn)
code.Preorder(pass, fn, (*ast.BinaryExpr)(nil))
return nil, nil
}
@ -616,14 +664,251 @@ func CheckInvisibleCharacters(pass *analysis.Pass) (interface{}, error) {
if lit.Kind != token.STRING {
return
}
for _, r := range lit.Value {
type invalid struct {
r rune
off int
}
var invalids []invalid
hasFormat := false
hasControl := false
for off, r := range lit.Value {
if unicode.Is(unicode.Cf, r) {
ReportNodef(pass, lit, "string literal contains the Unicode format character %U, consider using the %q escape sequence", r, r)
invalids = append(invalids, invalid{r, off})
hasFormat = true
} else if unicode.Is(unicode.Cc, r) && r != '\n' && r != '\t' && r != '\r' {
ReportNodef(pass, lit, "string literal contains the Unicode control character %U, consider using the %q escape sequence", r, r)
invalids = append(invalids, invalid{r, off})
hasControl = true
}
}
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),
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))
}
}
pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Preorder([]ast.Node{(*ast.BasicLit)(nil)}, fn)
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)
if decl.Doc == nil {
return
}
if !ast.IsExported(decl.Name.Name) {
return
}
kind := "function"
if decl.Recv != nil {
kind = "method"
switch T := decl.Recv.List[0].Type.(type) {
case *ast.StarExpr:
if !ast.IsExported(T.X.(*ast.Ident).Name) {
return
}
case *ast.Ident:
if !ast.IsExported(T.Name) {
return
}
default:
ExhaustiveTypeSwitch(T)
}
}
prefix := decl.Name.Name + " "
if !strings.HasPrefix(decl.Doc.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
if doc == nil {
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
if doc == nil {
return false
}
}
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, 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:
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
}
if genDecl.Doc == nil {
return false
}
prefix := name + " "
if !strings.HasPrefix(genDecl.Doc.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:
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

@ -4,14 +4,16 @@
package stylecheck
import (
"fmt"
"go/ast"
"go/token"
"strings"
"unicode"
"golang.org/x/tools/go/analysis"
"honnef.co/go/tools/code"
"honnef.co/go/tools/config"
. "honnef.co/go/tools/lint/lintdsl"
"honnef.co/go/tools/report"
)
// knownNameExceptions is a set of names that are known to be exempt from naming checks.
@ -46,7 +48,7 @@ func CheckNames(pass *analysis.Pass) (interface{}, error) {
// 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, "_") {
ReportfFG(pass, id.Pos(), "should not use ALL_CAPS in Go names; use CamelCase instead")
report.Report(pass, id, "should not use ALL_CAPS in Go names; use CamelCase instead", report.FilterGenerated())
return
}
@ -56,10 +58,10 @@ func CheckNames(pass *analysis.Pass) (interface{}, error) {
}
if len(id.Name) > 2 && strings.Contains(id.Name[1:len(id.Name)-1], "_") {
ReportfFG(pass, id.Pos(), "should not use underscores in Go names; %s %s should be %s", thing, id.Name, should)
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
}
ReportfFG(pass, id.Pos(), "%s %s should be %s", thing, id.Name, should)
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 {
@ -80,101 +82,111 @@ func CheckNames(pass *analysis.Pass) (interface{}, error) {
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, "_") {
ReportfFG(pass, f.Pos(), "should not use underscores in package names")
report.Report(pass, f, "should not use underscores in package names", report.FilterGenerated())
}
if strings.IndexFunc(f.Name.Name, unicode.IsUpper) != -1 {
ReportfFG(pass, f.Pos(), "should not use MixedCaps in package name; %s should be %s", f.Name.Name, strings.ToLower(f.Name.Name))
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())
}
}
ast.Inspect(f, func(node ast.Node) bool {
switch v := node.(type) {
case *ast.AssignStmt:
if v.Tok != token.DEFINE {
return true
}
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 true
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 IsInTest(pass, v) && (strings.HasPrefix(v.Name.Name, "Example") || strings.HasPrefix(v.Name.Name, "Test") || strings.HasPrefix(v.Name.Name, "Benchmark")) {
return true
}
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"
}
thing := "func"
if v.Recv != nil {
thing = "method"
}
if !isTechnicallyExported(v) {
check(v.Name, thing, initialisms)
}
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 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, 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 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", initialisms)
checkList(ft.Results, "interface method result", initialisms)
}
case *ast.RangeStmt:
if v.Tok == token.ASSIGN {
return true
}
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)
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)
}
}
}
return true
})
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
}