*: improve the linter and fix some bugs

.golangci.yml

@@ -13,6 +13,7 @@ linters:
     - bodyclose
     - testifylint
     - gofmt
+    - revive
   disable:
     - errcheck
 linters-settings:
@@ -52,3 +53,149 @@ linters-settings:
     rewrite-rules:
       - pattern: "interface{}"
         replacement: "any"
+  revive:
+    ignore-generated-header: false
+    severity: error
+    confidence: 0.8
+    rules:
+      - name: atomic
+        severity: warning
+        exclude: [""]
+        disabled: false
+      - name: blank-imports
+        severity: warning
+        exclude: [""]
+        disabled: false
+      - name: confusing-naming
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: confusing-results
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: context-as-argument
+        severity: warning
+        disabled: false
+        exclude: [""]
+        arguments:
+          - allowTypesBefore: "*testing.T,*github.com/user/repo/testing.Harness"
+      - name: datarace
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: defer
+        severity: warning
+        disabled: false
+        exclude: [""]
+        arguments:
+          - ["call-chain", "loop"]
+      - name: dot-imports
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: duplicated-imports
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: empty-block
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: empty-lines
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: error-return
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: error-strings
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: error-naming
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: exported
+        severity: warning
+        disabled: false
+        exclude: [""]
+        arguments:
+          - "checkPrivateReceivers"
+          - "sayRepetitiveInsteadOfStutters"
+      - name: identical-branches
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: if-return
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: modifies-parameter
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: optimize-operands-order
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: package-comments
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: range
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: range-val-in-closure
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: range-val-address
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: receiver-naming
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: indent-error-flow
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: superfluous-else
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: unnecessary-stmt
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: unreachable-code
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: unused-parameter
+        severity: warning
+        disabled: false
+        exclude: [""]
+        arguments:
+          - allowRegex: "^_"
+      - name: unused-receiver
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: useless-break
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: var-naming
+        severity: warning
+        disabled: false
+        exclude: [""]
+      - name: waitgroup-by-value
+        severity: warning
+        disabled: false
+        exclude: [""]

Makefile

@@ -184,9 +184,6 @@ static: install-tools pre-build
 	@ gofmt -s -l -d $(PACKAGE_DIRECTORIES) 2>&1 | awk '{ print } END { if (NR > 0) { exit 1 } }'
 	@ echo "golangci-lint ..."
 	@ golangci-lint run --verbose $(PACKAGE_DIRECTORIES) --allow-parallel-runners
-	@ echo "revive ..."
-	@ revive -formatter friendly -config revive.toml $(PACKAGES)
-
 	@ for mod in $(SUBMODULES); do cd $$mod && $(MAKE) static && cd $(ROOT_PATH) > /dev/null; done
 
 # Because CI downloads the dashboard code and runs gofmt, we can't add this check into static now.

client/Makefile

@@ -45,8 +45,6 @@ static: install-tools
 	@ gofmt -s -l -d . 2>&1 | awk '{ print } END { if (NR > 0) { exit 1 } }'
 	@ echo "golangci-lint ..."
 	@ golangci-lint run -c ../.golangci.yml --verbose ./... --allow-parallel-runners
-	@ echo "revive ..."
-	@ revive -formatter friendly -config ../revive.toml ./...
 
 tidy:
 	@ go mod tidy

client/client.go

@@ -438,12 +438,12 @@ func NewAPIContextV1() APIContext {
 }
 
 // GetAPIVersion returns the API version.
-func (apiCtx *apiContextV1) GetAPIVersion() (version APIVersion) {
+func (*apiContextV1) GetAPIVersion() (version APIVersion) {
 	return V1
 }
 
 // GetKeyspaceName returns the keyspace name.
-func (apiCtx *apiContextV1) GetKeyspaceName() (keyspaceName string) {
+func (*apiContextV1) GetKeyspaceName() (keyspaceName string) {
 	return ""
 }
 
@@ -460,7 +460,7 @@ func NewAPIContextV2(keyspaceName string) APIContext {
 }
 
 // GetAPIVersion returns the API version.
-func (apiCtx *apiContextV2) GetAPIVersion() (version APIVersion) {
+func (*apiContextV2) GetAPIVersion() (version APIVersion) {
 	return V2
 }
 
@@ -919,7 +919,7 @@ func handleRegionResponse(res *pdpb.GetRegionResponse) *Region {
 	return r
 }
 
-func (c *client) GetRegionFromMember(ctx context.Context, key []byte, memberURLs []string, opts ...GetRegionOption) (*Region, error) {
+func (c *client) GetRegionFromMember(ctx context.Context, key []byte, memberURLs []string, _ ...GetRegionOption) (*Region, error) {
 	if span := opentracing.SpanFromContext(ctx); span != nil && span.Tracer() != nil {
 		span = span.Tracer().StartSpan("pdclient.GetRegionFromMember", opentracing.ChildOf(span.Context()))
 		defer span.Finish()

client/keyspace_client.go

@@ -128,7 +128,7 @@ func (c *client) UpdateKeyspaceState(ctx context.Context, id uint32, state keysp
 // It returns a stream of slices of keyspace metadata.
 // The first message in stream contains all current keyspaceMeta,
 // all subsequent messages contains new put events for all keyspaces.
-func (c *client) WatchKeyspaces(ctx context.Context) (chan []*keyspacepb.KeyspaceMeta, error) {
+func (*client) WatchKeyspaces(context.Context) (chan []*keyspacepb.KeyspaceMeta, error) {
 	return nil, errors.Errorf("WatchKeyspaces unimplemented")
 }
 

client/mock_pd_service_discovery.go

@@ -56,19 +56,19 @@ func (m *mockPDServiceDiscovery) GetAllServiceClients() []ServiceClient {
 	return m.clients
 }
 
-func (m *mockPDServiceDiscovery) GetClusterID() uint64                           { return 0 }
-func (m *mockPDServiceDiscovery) GetKeyspaceID() uint32                          { return 0 }
-func (m *mockPDServiceDiscovery) GetKeyspaceGroupID() uint32                     { return 0 }
-func (m *mockPDServiceDiscovery) GetServiceURLs() []string                       { return nil }
-func (m *mockPDServiceDiscovery) GetServingEndpointClientConn() *grpc.ClientConn { return nil }
-func (m *mockPDServiceDiscovery) GetClientConns() *sync.Map                      { return nil }
-func (m *mockPDServiceDiscovery) GetServingURL() string                          { return "" }
-func (m *mockPDServiceDiscovery) GetBackupURLs() []string                        { return nil }
-func (m *mockPDServiceDiscovery) GetServiceClient() ServiceClient                { return nil }
-func (m *mockPDServiceDiscovery) GetOrCreateGRPCConn(url string) (*grpc.ClientConn, error) {
+func (*mockPDServiceDiscovery) GetClusterID() uint64                           { return 0 }
+func (*mockPDServiceDiscovery) GetKeyspaceID() uint32                          { return 0 }
+func (*mockPDServiceDiscovery) GetKeyspaceGroupID() uint32                     { return 0 }
+func (*mockPDServiceDiscovery) GetServiceURLs() []string                       { return nil }
+func (*mockPDServiceDiscovery) GetServingEndpointClientConn() *grpc.ClientConn { return nil }
+func (*mockPDServiceDiscovery) GetClientConns() *sync.Map                      { return nil }
+func (*mockPDServiceDiscovery) GetServingURL() string                          { return "" }
+func (*mockPDServiceDiscovery) GetBackupURLs() []string                        { return nil }
+func (*mockPDServiceDiscovery) GetServiceClient() ServiceClient                { return nil }
+func (*mockPDServiceDiscovery) GetOrCreateGRPCConn(string) (*grpc.ClientConn, error) {
 	return nil, nil
 }
-func (m *mockPDServiceDiscovery) ScheduleCheckMemberChanged()                        {}
-func (m *mockPDServiceDiscovery) CheckMemberChanged() error                          { return nil }
-func (m *mockPDServiceDiscovery) AddServingURLSwitchedCallback(callbacks ...func())  {}
-func (m *mockPDServiceDiscovery) AddServiceURLsSwitchedCallback(callbacks ...func()) {}
+func (*mockPDServiceDiscovery) ScheduleCheckMemberChanged()              {}
+func (*mockPDServiceDiscovery) CheckMemberChanged() error                { return nil }
+func (*mockPDServiceDiscovery) AddServingURLSwitchedCallback(...func())  {}
+func (*mockPDServiceDiscovery) AddServiceURLsSwitchedCallback(...func()) {}

client/pd_service_discovery.go

@@ -247,9 +247,9 @@ func (c *pdServiceClient) NeedRetry(pdErr *pdpb.Error, err error) bool {
 	return !(err == nil && pdErr == nil)
 }
 
-type errFn func(pdErr *pdpb.Error) bool
+type errFn func(*pdpb.Error) bool
 
-func emptyErrorFn(pdErr *pdpb.Error) bool {
+func emptyErrorFn(*pdpb.Error) bool {
 	return false
 }
 
@@ -618,7 +618,7 @@ func (c *pdServiceDiscovery) checkLeaderHealth(ctx context.Context) {
 }
 
 func (c *pdServiceDiscovery) checkFollowerHealth(ctx context.Context) {
-	c.followers.Range(func(key, value any) bool {
+	c.followers.Range(func(_, value any) bool {
 		// To ensure that the leader's healthy check is not delayed, shorten the duration.
 		ctx, cancel := context.WithTimeout(ctx, MemberHealthCheckInterval/3)
 		defer cancel()
@@ -661,7 +661,7 @@ func (c *pdServiceDiscovery) SetKeyspaceID(keyspaceID uint32) {
 }
 
 // GetKeyspaceGroupID returns the ID of the keyspace group
-func (c *pdServiceDiscovery) GetKeyspaceGroupID() uint32 {
+func (*pdServiceDiscovery) GetKeyspaceGroupID() uint32 {
 	// PD/API service only supports the default keyspace group
 	return defaultKeySpaceGroupID
 }

client/resource_group/controller/controller.go

@@ -398,8 +398,7 @@ func (c *ResourceGroupsController) Start(ctx context.Context) {
 				}
 
 			case gc := <-c.tokenBucketUpdateChan:
-				now := gc.run.now
-				go gc.handleTokenBucketUpdateEvent(c.loopCtx, now)
+				go gc.handleTokenBucketUpdateEvent(c.loopCtx)
 			}
 		}
 	}()
@@ -473,7 +472,7 @@ func (c *ResourceGroupsController) cleanUpResourceGroup() {
 }
 
 func (c *ResourceGroupsController) executeOnAllGroups(f func(controller *groupCostController)) {
-	c.groupsController.Range(func(name, value any) bool {
+	c.groupsController.Range(func(_, value any) bool {
 		f(value.(*groupCostController))
 		return true
 	})
@@ -504,7 +503,7 @@ func (c *ResourceGroupsController) handleTokenBucketResponse(resp []*rmpb.TokenB
 
 func (c *ResourceGroupsController) collectTokenBucketRequests(ctx context.Context, source string, typ selectType) {
 	c.run.currentRequests = make([]*rmpb.TokenBucketRequest, 0)
-	c.groupsController.Range(func(name, value any) bool {
+	c.groupsController.Range(func(_, value any) bool {
 		gc := value.(*groupCostController)
 		request := gc.collectRequestAndConsumption(typ)
 		if request != nil {
@@ -856,7 +855,7 @@ func (gc *groupCostController) resetEmergencyTokenAcquisition() {
 	}
 }
 
-func (gc *groupCostController) handleTokenBucketUpdateEvent(ctx context.Context, now time.Time) {
+func (gc *groupCostController) handleTokenBucketUpdateEvent(ctx context.Context) {
 	switch gc.mode {
 	case rmpb.GroupMode_RawMode:
 		for _, counter := range gc.run.resourceTokens {
@@ -873,7 +872,7 @@ func (gc *groupCostController) handleTokenBucketUpdateEvent(ctx context.Context,
 				counter.notify.setupNotificationCh = nil
 				threshold := counter.notify.setupNotificationThreshold
 				counter.notify.mu.Unlock()
-				counter.limiter.SetupNotificationThreshold(now, threshold)
+				counter.limiter.SetupNotificationThreshold(threshold)
 			case <-ctx.Done():
 				return
 			}
@@ -894,7 +893,7 @@ func (gc *groupCostController) handleTokenBucketUpdateEvent(ctx context.Context,
 				counter.notify.setupNotificationCh = nil
 				threshold := counter.notify.setupNotificationThreshold
 				counter.notify.mu.Unlock()
-				counter.limiter.SetupNotificationThreshold(now, threshold)
+				counter.limiter.SetupNotificationThreshold(threshold)
 			case <-ctx.Done():
 				return
 			}

client/resource_group/controller/limiter.go

@@ -218,7 +218,7 @@ func (lim *Limiter) Reserve(ctx context.Context, waitDuration time.Duration, now
 }
 
 // SetupNotificationThreshold enables the notification at the given threshold.
-func (lim *Limiter) SetupNotificationThreshold(now time.Time, threshold float64) {
+func (lim *Limiter) SetupNotificationThreshold(threshold float64) {
 	lim.mu.Lock()
 	defer lim.mu.Unlock()
 	lim.notifyThreshold = threshold

client/resource_group/controller/model.go

@@ -75,8 +75,7 @@ func newKVCalculator(cfg *RUConfig) *KVCalculator {
 }
 
 // Trickle ...
-func (kc *KVCalculator) Trickle(*rmpb.Consumption) {
-}
+func (*KVCalculator) Trickle(*rmpb.Consumption) {}
 
 // BeforeKVRequest ...
 func (kc *KVCalculator) BeforeKVRequest(consumption *rmpb.Consumption, req RequestInfo) {
@@ -166,11 +165,11 @@ func (dsc *SQLCalculator) Trickle(consumption *rmpb.Consumption) {
 }
 
 // BeforeKVRequest ...
-func (dsc *SQLCalculator) BeforeKVRequest(consumption *rmpb.Consumption, req RequestInfo) {
+func (*SQLCalculator) BeforeKVRequest(*rmpb.Consumption, RequestInfo) {
 }
 
 // AfterKVRequest ...
-func (dsc *SQLCalculator) AfterKVRequest(consumption *rmpb.Consumption, req RequestInfo, res ResponseInfo) {
+func (*SQLCalculator) AfterKVRequest(*rmpb.Consumption, RequestInfo, ResponseInfo) {
 }
 
 func getRUValueFromConsumption(custom *rmpb.Consumption, typ rmpb.RequestUnitType) float64 {

client/resource_group/controller/testutil.go

@@ -52,7 +52,7 @@ func (tri *TestRequestInfo) StoreID() uint64 {
 }
 
 // ReplicaNumber implements the RequestInfo interface.
-func (tri *TestRequestInfo) ReplicaNumber() int64 {
+func (*TestRequestInfo) ReplicaNumber() int64 {
 	return 1
 }
 

client/retry/backoff_test.go

@@ -95,7 +95,7 @@ func TestBackoffer(t *testing.T) {
 	// Test the retryable checker.
 	execCount = 0
 	bo = InitialBackoffer(base, max, total)
-	bo.SetRetryableChecker(func(err error) bool {
+	bo.SetRetryableChecker(func(error) bool {
 		return execCount < 2
 	})
 	err = bo.Exec(ctx, func() error {
@@ -169,7 +169,7 @@ func (w *testingWriter) Write(p []byte) (n int, err error) {
 	w.messages = append(w.messages, m)
 	return n, nil
 }
-func (w *testingWriter) Sync() error {
+func (*testingWriter) Sync() error {
 	return nil
 }