From 611c6f7c81185499a1bd4abde01ed2e20230bad1 Mon Sep 17 00:00:00 2001 From: chavacava Date: Sat, 14 Dec 2019 08:27:06 +0100 Subject: [PATCH] firts working version --- config.go | 1 + fixtures/cognitive-complexity.go | 204 ++++++++++++++++++++++++++++++ rule/cognitive-complexity.go | 165 ++++++++++++++++++++++++ test/cognitive-complexity_test.go | 14 ++ 4 files changed, 384 insertions(+) create mode 100644 fixtures/cognitive-complexity.go create mode 100644 rule/cognitive-complexity.go create mode 100644 test/cognitive-complexity_test.go diff --git a/config.go b/config.go index 508ba6e..06863fd 100644 --- a/config.go +++ b/config.go @@ -80,6 +80,7 @@ var allRules = append([]lint.Rule{ &rule.BareReturnRule{}, &rule.UnusedReceiverRule{}, &rule.UnhandledErrorRule{}, + &rule.CognitiveComplexityRule{}, }, defaultRules...) var allFormatters = []lint.Formatter{ diff --git a/fixtures/cognitive-complexity.go b/fixtures/cognitive-complexity.go new file mode 100644 index 0000000..dd20f0c --- /dev/null +++ b/fixtures/cognitive-complexity.go @@ -0,0 +1,204 @@ +// Test of cognitive complexity. + +// Package pkg ... +package pkg + +import ( + "fmt" + ast "go/ast" + "log" +) + +func f(x int) bool { // MATCH /function f has cognitive complexity 3 (> max enabled 0)/ + if x > 0 && true || false { + return true + } else { + log.Printf("non-positive x: %d", x) + } + return false +} + +func g(f func() bool) string { // MATCH /function g has cognitive complexity 1 (> max enabled 0)/ + if ok := f(); ok { + return "it's okay" + } else { + return "it's NOT okay!" + } +} + +func h(a, b, c, d, e, f bool) bool { // MATCH /function h has cognitive complexity 2 (> max enabled 0)/ + return a && b && c || d || e && f //FIXME: complexity should be 3 +} + +func i(a, b, c, d, e, f bool) bool { // MATCH /function i has cognitive complexity 2 (> max enabled 0)/ + result := a && b && c || d || e + return result +} + +func j(a, b, c, d, e, f bool) bool { // MATCH /function j has cognitive complexity 2 (> max enabled 0)/ + result := z(a && !(b && c)) + return result +} + +func k(a, b, c, d, e, f bool) bool { // MATCH /function k has cognitive complexity 1 (> max enabled 0)/ + switch expr { + case cond1: + case cond2: + default: + } + + return result +} + +func l() { // MATCH /function l has cognitive complexity 6 (> max enabled 0)/ + for i := 1; i <= max; i++ { + for j := 2; j < i; j++ { + if i%j == 0 { + continue + } + } + + total += i + } + return total +} + +func m() { // MATCH /function m has cognitive complexity 6 (> max enabled 0)/ + if i <= max { + if j < i { + if i%j == 0 { + return 0 + } + } + + total += i + } + return total +} + +func n() { // MATCH /function n has cognitive complexity 6 (> max enabled 0)/ + if i > max { + for j := 2; j < i; j++ { + if i%j == 0 { + continue + } + } + + total += i + } + return total +} + +func o() { // MATCH /function o has cognitive complexity 12 (> max enabled 0)/ + if i > max { + if j < i { + if i%j == 0 { + return + } + } + + total += i + } + + if i > max { + if j < i { + if i%j == 0 { + return + } + } + + total += i + } +} + +func p() { // MATCH /function p has cognitive complexity 1 (> max enabled 0)/ + switch n := n.(type) { + case *ast.IfStmt: + targets := []ast.Node{n.Cond, n.Body, n.Else} + v.walk(targets...) + return nil + case *ast.ForStmt: + v.walk(n.Body) + return nil + case *ast.TypeSwitchStmt: + v.walk(n.Body) + return nil + case *ast.BinaryExpr: + v.complexity += v.binExpComplexity(n) + return nil // skip visiting binexp sub-tree (already visited by binExpComplexity) + } +} + +func q() { // MATCH /function q has cognitive complexity 1 (> max enabled 0)/ + for _, t := range targets { + ast.Walk(v, t) + } +} + +func r() { // MATCH /function r has cognitive complexity 1 (> max enabled 0)/ + select { + case c <- x: + x, y = y, x+y + case <-quit: + fmt.Println("quit") + return + } +} + +func s() { // MATCH /function s has cognitive complexity 3 (> max enabled 0)/ +FirstLoop: + for i := 0; i < 10; i++ { + break + } + for i := 0; i < 10; i++ { + break FirstLoop + } +} + +func t() { // MATCH /function t has cognitive complexity 2 (> max enabled 0)/ +FirstLoop: + for i := 0; i < 10; i++ { + goto FirstLoop + } +} + +func u() { // MATCH /function u has cognitive complexity 3 (> max enabled 0)/ +FirstLoop: + for i := 0; i < 10; i++ { + continue + } + for i := 0; i < 10; i++ { + continue FirstLoop + } +} + +func v() { // MATCH /function v has cognitive complexity 2 (> max enabled 0)/ + myFunc := func(b bool) { + if b { + // do something + } + } +} + +func w() { // MATCH /function w has cognitive complexity 3 (> max enabled 0)/ + defer func(b bool) { + if b { + // do something + } + }(false || true) +} + +func sumOfPrimes(max int) int { // MATCH /function sumOfPrimes has cognitive complexity 7 (> max enabled 0)/ + total := 0 +OUT: + for i := 1; i <= max; i++ { + for j := 2; j < i; j++ { + if i%j == 0 { + continue OUT + } + } + + total += i + } + return total +} diff --git a/rule/cognitive-complexity.go b/rule/cognitive-complexity.go new file mode 100644 index 0000000..493c549 --- /dev/null +++ b/rule/cognitive-complexity.go @@ -0,0 +1,165 @@ +package rule + +import ( + "fmt" + "go/ast" + "go/token" + + "github.com/mgechev/revive/lint" +) + +// CognitiveComplexityRule lints given else constructs. +type CognitiveComplexityRule struct{} + +// Apply applies the rule to given file. +func (r *CognitiveComplexityRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + var failures []lint.Failure + + const expectedArgumentsCount = 1 + if len(arguments) < expectedArgumentsCount { + panic(fmt.Sprintf("not enough arguments for cognitive-complexity, expected %d, got %d", expectedArgumentsCount, len(arguments))) + } + complexity, ok := arguments[0].(int64) + if !ok { + panic(fmt.Sprintf("invalid argument type for cognitive-complexity, expected int64, got %T", arguments[0])) + } + + linter := cognitiveComplexityLinter{ + file: file, + maxComplexity: int(complexity), + onFailure: func(failure lint.Failure) { + failures = append(failures, failure) + }, + } + + linter.lint() + + return failures +} + +// Name returns the rule name. +func (r *CognitiveComplexityRule) Name() string { + return "cognitive-complexity" +} + +type cognitiveComplexityLinter struct { + file *lint.File + maxComplexity int + onFailure func(lint.Failure) +} + +func (w cognitiveComplexityLinter) lint() ast.Visitor { + f := w.file + for _, decl := range f.AST.Decls { + if fn, ok := decl.(*ast.FuncDecl); ok { + v := cognitiveComplexityVisitor{} + c := v.subTreeComplexity(fn.Body) + if c > w.maxComplexity { + w.onFailure(lint.Failure{ + Confidence: 1, + Category: "maintenance", + Failure: fmt.Sprintf("function %s has cognitive complexity %d (> max enabled %d)", funcName(fn), c, w.maxComplexity), + Node: fn, + }) + } + } + } + + return nil +} + +type cognitiveComplexityVisitor struct { + complexity int + nestingLevel int +} + +// complexity calculates the cognitive complexity of an AST-subtree. +func (v cognitiveComplexityVisitor) subTreeComplexity(n ast.Node) int { + ast.Walk(&v, n) + return v.complexity +} + +// Visit implements the ast.Visitor interface. +func (v *cognitiveComplexityVisitor) Visit(n ast.Node) ast.Visitor { + switch n := n.(type) { + case *ast.IfStmt: + targets := []ast.Node{n.Cond, n.Body, n.Else} + v.walk(1, targets...) + return nil + case *ast.ForStmt: + targets := []ast.Node{n.Cond, n.Body} + v.walk(1, targets...) + return nil + case *ast.RangeStmt: + v.walk(1, n.Body) + return nil + case *ast.SelectStmt: + v.walk(1, n.Body) + return nil + case *ast.SwitchStmt: + v.walk(1, n.Body) + return nil + case *ast.TypeSwitchStmt: + v.walk(1, n.Body) + return nil + case *ast.FuncLit: + v.walk(0, n.Body) // do not increment the complexity, just do the nesting + return nil + case *ast.BinaryExpr: + v.complexity += v.binExpComplexity(n) + return nil // skip visiting binexp sub-tree (already visited by binExpComplexity) + case *ast.BranchStmt: + if n.Label != nil { + v.complexity += 1 + } + } + // TODO handle (at least) direct recursion + + return v +} + +func (v *cognitiveComplexityVisitor) walk(complexityIncrement int, targets ...ast.Node) { + v.complexity += complexityIncrement + v.nestingLevel + nesting := v.nestingLevel + v.nestingLevel++ + + for _, t := range targets { + if t == nil { + continue + } + + ast.Walk(v, t) + } + + v.nestingLevel = nesting +} + +func (cognitiveComplexityVisitor) binExpComplexity(n *ast.BinaryExpr) int { + calculator := binExprComplexityCalculator{complexity: 0} + ast.Walk(&calculator, n) + + return calculator.complexity +} + +type binExprComplexityCalculator struct { + complexity int + currentOp token.Token +} + +func (v *binExprComplexityCalculator) Visit(n ast.Node) ast.Visitor { + switch n := n.(type) { + case *ast.UnaryExpr: + // TODO (chavacava): confirm if NOT should be taken into account + if n.Op == token.NOT { + v.complexity++ + } + case *ast.BinaryExpr: + isLogicOp := n.Op == token.LAND || n.Op == token.LOR + if isLogicOp && n.Op != v.currentOp { + v.complexity++ + v.currentOp = n.Op + } + } + + return v +} diff --git a/test/cognitive-complexity_test.go b/test/cognitive-complexity_test.go new file mode 100644 index 0000000..12e90b0 --- /dev/null +++ b/test/cognitive-complexity_test.go @@ -0,0 +1,14 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/lint" + "github.com/mgechev/revive/rule" +) + +func TestCognitiveComplexity(t *testing.T) { + testRule(t, "cognitive-complexity", &rule.CognitiveComplexityRule{}, &lint.RuleConfig{ + Arguments: []interface{}{int64(0)}, + }) +}