From 692dacb773b703162c091c2d8c59f9cd2d6801db Mon Sep 17 00:00:00 2001 From: Denis Isaev Date: Thu, 25 Apr 2019 09:27:35 +0300 Subject: [PATCH] Update go-critic New checkers were added: - badCall - dupImports - evalOrder - newDeref The following checkers were fixed/improved: - assignOp - caseOrder - commentedOutCode - deprecatedComment - dupArg - rangeValCopy - typeSwitchVar - wrapperFunc Relates: #429 --- .golangci.yml | 1 + README.md | 1 + go.mod | 2 +- go.sum | 9 +- .../go-critic/checkers/assignOp_checker.go | 23 ++- .../go-critic/checkers/badCall_checker.go | 63 +++++++ .../go-critic/checkers/caseOrder_checker.go | 8 + .../checkers/commentedOutCode_checker.go | 22 ++- .../checkers/deprecatedComment_checker.go | 1 + .../go-critic/checkers/dupArg_checker.go | 12 ++ .../go-critic/checkers/dupImports_checker.go | 63 +++++++ .../go-critic/checkers/evalOrder_checker.go | 87 +++++++++ .../checkers/internal/lintutil/zero_value.go | 94 ++++++++++ .../go-critic/checkers/newDeref_checker.go | 45 +++++ .../checkers/rangeValCopy_checker.go | 4 +- .../checkers/typeSwitchVar_checker.go | 4 +- .../go-critic/go-critic/checkers/utils.go | 176 +++++++++++++++++- .../go-critic/checkers/wrapperFunc_checker.go | 10 + vendor/modules.txt | 2 +- 19 files changed, 614 insertions(+), 13 deletions(-) create mode 100644 vendor/github.com/go-critic/go-critic/checkers/badCall_checker.go create mode 100644 vendor/github.com/go-critic/go-critic/checkers/dupImports_checker.go create mode 100644 vendor/github.com/go-critic/go-critic/checkers/evalOrder_checker.go create mode 100644 vendor/github.com/go-critic/go-critic/checkers/internal/lintutil/zero_value.go create mode 100644 vendor/github.com/go-critic/go-critic/checkers/newDeref_checker.go diff --git a/.golangci.yml b/.golangci.yml index 41fd316a..77f11362 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -38,6 +38,7 @@ linters-settings: - experimental disabled-checks: - wrapperFunc + - dupImport # https://github.com/go-critic/go-critic/issues/845 linters: enable-all: true diff --git a/README.md b/README.md index 8328bf6d..cc3dc1d3 100644 --- a/README.md +++ b/README.md @@ -848,6 +848,7 @@ linters-settings: - experimental disabled-checks: - wrapperFunc + - dupImport # https://github.com/go-critic/go-critic/issues/845 linters: enable-all: true diff --git a/go.mod b/go.mod index 1a89bf82..919e7b7c 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ require ( github.com/OpenPeeDeeP/depguard v0.0.0-20180806142446-a69c782687b2 github.com/StackExchange/wmi v0.0.0-20180116203802-5d049714c4a6 // indirect github.com/fatih/color v1.6.0 - github.com/go-critic/go-critic v0.0.0-20181204210945-ee9bf5809ead + github.com/go-critic/go-critic v0.0.0-20181204210945-c3db6069acc5 github.com/go-lintpack/lintpack v0.5.2 github.com/go-ole/go-ole v1.2.1 // indirect github.com/gobwas/glob v0.2.3 // indirect diff --git a/go.sum b/go.sum index de147eda..935d55d7 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,7 @@ github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/OpenPeeDeeP/depguard v0.0.0-20180806142446-a69c782687b2 h1:HTOmFEEYrWi4MW5ZKUx6xfeyM10Sx3kQF65xiQJMPYA= github.com/OpenPeeDeeP/depguard v0.0.0-20180806142446-a69c782687b2/go.mod h1:7/4sitnI9YlQgTLLk734QlzXT8DuHVnAyztLplQjk+o= +github.com/Quasilyte/go-consistent v0.0.0-20181230194409-8f8379e70f99/go.mod h1:ds1OLa3HF2x4OGKCx0pNTVL1s9Ii/2mT0Bg/8PtW6AM= github.com/StackExchange/wmi v0.0.0-20180116203802-5d049714c4a6 h1:fLjPD/aNc3UIOA6tDi6QXUemppXK3P9BI7mr2hd6gx8= github.com/StackExchange/wmi v0.0.0-20180116203802-5d049714c4a6/go.mod h1:3eOhrUMpNV+6aFIbp5/iudMxNCF27Vw2OZgy4xEx0Fg= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= @@ -10,8 +11,8 @@ github.com/fatih/color v1.6.0 h1:66qjqZk8kalYAvDRtM1AdAJQI0tj4Wrue3Eq3B3pmFU= github.com/fatih/color v1.6.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= -github.com/go-critic/go-critic v0.0.0-20181204210945-ee9bf5809ead h1:qwmAYufKDopQnFdeMw+iHJVxAd2CbF+VFKHyJJwnPKk= -github.com/go-critic/go-critic v0.0.0-20181204210945-ee9bf5809ead/go.mod h1:3MzXZKJdeXqdU9cj+rvZdNiN7SZ8V9OjybF8loZDmHU= +github.com/go-critic/go-critic v0.0.0-20181204210945-c3db6069acc5 h1:+7jr1oKB2or53kzZk2gwPbTa+Bo/rjSmdSIIlKmcSos= +github.com/go-critic/go-critic v0.0.0-20181204210945-c3db6069acc5/go.mod h1:Jc75BZJv2dNy7opKH6bF29VveDQHfGZ6Asn/3phBesg= github.com/go-lintpack/lintpack v0.5.2 h1:DI5mA3+eKdWeJ40nU4d6Wc26qmdG8RCi/btYq0TuRN0= github.com/go-lintpack/lintpack v0.5.2/go.mod h1:NwZuYi2nUHho8XEIZ6SIxihrnPoqBTDqfpXvXAN0sXM= github.com/go-ole/go-ole v1.2.1 h1:2lOsA72HgjxAuMlKpFiCbHTvu44PIVkZ5hqm3RSdI/E= @@ -24,6 +25,7 @@ github.com/go-toolsmith/astequal v0.0.0-20180903214952-dcb477bfacd6 h1:aTBUNRTat github.com/go-toolsmith/astequal v0.0.0-20180903214952-dcb477bfacd6/go.mod h1:H+xSiq0+LtiDC11+h1G32h7Of5O3CYFJ99GVbS5lDKY= github.com/go-toolsmith/astfmt v0.0.0-20180903215011-8f8ee99c3086 h1:EIMuvbE9fbtQtimdLe5yeXjuC5CeKbQt8zH6GwtIrhM= github.com/go-toolsmith/astfmt v0.0.0-20180903215011-8f8ee99c3086/go.mod h1:mP93XdblcopXwlyN4X4uodxXQhldPGZbcEJIimQHrkg= +github.com/go-toolsmith/astinfo v0.0.0-20180906194353-9809ff7efb21/go.mod h1:dDStQCHtmZpYOmjRP/8gHHnCCch3Zz3oEgCdZVdtweU= github.com/go-toolsmith/astp v0.0.0-20180903215135-0af7e3c24f30 h1:zRJPftZJNLPDiOtvYbFRwjSbaJAcVOf80TeEmWGe2kQ= github.com/go-toolsmith/astp v0.0.0-20180903215135-0af7e3c24f30/go.mod h1:SV2ur98SGypH1UjcPpCatrV5hPazG6+IfNHbkDXBRrk= github.com/go-toolsmith/pkgload v0.0.0-20181119091011-e9e65178eee8 h1:vVouagbdmqTVlCIAxpyYsNNTbkKZ3V66VpKOLU/s6W4= @@ -74,6 +76,7 @@ github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4 h1:zwtduBRr5SSW github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4/go.mod h1:Izgrg8RkN3rCIMLGE9CyYmU9pY2Jer6DgANEnZ/L/cQ= github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= +github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/hashicorp/hcl v0.0.0-20180404174102-ef8a98b0bbce h1:xdsDDbiBDQTKASoGEZ+pEmF1OnWuu8AQ9I8iNbHNeno= github.com/hashicorp/hcl v0.0.0-20180404174102-ef8a98b0bbce/go.mod h1:oZtUIOe8dh44I2q6ScRibXws4Ajl+d+nod3AaR9vL5w= github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI= @@ -112,6 +115,7 @@ github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+W github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v1.4.2 h1:3mYCb7aPxS/RU7TI1y4rkEn1oKmPRjNJLNEXgw7MH2I= github.com/onsi/gomega v1.4.2/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= +github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k= github.com/pelletier/go-toml v1.1.0 h1:cmiOvKzEunMsAxyhXSzpL5Q1CRKpVv0KQsnAIcSEVYM= github.com/pelletier/go-toml v1.1.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= github.com/pkg/errors v0.8.0 h1:WdK/asTD0HN+q6hsWO3/vpuAkAr+tw6aNJNDFFf0+qw= @@ -168,6 +172,7 @@ golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/tools v0.0.0-20170915040203-e531a2a1c15f/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20181024171208-a2dc47679d30/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20181117154741-2ddaf7f79a09/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20181205014116-22934f0fdb62/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190121143147-24cd39ecf745/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= diff --git a/vendor/github.com/go-critic/go-critic/checkers/assignOp_checker.go b/vendor/github.com/go-critic/go-critic/checkers/assignOp_checker.go index e4e82c98..eb342866 100644 --- a/vendor/github.com/go-critic/go-critic/checkers/assignOp_checker.go +++ b/vendor/github.com/go-critic/go-critic/checkers/assignOp_checker.go @@ -74,8 +74,29 @@ func (c *assignOpChecker) VisitStmt(stmt ast.Stmt) { } func (c *assignOpChecker) warn(cause *ast.AssignStmt, op token.Token, rhs ast.Expr) { + suggestion := c.simplify(cause, op, rhs) + c.ctx.Warn(cause, "replace `%s` with `%s`", cause, suggestion) +} + +func (c *assignOpChecker) simplify(cause *ast.AssignStmt, op token.Token, rhs ast.Expr) ast.Stmt { + if lit, ok := rhs.(*ast.BasicLit); ok && lit.Kind == token.INT && lit.Value == "1" { + switch op { + case token.ADD_ASSIGN: + return &ast.IncDecStmt{ + X: cause.Lhs[0], + TokPos: cause.TokPos, + Tok: token.INC, + } + case token.SUB_ASSIGN: + return &ast.IncDecStmt{ + X: cause.Lhs[0], + TokPos: cause.TokPos, + Tok: token.DEC, + } + } + } suggestion := astcopy.AssignStmt(cause) suggestion.Tok = op suggestion.Rhs[0] = rhs - c.ctx.Warn(cause, "replace `%s` with `%s`", cause, suggestion) + return suggestion } diff --git a/vendor/github.com/go-critic/go-critic/checkers/badCall_checker.go b/vendor/github.com/go-critic/go-critic/checkers/badCall_checker.go new file mode 100644 index 00000000..150cc690 --- /dev/null +++ b/vendor/github.com/go-critic/go-critic/checkers/badCall_checker.go @@ -0,0 +1,63 @@ +package checkers + +import ( + "go/ast" + + "github.com/go-lintpack/lintpack" + "github.com/go-lintpack/lintpack/astwalk" + "github.com/go-toolsmith/astcast" + "github.com/go-toolsmith/astcopy" +) + +func init() { + var info lintpack.CheckerInfo + info.Name = "badCall" + info.Tags = []string{"diagnostic", "experimental"} + info.Summary = "Detects suspicious function calls" + info.Before = `strings.Replace(s, from, to, 0)` + info.After = `strings.Replace(s, from, to, -1)` + + collection.AddChecker(&info, func(ctx *lintpack.CheckerContext) lintpack.FileWalker { + return astwalk.WalkerForExpr(&badCallChecker{ctx: ctx}) + }) +} + +type badCallChecker struct { + astwalk.WalkHandler + ctx *lintpack.CheckerContext +} + +func (c *badCallChecker) VisitExpr(expr ast.Expr) { + call := astcast.ToCallExpr(expr) + if len(call.Args) == 0 { + return + } + + // TODO(quasilyte): handle methods. + + switch qualifiedName(call.Fun) { + case "strings.Replace", "bytes.Replace": + if n := astcast.ToBasicLit(call.Args[3]); n.Value == "0" { + c.warnBadArg(n, "-1") + } + case "strings.SplitN", "bytes.SplitN": + if n := astcast.ToBasicLit(call.Args[2]); n.Value == "0" { + c.warnBadArg(n, "-1") + } + case "append": + if len(call.Args) == 1 { + c.warnAppend(call) + } + } +} + +func (c *badCallChecker) warnBadArg(badArg *ast.BasicLit, correction string) { + goodArg := astcopy.BasicLit(badArg) + goodArg.Value = correction + c.ctx.Warn(badArg, "suspicious arg %s, probably meant %s", + badArg, goodArg) +} + +func (c *badCallChecker) warnAppend(call *ast.CallExpr) { + c.ctx.Warn(call, "no-op append call, probably missing arguments") +} diff --git a/vendor/github.com/go-critic/go-critic/checkers/caseOrder_checker.go b/vendor/github.com/go-critic/go-critic/checkers/caseOrder_checker.go index 3da6f5c8..1ef4b53b 100644 --- a/vendor/github.com/go-critic/go-critic/checkers/caseOrder_checker.go +++ b/vendor/github.com/go-critic/go-critic/checkers/caseOrder_checker.go @@ -57,6 +57,10 @@ func (c *caseOrderChecker) checkTypeSwitch(s *ast.TypeSwitchStmt) { cc := cc.(*ast.CaseClause) for _, x := range cc.List { typ := c.ctx.TypesInfo.TypeOf(x) + if typ == nil { + c.warnTypeImpl(cc, x) + return + } for _, iface := range ifaces { if types.Implements(typ, iface.typ) { c.warnTypeSwitch(cc, x, iface.node) @@ -74,6 +78,10 @@ func (c *caseOrderChecker) warnTypeSwitch(cause, concrete, iface ast.Node) { c.ctx.Warn(cause, "case %s must go before the %s case", concrete, iface) } +func (c *caseOrderChecker) warnTypeImpl(cause, concrete ast.Node) { + c.ctx.Warn(cause, "type is not defined %s", concrete) +} + func (c *caseOrderChecker) checkSwitch(s *ast.SwitchStmt) { // TODO(Quasilyte): can handle expression cases that overlap. // Cases that have narrower value range should go before wider ones. diff --git a/vendor/github.com/go-critic/go-critic/checkers/commentedOutCode_checker.go b/vendor/github.com/go-critic/go-critic/checkers/commentedOutCode_checker.go index 77b36ef3..5a6c5c7f 100644 --- a/vendor/github.com/go-critic/go-critic/checkers/commentedOutCode_checker.go +++ b/vendor/github.com/go-critic/go-critic/checkers/commentedOutCode_checker.go @@ -1,6 +1,7 @@ package checkers import ( + "fmt" "go/ast" "go/token" "regexp" @@ -77,11 +78,26 @@ func (c *commentedOutCodeChecker) VisitLocalComment(cg *ast.CommentGroup) { } stmt := strparse.Stmt(s) - if stmt == strparse.BadStmt { - return // Most likely not a code + + if c.isPermittedStmt(stmt) { + return } - if !c.isPermittedStmt(stmt) { + if stmt != strparse.BadStmt { + c.warn(cg) + return + } + + // Don't try to parse one-liner as block statement + if len(cg.List) == 1 && !strings.Contains(s, "\n") { + return + } + + // Add parentheses to make block statement from + // multiple statements. + stmt = strparse.Stmt(fmt.Sprintf("{ %s }", s)) + + if stmt, ok := stmt.(*ast.BlockStmt); ok && len(stmt.List) != 0 { c.warn(cg) } } diff --git a/vendor/github.com/go-critic/go-critic/checkers/deprecatedComment_checker.go b/vendor/github.com/go-critic/go-critic/checkers/deprecatedComment_checker.go index d68e32fa..37675735 100644 --- a/vendor/github.com/go-critic/go-critic/checkers/deprecatedComment_checker.go +++ b/vendor/github.com/go-critic/go-critic/checkers/deprecatedComment_checker.go @@ -29,6 +29,7 @@ func FuncOld() int` regexp.MustCompile(`(?i)deprecated[.!]? use \S* instead`), regexp.MustCompile(`(?i)\[\[deprecated\]\].*`), regexp.MustCompile(`(?i)note: deprecated\b.*`), + regexp.MustCompile(`(?i)deprecated in.*`), // TODO(quasilyte): more of these? } diff --git a/vendor/github.com/go-critic/go-critic/checkers/dupArg_checker.go b/vendor/github.com/go-critic/go-critic/checkers/dupArg_checker.go index 431522d8..2e6a0cbf 100644 --- a/vendor/github.com/go-critic/go-critic/checkers/dupArg_checker.go +++ b/vendor/github.com/go-critic/go-critic/checkers/dupArg_checker.go @@ -2,9 +2,11 @@ package checkers import ( "go/ast" + "go/types" "github.com/go-lintpack/lintpack" "github.com/go-lintpack/lintpack/astwalk" + "github.com/go-toolsmith/astcast" "github.com/go-toolsmith/astequal" ) @@ -107,6 +109,16 @@ func (c *dupArgChecker) VisitExpr(expr ast.Expr) { return } + // TODO(quasilyte): this kind of check is needed in multiple + // places and the code is somewhat duplicated around. + // We probably need to stop using qualifiedName for non-experimental checkers. + if calledExpr, ok := call.Fun.(*ast.SelectorExpr); ok { + obj, ok := c.ctx.TypesInfo.ObjectOf(astcast.ToIdent(calledExpr.X)).(*types.PkgName) + if !ok || !isStdlibPkg(obj.Imported()) { + return + } + } + m := c.matchers[qualifiedName(call.Fun)] if m != nil && m(call) { c.warn(call) diff --git a/vendor/github.com/go-critic/go-critic/checkers/dupImports_checker.go b/vendor/github.com/go-critic/go-critic/checkers/dupImports_checker.go new file mode 100644 index 00000000..d531413a --- /dev/null +++ b/vendor/github.com/go-critic/go-critic/checkers/dupImports_checker.go @@ -0,0 +1,63 @@ +package checkers + +import ( + "fmt" + "go/ast" + + "github.com/go-lintpack/lintpack" +) + +func init() { + var info lintpack.CheckerInfo + info.Name = "dupImport" + info.Tags = []string{"style", "experimental"} + info.Summary = "Detects multiple imports of the same package under different aliases" + info.Before = ` +import ( + "fmt" + priting "fmt" // Imported the second time +)` + info.After = ` +import( + "fmt" +)` + + collection.AddChecker(&info, func(ctx *lintpack.CheckerContext) lintpack.FileWalker { + return &dupImportChecker{ctx: ctx} + }) +} + +type dupImportChecker struct { + ctx *lintpack.CheckerContext +} + +func (c *dupImportChecker) WalkFile(f *ast.File) { + imports := make(map[string][]*ast.ImportSpec) + for _, importDcl := range f.Imports { + pkg := importDcl.Path.Value + imports[pkg] = append(imports[pkg], importDcl) + } + + for _, importList := range imports { + if len(importList) == 1 { + continue + } + c.warn(importList) + } +} + +func (c *dupImportChecker) warn(importList []*ast.ImportSpec) { + msg := fmt.Sprintf("package is imported %d times under different aliases on lines", len(importList)) + for idx, importDcl := range importList { + switch { + case idx == len(importList)-1: + msg += " and" + case idx > 0: + msg += "," + } + msg += fmt.Sprintf(" %d", c.ctx.FileSet.Position(importDcl.Pos()).Line) + } + for _, importDcl := range importList { + c.ctx.Warn(importDcl, msg) + } +} diff --git a/vendor/github.com/go-critic/go-critic/checkers/evalOrder_checker.go b/vendor/github.com/go-critic/go-critic/checkers/evalOrder_checker.go new file mode 100644 index 00000000..f76519cd --- /dev/null +++ b/vendor/github.com/go-critic/go-critic/checkers/evalOrder_checker.go @@ -0,0 +1,87 @@ +package checkers + +import ( + "go/ast" + "go/token" + "go/types" + + "github.com/go-critic/go-critic/checkers/internal/lintutil" + "github.com/go-lintpack/lintpack" + "github.com/go-lintpack/lintpack/astwalk" + "github.com/go-toolsmith/astcast" + "github.com/go-toolsmith/astequal" + "github.com/go-toolsmith/typep" +) + +func init() { + var info lintpack.CheckerInfo + info.Name = "evalOrder" + info.Tags = []string{"diagnostic", "experimental"} + info.Summary = "Detects unwanted dependencies on the evaluation order" + info.Before = `return x, f(&x)` + info.After = ` +err := f(&x) +return x, err +` + + collection.AddChecker(&info, func(ctx *lintpack.CheckerContext) lintpack.FileWalker { + return astwalk.WalkerForStmt(&evalOrderChecker{ctx: ctx}) + }) +} + +type evalOrderChecker struct { + astwalk.WalkHandler + ctx *lintpack.CheckerContext +} + +func (c *evalOrderChecker) VisitStmt(stmt ast.Stmt) { + ret := astcast.ToReturnStmt(stmt) + if len(ret.Results) < 2 { + return + } + + // TODO(quasilyte): handle selector expressions like o.val in addition + // to bare identifiers. + addrTake := &ast.UnaryExpr{Op: token.AND} + for _, res := range ret.Results { + id, ok := res.(*ast.Ident) + if !ok { + continue + } + addrTake.X = id // addrTake is &id now + for _, res := range ret.Results { + call, ok := res.(*ast.CallExpr) + if !ok { + continue + } + + // 1. Check if there is a call in form of id.method() where + // method takes id by a pointer. + if sel, ok := call.Fun.(*ast.SelectorExpr); ok { + if astequal.Node(sel.X, id) && c.hasPtrRecv(sel.Sel) { + c.warn(call) + } + } + + // 2. Check that there is no call that uses &id as an argument. + dependency := lintutil.ContainsNode(call, func(n ast.Node) bool { + return astequal.Node(addrTake, n) + }) + if dependency { + c.warn(call) + } + } + } +} + +func (c *evalOrderChecker) hasPtrRecv(fn *ast.Ident) bool { + sig, ok := c.ctx.TypesInfo.TypeOf(fn).(*types.Signature) + if !ok { + return false + } + return typep.IsPointer(sig.Recv().Type()) +} + +func (c *evalOrderChecker) warn(call *ast.CallExpr) { + c.ctx.Warn(call, "may want to evaluate %s before the return statement", call) +} diff --git a/vendor/github.com/go-critic/go-critic/checkers/internal/lintutil/zero_value.go b/vendor/github.com/go-critic/go-critic/checkers/internal/lintutil/zero_value.go new file mode 100644 index 00000000..4370f581 --- /dev/null +++ b/vendor/github.com/go-critic/go-critic/checkers/internal/lintutil/zero_value.go @@ -0,0 +1,94 @@ +package lintutil + +import ( + "go/ast" + "go/constant" + "go/token" + "go/types" +) + +// IsZeroValue reports whether x represents zero value of its type. +// +// The functions is conservative and may return false for zero values +// if some cases are not handled in a comprehensive way +// but is should never return true for something that's not a proper zv. +func IsZeroValue(info *types.Info, x ast.Expr) bool { + switch x := x.(type) { + case *ast.BasicLit: + typ := info.TypeOf(x).Underlying().(*types.Basic) + v := info.Types[x].Value + var z constant.Value + switch { + case typ.Kind() == types.String: + z = constant.MakeString("") + case typ.Info()&types.IsInteger != 0: + z = constant.MakeInt64(0) + case typ.Info()&types.IsUnsigned != 0: + z = constant.MakeUint64(0) + case typ.Info()&types.IsFloat != 0: + z = constant.MakeFloat64(0) + default: + return false + } + return constant.Compare(v, token.EQL, z) + + case *ast.CompositeLit: + return len(x.Elts) == 0 + + default: + // Note that this function is not comprehensive. + return false + } +} + +// ZeroValueOf returns a zero value expression for typeExpr of type typ. +// If function can't find such a value, nil is returned. +func ZeroValueOf(typeExpr ast.Expr, typ types.Type) ast.Expr { + switch utyp := typ.Underlying().(type) { + case *types.Basic: + info := utyp.Info() + var zv ast.Expr + switch { + case info&types.IsInteger != 0: + zv = &ast.BasicLit{Kind: token.INT, Value: "0"} + case info&types.IsFloat != 0: + zv = &ast.BasicLit{Kind: token.FLOAT, Value: "0.0"} + case info&types.IsString != 0: + zv = &ast.BasicLit{Kind: token.STRING, Value: `""`} + case info&types.IsBoolean != 0: + zv = &ast.Ident{Name: "false"} + } + if isDefaultLiteralType(typ) { + return zv + } + return &ast.CallExpr{ + Fun: typeExpr, + Args: []ast.Expr{zv}, + } + + case *types.Slice, *types.Map, *types.Pointer, *types.Interface: + return &ast.CallExpr{ + Fun: typeExpr, + Args: []ast.Expr{&ast.Ident{Name: "nil"}}, + } + + case *types.Array, *types.Struct: + return &ast.CompositeLit{Type: typeExpr} + + default: + return nil + } +} + +func isDefaultLiteralType(typ types.Type) bool { + btyp, ok := typ.(*types.Basic) + if !ok { + return false + } + switch btyp.Kind() { + case types.Bool, types.Int, types.Float64, types.String: + return true + default: + return false + } +} diff --git a/vendor/github.com/go-critic/go-critic/checkers/newDeref_checker.go b/vendor/github.com/go-critic/go-critic/checkers/newDeref_checker.go new file mode 100644 index 00000000..75e7f642 --- /dev/null +++ b/vendor/github.com/go-critic/go-critic/checkers/newDeref_checker.go @@ -0,0 +1,45 @@ +package checkers + +import ( + "go/ast" + + "github.com/go-critic/go-critic/checkers/internal/lintutil" + "github.com/go-lintpack/lintpack" + "github.com/go-lintpack/lintpack/astwalk" + "github.com/go-toolsmith/astcast" + "golang.org/x/tools/go/ast/astutil" +) + +func init() { + var info lintpack.CheckerInfo + info.Name = "newDeref" + info.Tags = []string{"style", "experimental"} + info.Summary = "Detects immediate dereferencing of `new` expressions" + info.Before = `x := *new(bool)` + info.After = `x := false` + + collection.AddChecker(&info, func(ctx *lintpack.CheckerContext) lintpack.FileWalker { + return astwalk.WalkerForExpr(&newDerefChecker{ctx: ctx}) + }) +} + +type newDerefChecker struct { + astwalk.WalkHandler + ctx *lintpack.CheckerContext +} + +func (c *newDerefChecker) VisitExpr(expr ast.Expr) { + deref := astcast.ToStarExpr(expr) + call := astcast.ToCallExpr(deref.X) + if astcast.ToIdent(call.Fun).Name == "new" { + typ := c.ctx.TypesInfo.TypeOf(call.Args[0]) + zv := lintutil.ZeroValueOf(astutil.Unparen(call.Args[0]), typ) + if zv != nil { + c.warn(expr, zv) + } + } +} + +func (c *newDerefChecker) warn(cause, suggestion ast.Expr) { + c.ctx.Warn(cause, "replace `%s` with `%s`", cause, suggestion) +} diff --git a/vendor/github.com/go-critic/go-critic/checkers/rangeValCopy_checker.go b/vendor/github.com/go-critic/go-critic/checkers/rangeValCopy_checker.go index 0e243692..182538a9 100644 --- a/vendor/github.com/go-critic/go-critic/checkers/rangeValCopy_checker.go +++ b/vendor/github.com/go-critic/go-critic/checkers/rangeValCopy_checker.go @@ -70,6 +70,6 @@ func (c *rangeValCopyChecker) VisitStmt(stmt ast.Stmt) { } } -func (c *rangeValCopyChecker) warn(node ast.Node, size int64) { - c.ctx.Warn(node, "each iteration copies %d bytes (consider pointers or indexing)", size) +func (c *rangeValCopyChecker) warn(n ast.Node, size int64) { + c.ctx.Warn(n, "each iteration copies %d bytes (consider pointers or indexing)", size) } diff --git a/vendor/github.com/go-critic/go-critic/checkers/typeSwitchVar_checker.go b/vendor/github.com/go-critic/go-critic/checkers/typeSwitchVar_checker.go index 527383a7..a113597b 100644 --- a/vendor/github.com/go-critic/go-critic/checkers/typeSwitchVar_checker.go +++ b/vendor/github.com/go-critic/go-critic/checkers/typeSwitchVar_checker.go @@ -83,6 +83,6 @@ func (c *typeSwitchVarChecker) checkTypeSwitch(root *ast.TypeSwitchStmt) { } } -func (c *typeSwitchVarChecker) warn(node ast.Node, caseIndex int) { - c.ctx.Warn(node, "case %d can benefit from type switch with assignment", caseIndex) +func (c *typeSwitchVarChecker) warn(n ast.Node, caseIndex int) { + c.ctx.Warn(n, "case %d can benefit from type switch with assignment", caseIndex) } diff --git a/vendor/github.com/go-critic/go-critic/checkers/utils.go b/vendor/github.com/go-critic/go-critic/checkers/utils.go index f25a82ef..f8d9ff36 100644 --- a/vendor/github.com/go-critic/go-critic/checkers/utils.go +++ b/vendor/github.com/go-critic/go-critic/checkers/utils.go @@ -8,6 +8,180 @@ import ( "github.com/go-lintpack/lintpack" ) +// goStdlib contains `go list std` command output list. +// Used to detect packages that belong to standard Go packages distribution. +var goStdlib = map[string]bool{ + "archive/tar": true, + "archive/zip": true, + "bufio": true, + "bytes": true, + "compress/bzip2": true, + "compress/flate": true, + "compress/gzip": true, + "compress/lzw": true, + "compress/zlib": true, + "container/heap": true, + "container/list": true, + "container/ring": true, + "context": true, + "crypto": true, + "crypto/aes": true, + "crypto/cipher": true, + "crypto/des": true, + "crypto/dsa": true, + "crypto/ecdsa": true, + "crypto/elliptic": true, + "crypto/hmac": true, + "crypto/internal/randutil": true, + "crypto/internal/subtle": true, + "crypto/md5": true, + "crypto/rand": true, + "crypto/rc4": true, + "crypto/rsa": true, + "crypto/sha1": true, + "crypto/sha256": true, + "crypto/sha512": true, + "crypto/subtle": true, + "crypto/tls": true, + "crypto/x509": true, + "crypto/x509/pkix": true, + "database/sql": true, + "database/sql/driver": true, + "debug/dwarf": true, + "debug/elf": true, + "debug/gosym": true, + "debug/macho": true, + "debug/pe": true, + "debug/plan9obj": true, + "encoding": true, + "encoding/ascii85": true, + "encoding/asn1": true, + "encoding/base32": true, + "encoding/base64": true, + "encoding/binary": true, + "encoding/csv": true, + "encoding/gob": true, + "encoding/hex": true, + "encoding/json": true, + "encoding/pem": true, + "encoding/xml": true, + "errors": true, + "expvar": true, + "flag": true, + "fmt": true, + "go/ast": true, + "go/build": true, + "go/constant": true, + "go/doc": true, + "go/format": true, + "go/importer": true, + "go/internal/gccgoimporter": true, + "go/internal/gcimporter": true, + "go/internal/srcimporter": true, + "go/parser": true, + "go/printer": true, + "go/scanner": true, + "go/token": true, + "go/types": true, + "hash": true, + "hash/adler32": true, + "hash/crc32": true, + "hash/crc64": true, + "hash/fnv": true, + "html": true, + "html/template": true, + "image": true, + "image/color": true, + "image/color/palette": true, + "image/draw": true, + "image/gif": true, + "image/internal/imageutil": true, + "image/jpeg": true, + "image/png": true, + "index/suffixarray": true, + "internal/bytealg": true, + "internal/cpu": true, + "internal/nettrace": true, + "internal/poll": true, + "internal/race": true, + "internal/singleflight": true, + "internal/syscall/unix": true, + "internal/syscall/windows": true, + "internal/syscall/windows/registry": true, + "internal/syscall/windows/sysdll": true, + "internal/testenv": true, + "internal/testlog": true, + "internal/trace": true, + "io": true, + "io/ioutil": true, + "log": true, + "log/syslog": true, + "math": true, + "math/big": true, + "math/bits": true, + "math/cmplx": true, + "math/rand": true, + "mime": true, + "mime/multipart": true, + "mime/quotedprintable": true, + "net": true, + "net/http": true, + "net/http/cgi": true, + "net/http/cookiejar": true, + "net/http/fcgi": true, + "net/http/httptest": true, + "net/http/httptrace": true, + "net/http/httputil": true, + "net/http/internal": true, + "net/http/pprof": true, + "net/internal/socktest": true, + "net/mail": true, + "net/rpc": true, + "net/rpc/jsonrpc": true, + "net/smtp": true, + "net/textproto": true, + "net/url": true, + "os": true, + "os/exec": true, + "os/signal": true, + "os/signal/internal/pty": true, + "os/user": true, + "path": true, + "path/filepath": true, + "plugin": true, + "reflect": true, + "regexp": true, + "regexp/syntax": true, + "runtime": true, + "runtime/cgo": true, + "runtime/debug": true, + "runtime/internal/atomic": true, + "runtime/internal/sys": true, + "runtime/pprof": true, + "runtime/pprof/internal/profile": true, + "runtime/race": true, + "runtime/trace": true, + "sort": true, + "strconv": true, + "strings": true, + "sync": true, + "sync/atomic": true, + "syscall": true, + "testing": true, + "testing/internal/testdeps": true, + "testing/iotest": true, + "testing/quick": true, + "text/scanner": true, + "text/tabwriter": true, + "text/template": true, + "text/template/parse": true, + "time": true, + "unicode": true, + "unicode/utf16": true, + "unicode/utf8": true, + "unsafe": true, +} + var goBuiltins = map[string]bool{ // Types "bool": true, @@ -64,7 +238,7 @@ func isBuiltin(sym string) bool { // isStdlibPkg reports whether pkg is a package from the Go standard library. func isStdlibPkg(pkg *types.Package) bool { - return pkg != nil && pkg.Path() == pkg.Name() + return pkg != nil && goStdlib[pkg.Path()] } // isUnitTestFunc reports whether FuncDecl declares testing function. diff --git a/vendor/github.com/go-critic/go-critic/checkers/wrapperFunc_checker.go b/vendor/github.com/go-critic/go-critic/checkers/wrapperFunc_checker.go index e2308ff4..bba82e5e 100644 --- a/vendor/github.com/go-critic/go-critic/checkers/wrapperFunc_checker.go +++ b/vendor/github.com/go-critic/go-critic/checkers/wrapperFunc_checker.go @@ -39,6 +39,10 @@ func init() { "sync.WaitGroup.Add => WaitGroup.Done": { {0, "-1"}, }, + + "bytes.Buffer.Truncate => Buffer.Reset": { + {0, "0"}, + }, } pkgPatterns := map[string][]arg{ @@ -46,6 +50,9 @@ func init() { {0, "http.NotFound"}, }, + "strings.SplitN => strings.Split": { + {2, "-1"}, + }, "strings.Replace => strings.ReplaceAll": { {3, "-1"}, }, @@ -56,6 +63,9 @@ func init() { {0, "unicode.ToTitle"}, }, + "bytes.SplitN => bytes.Split": { + {2, "-1"}, + }, "bytes.Replace => bytes.ReplaceAll": { {3, "-1"}, }, diff --git a/vendor/modules.txt b/vendor/modules.txt index 7c79ac86..bc15d808 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -10,7 +10,7 @@ github.com/davecgh/go-spew/spew github.com/fatih/color # github.com/fsnotify/fsnotify v1.4.7 github.com/fsnotify/fsnotify -# github.com/go-critic/go-critic v0.0.0-20181204210945-ee9bf5809ead +# github.com/go-critic/go-critic v0.0.0-20181204210945-c3db6069acc5 github.com/go-critic/go-critic/checkers github.com/go-critic/go-critic/checkers/internal/lintutil # github.com/go-lintpack/lintpack v0.5.2