diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 01c370a..e9eea04 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,6 +1,12 @@ name: Checks + on: - [pull_request] + push: + branches: + - master + pull_request: + branches: + - master jobs: test: @@ -11,7 +17,7 @@ jobs: matrix: os: [ubuntu-latest] os-version: ['stable'] - go-version: ['1.19', '1.20', '1.21', '1.22'] + go-version: ['1.19', '1.20', '1.21', '1.22', '1.23'] steps: - name: Checkout diff --git a/README.md b/README.md index f063256..415e81e 100644 --- a/README.md +++ b/README.md @@ -169,14 +169,17 @@ Usage: Flags: - -over N show functions with complexity > N only - and return exit code 1 if the output 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 - -json encode the output as JSON - -f format string the format to use - (default "{{.PkgName}}.{{.FuncName}}:{{.Complexity}}:{{.Pos}}") + -over N show functions with complexity > N only + and return exit code 1 if the output 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 + -test indicates whether test files should be included + -json encode the output as JSON + -d enable diagnostic output + -f format string the format to use + (default "{{.Complexity}} {{.PkgName}} {{.FuncName}} {{.Pos}}") + -ignore expr ignore files matching the given regexp The (default) output fields for each line are: @@ -191,10 +194,24 @@ or equal to The struct being passed to the template is: type Stat struct { - PkgName string - FuncName string - Complexity int - Pos token.Position + PkgName string + FuncName string + Complexity int + Pos token.Position + Diagnostics []Diagnostics + } + + type Diagnostic struct { + Inc string + Nesting int + Text string + Pos DiagnosticPosition + } + + type DiagnosticPosition struct { + Offset int + Line int + Column int } ``` @@ -223,6 +240,76 @@ func IgnoreMe() { } ``` +## Diagnostic +To understand how the complexity are calculated, we can enable the diagnostic by using `-d` flag. + +Example: +```shell +$ gocognit -json -d . +``` + +It will show the diagnostic output in JSON format +
+ +JSON Output + +```json +[ + { + "PkgName": "prime", + "FuncName": "SumOfPrimes", + "Complexity": 7, + "Pos": { + "Filename": "prime.go", + "Offset": 15, + "Line": 3, + "Column": 1 + }, + "Diagnostics": [ + { + "Inc": 1, + "Text": "for", + "Pos": { + "Offset": 69, + "Line": 7, + "Column": 2 + } + }, + { + "Inc": 2, + "Nesting": 1, + "Text": "for", + "Pos": { + "Offset": 104, + "Line": 8, + "Column": 3 + } + }, + { + "Inc": 3, + "Nesting": 2, + "Text": "if", + "Pos": { + "Offset": 152, + "Line": 9, + "Column": 4 + } + }, + { + "Inc": 1, + "Text": "continue", + "Pos": { + "Offset": 190, + "Line": 10, + "Column": 5 + } + } + ] + } +] +``` +
+ ## 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. diff --git a/cmd/gocognit/main.go b/cmd/gocognit/main.go index 469c3b2..b617ee6 100644 --- a/cmd/gocognit/main.go +++ b/cmd/gocognit/main.go @@ -10,8 +10,10 @@ // -over N show functions with complexity > N only and return exit code 1 if the output 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 +// -test indicates whether test files should be included // -json encode the output as JSON -// -f format string the format to use (default "{{.PkgName}}.{{.FuncName}}:{{.Complexity}}:{{.Pos}}") +// -d enable diagnostic output +// -f format string the format to use (default "{{.Complexity}} {{.PkgName}} {{.FuncName}} {{.Pos}}") // // The (default) output fields for each line are: // @@ -29,8 +31,22 @@ // PkgName string // FuncName string // Complexity int +// Diagnostics []Diagnostic // Pos token.Position // } +// +// type Diagnostic struct { +// Inc string +// Nesting int +// Text string +// Pos DiagnosticPosition +// } +// +// type DiagnosticPosition struct { +// Offset int +// Line int +// Column int +// } package main import ( @@ -59,15 +75,17 @@ Usage: Flags: - -over N show functions with complexity > N only - and return exit code 1 if the output 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 - -test indicates whether test files should be included - -json encode the output as JSON - -f format string the format to use - (default "{{.PkgName}}.{{.FuncName}}:{{.Complexity}}:{{.Pos}}") + -over N show functions with complexity > N only + and return exit code 1 if the output 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 + -test indicates whether test files should be included + -json encode the output as JSON + -d enable diagnostic output + -f format string the format to use + (default "{{.Complexity}} {{.PkgName}} {{.FuncName}} {{.Pos}}") + -ignore expr ignore files matching the given regexp The (default) output fields for each line are: @@ -82,10 +100,24 @@ or equal to The struct being passed to the template is: type Stat struct { - PkgName string - FuncName string - Complexity int - Pos token.Position + PkgName string + FuncName string + Complexity int + Pos token.Position + Diagnostics []Diagnostics + } + + type Diagnostic struct { + Inc string + Nesting int + Text string + Pos DiagnosticPosition + } + + type DiagnosticPosition struct { + Offset int + Line int + Column int } ` @@ -103,13 +135,14 @@ func usage() { func main() { var ( - over int - top int - avg bool - includeTests bool - format string - jsonEncode bool - ignoreExpr string + over int + top int + avg bool + includeTests bool + format string + jsonEncode bool + enableDiagnostics bool + ignoreExpr string ) flag.IntVar(&over, "over", defaultOverFlagVal, "show functions with complexity > N only") @@ -118,6 +151,7 @@ func main() { flag.BoolVar(&includeTests, "test", true, "indicates whether test files should be included") flag.StringVar(&format, "f", defaultFormat, "the format to use") flag.BoolVar(&jsonEncode, "json", false, "encode the output as JSON") + flag.BoolVar(&enableDiagnostics, "d", false, "enable diagnostic output") flag.StringVar(&ignoreExpr, "ignore", "", "ignore files matching the given regexp") log.SetFlags(0) @@ -136,7 +170,7 @@ func main() { log.Fatal(err) } - stats, err := analyze(args, includeTests) + stats, err := analyze(args, includeTests, enableDiagnostics) if err != nil { log.Fatal(err) } @@ -170,19 +204,19 @@ func main() { } } -func analyzePath(path string, includeTests bool) ([]gocognit.Stat, error) { +func analyzePath(path string, includeTests bool, includeDiagnostic bool) ([]gocognit.Stat, error) { if isDir(path) { - return analyzeDir(path, includeTests, nil) + return analyzeDir(path, includeTests, nil, includeDiagnostic) } - return analyzeFile(path, nil) + return analyzeFile(path, nil, includeDiagnostic) } -func analyze(paths []string, includeTests bool) (stats []gocognit.Stat, err error) { +func analyze(paths []string, includeTests bool, includeDiagnostic bool) (stats []gocognit.Stat, err error) { var out []gocognit.Stat for _, path := range paths { - stats, err := analyzePath(path, includeTests) + stats, err := analyzePath(path, includeTests, includeDiagnostic) if err != nil { return nil, err } @@ -199,7 +233,7 @@ func isDir(filename string) bool { return err == nil && fi.IsDir() } -func analyzeFile(fname string, stats []gocognit.Stat) ([]gocognit.Stat, error) { +func analyzeFile(fname string, stats []gocognit.Stat, includeDiagnostic bool) ([]gocognit.Stat, error) { fset := token.NewFileSet() f, err := parser.ParseFile(fset, fname, nil, parser.ParseComments) @@ -207,10 +241,10 @@ func analyzeFile(fname string, stats []gocognit.Stat) ([]gocognit.Stat, error) { return nil, err } - return gocognit.ComplexityStats(f, fset, stats), nil + return gocognit.ComplexityStatsWithDiagnostic(f, fset, stats, includeDiagnostic), nil } -func analyzeDir(dirname string, includeTests bool, stats []gocognit.Stat) ([]gocognit.Stat, error) { +func analyzeDir(dirname string, includeTests bool, stats []gocognit.Stat, trace bool) ([]gocognit.Stat, error) { err := filepath.Walk(dirname, func(path string, info os.FileInfo, err error) error { if err != nil { return err @@ -228,7 +262,7 @@ func analyzeDir(dirname string, includeTests bool, stats []gocognit.Stat) ([]goc return nil } - stats, err = analyzeFile(path, stats) + stats, err = analyzeFile(path, stats, trace) if err != nil { return err } diff --git a/gocognit.go b/gocognit.go index 126452e..e51ee2a 100644 --- a/gocognit.go +++ b/gocognit.go @@ -4,6 +4,7 @@ import ( "fmt" "go/ast" "go/token" + "strconv" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" @@ -12,10 +13,58 @@ import ( // Stat is statistic of the complexity. type Stat struct { - PkgName string - FuncName string - Complexity int - Pos token.Position + PkgName string + FuncName string + Complexity int + Pos token.Position + Diagnostics []Diagnostic `json:",omitempty"` +} + +// Diagnostic contains information how the complexity increase. +type Diagnostic struct { + Inc int + Nesting int `json:",omitempty"` + Text string + Pos DiagnosticPosition +} + +// DiagnosticPosition is the position of the diagnostic. +type DiagnosticPosition struct { + Offset int // offset, starting at 0 + Line int // line number, starting at 1 + Column int // column number, starting at 1 (byte count) +} + +func (pos DiagnosticPosition) isValid() bool { + return pos.Line > 0 +} + +func (pos DiagnosticPosition) String() string { + var s string + if pos.isValid() { + if s != "" { + s += ":" + } + + s += strconv.Itoa(pos.Line) + if pos.Column != 0 { + s += fmt.Sprintf(":%d", pos.Column) + } + } + + if s == "" { + s = "-" + } + + return s +} + +func (d Diagnostic) String() string { + if d.Nesting == 0 { + return fmt.Sprintf("+%d", d.Inc) + } + + return fmt.Sprintf("+%d (nesting=%d)", d.Inc, d.Nesting) } func (s Stat) String() string { @@ -24,6 +73,11 @@ func (s Stat) String() string { // ComplexityStats builds the complexity statistics. func ComplexityStats(f *ast.File, fset *token.FileSet, stats []Stat) []Stat { + return ComplexityStatsWithDiagnostic(f, fset, stats, false) +} + +// ComplexityStatsWithDiagnostic builds the complexity statistics with diagnostic. +func ComplexityStatsWithDiagnostic(f *ast.File, fset *token.FileSet, stats []Stat, enableDiagnostics bool) []Stat { for _, decl := range f.Decls { if fn, ok := decl.(*ast.FuncDecl); ok { d := parseDirective(fn.Doc) @@ -31,11 +85,14 @@ func ComplexityStats(f *ast.File, fset *token.FileSet, stats []Stat) []Stat { continue } + res := ScanComplexity(fn, enableDiagnostics) + stats = append(stats, Stat{ - PkgName: f.Name.Name, - FuncName: funcName(fn), - Complexity: Complexity(fn), - Pos: fset.Position(fn.Pos()), + PkgName: f.Name.Name, + FuncName: funcName(fn), + Complexity: res.Complexity, + Diagnostics: generateDiagnostics(fset, res.Diagnostics), + Pos: fset.Position(fn.Pos()), }) } } @@ -43,6 +100,28 @@ func ComplexityStats(f *ast.File, fset *token.FileSet, stats []Stat) []Stat { return stats } +func generateDiagnostics(fset *token.FileSet, diags []diagnostic) []Diagnostic { + out := make([]Diagnostic, 0, len(diags)) + + for _, diag := range diags { + pos := fset.Position(diag.Pos) + diagPos := DiagnosticPosition{ + Offset: pos.Offset, + Line: pos.Line, + Column: pos.Column, + } + + out = append(out, Diagnostic{ + Inc: diag.Inc, + Nesting: diag.Nesting, + Text: diag.Text, + Pos: diagPos, + }) + } + + return out +} + type directive struct { Ignore bool } @@ -77,13 +156,36 @@ func funcName(fn *ast.FuncDecl) string { // Complexity calculates the cognitive complexity of a function. func Complexity(fn *ast.FuncDecl) int { + res := ScanComplexity(fn, false) + + return res.Complexity +} + +// ScanComplexity scans the function declaration. +func ScanComplexity(fn *ast.FuncDecl, includeDiagnostics bool) ScanResult { v := complexityVisitor{ - name: fn.Name, + name: fn.Name, + diagnosticsEnabled: includeDiagnostics, } ast.Walk(&v, fn) - return v.complexity + return ScanResult{ + Diagnostics: v.diagnostics, + Complexity: v.complexity, + } +} + +type ScanResult struct { + Diagnostics []diagnostic + Complexity int +} + +type diagnostic struct { + Inc int + Nesting int + Text string + Pos token.Pos } type complexityVisitor struct { @@ -92,6 +194,9 @@ type complexityVisitor struct { nesting int elseNodes map[ast.Node]bool calculatedExprs map[ast.Expr]bool + + diagnosticsEnabled bool + diagnostics []diagnostic } func (v *complexityVisitor) incNesting() { @@ -102,12 +207,33 @@ func (v *complexityVisitor) decNesting() { v.nesting-- } -func (v *complexityVisitor) incComplexity() { +func (v *complexityVisitor) incComplexity(text string, pos token.Pos) { v.complexity++ + + if !v.diagnosticsEnabled { + return + } + + v.diagnostics = append(v.diagnostics, diagnostic{ + Inc: 1, + Text: text, + Pos: pos, + }) } -func (v *complexityVisitor) nestIncComplexity() { +func (v *complexityVisitor) nestIncComplexity(text string, pos token.Pos) { v.complexity += (v.nesting + 1) + + if !v.diagnosticsEnabled { + return + } + + v.diagnostics = append(v.diagnostics, diagnostic{ + Inc: v.nesting + 1, + Nesting: v.nesting, + Text: text, + Pos: pos, + }) } func (v *complexityVisitor) markAsElseNode(n ast.Node) { @@ -171,7 +297,7 @@ func (v *complexityVisitor) Visit(n ast.Node) ast.Visitor { } func (v *complexityVisitor) visitIfStmt(n *ast.IfStmt) ast.Visitor { - v.incIfComplexity(n) + v.incIfComplexity(n, "if", n.Pos()) if n := n.Init; n != nil { ast.Walk(v, n) @@ -184,7 +310,7 @@ func (v *complexityVisitor) visitIfStmt(n *ast.IfStmt) ast.Visitor { v.decNesting() if _, ok := n.Else.(*ast.BlockStmt); ok { - v.incComplexity() + v.incComplexity("else", n.Else.Pos()) ast.Walk(v, n.Else) } else if _, ok := n.Else.(*ast.IfStmt); ok { @@ -196,7 +322,7 @@ func (v *complexityVisitor) visitIfStmt(n *ast.IfStmt) ast.Visitor { } func (v *complexityVisitor) visitSwitchStmt(n *ast.SwitchStmt) ast.Visitor { - v.nestIncComplexity() + v.nestIncComplexity("switch", n.Pos()) if n := n.Init; n != nil { ast.Walk(v, n) @@ -214,7 +340,7 @@ func (v *complexityVisitor) visitSwitchStmt(n *ast.SwitchStmt) ast.Visitor { } func (v *complexityVisitor) visitTypeSwitchStmt(n *ast.TypeSwitchStmt) ast.Visitor { - v.nestIncComplexity() + v.nestIncComplexity("switch", n.Pos()) if n := n.Init; n != nil { ast.Walk(v, n) @@ -232,7 +358,7 @@ func (v *complexityVisitor) visitTypeSwitchStmt(n *ast.TypeSwitchStmt) ast.Visit } func (v *complexityVisitor) visitSelectStmt(n *ast.SelectStmt) ast.Visitor { - v.nestIncComplexity() + v.nestIncComplexity("select", n.Pos()) v.incNesting() ast.Walk(v, n.Body) @@ -242,7 +368,7 @@ func (v *complexityVisitor) visitSelectStmt(n *ast.SelectStmt) ast.Visitor { } func (v *complexityVisitor) visitForStmt(n *ast.ForStmt) ast.Visitor { - v.nestIncComplexity() + v.nestIncComplexity("for", n.Pos()) if n := n.Init; n != nil { ast.Walk(v, n) @@ -264,7 +390,7 @@ func (v *complexityVisitor) visitForStmt(n *ast.ForStmt) ast.Visitor { } func (v *complexityVisitor) visitRangeStmt(n *ast.RangeStmt) ast.Visitor { - v.nestIncComplexity() + v.nestIncComplexity("for", n.Pos()) if n := n.Key; n != nil { ast.Walk(v, n) @@ -289,13 +415,15 @@ func (v *complexityVisitor) visitFuncLit(n *ast.FuncLit) ast.Visitor { 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() + v.incComplexity(n.Tok.String(), n.Pos()) } + return v } @@ -306,8 +434,7 @@ func (v *complexityVisitor) visitBinaryExpr(n *ast.BinaryExpr) ast.Visitor { var lastOp token.Token for _, op := range ops { if lastOp != op { - v.incComplexity() - + v.incComplexity(op.String(), n.OpPos) lastOp = op } } @@ -321,7 +448,7 @@ func (v *complexityVisitor) visitCallExpr(n *ast.CallExpr) ast.Visitor { obj, name := callIdent.Obj, callIdent.Name if obj == v.name.Obj && name == v.name.Name { // called by same function directly (direct recursion) - v.incComplexity() + v.incComplexity(name, n.Pos()) } } @@ -337,11 +464,11 @@ func (v *complexityVisitor) collectBinaryOps(exp ast.Expr) []token.Token { return nil } -func (v *complexityVisitor) incIfComplexity(n *ast.IfStmt) { +func (v *complexityVisitor) incIfComplexity(n *ast.IfStmt, text string, pos token.Pos) { if v.markedAsElseNode(n) { - v.incComplexity() + v.incComplexity(text, pos) } else { - v.nestIncComplexity() + v.nestIncComplexity(text, pos) } }