Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linter, *: enable linters appends and unusedwrite #52452

Merged
merged 1 commit into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions br/pkg/stream/rewrite_meta_rawkv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove this ID?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it's not used in this test 🤔 .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that test passes. I guess this will probably be fine.

}
db2 := model.DBInfo{}

// construct the SchemaReplace
dbMap := make(map[UpstreamID]*DBReplace)
Expand Down
2 changes: 2 additions & 0 deletions build/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
4 changes: 1 addition & 3 deletions build/linter/misspell/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions build/nogo_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
}
4 changes: 1 addition & 3 deletions pkg/ddl/stat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 12 additions & 2 deletions pkg/executor/aggfuncs/func_group_concat.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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] {
Expand Down
2 changes: 0 additions & 2 deletions pkg/executor/test/aggregate/aggregate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions pkg/planner/core/logical_plans_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/planner/core/memtable_predicate_extractor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 6 additions & 12 deletions pkg/planner/funcdep/fd_graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
10 changes: 5 additions & 5 deletions pkg/server/conn_stmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down Expand Up @@ -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())),
Expand Down
2 changes: 1 addition & 1 deletion pkg/sessionctx/variable/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,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
}
}
Expand Down
Loading