improve the linter and fix some bugs

Signed-off-by: Ryan Leung <[email protected]>
tikv · Apr 2, 2024 · d2cdcda · d2cdcda
1 parent 6fe44d7
commit d2cdcda
Show file tree

Hide file tree

Showing 249 changed files with 1,100 additions and 1,027 deletions.
diff --git a/.golangci.yml b/.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: [""]
diff --git a/Makefile b/Makefile
@@ -181,9 +181,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.

diff --git a/client/Makefile b/client/Makefile
@@ -35,8 +35,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

diff --git a/client/client.go b/client/client.go
@@ -431,12 +431,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 ""
 }
 
@@ -453,7 +453,7 @@ func NewAPIContextV2(keyspaceName string) APIContext {
 }
 
 // GetAPIVersion returns the API version.
-func (apiCtx *apiContextV2) GetAPIVersion() (version APIVersion) {
+func (*apiContextV2) GetAPIVersion() (version APIVersion) {
 	return V2
 }
 
@@ -912,7 +912,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()

diff --git a/client/keyspace_client.go b/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")
 }
 

diff --git a/client/mock_pd_service_discovery.go b/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()) {}
diff --git a/client/pd_service_discovery.go b/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
 }

diff --git a/client/resource_group/controller/controller.go b/client/resource_group/controller/controller.go
@@ -386,8 +386,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)
 			}
 		}
 	}()
@@ -461,7 +460,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
 	})
@@ -492,7 +491,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 {
@@ -844,7 +843,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 {
@@ -861,7 +860,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
 			}
@@ -882,7 +881,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
 			}

diff --git a/client/resource_group/controller/limiter.go b/client/resource_group/controller/limiter.go
@@ -217,7 +217,8 @@ 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) {
+// FIXME: is it expected?
+func (lim *Limiter) SetupNotificationThreshold(threshold float64) {
 	lim.mu.Lock()
 	defer lim.mu.Unlock()
 	lim.notifyThreshold = threshold

diff --git a/client/resource_group/controller/model.go b/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 {

diff --git a/client/resource_group/controller/testutil.go b/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
 }
 

diff --git a/client/retry/backoff_test.go b/client/retry/backoff_test.go
@@ -92,7 +92,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 {

diff --git a/client/testutil/check_env_dummy.go b/client/testutil/check_env_dummy.go
@@ -16,6 +16,6 @@
 
 package testutil
 
-func environmentCheck(addr string) bool {
+func environmentCheck(_ string) bool {
 	return true
 }