diff --git a/BUILD.bazel b/BUILD.bazel index 9fc079a46..25783b456 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -115,18 +115,19 @@ nogo( "@org_golang_x_tools//go/analysis/passes/assign:go_default_library", "@org_golang_x_tools//go/analysis/passes/inspect:go_default_library", "@org_golang_x_tools//go/analysis/passes/asmdecl:go_default_library", - "//tools/analyzers/maligned:go_default_library", + "//tools/analyzers/comparesame:go_default_library", "//tools/analyzers/cryptorand:go_default_library", "//tools/analyzers/errcheck:go_default_library", "//tools/analyzers/featureconfig:go_default_library", - "//tools/analyzers/comparesame:go_default_library", - "//tools/analyzers/shadowpredecl:go_default_library", - "//tools/analyzers/nop:go_default_library", - "//tools/analyzers/slicedirect:go_default_library", - "//tools/analyzers/interfacechecker:go_default_library", + "//tools/analyzers/gocognit:go_default_library", "//tools/analyzers/ineffassign:go_default_library", + "//tools/analyzers/interfacechecker:go_default_library", + "//tools/analyzers/maligned:go_default_library", + "//tools/analyzers/nop:go_default_library", "//tools/analyzers/properpermissions:go_default_library", "//tools/analyzers/recursivelock:go_default_library", + "//tools/analyzers/shadowpredecl:go_default_library", + "//tools/analyzers/slicedirect:go_default_library", "//tools/analyzers/uintcast:go_default_library", ] + select({ # nogo checks that fail with coverage enabled. diff --git a/deps.bzl b/deps.bzl index 722017f43..d93e275fc 100644 --- a/deps.bzl +++ b/deps.bzl @@ -3756,6 +3756,13 @@ def prysm_deps(): sum = "h1:qph92Y649prgesehzOrQjdWyxFOp/QVM+6imKHad91M=", version = "v2.3.0", ) + go_repository( + name = "com_github_uudashr_gocognit", + importpath = "github.com/uudashr/gocognit", + sum = "h1:rrSex7oHr3/pPLQ0xoWq108XMU8s678FJcQ+aSfOHa4=", + version = "v1.0.5", + ) + go_repository( name = "com_github_valyala_bytebufferpool", importpath = "github.com/valyala/bytebufferpool", diff --git a/go.mod b/go.mod index eb30d2351..9b6789bf6 100644 --- a/go.mod +++ b/go.mod @@ -77,6 +77,7 @@ require ( github.com/trailofbits/go-mutexasserts v0.0.0-20200708152505-19999e7d3cef github.com/tyler-smith/go-bip39 v1.1.0 github.com/urfave/cli/v2 v2.3.0 + github.com/uudashr/gocognit v1.0.5 github.com/wealdtech/go-bytesutil v1.1.1 github.com/wealdtech/go-eth2-util v1.6.3 github.com/wealdtech/go-eth2-wallet-encryptor-keystorev4 v1.1.3 diff --git a/go.sum b/go.sum index a77fc59a9..a75fe3b0f 100644 --- a/go.sum +++ b/go.sum @@ -1358,6 +1358,8 @@ github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijb github.com/urfave/cli v1.22.1/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0= github.com/urfave/cli/v2 v2.3.0 h1:qph92Y649prgesehzOrQjdWyxFOp/QVM+6imKHad91M= github.com/urfave/cli/v2 v2.3.0/go.mod h1:LJmUH05zAU44vOAcrfzZQKsZbVcdbOG8rtL3/XcUArI= +github.com/uudashr/gocognit v1.0.5 h1:rrSex7oHr3/pPLQ0xoWq108XMU8s678FJcQ+aSfOHa4= +github.com/uudashr/gocognit v1.0.5/go.mod h1:wgYz0mitoKOTysqxTDMOUXg+Jb5SvtihkfmugIZYpEA= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/valyala/fasttemplate v1.0.1/go.mod h1:UQGH1tvbgY+Nz5t2n7tXsz52dQxojPUpymEIMZ47gx8= github.com/valyala/fasttemplate v1.2.1/go.mod h1:KHLXt3tVN2HBp8eijSv/kGJopbvo7S+qRAEEKiv+SiQ= @@ -1804,6 +1806,7 @@ golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4f golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= golang.org/x/tools v0.1.1-0.20210205202024-ef80cdb6ec6d/go.mod h1:9bzcO0MWcOuT0tm1iBGzDVPshzfwoVvREIui8C+MHqU= golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= +golang.org/x/tools v0.1.4/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.8 h1:P1HhGGuLW4aAclzjtmJdf0mJOjVUZUzOTqkAkWL+l6w= golang.org/x/tools v0.1.8/go.mod h1:nABZi5QlRsZVlzPpHl034qft6wpY4eDcsTt5AaioBiU= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/nogo_config.json b/nogo_config.json index d860436bd..ef538c359 100644 --- a/nogo_config.json +++ b/nogo_config.json @@ -170,5 +170,13 @@ "rules_go_work-.*": "Third party code", ".*_test\\.go": "Tests are ok" } + }, + "gocognit": { + "exclude_files": { + "external/.*": "Third party code", + "rules_go_work-.*": "Third party code", + ".*\\.pb.*.go": "Generated code is ok", + ".*generated\\.ssz\\.go": "Generated code is ok" + } } } diff --git a/tools/analyzers/gocognit/BUILD.bazel b/tools/analyzers/gocognit/BUILD.bazel new file mode 100644 index 000000000..5c4089d36 --- /dev/null +++ b/tools/analyzers/gocognit/BUILD.bazel @@ -0,0 +1,14 @@ +load("@prysm//tools/go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["analyzer.go"], + importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/gocognit", + visibility = ["//visibility:public"], + deps = [ + "@com_github_uudashr_gocognit//:go_default_library", + "@org_golang_x_tools//go/analysis:go_default_library", + "@org_golang_x_tools//go/analysis/passes/inspect:go_default_library", + "@org_golang_x_tools//go/ast/inspector:go_default_library", + ], +) diff --git a/tools/analyzers/gocognit/CognitiveComplexity.pdf b/tools/analyzers/gocognit/CognitiveComplexity.pdf new file mode 100644 index 000000000..24ce35f97 Binary files /dev/null and b/tools/analyzers/gocognit/CognitiveComplexity.pdf differ diff --git a/tools/analyzers/gocognit/README.md b/tools/analyzers/gocognit/README.md new file mode 100644 index 000000000..c7cbf8b32 --- /dev/null +++ b/tools/analyzers/gocognit/README.md @@ -0,0 +1,195 @@ +> Copied from https://github.com/uudashr/gocognit/blob/5bf67146515e79acd2a8d5728deafa9d91ad48db/README.md +> License: MIT + +[![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](./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 install github.com/uudashr/gocognit/cmd/gocognit@latest +``` + +or + +``` +$ 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](./CognitiveComplexity.pdf) white paper by G. Ann Campbell. diff --git a/tools/analyzers/gocognit/analyzer.go b/tools/analyzers/gocognit/analyzer.go new file mode 100644 index 000000000..6cef89591 --- /dev/null +++ b/tools/analyzers/gocognit/analyzer.go @@ -0,0 +1,90 @@ +package gocognit + +import ( + "errors" + "fmt" + "go/ast" + + "github.com/uudashr/gocognit" + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +// Doc explaining the tool. +const Doc = "Tool to ensure go code does not have high cognitive complexity." + +// Analyzer runs static analysis. +var Analyzer = &analysis.Analyzer{ + Name: "gocognit", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +// Recommended thresholds according to the 2008 presentation titled +// "Software Quality Metrics to Identify Risk" by Thomas McCabe Jr. +// +// 1 - 10 Simple procedure, little risk +// 11 - 20 More complex, moderate risk +// 21 - 50 Complex, high risk +// > 50 Untestable code, very high risk +// +// This threshold should be lowered to 50 over time. +const over = 130 + +func run(pass *analysis.Pass) (interface{}, error) { + inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + if !ok { + return nil, errors.New("analyzer is not type *inspector.Inspector") + } + + nodeFilter := []ast.Node{ + (*ast.FuncDecl)(nil), + } + inspect.Preorder(nodeFilter, func(n ast.Node) { + fnDecl, ok := n.(*ast.FuncDecl) + if !ok { + return + } + + fnName := funcName(fnDecl) + fnComplexity := gocognit.Complexity(fnDecl) + + if fnComplexity > over { + pass.Reportf(fnDecl.Pos(), "cognitive complexity %d of func %s is high (> %d)", fnComplexity, fnName, over) + } + }) + + return nil, nil +} + +// funcName returns the name representation of a function or method: +// "(Type).Name" for methods or simply "Name" for functions. +// +// Copied from https://github.com/uudashr/gocognit/blob/5bf67146515e79acd2a8d5728deafa9d91ad48db/gocognit.go +// License: MIT +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). +// +// Copied from https://github.com/uudashr/gocognit/blob/5bf67146515e79acd2a8d5728deafa9d91ad48db/gocognit.go +// License: MIT +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" +}