upgrade ruleguard and integrate into golang-ci (#17476)

Signed-off-by: Enwei Jiao <jiaoew2011@gmail.com>

.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

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:
+verifiers: build-cpp getdeps cppcheck fmt static-check
-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
 
 # Build various components locally.
 binlog:

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

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=

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