diff --git a/.golangci.example.yml b/.golangci.example.yml index c2d24c33749c..5df0f59c45f1 100644 --- a/.golangci.example.yml +++ b/.golangci.example.yml @@ -116,6 +116,9 @@ linters-settings: gocyclo: # minimal code complexity to report, 30 by default (but we recommend 10-20) min-complexity: 10 + gocognit: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 10 maligned: # print struct with more effective memory layout or not, false by default suggest-new: true diff --git a/README.md b/README.md index 1e4d4867bc1d..66d1780d87b3 100644 --- a/README.md +++ b/README.md @@ -201,6 +201,7 @@ dupl: Tool for code clone detection [fast: true, auto-fix: false] funlen: Tool for detection of long functions [fast: true, auto-fix: false] gochecknoglobals: Checks that no globals are present in Go code [fast: true, auto-fix: false] gochecknoinits: Checks that no init functions are present in Go code [fast: true, auto-fix: false] +gocognit: Computes and checks the cognitive complexity of functions [fast: true, auto-fix: false] goconst: Finds repeated strings that could be replaced by a constant [fast: true, auto-fix: false] gocritic: The most opinionated Go source code linter [fast: true, auto-fix: false] gocyclo: Computes and checks the cyclomatic complexity of functions [fast: true, auto-fix: false] @@ -449,6 +450,7 @@ golangci-lint help linters - [dupl](https://github.com/mibk/dupl) - Tool for code clone detection - [goconst](https://github.com/jgautheron/goconst) - Finds repeated strings that could be replaced by a constant - [gocyclo](https://github.com/alecthomas/gocyclo) - Computes and checks the cyclomatic complexity of functions +- [gocognit](https://github.com/uudashr/gocognit) - Computes and checks the cognitive complexity of functions - [gofmt](https://golang.org/cmd/gofmt/) - Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification - [goimports](https://godoc.org/golang.org/x/tools/cmd/goimports) - Goimports does everything that gofmt does. Additionally it checks unused imports - [maligned](https://github.com/mdempsky/maligned) - Tool to detect Go structs that would take less memory if their fields were sorted @@ -703,6 +705,9 @@ linters-settings: gocyclo: # minimal code complexity to report, 30 by default (but we recommend 10-20) min-complexity: 10 + gocognit: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 10 maligned: # print struct with more effective memory layout or not, false by default suggest-new: true @@ -1108,6 +1113,7 @@ Thanks to developers and authors of used linters: - [jgautheron](https://github.com/jgautheron) - [remyoudompheng](https://github.com/remyoudompheng) - [alecthomas](https://github.com/alecthomas) +- [uudashr](https://github.com/uudashr) - [OpenPeeDeeP](https://github.com/OpenPeeDeeP) - [client9](https://github.com/client9) - [walle](https://github.com/walle) diff --git a/go.mod b/go.mod index 51e7d8441ceb..4e9479247785 100644 --- a/go.mod +++ b/go.mod @@ -37,6 +37,7 @@ require ( github.com/timakin/bodyclose v0.0.0-20190930140734-f7f2e9bca95e github.com/ultraware/funlen v0.0.2 github.com/ultraware/whitespace v0.0.3 + github.com/uudashr/gocognit v0.0.0-20190926065955-1655d0de0517 github.com/valyala/quicktemplate v1.2.0 golang.org/x/tools v0.0.0-20190912215617-3720d1ec3678 gopkg.in/yaml.v2 v2.2.2 diff --git a/go.sum b/go.sum index 570a2b6284a2..b51dbe75772d 100644 --- a/go.sum +++ b/go.sum @@ -234,6 +234,8 @@ github.com/ultraware/funlen v0.0.2 h1:Av96YVBwwNSe4MLR7iI/BIa3VyI7/djnto/pK3Uxbd github.com/ultraware/funlen v0.0.2/go.mod h1:Dp4UiAus7Wdb9KUZsYWZEWiRzGuM2kXM1lPbfaF6xhA= github.com/ultraware/whitespace v0.0.3 h1:S5BCRRB5sttNy0bSOhbpw+0mb+cHiCmWfrvxpEzuUk0= github.com/ultraware/whitespace v0.0.3/go.mod h1:aVMh/gQve5Maj9hQ/hg+F75lr/X5A89uZnzAmWSineA= +github.com/uudashr/gocognit v0.0.0-20190926065955-1655d0de0517 h1:ChMKTho2hWKpks/nD/FL2KqM1wuVt62oJeiE8+eFpGs= +github.com/uudashr/gocognit v0.0.0-20190926065955-1655d0de0517/go.mod h1:j44Ayx2KW4+oB6SWMv8KsmHzZrOInQav7D3cQMJ5JUM= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/valyala/fasthttp v1.2.0/go.mod h1:4vX61m6KN+xDduDNwXrhIAVZaZaZiQ1luJk8LWSxF3s= diff --git a/pkg/config/config.go b/pkg/config/config.go index f07037d947b2..7f97490884e7 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -138,6 +138,9 @@ type LintersSettings struct { Gocyclo struct { MinComplexity int `mapstructure:"min-complexity"` } + Gocognit struct { + MinComplexity int `mapstructure:"min-complexity"` + } Varcheck struct { CheckExportedFields bool `mapstructure:"exported-fields"` } diff --git a/pkg/golinters/gocognit.go b/pkg/golinters/gocognit.go new file mode 100644 index 000000000000..8a3b65cd28d9 --- /dev/null +++ b/pkg/golinters/gocognit.go @@ -0,0 +1,69 @@ +// nolint:dupl +package golinters + +import ( + "fmt" + "sort" + "sync" + + "github.com/uudashr/gocognit" + "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" + "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/result" +) + +const gocognitName = "gocognit" + +func NewGocognit() *goanalysis.Linter { + var mu sync.Mutex + var resIssues []result.Issue + + analyzer := &analysis.Analyzer{ + Name: goanalysis.TheOnlyAnalyzerName, + Doc: goanalysis.TheOnlyanalyzerDoc, + } + return goanalysis.NewLinter( + gocognitName, + "Computes and checks the cognitive complexity of functions", + []*analysis.Analyzer{analyzer}, + nil, + ).WithContextSetter(func(lintCtx *linter.Context) { + analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { + var stats []gocognit.Stat + for _, f := range pass.Files { + stats = gocognit.ComplexityStats(f, pass.Fset, stats) + } + if len(stats) == 0 { + return nil, nil + } + + sort.Slice(stats, func(i, j int) bool { + return stats[i].Complexity > stats[j].Complexity + }) + + res := make([]result.Issue, 0, len(stats)) + for _, s := range stats { + if s.Complexity <= lintCtx.Settings().Gocognit.MinComplexity { + break // Break as the stats is already sorted from greatest to least + } + + res = append(res, result.Issue{ + Pos: s.Pos, + Text: fmt.Sprintf("cognitive complexity %d of func %s is high (> %d)", + s.Complexity, formatCode(s.FuncName, lintCtx.Cfg), lintCtx.Settings().Gocognit.MinComplexity), + FromLinter: gocognitName, + }) + } + + mu.Lock() + resIssues = append(resIssues, res...) + mu.Unlock() + + return nil, nil + } + }).WithIssuesReporter(func(*linter.Context) []result.Issue { + return resIssues + }).WithLoadMode(goanalysis.LoadModeSyntax) +} diff --git a/pkg/golinters/gocyclo.go b/pkg/golinters/gocyclo.go index 60d439fecbc8..8e11775b2f85 100644 --- a/pkg/golinters/gocyclo.go +++ b/pkg/golinters/gocyclo.go @@ -1,3 +1,4 @@ +// nolint:dupl package golinters import ( diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 72f88573b33c..89b4b7af4b9c 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -137,6 +137,9 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { linter.NewConfig(golinters.NewGocyclo()). WithPresets(linter.PresetComplexity). WithURL("https://github.com/alecthomas/gocyclo"), + linter.NewConfig(golinters.NewGocognit()). + WithPresets(linter.PresetComplexity). + WithURL("https://github.com/uudashr/gocognit"), linter.NewConfig(golinters.NewTypecheck()). WithLoadForGoAnalysis(). WithPresets(linter.PresetBugs). diff --git a/test/testdata/gocognit.go b/test/testdata/gocognit.go new file mode 100644 index 000000000000..f73420b4ff60 --- /dev/null +++ b/test/testdata/gocognit.go @@ -0,0 +1,23 @@ +//args: -Egocognit +//config: linters-settings.gocognit.min-complexity=2 +package testdata + +func GocognitGetWords(number int) string { // ERROR "cognitive complexity 4 of func .* is high .*" + if number == 1 { // +1 + return "one" + } else if number == 2 { // +1 + return "a couple" + } else if number == 3 { // +1 + return "a few" + } else { // +1 + return "lots" + } +} // total complexity = 4 + +func GoCognitFact(n int) int { // ERROR "cognitive complexity 3 of func .* is high .*" + if n <= 1 { // +1 + return 1 + } else { // +1 + return n + GoCognitFact(n-1) // +1 + } +} // total complexity = 3 diff --git a/vendor/github.com/uudashr/gocognit/LICENSE b/vendor/github.com/uudashr/gocognit/LICENSE new file mode 100644 index 000000000000..75d4b9c98b93 --- /dev/null +++ b/vendor/github.com/uudashr/gocognit/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2019 Nuruddin Ashr + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/vendor/github.com/uudashr/gocognit/README.md b/vendor/github.com/uudashr/gocognit/README.md new file mode 100644 index 000000000000..4a8846907fe5 --- /dev/null +++ b/vendor/github.com/uudashr/gocognit/README.md @@ -0,0 +1,185 @@ +[![GoDoc](https://godoc.org/github.com/uudashr/gocognit?status.svg)](https://godoc.org/github.com/uudashr/gocognit) +# Gocognit +Gocognit calculates cognitive complexities of functions in Go source code. A measurement of how hard does the code is intuitively to understand. + +## Understanding the complexity + +Given code using `if` statement, +```go +func GetWords(number int) string { + if number == 1 { // +1 + return "one" + } else if number == 2 { // +1 + return "a couple" + } else if number == 3 { // +1 + return "a few" + } else { // +1 + return "lots" + } +} // Cognitive complexity = 4 +``` + +Above code can be refactored using `switch` statement, +```go +func GetWords(number int) string { + switch number { // +1 + case 1: + return "one" + case 2: + return "a couple" + case 3: + return "a few" + default: + return "lots" + } +} // Cognitive complexity = 1 +``` + +As you see above codes are the same, but the second code are easier to understand, that is why the cognitive complexity score are lower compare to the first one. + +## Comparison with cyclometic complexity + +### Example 1 +#### Cyclometic complexity +```go +func GetWords(number int) string { // +1 + switch number { + case 1: // +1 + return "one" + case 2: // +1 + return "a couple" + case 3: // +1 + return "a few" + default: + return "lots" + } +} // Cyclomatic complexity = 4 +``` + +#### Cognitive complexity +```go +func GetWords(number int) string { + switch number { // +1 + case 1: + return "one" + case 2: + return "a couple" + case 3: + return "a few" + default: + return "lots" + } +} // Cognitive complexity = 1 +``` + +Cognitive complexity give lower score compare to cyclomatic complexity. + +### Example 2 +#### Cyclomatic complexity +```go +func SumOfPrimes(max int) int { // +1 + var total int + +OUT: + for i := 1; i < max; i++ { // +1 + for j := 2; j < i; j++ { // +1 + if i%j == 0 { // +1 + continue OUT + } + } + total += i + } + + return total +} // Cyclomatic complexity = 4 +``` + +#### Cognitive complexity +```go +func SumOfPrimes(max int) int { + var total int + +OUT: + for i := 1; i < max; i++ { // +1 + for j := 2; j < i; j++ { // +2 (nesting = 1) + if i%j == 0 { // +3 (nesting = 2) + continue OUT // +1 + } + } + total += i + } + + return total +} // Cognitive complexity = 7 +``` + +Cognitive complexity give higher score compare to cyclomatic complexity. + +## Rules + +The cognitive complexity of a function is calculated according to the +following rules: +> Note: these rules are specific for Go, please see the [original whitepaper](https://www.sonarsource.com/docs/CognitiveComplexity.pdf) for more complete reference. + +### Increments +There is an increment for each of the following: +1. `if`, `else if`, `else` +2. `switch`, `select` +3. `for` +4. `goto` LABEL, `break` LABEL, `continue` LABEL +5. sequence of binary logical operators +6. each method in a recursion cycle + +### Nesting level +The following structures increment the nesting level: +1. `if`, `else if`, `else` +2. `switch`, `select` +3. `for` +4. function literal or lambda + +### Nesting increments +The following structures receive a nesting increment commensurate with their nested depth inside nesting structures: +1. `if` +2. `switch`, `select` +3. `for` + +## Installation +``` +$ go get github.com/uudashr/gocognit/cmd/gocognit +``` + +## Usage + +``` +$ gocognit +Calculate cognitive complexities of Go functions. +Usage: + gocognit [flags] ... +Flags: + -over N show functions with complexity > N only and + return exit code 1 if the set is non-empty + -top N show the top N most complex functions only + -avg show the average complexity over all functions, + not depending on whether -over or -top are set +The output fields for each line are: + +``` + +Examples: + +``` +$ gocognit . +$ gocognit main.go +$ gocognit -top 10 src/ +$ gocognit -over 25 docker +$ gocognit -avg . +``` + +The output fields for each line are: +``` + +``` + +## Related project +- [Gocyclo](https://github.com/fzipp/gocyclo) where the code are based on. +- [Cognitive Complexity: A new way of measuring understandability](https://www.sonarsource.com/docs/CognitiveComplexity.pdf) white paper by G. Ann Campbell. \ No newline at end of file diff --git a/vendor/github.com/uudashr/gocognit/go.mod b/vendor/github.com/uudashr/gocognit/go.mod new file mode 100644 index 000000000000..1a68d3880bd1 --- /dev/null +++ b/vendor/github.com/uudashr/gocognit/go.mod @@ -0,0 +1,3 @@ +module github.com/uudashr/gocognit + +go 1.13 diff --git a/vendor/github.com/uudashr/gocognit/gocognit.go b/vendor/github.com/uudashr/gocognit/gocognit.go new file mode 100644 index 000000000000..71cafd4fabf4 --- /dev/null +++ b/vendor/github.com/uudashr/gocognit/gocognit.go @@ -0,0 +1,298 @@ +package gocognit + +import ( + "fmt" + "go/ast" + "go/token" +) + +// Stat is statistic of the complexity. +type Stat struct { + PkgName string + FuncName string + Complexity int + Pos token.Position +} + +func (s Stat) String() string { + return fmt.Sprintf("%d %s %s %s", s.Complexity, s.PkgName, s.FuncName, s.Pos) +} + +// ComplexityStats builds the complexity statistics. +func ComplexityStats(f *ast.File, fset *token.FileSet, stats []Stat) []Stat { + for _, decl := range f.Decls { + if fn, ok := decl.(*ast.FuncDecl); ok { + stats = append(stats, Stat{ + PkgName: f.Name.Name, + FuncName: funcName(fn), + Complexity: Complexity(fn), + Pos: fset.Position(fn.Pos()), + }) + } + } + return stats +} + +// funcName returns the name representation of a function or method: +// "(Type).Name" for methods or simply "Name" for functions. +func funcName(fn *ast.FuncDecl) string { + if fn.Recv != nil { + if fn.Recv.NumFields() > 0 { + typ := fn.Recv.List[0].Type + return fmt.Sprintf("(%s).%s", recvString(typ), fn.Name) + } + } + return fn.Name.Name +} + +// recvString returns a string representation of recv of the +// form "T", "*T", or "BADRECV" (if not a proper receiver type). +func recvString(recv ast.Expr) string { + switch t := recv.(type) { + case *ast.Ident: + return t.Name + case *ast.StarExpr: + return "*" + recvString(t.X) + } + return "BADRECV" +} + +// Complexity calculates the cognitive complexity of a function. +func Complexity(fn *ast.FuncDecl) int { + v := complexityVisitor{ + name: fn.Name, + } + ast.Walk(&v, fn) + return v.complexity +} + +type complexityVisitor struct { + name *ast.Ident + complexity int + nesting int + elseNodes map[ast.Node]bool + calculatedExprs map[ast.Expr]bool +} + +func (v *complexityVisitor) incNesting() { + v.nesting++ +} + +func (v *complexityVisitor) decNesting() { + v.nesting-- +} + +func (v *complexityVisitor) incComplexity() { + v.complexity++ +} + +func (v *complexityVisitor) nestIncComplexity() { + v.complexity += (v.nesting + 1) +} + +func (v *complexityVisitor) markAsElseNode(n ast.Node) { + if v.elseNodes == nil { + v.elseNodes = make(map[ast.Node]bool) + } + + v.elseNodes[n] = true +} + +func (v *complexityVisitor) markedAsElseNode(n ast.Node) bool { + if v.elseNodes == nil { + return false + } + + return v.elseNodes[n] +} + +func (v *complexityVisitor) markCalculated(e ast.Expr) { + if v.calculatedExprs == nil { + v.calculatedExprs = make(map[ast.Expr]bool) + } + + v.calculatedExprs[e] = true +} + +func (v *complexityVisitor) isCalculated(e ast.Expr) bool { + if v.calculatedExprs == nil { + return false + } + + return v.calculatedExprs[e] +} + +// Visit implements the ast.Visitor interface. +func (v *complexityVisitor) Visit(n ast.Node) ast.Visitor { + switch n := n.(type) { + case *ast.IfStmt: + return v.visitIfStmt(n) + case *ast.SwitchStmt: + return v.visitSwitchStmt(n) + case *ast.SelectStmt: + return v.visitSelectStmt(n) + case *ast.ForStmt: + return v.visitForStmt(n) + case *ast.FuncLit: + return v.visitFuncLit(n) + case *ast.BranchStmt: + return v.visitBranchStmt(n) + case *ast.BinaryExpr: + return v.visitBinaryExpr(n) + case *ast.CallExpr: + return v.visitCallExpr(n) + } + return v +} + +func (v *complexityVisitor) visitIfStmt(n *ast.IfStmt) ast.Visitor { + v.incIfComplexity(n) + + if n.Init != nil { + ast.Walk(v, n.Init) + } + + ast.Walk(v, n.Cond) + + v.incNesting() + ast.Walk(v, n.Body) + v.decNesting() + + if _, ok := n.Else.(*ast.BlockStmt); ok { + v.incComplexity() + + v.incNesting() + ast.Walk(v, n.Else) + v.decNesting() + } else if _, ok := n.Else.(*ast.IfStmt); ok { + v.markAsElseNode(n.Else) + ast.Walk(v, n.Else) + } + return nil +} + +func (v *complexityVisitor) visitSwitchStmt(n *ast.SwitchStmt) ast.Visitor { + v.nestIncComplexity() + + if n.Init != nil { + ast.Walk(v, n.Init) + } + + if n.Tag != nil { + ast.Walk(v, n.Tag) + } + + v.incNesting() + ast.Walk(v, n.Body) + v.decNesting() + return nil +} + +func (v *complexityVisitor) visitSelectStmt(n *ast.SelectStmt) ast.Visitor { + v.nestIncComplexity() + + v.incNesting() + ast.Walk(v, n.Body) + v.decNesting() + return nil +} + +func (v *complexityVisitor) visitForStmt(n *ast.ForStmt) ast.Visitor { + v.nestIncComplexity() + + if n.Init != nil { + ast.Walk(v, n.Init) + } + + if n.Cond != nil { + ast.Walk(v, n.Cond) + } + + if n.Post != nil { + ast.Walk(v, n.Post) + } + + v.incNesting() + ast.Walk(v, n.Body) + v.decNesting() + return nil +} + +func (v *complexityVisitor) visitFuncLit(n *ast.FuncLit) ast.Visitor { + ast.Walk(v, n.Type) + + v.incNesting() + ast.Walk(v, n.Body) + v.decNesting() + return nil +} + +func (v *complexityVisitor) visitBranchStmt(n *ast.BranchStmt) ast.Visitor { + if n.Label != nil { + v.incComplexity() + } + return v +} + +func (v *complexityVisitor) visitBinaryExpr(n *ast.BinaryExpr) ast.Visitor { + if (n.Op == token.LAND || n.Op == token.LOR) && !v.isCalculated(n) { + ops := v.collectBinaryOps(n) + + var lastOp token.Token + for _, op := range ops { + if lastOp != op { + v.incComplexity() + lastOp = op + } + } + } + return v +} + +func (v *complexityVisitor) visitCallExpr(n *ast.CallExpr) ast.Visitor { + if name, ok := n.Fun.(*ast.Ident); ok { + if name.Obj == v.name.Obj && name.Name == v.name.Name { + v.incComplexity() + } + } + return v +} + +func (v *complexityVisitor) collectBinaryOps(exp ast.Expr) []token.Token { + v.markCalculated(exp) + switch exp := exp.(type) { + case *ast.BinaryExpr: + return mergeBinaryOps(v.collectBinaryOps(exp.X), exp.Op, v.collectBinaryOps(exp.Y)) + case *ast.ParenExpr: + // interest only on what inside paranthese + return v.collectBinaryOps(exp.X) + default: + return []token.Token{} + } +} + +func (v *complexityVisitor) incIfComplexity(n *ast.IfStmt) { + if v.markedAsElseNode(n) { + v.incComplexity() + } else { + v.nestIncComplexity() + } +} + +func mergeBinaryOps(x []token.Token, op token.Token, y []token.Token) []token.Token { + var out []token.Token + if len(x) != 0 { + out = append(out, x...) + } + out = append(out, op) + if len(y) != 0 { + out = append(out, y...) + } + return out +} + +func walkExprList(v ast.Visitor, list []ast.Expr) { + for _, x := range list { + ast.Walk(v, x) + } +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 77b1086d6513..0d480b85839b 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -170,6 +170,8 @@ github.com/timakin/bodyclose/passes/bodyclose github.com/ultraware/funlen # github.com/ultraware/whitespace v0.0.3 github.com/ultraware/whitespace +# github.com/uudashr/gocognit v0.0.0-20190926065955-1655d0de0517 +github.com/uudashr/gocognit # github.com/valyala/bytebufferpool v1.0.0 github.com/valyala/bytebufferpool # github.com/valyala/quicktemplate v1.2.0