linter, *: enable linters appends and unusedwrite (pingcap#52452)

ref pingcap#52449, close pingcap#52450

br/pkg/stream/rewrite_meta_rawkv_test.go

@@ -468,18 +468,14 @@ func TestRewriteTableInfoForExchangePartition(t *testing.T) {
 		Name:      model.NewCIStr(tableName1),
 		Partition: &pi,
 	}
-	db1 := model.DBInfo{
-		ID: dbID1,
-	}
+	db1 := model.DBInfo{}
 
 	// construct table t2 without partition.
 	t2 := model.TableInfo{
 		ID:   tableID2,
 		Name: model.NewCIStr(tableName2),
 	}
-	db2 := model.DBInfo{
-		ID: dbID2,
-	}
+	db2 := model.DBInfo{}
 
 	// construct the SchemaReplace
 	dbMap := make(map[UpstreamID]*DBReplace)

build/BUILD.bazel

@@ -105,6 +105,7 @@ nogo(
     config = ":nogo_config.json",
     visibility = ["//visibility:public"],  # must have public visibility
     deps = [
+               "@org_golang_x_tools//go/analysis/passes/appends:go_default_library",
                "@org_golang_x_tools//go/analysis/passes/asmdecl:go_default_library",
                "@org_golang_x_tools//go/analysis/passes/assign:go_default_library",
                "@org_golang_x_tools//go/analysis/passes/atomic:go_default_library",
@@ -142,6 +143,7 @@ nogo(
                "@org_golang_x_tools//go/analysis/passes/unreachable:go_default_library",
                "@org_golang_x_tools//go/analysis/passes/unsafeptr:go_default_library",
                "@org_golang_x_tools//go/analysis/passes/unusedresult:go_default_library",
+               "@org_golang_x_tools//go/analysis/passes/unusedwrite:go_default_library",
                "//build/linter/asciicheck",
                "//build/linter/bodyclose",
                "//build/linter/bootstrap",

build/linter/misspell/analyzer.go

@@ -50,9 +50,7 @@ func run(pass *analysis.Pass) (any, error) {
 	}
 
 	// Figure out regional variations
-	settings := &Misspell{
-		Locale: "",
-	}
+	settings := &Misspell{}
 
 	if len(settings.IgnoreWords) != 0 {
 		r.RemoveRule(settings.IgnoreWords)

build/nogo_config.json

@@ -1348,5 +1348,19 @@
     "only_files": {
       "pkg/session/": "bootstrap code"
     }
+  },
+  "appends": {
+    "exclude_files": {
+      "pkg/parser/parser.go": "parser/parser.go code",
+      "external/": "no need to vet third party code",
+      ".*_generated\\.go$": "ignore generated code"
+    }
+  },
+  "unusedwrite": {
+    "exclude_files": {
+      "pkg/parser/parser.go": "parser/parser.go code",
+      "external/": "no need to vet third party code",
+      ".*_generated\\.go$": "ignore generated code"
+    }
   }
 }

pkg/ddl/stat_test.go

@@ -46,9 +46,7 @@ func TestGetDDLInfo(t *testing.T) {
 	require.NoError(t, err)
 
 	dbInfo2 := &model.DBInfo{
-		ID:    2,
-		Name:  model.NewCIStr("b"),
-		State: model.StateNone,
+		ID: 2,
 	}
 	job := &model.Job{
 		ID:       1,

pkg/executor/aggfuncs/func_group_concat.go

@@ -303,7 +303,8 @@ type topNRows struct {
 	rows []sortRow
 	desc []bool
 	sctx AggFuncUpdateContext
-	err  error
+	// TODO: this err is never assigned now. Please choose to make use of it or just remove it.
+	err error
 
 	currSize  uint64
 	limitSize uint64
@@ -325,7 +326,16 @@ func (h topNRows) Less(i, j int) bool {
 	for k := 0; k < n; k++ {
 		ret, err := h.rows[i].byItems[k].Compare(h.sctx.TypeCtx(), h.rows[j].byItems[k], h.collators[k])
 		if err != nil {
-			h.err = err
+			// TODO: check whether it's appropriate to just ignore the error here.
+			//
+			// Previously, the error is assigned to `h.err` and hope it can be accessed from outside. However,
+			// the `h` is copied when calling this method, and the assignment to `h.err` is meaningless.
+			//
+			// The linter `unusedwrite` found this issue. Therefore, the unused write to `h.err` is removed and
+			// it doesn't change the behavior. But we need to confirm whether it's correct to just ignore the error
+			// here.
+			//
+			// Ref https://github.com/pingcap/tidb/issues/52449
 			return false
 		}
 		if h.desc[k] {

pkg/executor/test/aggregate/aggregate_test.go

@@ -38,11 +38,9 @@ import (
 func TestHashAggRuntimeStat(t *testing.T) {
 	partialInfo := &aggregate.AggWorkerInfo{
 		Concurrency: 5,
-		WallTime:    int64(time.Second * 20),
 	}
 	finalInfo := &aggregate.AggWorkerInfo{
 		Concurrency: 8,
-		WallTime:    int64(time.Second * 10),
 	}
 	stats := &aggregate.HashAggRuntimeStats{
 		PartialConcurrency: 5,

pkg/planner/core/logical_plans_test.go

@@ -91,8 +91,8 @@ func createPlannerSuite() (s *plannerSuite) {
 		if pi == nil {
 			continue
 		}
-		for _, def := range pi.Definitions {
-			def.ID = id
+		for i := range pi.Definitions {
+			pi.Definitions[i].ID = id
 			id += 1
 		}
 	}

pkg/planner/core/memtable_predicate_extractor.go

@@ -1354,7 +1354,7 @@ func (e *SlowQueryExtractor) decodeBytesToTime(bs []byte) (int64, error) {
 
 func (*SlowQueryExtractor) decodeToTime(handle kv.Handle) (int64, error) {
 	tp := types.NewFieldType(mysql.TypeDatetime)
-	col := rowcodec.ColInfo{ID: 0, Ft: tp}
+	col := rowcodec.ColInfo{Ft: tp}
 	chk := chunk.NewChunkWithCapacity([]*types.FieldType{tp}, 1)
 	coder := codec.NewDecoder(chk, nil)
 	_, err := coder.DecodeOne(handle.EncodedCol(0), 0, col.Ft)

pkg/planner/funcdep/fd_graph_test.go

@@ -26,22 +26,16 @@ func TestAddStrictFunctionalDependency(t *testing.T) {
 		fdEdges: []*fdEdge{},
 	}
 	fe1 := &fdEdge{
-		from:   intset.NewFastIntSet(1, 2), // AB -> CDEFG
-		to:     intset.NewFastIntSet(3, 4, 5, 6, 7),
-		strict: true,
-		equiv:  false,
+		from: intset.NewFastIntSet(1, 2), // AB -> CDEFG
+		to:   intset.NewFastIntSet(3, 4, 5, 6, 7),
 	}
 	fe2 := &fdEdge{
-		from:   intset.NewFastIntSet(1, 2), // AB -> CD
-		to:     intset.NewFastIntSet(3, 4),
-		strict: true,
-		equiv:  false,
+		from: intset.NewFastIntSet(1, 2), // AB -> CD
+		to:   intset.NewFastIntSet(3, 4),
 	}
 	fe3 := &fdEdge{
-		from:   intset.NewFastIntSet(1, 2), // AB -> EF
-		to:     intset.NewFastIntSet(5, 6),
-		strict: true,
-		equiv:  false,
+		from: intset.NewFastIntSet(1, 2), // AB -> EF
+		to:   intset.NewFastIntSet(5, 6),
 	}
 	// fd: AB -> CDEFG implies all of others.
 	assertF := func() {

pkg/server/conn_stmt_test.go

@@ -357,8 +357,8 @@ func TestCursorFetchReset(t *testing.T) {
 	require.NoError(t, c.flush(context.Background()))
 	require.Equal(t, expected, out.Bytes())
 	// reset the statement
-	require.NoError(t, c.Dispatch(ctx, append(
-		appendUint32([]byte{mysql.ComStmtReset}, uint32(stmt.ID())),
+	require.NoError(t, c.Dispatch(ctx, appendUint32(
+		[]byte{mysql.ComStmtReset}, uint32(stmt.ID()),
 	)))
 	// the following fetch will fail
 	require.Error(t, c.Dispatch(ctx, appendUint32(appendUint32([]byte{mysql.ComStmtFetch}, uint32(stmt.ID())), 1)))
@@ -446,9 +446,9 @@ func TestCursorFetchSendLongDataReset(t *testing.T) {
 	// send a parameter to the server
 	require.NoError(t, dispatchSendLongData(c, stmt.ID(), 0, appendUint64([]byte{}, 1)))
 	// reset the statement
-	require.NoError(t, c.Dispatch(ctx, append(
-		appendUint32([]byte{mysql.ComStmtReset}, uint32(stmt.ID())),
-	)))
+	require.NoError(t, c.Dispatch(ctx, appendUint32(
+		[]byte{mysql.ComStmtReset}, uint32(stmt.ID())),
+	))
 	// execute directly will fail
 	require.Error(t, c.Dispatch(ctx, append(
 		appendUint32([]byte{mysql.ComStmtExecute}, uint32(stmt.ID())),

pkg/sessionctx/variable/session.go

@@ -450,7 +450,7 @@ func (tc *TransactionContext) ReleaseSavepoint(name string) bool {
 	name = strings.ToLower(name)
 	for i, sp := range tc.Savepoints {
 		if sp.Name == name {
-			tc.Savepoints = append(tc.Savepoints[:i])
+			tc.Savepoints = tc.Savepoints[:i]
 			return true
 		}
 	}