diff --git a/go.mod b/go.mod index 156ec3d3..94c6ade3 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/golangci/dupl v0.0.0-20180902072040-3e9179ac440a github.com/golangci/errcheck v0.0.0-20181003203344-ef45e06d44b6 github.com/golangci/go-misc v0.0.0-20180628070357-927a3d87b613 - github.com/golangci/go-tools v0.0.0-20180109140146-35a9f45a5db0 + github.com/golangci/go-tools v0.0.0-20180109140146-af6baa5dc196 github.com/golangci/goconst v0.0.0-20180610141641-041c5f2b40f3 github.com/golangci/gocyclo v0.0.0-20180528134321-2becd97e67ee github.com/golangci/gofmt v0.0.0-20181105071733-0b8337e80d98 diff --git a/go.sum b/go.sum index e89b4579..6e328cda 100644 --- a/go.sum +++ b/go.sum @@ -48,8 +48,8 @@ github.com/golangci/errcheck v0.0.0-20181003203344-ef45e06d44b6 h1:i2jIkQFb8RG45 github.com/golangci/errcheck v0.0.0-20181003203344-ef45e06d44b6/go.mod h1:DbHgvLiFKX1Sh2T1w8Q/h4NAI8MHIpzCdnBUDTXU3I0= github.com/golangci/go-misc v0.0.0-20180628070357-927a3d87b613 h1:9kfjN3AdxcbsZBf8NjltjWihK2QfBBBZuv91cMFfDHw= github.com/golangci/go-misc v0.0.0-20180628070357-927a3d87b613/go.mod h1:SyvUF2NxV+sN8upjjeVYr5W7tyxaT1JVtvhKhOn2ii8= -github.com/golangci/go-tools v0.0.0-20180109140146-35a9f45a5db0 h1:tYc7NX0EeSyW9wFF0EXPB57lAiKRPwKZxVkaTsGuB9k= -github.com/golangci/go-tools v0.0.0-20180109140146-35a9f45a5db0/go.mod h1:unzUULGw35sjyOYjUt0jMTXqHlZPpPc6e+xfO4cd6mM= +github.com/golangci/go-tools v0.0.0-20180109140146-af6baa5dc196 h1:9rtVlONXLF1rJZzvLt4tfOXtnAFUEhxCJ64Ibzj6ECo= +github.com/golangci/go-tools v0.0.0-20180109140146-af6baa5dc196/go.mod h1:unzUULGw35sjyOYjUt0jMTXqHlZPpPc6e+xfO4cd6mM= github.com/golangci/goconst v0.0.0-20180610141641-041c5f2b40f3 h1:pe9JHs3cHHDQgOFXJJdYkK6fLz2PWyYtP4hthoCMvs8= github.com/golangci/goconst v0.0.0-20180610141641-041c5f2b40f3/go.mod h1:JXrF4TWy4tXYn62/9x8Wm/K/dm06p8tCKwFRDPZG/1o= github.com/golangci/gocyclo v0.0.0-20180528134321-2becd97e67ee h1:J2XAy40+7yz70uaOiMbNnluTg7gyQhtGqLQncQh+4J8= diff --git a/test/testdata/gosimple.go b/test/testdata/gosimple.go index cc183033..258402da 100644 --- a/test/testdata/gosimple.go +++ b/test/testdata/gosimple.go @@ -1,25 +1,30 @@ //args: -Egosimple package testdata -import "strings" +import ( + "log" +) -func Gosimple(id1, s1 string) string { - if strings.HasPrefix(id1, s1) { // ERROR "should replace.*with.*strings.TrimPrefix" - id1 = strings.TrimPrefix(id1, s1) +func Gosimple(ss []string) { + if ss != nil { // ERROR "S1031: unnecessary nil check around range" + for _, s := range ss { + log.Printf(s) + } } - return id1 } -func GosimpleNolintGosimple(id1, s1 string) string { - if strings.HasPrefix(id1, s1) { //nolint:gosimple - id1 = strings.TrimPrefix(id1, s1) +func GosimpleNolintGosimple(ss []string) { + if ss != nil { //nolint:gosimple + for _, s := range ss { + log.Printf(s) + } } - return id1 } -func GosimpleNolintMegacheck(id1, s1 string) string { - if strings.HasPrefix(id1, s1) { //nolint:megacheck - id1 = strings.TrimPrefix(id1, s1) +func GosimpleNolintMegacheck(ss []string) { + if ss != nil { //nolint:megacheck + for _, s := range ss { + log.Printf(s) + } } - return id1 } diff --git a/test/testdata/stylecheck.go b/test/testdata/stylecheck.go index 20ebad36..a0d73053 100644 --- a/test/testdata/stylecheck.go +++ b/test/testdata/stylecheck.go @@ -2,19 +2,34 @@ package testdata func Stylecheck(x int) { - if 0 == x { // ERROR "don't use Yoda conditions" - panic(x) + switch x { + case 1: + return + default: // ERROR "ST1015: default case should be first or last in switch statement" + return + case 2: + return } } func StylecheckNolintStylecheck(x int) { - if 0 == x { //nolint:stylecheck - panic(x) + switch x { + case 1: + return + default: //nolint:stylecheck + return + case 2: + return } } func StylecheckNolintMegacheck(x int) { - if 0 == x { //nolint:megacheck // ERROR "don't use Yoda conditions" - panic(x) + switch x { + case 1: + return + default: //nolint:megacheck // ERROR "ST1015: default case should be first or last in switch statement" + return + case 2: + return } } diff --git a/vendor/github.com/golangci/go-tools/arg/arg.go b/vendor/github.com/golangci/go-tools/arg/arg.go index 6f3aafc4..d9e42dbe 100644 --- a/vendor/github.com/golangci/go-tools/arg/arg.go +++ b/vendor/github.com/golangci/go-tools/arg/arg.go @@ -12,7 +12,6 @@ var args = map[string]int{ "encoding/binary.Write.data": 2, "errors.New.text": 0, "fmt.Printf.format": 0, - "fmt.Fprintf.format": 1, "fmt.Sprintf.a[0]": 1, "fmt.Sprintf.format": 0, "len.v": 0, diff --git a/vendor/github.com/golangci/go-tools/lint/generated.go b/vendor/github.com/golangci/go-tools/lint/generated.go index d407223e..58b23f68 100644 --- a/vendor/github.com/golangci/go-tools/lint/generated.go +++ b/vendor/github.com/golangci/go-tools/lint/generated.go @@ -7,6 +7,8 @@ import ( ) var ( + // used by cgo before Go 1.11 + oldCgo = []byte("// Created by cgo - DO NOT EDIT") prefix = []byte("// Code generated ") suffix = []byte(" DO NOT EDIT.") nl = []byte("\n") @@ -25,6 +27,9 @@ func isGenerated(r io.Reader) bool { if bytes.HasPrefix(s, prefix) && bytes.HasSuffix(s, suffix) { return true } + if bytes.Equal(s, oldCgo) { + return true + } if err == io.EOF { break } diff --git a/vendor/github.com/golangci/go-tools/lint/lintdsl/lintdsl.go b/vendor/github.com/golangci/go-tools/lint/lintdsl/lintdsl.go index 4ab5d840..f56eec6b 100644 --- a/vendor/github.com/golangci/go-tools/lint/lintdsl/lintdsl.go +++ b/vendor/github.com/golangci/go-tools/lint/lintdsl/lintdsl.go @@ -233,26 +233,15 @@ func IsGoVersion(j *lint.Job, minor int) bool { } func CallNameAST(j *lint.Job, call *ast.CallExpr) string { - switch fun := call.Fun.(type) { - case *ast.SelectorExpr: - fn, ok := ObjectOf(j, fun.Sel).(*types.Func) - if !ok { - return "" - } - return fn.FullName() - case *ast.Ident: - obj := ObjectOf(j, fun) - switch obj := obj.(type) { - case *types.Func: - return obj.FullName() - case *types.Builtin: - return obj.Name() - default: - return "" - } - default: + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { return "" } + fn, ok := j.NodePackage(call).TypesInfo.ObjectOf(sel.Sel).(*types.Func) + if !ok { + return "" + } + return fn.FullName() } func IsCallToAST(j *lint.Job, node ast.Node, name string) bool { @@ -332,11 +321,3 @@ func GroupSpecs(j *lint.Job, specs []ast.Spec) [][]ast.Spec { return groups } - -func IsObject(obj types.Object, name string) bool { - var path string - if pkg := obj.Pkg(); pkg != nil { - path = pkg.Path() + "." - } - return path+obj.Name() == name -} diff --git a/vendor/github.com/golangci/go-tools/lint/lintutil/util.go b/vendor/github.com/golangci/go-tools/lint/lintutil/util.go index a667c6a8..93edd41d 100644 --- a/vendor/github.com/golangci/go-tools/lint/lintutil/util.go +++ b/vendor/github.com/golangci/go-tools/lint/lintutil/util.go @@ -309,7 +309,7 @@ func Lint(cs []lint.Checker, paths []string, opt *Options) ([]lint.Problem, erro return problems, nil } -var posRe = regexp.MustCompile(`^(.+?):(\d+):(\d+)?$`) +var posRe = regexp.MustCompile(`^(.+?):(\d+)(?::(\d+)?)?$`) func parsePos(pos string) token.Position { if pos == "-" || pos == "" { diff --git a/vendor/github.com/golangci/go-tools/simple/lint.go b/vendor/github.com/golangci/go-tools/simple/lint.go index 2c47c93f..67eebe39 100644 --- a/vendor/github.com/golangci/go-tools/simple/lint.go +++ b/vendor/github.com/golangci/go-tools/simple/lint.go @@ -62,8 +62,6 @@ func (c *Checker) Checks() []lint.Check { {ID: "S1030", FilterGenerated: true, Fn: c.LintBytesBufferConversions}, {ID: "S1031", FilterGenerated: true, Fn: c.LintNilCheckAroundRange}, {ID: "S1032", FilterGenerated: true, Fn: c.LintSortHelpers}, - {ID: "S1033", FilterGenerated: true, Fn: c.LintGuardedDelete}, - {ID: "S1034", FilterGenerated: true, Fn: c.LintSimplifyTypeSwitch}, } } @@ -1087,26 +1085,22 @@ func (c *Checker) LintTrim(j *lint.Job) { if !ok { return true } - switch { - case IsCallToAST(j, condCall, "strings.HasPrefix"): + call, ok := condCall.Fun.(*ast.SelectorExpr) + if !ok { + return true + } + if IsIdent(call.X, "strings") { pkg = "strings" + } else if IsIdent(call.X, "bytes") { + pkg = "bytes" + } else { + return true + } + if IsIdent(call.Sel, "HasPrefix") { fun = "HasPrefix" - case IsCallToAST(j, condCall, "strings.HasSuffix"): - pkg = "strings" + } else if IsIdent(call.Sel, "HasSuffix") { fun = "HasSuffix" - case IsCallToAST(j, condCall, "strings.Contains"): - pkg = "strings" - fun = "Contains" - case IsCallToAST(j, condCall, "bytes.HasPrefix"): - pkg = "bytes" - fun = "HasPrefix" - case IsCallToAST(j, condCall, "bytes.HasSuffix"): - pkg = "bytes" - fun = "HasSuffix" - case IsCallToAST(j, condCall, "bytes.Contains"): - pkg = "bytes" - fun = "Contains" - default: + } else { return true } @@ -1123,121 +1117,102 @@ func (c *Checker) LintTrim(j *lint.Job) { if !sameNonDynamic(condCall.Args[0], assign.Lhs[0]) { return true } - - switch rhs := assign.Rhs[0].(type) { - case *ast.CallExpr: - if len(rhs.Args) < 2 || !sameNonDynamic(condCall.Args[0], rhs.Args[0]) || !sameNonDynamic(condCall.Args[1], rhs.Args[1]) { - return true - } - if IsCallToAST(j, condCall, "strings.HasPrefix") && IsCallToAST(j, rhs, "strings.TrimPrefix") || - IsCallToAST(j, condCall, "strings.HasSuffix") && IsCallToAST(j, rhs, "strings.TrimSuffix") || - IsCallToAST(j, condCall, "strings.Contains") && IsCallToAST(j, rhs, "strings.Replace") || - IsCallToAST(j, condCall, "bytes.HasPrefix") && IsCallToAST(j, rhs, "bytes.TrimPrefix") || - IsCallToAST(j, condCall, "bytes.HasSuffix") && IsCallToAST(j, rhs, "bytes.TrimSuffix") || - IsCallToAST(j, condCall, "bytes.Contains") && IsCallToAST(j, rhs, "bytes.Replace") { - j.Errorf(ifstmt, "should replace this if statement with an unconditional %s", CallNameAST(j, rhs)) - } + slice, ok := assign.Rhs[0].(*ast.SliceExpr) + if !ok { return true - case *ast.SliceExpr: - slice := rhs - if !ok { + } + if slice.Slice3 { + return true + } + if !sameNonDynamic(slice.X, condCall.Args[0]) { + return true + } + var index ast.Expr + switch fun { + case "HasPrefix": + // TODO(dh) We could detect a High that is len(s), but another + // rule will already flag that, anyway. + if slice.High != nil { return true } - if slice.Slice3 { - return true - } - if !sameNonDynamic(slice.X, condCall.Args[0]) { - return true - } - var index ast.Expr - switch fun { - case "HasPrefix": - // TODO(dh) We could detect a High that is len(s), but another - // rule will already flag that, anyway. - if slice.High != nil { + index = slice.Low + case "HasSuffix": + if slice.Low != nil { + n, ok := ExprToInt(j, slice.Low) + if !ok || n != 0 { return true } - index = slice.Low - case "HasSuffix": - if slice.Low != nil { - n, ok := ExprToInt(j, slice.Low) - if !ok || n != 0 { - return true - } - } - index = slice.High } + index = slice.High + } - switch index := index.(type) { - case *ast.CallExpr: - if fun != "HasPrefix" { - return true - } - if fn, ok := index.Fun.(*ast.Ident); !ok || fn.Name != "len" { - return true - } - if len(index.Args) != 1 { - return true - } - id3 := index.Args[Arg("len.v")] - switch oid3 := condCall.Args[1].(type) { - case *ast.BasicLit: - if pkg != "strings" { - return false - } - lit, ok := id3.(*ast.BasicLit) - if !ok { - return true - } - s1, ok1 := ExprToString(j, lit) - s2, ok2 := ExprToString(j, condCall.Args[1]) - if !ok1 || !ok2 || s1 != s2 { - return true - } - default: - if !sameNonDynamic(id3, oid3) { - return true - } - } - case *ast.BasicLit, *ast.Ident: - if fun != "HasPrefix" { - return true - } + switch index := index.(type) { + case *ast.CallExpr: + if fun != "HasPrefix" { + return true + } + if fn, ok := index.Fun.(*ast.Ident); !ok || fn.Name != "len" { + return true + } + if len(index.Args) != 1 { + return true + } + id3 := index.Args[Arg("len.v")] + switch oid3 := condCall.Args[1].(type) { + case *ast.BasicLit: if pkg != "strings" { + return false + } + lit, ok := id3.(*ast.BasicLit) + if !ok { return true } - string, ok1 := ExprToString(j, condCall.Args[1]) - int, ok2 := ExprToInt(j, slice.Low) - if !ok1 || !ok2 || int != int64(len(string)) { - return true - } - case *ast.BinaryExpr: - if fun != "HasSuffix" { - return true - } - if index.Op != token.SUB { - return true - } - if !isLenOnIdent(index.X, condCall.Args[0]) || - !isLenOnIdent(index.Y, condCall.Args[1]) { + s1, ok1 := ExprToString(j, lit) + s2, ok2 := ExprToString(j, condCall.Args[1]) + if !ok1 || !ok2 || s1 != s2 { return true } default: + if !sameNonDynamic(id3, oid3) { + return true + } + } + case *ast.BasicLit, *ast.Ident: + if fun != "HasPrefix" { return true } - - var replacement string - switch fun { - case "HasPrefix": - replacement = "TrimPrefix" - case "HasSuffix": - replacement = "TrimSuffix" + if pkg != "strings" { + return true + } + string, ok1 := ExprToString(j, condCall.Args[1]) + int, ok2 := ExprToInt(j, slice.Low) + if !ok1 || !ok2 || int != int64(len(string)) { + return true + } + case *ast.BinaryExpr: + if fun != "HasSuffix" { + return true + } + if index.Op != token.SUB { + return true + } + if !isLenOnIdent(index.X, condCall.Args[0]) || + !isLenOnIdent(index.Y, condCall.Args[1]) { + return true } - j.Errorf(ifstmt, "should replace this if statement with an unconditional %s.%s", pkg, replacement) - return true default: return true } + + var replacement string + switch fun { + case "HasPrefix": + replacement = "TrimPrefix" + case "HasSuffix": + replacement = "TrimSuffix" + } + j.Errorf(ifstmt, "should replace this if statement with an unconditional %s.%s", pkg, replacement) + return true } for _, f := range j.Program.Files { ast.Inspect(f, fn) @@ -1405,7 +1380,7 @@ func (c *Checker) LintAssertNotNil(j *lint.Job) { } return true } - fn1 := func(node ast.Node) bool { + fn := func(node ast.Node) bool { ifstmt, ok := node.(*ast.IfStmt) if !ok { return true @@ -1437,71 +1412,6 @@ func (c *Checker) LintAssertNotNil(j *lint.Job) { j.Errorf(ifstmt, "when %s is true, %s can't be nil", Render(j, assignIdent), Render(j, assertIdent)) return true } - fn2 := func(node ast.Node) bool { - // Check that outer ifstmt is an 'if x != nil {}' - ifstmt, ok := node.(*ast.IfStmt) - if !ok { - return true - } - if ifstmt.Init != nil { - return true - } - if ifstmt.Else != nil { - return true - } - if len(ifstmt.Body.List) != 1 { - return true - } - binop, ok := ifstmt.Cond.(*ast.BinaryExpr) - if !ok { - return true - } - if binop.Op != token.NEQ { - return true - } - lhs, ok := binop.X.(*ast.Ident) - if !ok { - return true - } - if !IsNil(j, binop.Y) { - return true - } - - // Check that inner ifstmt is an `if _, ok := x.(T); ok {}` - ifstmt, ok = ifstmt.Body.List[0].(*ast.IfStmt) - if !ok { - return true - } - assign, ok := ifstmt.Init.(*ast.AssignStmt) - if !ok || len(assign.Lhs) != 2 || len(assign.Rhs) != 1 || !IsBlank(assign.Lhs[0]) { - return true - } - assert, ok := assign.Rhs[0].(*ast.TypeAssertExpr) - if !ok { - return true - } - assertIdent, ok := assert.X.(*ast.Ident) - if !ok { - return true - } - if lhs.Obj != assertIdent.Obj { - return true - } - assignIdent, ok := assign.Lhs[1].(*ast.Ident) - if !ok { - return true - } - if !isOKCheck(assignIdent, ifstmt.Cond) { - return true - } - j.Errorf(ifstmt, "when %s is true, %s can't be nil", Render(j, assignIdent), Render(j, assertIdent)) - return true - } - fn := func(node ast.Node) bool { - b1 := fn1(node) - b2 := fn2(node) - return b1 || b2 - } for _, f := range j.Program.Files { ast.Inspect(f, fn) } @@ -1822,143 +1732,3 @@ func (c *Checker) LintSortHelpers(j *lint.Job) { ast.Inspect(f, fnFuncs) } } - -func (c *Checker) LintGuardedDelete(j *lint.Job) { - isCommaOkMapIndex := func(stmt ast.Stmt) (b *ast.Ident, m ast.Expr, key ast.Expr, ok bool) { - // Has to be of the form `_, = [] - - assign, ok := stmt.(*ast.AssignStmt) - if !ok { - return nil, nil, nil, false - } - if len(assign.Lhs) != 2 || len(assign.Rhs) != 1 { - return nil, nil, nil, false - } - if !IsBlank(assign.Lhs[0]) { - return nil, nil, nil, false - } - ident, ok := assign.Lhs[1].(*ast.Ident) - if !ok { - return nil, nil, nil, false - } - index, ok := assign.Rhs[0].(*ast.IndexExpr) - if !ok { - return nil, nil, nil, false - } - if _, ok := TypeOf(j, index.X).(*types.Map); !ok { - return nil, nil, nil, false - } - key = index.Index - return ident, index.X, key, true - } - fn := func(node ast.Node) bool { - stmt, ok := node.(*ast.IfStmt) - if !ok { - return true - } - if len(stmt.Body.List) != 1 { - return true - } - expr, ok := stmt.Body.List[0].(*ast.ExprStmt) - if !ok { - return true - } - call, ok := expr.X.(*ast.CallExpr) - if !ok { - return true - } - if !IsCallToAST(j, call, "delete") { - return true - } - b, m, key, ok := isCommaOkMapIndex(stmt.Init) - if !ok { - return true - } - if cond, ok := stmt.Cond.(*ast.Ident); !ok || ObjectOf(j, cond) != ObjectOf(j, b) { - return true - } - if Render(j, call.Args[0]) != Render(j, m) || Render(j, call.Args[1]) != Render(j, key) { - return true - } - j.Errorf(stmt, "unnecessary guard around call to delete") - return true - } - for _, f := range j.Program.Files { - ast.Inspect(f, fn) - } -} - -func (c *Checker) LintSimplifyTypeSwitch(j *lint.Job) { - fn := func(node ast.Node) bool { - stmt, ok := node.(*ast.TypeSwitchStmt) - if !ok { - return true - } - if stmt.Init != nil { - // bailing out for now, can't anticipate how type switches with initializers are being used - return true - } - expr, ok := stmt.Assign.(*ast.ExprStmt) - if !ok { - // the user is in fact assigning the result - return true - } - assert := expr.X.(*ast.TypeAssertExpr) - ident, ok := assert.X.(*ast.Ident) - if !ok { - return true - } - x := ObjectOf(j, ident) - var allOffenders []ast.Node - for _, clause := range stmt.Body.List { - clause := clause.(*ast.CaseClause) - if len(clause.List) != 1 { - continue - } - hasUnrelatedAssertion := false - var offenders []ast.Node - ast.Inspect(clause, func(node ast.Node) bool { - assert2, ok := node.(*ast.TypeAssertExpr) - if !ok { - return true - } - ident, ok := assert2.X.(*ast.Ident) - if !ok { - hasUnrelatedAssertion = true - return false - } - if ObjectOf(j, ident) != x { - hasUnrelatedAssertion = true - return false - } - - if !types.Identical(TypeOf(j, clause.List[0]), TypeOf(j, assert2.Type)) { - hasUnrelatedAssertion = true - return false - } - offenders = append(offenders, assert2) - return true - }) - if !hasUnrelatedAssertion { - // don't flag cases that have other type assertions - // unrelated to the one in the case clause. often - // times, this is done for symmetry, when two - // different values have to be asserted to the same - // type. - allOffenders = append(allOffenders, offenders...) - } - } - if len(allOffenders) != 0 { - at := "" - for _, offender := range allOffenders { - pos := j.Program.DisplayPosition(offender.Pos()) - at += "\n\t" + pos.String() - } - j.Errorf(expr, "assigning the result of this type assertion to a variable (switch %s := %s.(type)) could eliminate the following type assertions:%s", Render(j, ident), Render(j, ident), at) - } - return true - } - for _, f := range j.Program.Files { - ast.Inspect(f, fn) - } -} diff --git a/vendor/github.com/golangci/go-tools/staticcheck/lint.go b/vendor/github.com/golangci/go-tools/staticcheck/lint.go index 46c869bb..ef4c893e 100644 --- a/vendor/github.com/golangci/go-tools/staticcheck/lint.go +++ b/vendor/github.com/golangci/go-tools/staticcheck/lint.go @@ -257,7 +257,7 @@ func (c *Checker) Checks() []lint.Check { {ID: "SA4000", FilterGenerated: false, Fn: c.CheckLhsRhsIdentical}, {ID: "SA4001", FilterGenerated: false, Fn: c.CheckIneffectiveCopy}, {ID: "SA4002", FilterGenerated: false, Fn: c.CheckDiffSizeComparison}, - {ID: "SA4003", FilterGenerated: false, Fn: c.CheckExtremeComparison}, + {ID: "SA4003", FilterGenerated: false, Fn: c.CheckUnsignedComparison}, {ID: "SA4004", FilterGenerated: false, Fn: c.CheckIneffectiveLoop}, {ID: "SA4006", FilterGenerated: false, Fn: c.CheckUnreadVariableValues}, {ID: "SA4008", FilterGenerated: false, Fn: c.CheckLoopCondition}, @@ -272,7 +272,6 @@ func (c *Checker) Checks() []lint.Check { {ID: "SA4017", FilterGenerated: false, Fn: c.CheckPureFunctions}, {ID: "SA4018", FilterGenerated: true, Fn: c.CheckSelfAssignment}, {ID: "SA4019", FilterGenerated: true, Fn: c.CheckDuplicateBuildConstraints}, - {ID: "SA4020", FilterGenerated: false, Fn: c.CheckUnreachableTypeCases}, {ID: "SA5000", FilterGenerated: false, Fn: c.CheckNilMaps}, {ID: "SA5001", FilterGenerated: false, Fn: c.CheckEarlyDefer}, @@ -287,7 +286,6 @@ func (c *Checker) Checks() []lint.Check { {ID: "SA6002", FilterGenerated: false, Fn: c.callChecker(checkSyncPoolValueRules)}, {ID: "SA6003", FilterGenerated: false, Fn: c.CheckRangeStringRunes}, // {ID: "SA6004", FilterGenerated: false, Fn: c.CheckSillyRegexp}, - {ID: "SA6005", FilterGenerated: false, Fn: c.CheckToLowerToUpperComparison}, {ID: "SA9001", FilterGenerated: false, Fn: c.CheckDubiousDeferInChannelRangeLoop}, {ID: "SA9002", FilterGenerated: false, Fn: c.CheckNonOctalFileMode}, @@ -654,21 +652,14 @@ func (c *Checker) CheckInfiniteEmptyLoop(j *lint.Job) { // is dynamic and the loop might terminate. Similarly for // channel receives. + if loop.Cond != nil && hasSideEffects(loop.Cond) { + return true + } + + j.Errorf(loop, "this loop will spin, using 100%% CPU") if loop.Cond != nil { - if hasSideEffects(loop.Cond) { - return true - } - if ident, ok := loop.Cond.(*ast.Ident); ok { - if k, ok := ObjectOf(j, ident).(*types.Const); ok { - if !constant.BoolVal(k.Val()) { - // don't flag `for false {}` loops. They're a debug aid. - return true - } - } - } j.Errorf(loop, "loop condition never changes or has a race condition") } - j.Errorf(loop, "this loop will spin, using 100%% CPU") return true } @@ -872,7 +863,7 @@ func (c *Checker) CheckLhsRhsIdentical(j *lint.Job) { } switch op.Op { case token.EQL, token.NEQ: - if basic, ok := TypeOf(j, op.X).Underlying().(*types.Basic); ok { + if basic, ok := TypeOf(j, op.X).(*types.Basic); ok { if kind := basic.Kind(); kind == types.Float32 || kind == types.Float64 { // f == f and f != f might be used to check for NaN return true @@ -964,24 +955,19 @@ func (c *Checker) CheckUnsafePrintf(j *lint.Job) { if !ok { return true } - var arg int - if IsCallToAnyAST(j, call, "fmt.Printf", "fmt.Sprintf", "log.Printf") { - arg = Arg("fmt.Printf.format") - } else if IsCallToAnyAST(j, call, "fmt.Fprintf") { - arg = Arg("fmt.Fprintf.format") - } else { + if !IsCallToAnyAST(j, call, "fmt.Printf", "fmt.Sprintf", "log.Printf") { return true } - if len(call.Args) != arg+1 { + if len(call.Args) != 1 { return true } - switch call.Args[arg].(type) { + switch call.Args[Arg("fmt.Printf.format")].(type) { case *ast.CallExpr, *ast.Ident: default: return true } - j.Errorf(call.Args[arg], - "printf-style function with dynamic format string and no further arguments should use print-style function instead") + j.Errorf(call.Args[Arg("fmt.Printf.format")], + "printf-style function with dynamic first argument and no further arguments should use print-style function instead") return true } for _, f := range j.Program.Files { @@ -1396,15 +1382,7 @@ func (c *Checker) CheckNilMaps(j *lint.Job) { } } -func (c *Checker) CheckExtremeComparison(j *lint.Job) { - isobj := func(expr ast.Expr, name string) bool { - sel, ok := expr.(*ast.SelectorExpr) - if !ok { - return false - } - return IsObject(ObjectOf(j, sel.Sel), name) - } - +func (c *Checker) CheckUnsignedComparison(j *lint.Job) { fn := func(node ast.Node) bool { expr, ok := node.(*ast.BinaryExpr) if !ok { @@ -1415,68 +1393,19 @@ func (c *Checker) CheckExtremeComparison(j *lint.Job) { if !ok { return true } - - var max string - var min string - - switch basic.Kind() { - case types.Uint8: - max = "math.MaxUint8" - case types.Uint16: - max = "math.MaxUint16" - case types.Uint32: - max = "math.MaxUint32" - case types.Uint64: - max = "math.MaxUint64" - case types.Uint: - max = "math.MaxUint64" - - case types.Int8: - min = "math.MinInt8" - max = "math.MaxInt8" - case types.Int16: - min = "math.MinInt16" - max = "math.MaxInt16" - case types.Int32: - min = "math.MinInt32" - max = "math.MaxInt32" - case types.Int64: - min = "math.MinInt64" - max = "math.MaxInt64" - case types.Int: - min = "math.MinInt64" - max = "math.MaxInt64" + if (basic.Info() & types.IsUnsigned) == 0 { + return true } - - if (expr.Op == token.GTR || expr.Op == token.GEQ) && isobj(expr.Y, max) || - (expr.Op == token.LSS || expr.Op == token.LEQ) && isobj(expr.X, max) { - j.Errorf(expr, "no value of type %s is greater than %s", basic, max) + lit, ok := expr.Y.(*ast.BasicLit) + if !ok || lit.Value != "0" { + return true } - if expr.Op == token.LEQ && isobj(expr.Y, max) || - expr.Op == token.GEQ && isobj(expr.X, max) { - j.Errorf(expr, "every value of type %s is <= %s", basic, max) + switch expr.Op { + case token.GEQ: + j.Errorf(expr, "unsigned values are always >= 0") + case token.LSS: + j.Errorf(expr, "unsigned values are never < 0") } - - if (basic.Info() & types.IsUnsigned) != 0 { - if (expr.Op == token.LSS || expr.Op == token.LEQ) && IsIntLiteral(expr.Y, "0") || - (expr.Op == token.GTR || expr.Op == token.GEQ) && IsIntLiteral(expr.X, "0") { - j.Errorf(expr, "no value of type %s is less than 0", basic) - } - if expr.Op == token.GEQ && IsIntLiteral(expr.Y, "0") || - expr.Op == token.LEQ && IsIntLiteral(expr.X, "0") { - j.Errorf(expr, "every value of type %s is >= 0", basic) - } - } else { - if (expr.Op == token.LSS || expr.Op == token.LEQ) && isobj(expr.Y, min) || - (expr.Op == token.GTR || expr.Op == token.GEQ) && isobj(expr.X, min) { - j.Errorf(expr, "no value of type %s is less than %s", basic, min) - } - if expr.Op == token.GEQ && isobj(expr.Y, min) || - expr.Op == token.LEQ && isobj(expr.X, min) { - j.Errorf(expr, "every value of type %s is >= %s", basic, min) - } - } - return true } for _, f := range j.Program.Files { @@ -2887,122 +2816,3 @@ func (c *Checker) CheckTimerResetReturnValue(j *lint.Job) { } } } - -func (c *Checker) CheckToLowerToUpperComparison(j *lint.Job) { - fn := func(node ast.Node) bool { - binExpr, ok := node.(*ast.BinaryExpr) - if !ok { - return true - } - - var negative bool - switch binExpr.Op { - case token.EQL: - negative = false - case token.NEQ: - negative = true - default: - return true - } - - const ( - lo = "strings.ToLower" - up = "strings.ToUpper" - ) - - var call string - if IsCallToAST(j, binExpr.X, lo) && IsCallToAST(j, binExpr.Y, lo) { - call = lo - } else if IsCallToAST(j, binExpr.X, up) && IsCallToAST(j, binExpr.Y, up) { - call = up - } else { - return true - } - - bang := "" - if negative { - bang = "!" - } - - j.Errorf(binExpr, "should use %sstrings.EqualFold(a, b) instead of %s(a) %s %s(b)", bang, call, binExpr.Op, call) - return true - } - - for _, f := range j.Program.Files { - ast.Inspect(f, fn) - } -} - -func (c *Checker) CheckUnreachableTypeCases(j *lint.Job) { - // Check if T subsumes V in a type switch. T subsumes V if T is an interface and T's method set is a subset of V's method set. - subsumes := func(T, V types.Type) bool { - tIface, ok := T.Underlying().(*types.Interface) - if !ok { - return false - } - - return types.Implements(V, tIface) - } - - subsumesAny := func(Ts, Vs []types.Type) (types.Type, types.Type, bool) { - for _, T := range Ts { - for _, V := range Vs { - if subsumes(T, V) { - return T, V, true - } - } - } - - return nil, nil, false - } - - fn := func(node ast.Node) bool { - tsStmt, ok := node.(*ast.TypeSwitchStmt) - if !ok { - return true - } - - type ccAndTypes struct { - cc *ast.CaseClause - types []types.Type - } - - // All asserted types in the order of case clauses. - ccs := make([]ccAndTypes, 0, len(tsStmt.Body.List)) - for _, stmt := range tsStmt.Body.List { - cc, _ := stmt.(*ast.CaseClause) - - // Exclude the 'default' case. - if len(cc.List) == 0 { - continue - } - - Ts := make([]types.Type, len(cc.List)) - for i, expr := range cc.List { - Ts[i] = TypeOf(j, expr) - } - - ccs = append(ccs, ccAndTypes{cc: cc, types: Ts}) - } - - if len(ccs) <= 1 { - // Zero or one case clauses, nothing to check. - return true - } - - // Check if case clauses following cc have types that are subsumed by cc. - for i, cc := range ccs[:len(ccs)-1] { - for _, next := range ccs[i+1:] { - if T, V, yes := subsumesAny(cc.types, next.types); yes { - j.Errorf(next.cc, "unreachable case clause: %s will always match before %s", T.String(), V.String()) - } - } - } - - return true - } - - for _, f := range j.Program.Files { - ast.Inspect(f, fn) - } -} diff --git a/vendor/github.com/golangci/go-tools/stylecheck/lint.go b/vendor/github.com/golangci/go-tools/stylecheck/lint.go index 38f9d666..c0601861 100644 --- a/vendor/github.com/golangci/go-tools/stylecheck/lint.go +++ b/vendor/github.com/golangci/go-tools/stylecheck/lint.go @@ -48,7 +48,6 @@ func (c *Checker) Checks() []lint.Check { {ID: "ST1013", FilterGenerated: true, Fn: c.CheckHTTPStatusCodes}, {ID: "ST1015", FilterGenerated: true, Fn: c.CheckDefaultCaseOrder}, {ID: "ST1016", FilterGenerated: false, Fn: c.CheckReceiverNamesIdentical}, - {ID: "ST1017", FilterGenerated: true, Fn: c.CheckYodaConditions}, } } @@ -624,27 +623,3 @@ func (c *Checker) CheckDefaultCaseOrder(j *lint.Job) { ast.Inspect(f, fn) } } - -func (c *Checker) CheckYodaConditions(j *lint.Job) { - fn := func(node ast.Node) bool { - cond, ok := node.(*ast.BinaryExpr) - if !ok { - return true - } - if cond.Op != token.EQL && cond.Op != token.NEQ { - return true - } - if _, ok := cond.X.(*ast.BasicLit); !ok { - return true - } - if _, ok := cond.Y.(*ast.BasicLit); ok { - // Don't flag lit == lit conditions, just in case - return true - } - j.Errorf(cond, "don't use Yoda conditions") - return true - } - for _, f := range j.Program.Files { - ast.Inspect(f, fn) - } -} diff --git a/vendor/github.com/golangci/go-tools/unused/implements.go b/vendor/github.com/golangci/go-tools/unused/implements.go new file mode 100644 index 00000000..78a54563 --- /dev/null +++ b/vendor/github.com/golangci/go-tools/unused/implements.go @@ -0,0 +1,79 @@ +package unused + +import "go/types" + +// lookupMethod returns the index of and method with matching package and name, or (-1, nil). +func lookupMethod(T *types.Interface, pkg *types.Package, name string) (int, *types.Func) { + if name != "_" { + for i := 0; i < T.NumMethods(); i++ { + m := T.Method(i) + if sameId(m, pkg, name) { + return i, m + } + } + } + return -1, nil +} + +func sameId(obj types.Object, pkg *types.Package, name string) bool { + // spec: + // "Two identifiers are different if they are spelled differently, + // or if they appear in different packages and are not exported. + // Otherwise, they are the same." + if name != obj.Name() { + return false + } + // obj.Name == name + if obj.Exported() { + return true + } + // not exported, so packages must be the same (pkg == nil for + // fields in Universe scope; this can only happen for types + // introduced via Eval) + if pkg == nil || obj.Pkg() == nil { + return pkg == obj.Pkg() + } + // pkg != nil && obj.pkg != nil + return pkg.Path() == obj.Pkg().Path() +} + +func (c *Checker) implements(V types.Type, T *types.Interface) bool { + // fast path for common case + if T.Empty() { + return true + } + + if ityp, _ := V.Underlying().(*types.Interface); ityp != nil { + for i := 0; i < T.NumMethods(); i++ { + m := T.Method(i) + _, obj := lookupMethod(ityp, m.Pkg(), m.Name()) + switch { + case obj == nil: + return false + case !types.Identical(obj.Type(), m.Type()): + return false + } + } + return true + } + + // A concrete type implements T if it implements all methods of T. + ms := c.msCache.MethodSet(V) + for i := 0; i < T.NumMethods(); i++ { + m := T.Method(i) + sel := ms.Lookup(m.Pkg(), m.Name()) + if sel == nil { + return false + } + + f, _ := sel.Obj().(*types.Func) + if f == nil { + return false + } + + if !types.Identical(f.Type(), m.Type()) { + return false + } + } + return true +} diff --git a/vendor/github.com/golangci/go-tools/unused/unused.go b/vendor/github.com/golangci/go-tools/unused/unused.go index 180fa1a1..d80b6622 100644 --- a/vendor/github.com/golangci/go-tools/unused/unused.go +++ b/vendor/github.com/golangci/go-tools/unused/unused.go @@ -265,6 +265,7 @@ func (c *Checker) Check(prog *lint.Program) []Unused { unused = append(unused, Unused{Obj: obj, Position: pos}) } + return unused } @@ -552,10 +553,22 @@ func (c *Checker) processTypes(pkg *lint.Pkg) { for i := 0; i < iface.NumEmbeddeds(); i++ { c.graph.markUsedBy(iface.Embedded(i), iface) } + namedLoop: for obj, objPtr := range named { - if !types.Implements(obj, iface) && !types.Implements(objPtr, iface) { - continue + switch obj.Underlying().(type) { + case *types.Interface: + // pointers to interfaces have no methods, only checking non-pointer + if !c.implements(obj, iface) { + continue namedLoop + } + default: + // pointer receivers include the method set of non-pointer receivers, + // only checking pointer + if !c.implements(objPtr, iface) { + continue namedLoop + } } + ifaceMethods := make(map[string]struct{}, iface.NumMethods()) n := iface.NumMethods() for i := 0; i < n; i++ { @@ -594,15 +607,15 @@ func (c *Checker) processTypes(pkg *lint.Pkg) { func (c *Checker) processSelections(pkg *lint.Pkg) { fn := func(expr *ast.SelectorExpr, sel *types.Selection, offset int) { scope := pkg.Types.Scope().Innermost(expr.Pos()) - c.graph.markUsedBy(expr.X, c.topmostScope(scope, pkg.Types)) - c.graph.markUsedBy(sel.Obj(), expr.X) + c.graph.markUsedBy(sel, c.topmostScope(scope, pkg.Types)) + c.graph.markUsedBy(sel.Obj(), sel) if len(sel.Index()) > 1 { typ := sel.Recv() indices := sel.Index() for _, idx := range indices[:len(indices)-offset] { obj := getField(typ, idx) typ = obj.Type() - c.graph.markUsedBy(obj, expr.X) + c.graph.markUsedBy(obj, sel) } } } diff --git a/vendor/github.com/golangci/go-tools/version/version.go b/vendor/github.com/golangci/go-tools/version/version.go index 232cf7e7..511fb0bd 100644 --- a/vendor/github.com/golangci/go-tools/version/version.go +++ b/vendor/github.com/golangci/go-tools/version/version.go @@ -6,7 +6,7 @@ import ( "path/filepath" ) -const Version = "devel" +const Version = "2019.1.1" func Print() { if Version == "devel" { diff --git a/vendor/modules.txt b/vendor/modules.txt index 218688ac..4ac00d39 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -61,7 +61,7 @@ github.com/golangci/errcheck/golangci github.com/golangci/errcheck/internal/errcheck # github.com/golangci/go-misc v0.0.0-20180628070357-927a3d87b613 github.com/golangci/go-misc/deadcode -# github.com/golangci/go-tools v0.0.0-20180109140146-35a9f45a5db0 +# github.com/golangci/go-tools v0.0.0-20180109140146-af6baa5dc196 github.com/golangci/go-tools/config github.com/golangci/go-tools/lint github.com/golangci/go-tools/lint/lintutil