From e73dd3e4bd9e8f4b2bee878e3f8ec846f079c48b Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Wed, 29 Jun 2022 17:55:03 +0800 Subject: [PATCH 1/5] *: enable unconvert Signed-off-by: Weizhen Wang --- DEPS.bzl | 7 +++++ br/pkg/lightning/mydump/BUILD.bazel | 1 - build/BUILD.bazel | 1 + build/linter/unconvert/BUILD.bazel | 14 +++++++++ build/linter/unconvert/analysis.go | 47 +++++++++++++++++++++++++++++ build/linter/util/BUILD.bazel | 1 + build/linter/util/util.go | 19 ++++++++++++ build/nogo_config.json | 6 ++++ executor/seqtest/BUILD.bazel | 1 + go.mod | 3 ++ go.sum | 3 ++ sessiontxn/isolation/BUILD.bazel | 2 ++ testkit/testfork/BUILD.bazel | 1 + 13 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 build/linter/unconvert/BUILD.bazel create mode 100644 build/linter/unconvert/analysis.go diff --git a/DEPS.bzl b/DEPS.bzl index 65f9d5b4b657c..0bcdc6e81964a 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -942,6 +942,13 @@ def go_deps(): sum = "h1:leSNB7iYzLYSSx3J/s5sVf4Drkc68W2wm4Ixh/mr0us=", version = "v0.0.0-20180630174525-215b22d4de21", ) + go_repository( + name = "com_github_golangci_unconvert", + build_file_proto_mode = "disable", + importpath = "github.com/golangci/unconvert", + sum = "h1:zwtduBRr5SSWhqsYNgcuWO2kFlpdOZbP0+yRjmvPGys=", + version = "v0.0.0-20180507085042-28b1c447d1f4", + ) go_repository( name = "com_github_gomodule_redigo", diff --git a/br/pkg/lightning/mydump/BUILD.bazel b/br/pkg/lightning/mydump/BUILD.bazel index 24d3545571ac2..1736f87af679f 100644 --- a/br/pkg/lightning/mydump/BUILD.bazel +++ b/br/pkg/lightning/mydump/BUILD.bazel @@ -31,7 +31,6 @@ go_library( "//util/slice", "//util/table-filter", "@com_github_pingcap_errors//:errors", - "@com_github_pingcap_failpoint//:failpoint", "@com_github_xitongsys_parquet_go//parquet", "@com_github_xitongsys_parquet_go//reader", "@com_github_xitongsys_parquet_go//source", diff --git a/build/BUILD.bazel b/build/BUILD.bazel index b283c4b2e0f20..cddd70f8d5876 100644 --- a/build/BUILD.bazel +++ b/build/BUILD.bazel @@ -110,5 +110,6 @@ nogo( "//build/linter/gofmt:gofmt", "//build/linter/ineffassign:ineffassign", "//build/linter/prealloc:prealloc", + "//build/linter/unconvert:unconvert", ] + staticcheck_analyzers(STATICHECK_ANALYZERS), ) diff --git a/build/linter/unconvert/BUILD.bazel b/build/linter/unconvert/BUILD.bazel new file mode 100644 index 0000000000000..23ececa09cd70 --- /dev/null +++ b/build/linter/unconvert/BUILD.bazel @@ -0,0 +1,14 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "unconvert", + srcs = ["analysis.go"], + importpath = "github.com/pingcap/tidb/build/linter/unconvert", + visibility = ["//visibility:public"], + deps = [ + "//build/linter/util", + "@com_github_golangci_unconvert//:unconvert", + "@org_golang_x_tools//go/analysis", + "@org_golang_x_tools//go/loader", + ], +) diff --git a/build/linter/unconvert/analysis.go b/build/linter/unconvert/analysis.go new file mode 100644 index 0000000000000..938dac5a9d155 --- /dev/null +++ b/build/linter/unconvert/analysis.go @@ -0,0 +1,47 @@ +package unconvert + +import ( + "fmt" + "go/token" + "go/types" + + unconvertAPI "github.com/golangci/unconvert" + "github.com/pingcap/tidb/build/linter/util" + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/loader" +) + +type Unconvert struct{} + +const Name = "unconvert" + +var Analyzer = &analysis.Analyzer{ + Name: Name, + Doc: "Remove unnecessary type conversions", + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + var createdPkgs []*loader.PackageInfo + createdPkgs = append(createdPkgs, util.MakeFakeLoaderPackageInfo(pass)) + allPkgs := map[*types.Package]*loader.PackageInfo{} + for _, pkg := range createdPkgs { + pkg := pkg + allPkgs[pkg.Pkg] = pkg + } + prog := &loader.Program{ + Fset: pass.Fset, + Imported: nil, // not used without .Created in any linter + Created: createdPkgs, // all initial packages + AllPackages: allPkgs, // all initial packages and their depndencies + } + positions := unconvertAPI.Run(prog) + if len(positions) == 0 { + return nil, nil + } + + for _, pos := range positions { + pass.Reportf(token.Pos(pos.Offset), fmt.Sprintf("[%s] Unnecessary conversion", Name)) + } + return nil, nil +} diff --git a/build/linter/util/BUILD.bazel b/build/linter/util/BUILD.bazel index 4ac3fec064d07..f8e81695c03eb 100644 --- a/build/linter/util/BUILD.bazel +++ b/build/linter/util/BUILD.bazel @@ -8,5 +8,6 @@ go_library( deps = [ "@co_honnef_go_tools//analysis/report", "@org_golang_x_tools//go/analysis", + "@org_golang_x_tools//go/loader", ], ) diff --git a/build/linter/util/util.go b/build/linter/util/util.go index d476173a973a0..2265efbe5c1c8 100644 --- a/build/linter/util/util.go +++ b/build/linter/util/util.go @@ -22,6 +22,7 @@ import ( "strings" "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/loader" "honnef.co/go/tools/analysis/report" ) @@ -147,3 +148,21 @@ func FormatCode(code string) string { return fmt.Sprintf("`%s`", code) } + +func MakeFakeLoaderPackageInfo(pass *analysis.Pass) *loader.PackageInfo { + var errs []error + + typeInfo := pass.TypesInfo + + return &loader.PackageInfo{ + Pkg: pass.Pkg, + Importable: true, // not used + TransitivelyErrorFree: true, // not used + + // use compiled (preprocessed) go files AST; + // AST linters use not preprocessed go files AST + Files: pass.Files, + Errors: errs, + Info: *typeInfo, + } +} diff --git a/build/nogo_config.json b/build/nogo_config.json index 8f7f4e940a597..ada308fba1104 100644 --- a/build/nogo_config.json +++ b/build/nogo_config.json @@ -210,6 +210,12 @@ ".*_generated\\.go$": "ignore generated code" } }, + "unconvert": { + "exclude_files": { + "/external/": "no need to vet third party code", + ".*_generated\\.go$": "ignore generated code" + } + }, "unmarshal": { "exclude_files": { "/external/": "no need to vet third party code", diff --git a/executor/seqtest/BUILD.bazel b/executor/seqtest/BUILD.bazel index 3f0a4bfd1291d..c248e2e1fd30a 100644 --- a/executor/seqtest/BUILD.bazel +++ b/executor/seqtest/BUILD.bazel @@ -17,6 +17,7 @@ go_test( "//kv", "//meta/autoid", "//metrics", + "//parser/ast", "//parser/model", "//parser/mysql", "//parser/terror", diff --git a/go.mod b/go.mod index 8943a2403025a..443d6ee0d5431 100644 --- a/go.mod +++ b/go.mod @@ -99,11 +99,14 @@ require ( github.com/aliyun/alibaba-cloud-sdk-go v1.61.1581 github.com/charithe/durationcheck v0.0.9 github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a + github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4 github.com/gordonklaus/ineffassign v0.0.0-20210914165742-4cc7213b9bc8 github.com/kyoh86/exportloopref v0.1.8 honnef.co/go/tools v0.0.1-2020.1.4 ) +require github.com/kisielk/gotool v1.0.0 // indirect + require ( cloud.google.com/go v0.100.2 // indirect cloud.google.com/go/compute v1.2.0 // indirect diff --git a/go.sum b/go.sum index 2dcc2c3e4cfeb..395e388d04d89 100644 --- a/go.sum +++ b/go.sum @@ -347,6 +347,8 @@ github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a h1:iR3fYXUjHCR97qWS github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a/go.mod h1:9qCChq59u/eW8im404Q2WWTrnBUQKjpNYKMbU4M7EFU= github.com/golangci/prealloc v0.0.0-20180630174525-215b22d4de21 h1:leSNB7iYzLYSSx3J/s5sVf4Drkc68W2wm4Ixh/mr0us= github.com/golangci/prealloc v0.0.0-20180630174525-215b22d4de21/go.mod h1:tf5+bzsHdTM0bsB7+8mt0GUMvjCgwLpTapNZHU8AajI= +github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4 h1:zwtduBRr5SSWhqsYNgcuWO2kFlpdOZbP0+yRjmvPGys= +github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4/go.mod h1:Izgrg8RkN3rCIMLGE9CyYmU9pY2Jer6DgANEnZ/L/cQ= github.com/gomodule/redigo v1.7.1-0.20190724094224-574c33c3df38/go.mod h1:B4C85qUVwatsJoIUNIfCRsp7qO0iAmpGFZ4EELWSbC4= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= @@ -511,6 +513,7 @@ github.com/kataras/pio v0.0.0-20190103105442-ea782b38602d/go.mod h1:NV88laa9UiiD github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q= github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= +github.com/kisielk/gotool v1.0.0 h1:AV2c/EiW3KqPNT9ZKl07ehoAGi4C5/01Cfbblndcapg= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.8.2/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A= github.com/klauspost/compress v1.9.0/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A= diff --git a/sessiontxn/isolation/BUILD.bazel b/sessiontxn/isolation/BUILD.bazel index 1345c09f08b21..16ebeaecfb9a0 100644 --- a/sessiontxn/isolation/BUILD.bazel +++ b/sessiontxn/isolation/BUILD.bazel @@ -48,10 +48,12 @@ go_test( "//parser", "//parser/ast", "//planner", + "//session", "//sessionctx", "//sessiontxn", "//testkit", "//testkit/testsetup", + "//types", "@com_github_pingcap_errors//:errors", "@com_github_pingcap_failpoint//:failpoint", "@com_github_pingcap_kvproto//pkg/kvrpcpb", diff --git a/testkit/testfork/BUILD.bazel b/testkit/testfork/BUILD.bazel index 743bd70da5b0d..89d1e33ac65a5 100644 --- a/testkit/testfork/BUILD.bazel +++ b/testkit/testfork/BUILD.bazel @@ -15,4 +15,5 @@ go_test( name = "testfork_test", srcs = ["fork_test.go"], embed = [":testfork"], + deps = ["@com_github_stretchr_testify//require"], ) From 205d67f60614aef85ce0e449c8def101b30234ab Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Wed, 29 Jun 2022 18:06:44 +0800 Subject: [PATCH 2/5] *: enable unconvert Signed-off-by: Weizhen Wang --- build/linter/unconvert/analysis.go | 14 ++++++++++++++ build/linter/util/util.go | 1 + build/nogo_config.json | 1 + 3 files changed, 16 insertions(+) diff --git a/build/linter/unconvert/analysis.go b/build/linter/unconvert/analysis.go index 938dac5a9d155..28935bf69e65e 100644 --- a/build/linter/unconvert/analysis.go +++ b/build/linter/unconvert/analysis.go @@ -1,3 +1,17 @@ +// Copyright 2022 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package unconvert import ( diff --git a/build/linter/util/util.go b/build/linter/util/util.go index 2265efbe5c1c8..5b58883576d60 100644 --- a/build/linter/util/util.go +++ b/build/linter/util/util.go @@ -149,6 +149,7 @@ func FormatCode(code string) string { return fmt.Sprintf("`%s`", code) } +// MakeFakeLoaderPackageInfo creates a fake loader.PackageInfo for a given package. func MakeFakeLoaderPackageInfo(pass *analysis.Pass) *loader.PackageInfo { var errs []error diff --git a/build/nogo_config.json b/build/nogo_config.json index ada308fba1104..27ecd053d462a 100644 --- a/build/nogo_config.json +++ b/build/nogo_config.json @@ -213,6 +213,7 @@ "unconvert": { "exclude_files": { "/external/": "no need to vet third party code", + ".*\\.pb\\.go$": "generated code", ".*_generated\\.go$": "ignore generated code" } }, From e71a3c409903e782de9d500361b17ca966bf6170 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Wed, 29 Jun 2022 18:16:11 +0800 Subject: [PATCH 3/5] *: enable unconvert Signed-off-by: Weizhen Wang --- build/nogo_config.json | 1 + 1 file changed, 1 insertion(+) diff --git a/build/nogo_config.json b/build/nogo_config.json index 27ecd053d462a..dbc944c1b9486 100644 --- a/build/nogo_config.json +++ b/build/nogo_config.json @@ -214,6 +214,7 @@ "exclude_files": { "/external/": "no need to vet third party code", ".*\\.pb\\.go$": "generated code", + "/cgo/": "no need to vet third party code for cgo", ".*_generated\\.go$": "ignore generated code" } }, From 472da15f3ec8c86d7ef53a4e5c4ac0de6e05c91d Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Wed, 29 Jun 2022 18:23:23 +0800 Subject: [PATCH 4/5] *: enable unconvert Signed-off-by: Weizhen Wang --- build/nogo_config.json | 1 + 1 file changed, 1 insertion(+) diff --git a/build/nogo_config.json b/build/nogo_config.json index dbc944c1b9486..5200c55164431 100644 --- a/build/nogo_config.json +++ b/build/nogo_config.json @@ -214,6 +214,7 @@ "exclude_files": { "/external/": "no need to vet third party code", ".*\\.pb\\.go$": "generated code", + "parser/parser.go": "generated code", "/cgo/": "no need to vet third party code for cgo", ".*_generated\\.go$": "ignore generated code" } From e43a93a20ee3e179152049e8db405782bbee7a47 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Wed, 29 Jun 2022 18:38:28 +0800 Subject: [PATCH 5/5] *: enable unconvert Signed-off-by: Weizhen Wang --- build/linter/unconvert/analysis.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build/linter/unconvert/analysis.go b/build/linter/unconvert/analysis.go index 28935bf69e65e..89da08bccd4aa 100644 --- a/build/linter/unconvert/analysis.go +++ b/build/linter/unconvert/analysis.go @@ -25,10 +25,10 @@ import ( "golang.org/x/tools/go/loader" ) -type Unconvert struct{} - +// Name is the name of the analyzer. const Name = "unconvert" +// Analyzer is the analyzer struct of unconvert. var Analyzer = &analysis.Analyzer{ Name: Name, Doc: "Remove unnecessary type conversions",