diff --git a/.golangci.yml b/.golangci.yml index fba5c92e7c..951c39821a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -7,10 +7,6 @@ run: - scripts - internal/core -linters-settings: - misspell: - locale: US - linters: disable-all: true enable: @@ -22,6 +18,18 @@ linters: - gosimple - gosec - revive + - gocritic + +linters-settings: + misspell: + locale: US + gocritic: + enabled-checks: + - ruleguard + settings: + ruleguard: + failOnError: true + rules: "rules.go" issues: exclude-use-default: false diff --git a/Makefile b/Makefile index 71a8f796c9..a2ea37880d 100644 --- a/Makefile +++ b/Makefile @@ -40,7 +40,6 @@ get-build-deps: getdeps: @mkdir -p ${GOPATH}/bin @which golangci-lint 1>/dev/null || (echo "Installing golangci-lint" && curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOPATH)/bin v1.43.0) - @which ruleguard 1>/dev/null || (echo "Installing ruleguard" && go get github.com/quasilyte/go-ruleguard/cmd/ruleguard@v0.2.1) tools/bin/revive: tools/check/go.mod cd tools/check; \ @@ -81,28 +80,7 @@ static-check: @GO111MODULE=on ${GOPATH}/bin/golangci-lint run --timeout=30m --config ./.golangci.yml ./cmd/... # @GO111MODULE=on ${GOPATH}/bin/golangci-lint run --timeout=30m --config ./.golangci.yml ./tests/go_client/... -ruleguard: -ifdef GO_DIFF_FILES - @echo "Running $@ check" - @${GOPATH}/bin/ruleguard -rules ruleguard.rules.go $(GO_DIFF_FILES) -else - @echo "Running $@ check" -ifeq ($(OS),Darwin) # MacOS X -ifeq ($(ARCH),arm64) - @${GOPATH}/bin/darwin_arm64/ruleguard -rules ruleguard.rules.go ./internal/... - @${GOPATH}/bin/darwin_arm64/ruleguard -rules ruleguard.rules.go ./cmd/... -else - @${GOPATH}/bin/ruleguard -rules ruleguard.rules.go ./internal/... - @${GOPATH}/bin/ruleguard -rules ruleguard.rules.go ./cmd/... -endif -else - @${GOPATH}/bin/ruleguard -rules ruleguard.rules.go ./internal/... - @${GOPATH}/bin/ruleguard -rules ruleguard.rules.go ./cmd/... -endif - #@${GOPATH}/bin/ruleguard -rules ruleguard.rules.go ./tests/go/... -endif - -verifiers: build-cpp getdeps cppcheck fmt static-check ruleguard +verifiers: build-cpp getdeps cppcheck fmt static-check # Build various components locally. binlog: diff --git a/go.mod b/go.mod index 4e9f3393bf..32efc6c20f 100644 --- a/go.mod +++ b/go.mod @@ -35,6 +35,7 @@ require ( github.com/pierrec/lz4 v2.5.2+incompatible // indirect github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.11.0 + github.com/quasilyte/go-ruleguard/dsl v0.3.21 // indirect github.com/sbinet/npyio v0.6.0 github.com/shirou/gopsutil v3.21.8+incompatible github.com/spaolacci/murmur3 v1.1.0 diff --git a/go.sum b/go.sum index c1125f271b..59cb67fccf 100644 --- a/go.sum +++ b/go.sum @@ -590,6 +590,8 @@ github.com/prometheus/procfs v0.1.3/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4O github.com/prometheus/procfs v0.6.0 h1:mxy4L2jP6qMonqmq+aTtOx1ifVWUgG/TAmntgbh3xv4= github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA= github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU= +github.com/quasilyte/go-ruleguard/dsl v0.3.21 h1:vNkC6fC6qMLzCOGbnIHOd5ixUGgTbp3Z4fGnUgULlDA= +github.com/quasilyte/go-ruleguard/dsl v0.3.21/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU= github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4= github.com/rivo/tview v0.0.0-20200219210816-cd38d7432498/go.mod h1:6lkG1x+13OShEf0EaOCaTQYyB7d5nSbb181KtjlS+84= github.com/rivo/uniseg v0.1.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= diff --git a/ruleguard.rules.go b/rules.go similarity index 91% rename from ruleguard.rules.go rename to rules.go index d16ed24c82..bdec749333 100644 --- a/ruleguard.rules.go +++ b/rules.go @@ -19,13 +19,13 @@ package gorules import ( - "github.com/quasilyte/go-ruleguard/dsl/fluent" + "github.com/quasilyte/go-ruleguard/dsl" ) // This is a collection of rules for ruleguard: https://github.com/quasilyte/go-ruleguard // Remove extra conversions: mdempsky/unconvert -func unconvert(m fluent.Matcher) { +func unconvert(m dsl.Matcher) { m.Match("int($x)").Where(m["x"].Type.Is("int") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") m.Match("float32($x)").Where(m["x"].Type.Is("float32") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") @@ -50,14 +50,14 @@ func unconvert(m fluent.Matcher) { // Don't use == or != with time.Time // https://github.com/dominikh/go-tools/issues/47 : Wontfix -func timeeq(m fluent.Matcher) { +func timeeq(m dsl.Matcher) { m.Match("$t0 == $t1").Where(m["t0"].Type.Is("time.Time")).Report("using == with time.Time") m.Match("$t0 != $t1").Where(m["t0"].Type.Is("time.Time")).Report("using != with time.Time") m.Match(`map[$k]$v`).Where(m["k"].Type.Is("time.Time")).Report("map with time.Time keys are easy to misuse") } // err but no an error -func errnoterror(m fluent.Matcher) { +func errnoterror(m dsl.Matcher) { // Would be easier to check for all err identifiers instead, but then how do we get the type from m[] ? @@ -83,7 +83,7 @@ func errnoterror(m fluent.Matcher) { } // Identical if and else bodies -func ifbodythenbody(m fluent.Matcher) { +func ifbodythenbody(m dsl.Matcher) { m.Match("if $*_ { $body } else { $body }"). Report("identical if and else bodies") @@ -95,7 +95,7 @@ func ifbodythenbody(m fluent.Matcher) { // Odd inequality: A - B < 0 instead of != // Too many false positives. /* -func subtractnoteq(m fluent.Matcher) { +func subtractnoteq(m dsl.Matcher) { m.Match("$a - $b < 0").Report("consider $a != $b") m.Match("$a - $b > 0").Report("consider $a != $b") m.Match("0 < $a - $b").Report("consider $a != $b") @@ -104,12 +104,12 @@ func subtractnoteq(m fluent.Matcher) { */ // Self-assignment -func selfassign(m fluent.Matcher) { +func selfassign(m dsl.Matcher) { m.Match("$x = $x").Report("useless self-assignment") } // Odd nested ifs -func oddnestedif(m fluent.Matcher) { +func oddnestedif(m dsl.Matcher) { m.Match("if $x { if $x { $*_ }; $*_ }", "if $x == $y { if $x != $y {$*_ }; $*_ }", "if $x != $y { if $x == $y {$*_ }; $*_ }", @@ -126,7 +126,7 @@ func oddnestedif(m fluent.Matcher) { } // odd bitwise expressions -func oddbitwise(m fluent.Matcher) { +func oddbitwise(m dsl.Matcher) { m.Match("$x | $x", "$x | ^$x", "^$x | $x"). @@ -142,7 +142,7 @@ func oddbitwise(m fluent.Matcher) { } // odd sequence of if tests with return -func ifreturn(m fluent.Matcher) { +func ifreturn(m dsl.Matcher) { m.Match("if $x { return $*_ }; if $x {$*_ }").Report("odd sequence of if test") m.Match("if $x { return $*_ }; if !$x {$*_ }").Report("odd sequence of if test") m.Match("if !$x { return $*_ }; if $x {$*_ }").Report("odd sequence of if test") @@ -151,7 +151,7 @@ func ifreturn(m fluent.Matcher) { } -func oddifsequence(m fluent.Matcher) { +func oddifsequence(m dsl.Matcher) { /* m.Match("if $x { $*_ }; if $x {$*_ }").Report("odd sequence of if test") @@ -167,7 +167,7 @@ func oddifsequence(m fluent.Matcher) { } // odd sequence of nested if tests -func nestedifsequence(m fluent.Matcher) { +func nestedifsequence(m dsl.Matcher) { /* m.Match("if $x < $y { if $x >= $y {$*_ }; $*_ }").Report("odd sequence of nested if tests") m.Match("if $x <= $y { if $x > $y {$*_ }; $*_ }").Report("odd sequence of nested if tests") @@ -177,11 +177,11 @@ func nestedifsequence(m fluent.Matcher) { } // odd sequence of assignments -func identicalassignments(m fluent.Matcher) { +func identicalassignments(m dsl.Matcher) { m.Match("$x = $y; $y = $x").Report("odd sequence of assignments") } -func oddcompoundop(m fluent.Matcher) { +func oddcompoundop(m dsl.Matcher) { m.Match("$x += $x + $_", "$x += $x - $_"). Report("odd += expression") @@ -191,13 +191,13 @@ func oddcompoundop(m fluent.Matcher) { Report("odd -= expression") } -func constswitch(m fluent.Matcher) { +func constswitch(m dsl.Matcher) { m.Match("switch $x { $*_ }", "switch $*_; $x { $*_ }"). Where(m["x"].Const && !m["x"].Text.Matches(`^runtime\.`)). Report("constant switch") } -func oddcomparisons(m fluent.Matcher) { +func oddcomparisons(m dsl.Matcher) { m.Match( "$x - $y == 0", "$x - $y != 0", @@ -210,7 +210,7 @@ func oddcomparisons(m fluent.Matcher) { ).Report("odd comparison") } -func oddmathbits(m fluent.Matcher) { +func oddmathbits(m dsl.Matcher) { m.Match( "64 - bits.LeadingZeros64($x)", "32 - bits.LeadingZeros32($x)", @@ -219,7 +219,7 @@ func oddmathbits(m fluent.Matcher) { ).Report("odd math/bits expression: use bits.Len*() instead?") } -func floateq(m fluent.Matcher) { +func floateq(m dsl.Matcher) { m.Match( "$x == $y", "$x != $y", @@ -244,7 +244,7 @@ func floateq(m fluent.Matcher) { } -func badexponent(m fluent.Matcher) { +func badexponent(m dsl.Matcher) { m.Match( "2 ^ $x", "10 ^ $x", @@ -252,7 +252,7 @@ func badexponent(m fluent.Matcher) { Report("caret (^) is not exponentiation") } -func floatloop(m fluent.Matcher) { +func floatloop(m dsl.Matcher) { m.Match( "for $i := $x; $i < $y; $i += $z { $*_ }", "for $i = $x; $i < $y; $i += $z { $*_ }", @@ -268,7 +268,7 @@ func floatloop(m fluent.Matcher) { Report("floating point for loop counter") } -func urlredacted(m fluent.Matcher) { +func urlredacted(m dsl.Matcher) { m.Match( "log.Println($x, $*_)", @@ -287,7 +287,7 @@ func urlredacted(m fluent.Matcher) { Report("consider $x.Redacted() when outputting URLs") } -func sprinterr(m fluent.Matcher) { +func sprinterr(m dsl.Matcher) { m.Match(`fmt.Sprint($err)`, `fmt.Sprintf("%s", $err)`, `fmt.Sprintf("%v", $err)`, @@ -297,7 +297,7 @@ func sprinterr(m fluent.Matcher) { } -func largeloopcopy(m fluent.Matcher) { +func largeloopcopy(m dsl.Matcher) { m.Match( `for $_, $v := range $_ { $*_ }`, ). @@ -305,7 +305,7 @@ func largeloopcopy(m fluent.Matcher) { Report(`loop copies large value each iteration`) } -func joinpath(m fluent.Matcher) { +func joinpath(m dsl.Matcher) { m.Match( `strings.Join($_, "/")`, `strings.Join($_, "\\")`, @@ -314,7 +314,7 @@ func joinpath(m fluent.Matcher) { Report(`did you mean path.Join() or filepath.Join() ?`) } -func readfull(m fluent.Matcher) { +func readfull(m dsl.Matcher) { m.Match(`$n, $err := io.ReadFull($_, $slice) if $err != nil || $n != len($slice) { $*_ @@ -346,7 +346,7 @@ func readfull(m fluent.Matcher) { ).Report("io.ReadFull() returns err == nil iff n == len(slice)") } -func nilerr(m fluent.Matcher) { +func nilerr(m dsl.Matcher) { m.Match( `if err == nil { return err }`, `if err == nil { return $*_, err }`, @@ -355,7 +355,7 @@ func nilerr(m fluent.Matcher) { } -func mailaddress(m fluent.Matcher) { +func mailaddress(m dsl.Matcher) { m.Match( "fmt.Sprintf(`\"%s\" <%s>`, $NAME, $EMAIL)", "fmt.Sprintf(`\"%s\"<%s>`, $NAME, $EMAIL)", @@ -371,7 +371,7 @@ func mailaddress(m fluent.Matcher) { } -func errnetclosed(m fluent.Matcher) { +func errnetclosed(m dsl.Matcher) { m.Match( `strings.Contains($err.Error(), $text)`, ). @@ -381,7 +381,7 @@ func errnetclosed(m fluent.Matcher) { } -func httpheaderadd(m fluent.Matcher) { +func httpheaderadd(m dsl.Matcher) { m.Match( `$H.Add($KEY, $VALUE)`, ). @@ -390,7 +390,7 @@ func httpheaderadd(m fluent.Matcher) { Suggest(`$H.Set($KEY, $VALUE)`) } -func hmacnew(m fluent.Matcher) { +func hmacnew(m dsl.Matcher) { m.Match("hmac.New(func() hash.Hash { return $x }, $_)", `$f := func() hash.Hash { return $x } $*_ @@ -399,13 +399,13 @@ func hmacnew(m fluent.Matcher) { Report("invalid hash passed to hmac.New()") } -func writestring(m fluent.Matcher) { +func writestring(m dsl.Matcher) { m.Match(`io.WriteString($w, string($b))`). Where(m["b"].Type.Is("[]byte")). Suggest("$w.Write($b)") } -func badlock(m fluent.Matcher) { +func badlock(m dsl.Matcher) { // Shouldn't give many false positives without type filter // as Lock+Unlock pairs in combination with defer gives us pretty // a good chance to guess correctly. If we constrain the type to sync.Mutex