build: refine build and lint (#1584)

pingcap · Apr 15, 2021 · ae41881 · ae41881
1 parent f7e6f8e
commit ae41881
Show file tree

Hide file tree

Showing 314 changed files with 3,532 additions and 2,617 deletions.
diff --git a/.github/workflows/check-and-build.yml b/.github/workflows/check-and-build.yml
@@ -43,7 +43,7 @@ jobs:
         key: ${{ runner.os }}-dm-tools-${{ hashFiles('**/tools/go.sum') }}
 
     - name: Build
-      run: make build
+      run: make build nolint=true
 
   lint:
     name: Lint
@@ -56,11 +56,5 @@ jobs:
       - name: GolangCI Lint
         uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.31
-          args: --timeout 10m0s --skip-dirs ^_tool/
-
-      - name: Revive Lint
-        uses: morphy2k/revive-action@v1
-        with:
-          config: .revive.toml
-          exclude: _tool/...
+          version: latest
+          args: --config .golangci.yml --timeout 10m0s --skip-dirs ^_tool/
diff --git a/.golangci.yml b/.golangci.yml
@@ -0,0 +1,123 @@
+linters:
+  disable-all: true
+  enable:
+    - asciicheck
+    - bodyclose
+    - deadcode
+    - depguard
+    - dogsled
+    - dupl
+    - errcheck
+    - exportloopref
+    - gocritic
+    - godot
+    - golint
+    - goprintffuncname
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - nakedret
+    - noctx
+    - nolintlint
+    - prealloc
+    - revive
+    - rowserrcheck
+    - staticcheck
+    - structcheck
+    - stylecheck
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - varcheck
+    - whitespace
+    - durationcheck
+    - gofumpt
+    - goheader
+    - gomodguard
+    - ifshort
+    - importas
+    - makezero
+    - nilerr
+    - predeclared
+    - sqlclosecheck
+    - thelper
+    - tparallel
+    - wastedassign
+
+  # don't enable:
+  # - testpackage
+  # - lll
+  # - wsl
+  # - gochecknoglobals
+  # - godox
+  # - gomnd
+  # - goerr113
+  # - exhaustive
+  # - wrapcheck
+  # - nlreturn
+  # - exhaustivestruct
+  # - errorlint
+  # - forcetypeassert
+  # - paralleltest
+  # - forbidigo
+  # - gosec
+  # - goconst
+  # - interfacer
+  # - scopelint
+
+  # already cover:
+  # - gci
+  # - goimports
+  # - gofmt
+
+  # better to fix them
+  # - funlen
+  # - gocognit
+  # - nestif
+  # - gocyclo
+  # - cyclop
+  # - gomoddirectives
+  # - gochecknoinits
+  # - maligned
+
+linters-settings:
+  govet:
+    # report about shadowed variables
+    check-shadowing: true
+  revive:
+    ignoreGeneratedHeader: false
+    severity: "error"
+    confidence: 0.8
+    errorCode: -1
+    warningCode: -1
+    rules:
+      - name: blank-imports
+      - name: context-as-argument
+      - name: dot-imports
+      - name: error-return
+      - name: error-strings
+      - name: error-naming
+      - name: exported
+      - name: if-return
+      - name: var-naming
+      - name: package-comments
+      - name: range
+      - name: receiver-naming
+      - name: indent-error-flow
+      - name: superfluous-else
+      - name: modifies-parameter
+      - name: unreachable-code
+
+issues:
+  include:
+    - EXC0002 # golint
+    - EXC0003
+
+  # Fix found issues (if it's supported by the linter)
+  fix: true
+
+run:
+  # timeout for analysis, e.g. 30s, 5m, default is 1m
+  timeout: 5m
diff --git a/.revive.toml b/.revive.toml
diff --git a/Makefile b/Makefile
@@ -8,7 +8,9 @@ CURDIR   := $(shell pwd)
 GO       := GO111MODULE=on go
 GOBUILD  := CGO_ENABLED=0 $(GO) build
 GOTEST   := CGO_ENABLED=1 $(GO) test
+PACKAGE_NAME := github.com/pingcap/dm
 PACKAGES  := $$(go list ./... | grep -vE 'tests|cmd|vendor|pb|pbmock|_tools')
+PACKAGE_DIRECTORIES := $$(echo "$(PACKAGES)" | sed 's/github.com\/pingcap\/dm\/*//')
 PACKAGES_RELAY := $$(go list ./... | grep 'github.com/pingcap/dm/relay')
 PACKAGES_SYNCER := $$(go list ./... | grep 'github.com/pingcap/dm/syncer')
 PACKAGES_PKG_BINLOG := $$(go list ./... | grep 'github.com/pingcap/dm/pkg/binlog')
@@ -80,15 +82,7 @@ dm-portal-frontend:
 
 tools_setup:
 	@echo "setup tools"
-	cd tools && $(GOBUILD) -o bin/errcheck github.com/kisielk/errcheck
-	cd tools && $(GOBUILD) -o bin/failpoint-ctl github.com/pingcap/failpoint/failpoint-ctl
-	cd tools && $(GOBUILD) -o bin/gocovmerge github.com/zhouqiang-cl/gocovmerge
-	cd tools && $(GOBUILD) -o bin/golint golang.org/x/lint/golint
-	cd tools && $(GOBUILD) -o bin/goveralls github.com/mattn/goveralls
-	cd tools && $(GOBUILD) -o bin/mockgen github.com/golang/mock/mockgen
-	cd tools && $(GOBUILD) -o bin/protoc-gen-gogofaster github.com/gogo/protobuf/protoc-gen-gogofaster
-	cd tools && $(GOBUILD) -o bin/protoc-gen-grpc-gateway github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway
-	cd tools && $(GOBUILD) -o bin/statik github.com/rakyll/statik
+	@cd tools && make
 
 generate_proto: tools_setup
 	./generate-dm.sh
@@ -128,33 +122,26 @@ unit_test_pkg_binlog: tools_setup
 unit_test_others: tools_setup
 	$(call run_unit_test,$(PACKAGES_OTHERS),unit_test_others)
 
-check: tools_setup fmt lint vet terror_check tidy_mod
+check: tools_setup fmt lint terror_check tidy_mod
 
 fmt:
-	@echo "gofmt (simplify)"
-	@ gofmt -s -l -w $(FILES) 2>&1 | awk '{print} END{if(NR>0) {exit 1}}'
-
-errcheck: tools_setup
-	@echo "errcheck"
-	tools/bin/errcheck -blank $(PACKAGES) | grep -v "_test\.go" | awk '{print} END{if(NR>0) {exit 1}}'
+	@echo "gofumports"
+	tools/bin/gofumports -w -d -local $(PACKAGE_NAME) $(PACKAGE_DIRECTORIES) 2>&1 | awk '{print} END{if(NR>0) {exit 1}}'
 
 lint: tools_setup
-	@echo "golint"
-	tools/bin/golint -set_exit_status $(PACKAGES)
-
-vet:
-	$(GO) build -o bin/shadow golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow
-	@echo "vet"
-	@$(GO) vet -composites=false $(PACKAGES)
-	@$(GO) vet -vettool=$(CURDIR)/bin/shadow $(PACKAGES) || true
+	@if [[ "${nolint}" != "true" ]]; then\
+		echo "golangci-lint"; \
+		tools/bin/golangci-lint run --config=$(CURDIR)/.golangci.yml --issues-exit-code=1 $(PACKAGE_DIRECTORIES); \
+	fi
 
 terror_check:
 	@echo "check terror conflict"
 	_utils/terror_gen/check.sh
 
 tidy_mod:
 	@echo "tidy go.mod"
-	_utils/mod_check/check.sh
+	$(GO) mod tidy
+	git diff --exit-code go.mod go.sum 
 
 dm_integration_test_build: tools_setup
 	$(FAILPOINT_ENABLE)
@@ -209,14 +196,6 @@ else
 	go tool cover -html "$(TEST_DIR)/unit_test.out" -o "$(TEST_DIR)/unit_test_cov.html"
 endif
 
-check-static:
-	@echo "gometalinter"
-	gometalinter --disable-all --deadline 120s \
-	  --enable misspell \
-	  --enable megacheck \
-	  --enable ineffassign \
-	  ./...
-
 failpoint-enable: tools_setup
 	$(FAILPOINT_ENABLE)
 

diff --git a/_utils/mod_check/check.sh b/_utils/mod_check/check.sh
diff --git a/chaos/cases/config.go b/chaos/cases/config.go
@@ -75,8 +75,7 @@ func newConfig() *config {
 
 // parse parses flag definitions from the argument list.
 func (c *config) parse(args []string) error {
-	err := c.FlagSet.Parse(args)
-	if err != nil {
+	if err := c.FlagSet.Parse(args); err != nil {
 		return err
 	}
 

diff --git a/chaos/cases/generator.go b/chaos/cases/generator.go
@@ -40,7 +40,7 @@ type SQLs []SQL
 type Case []SQLs
 
 var (
-	// add some flags column, so we can make sure the order of new column in optimsitic
+	// add some flags column, so we can make sure the order of new column in optimsitic.
 	preSQLs1 = SQLs{
 		{"ALTER TABLE %s.%s ADD COLUMN case3_flag1 INT, ADD COLUMN case3_flag2 INT, ADD COLUMN case3_flag3 INT", source1},
 		{"ALTER TABLE %s.%s ADD COLUMN case3_flag1 INT, ADD COLUMN case3_flag2 INT, ADD COLUMN case3_flag3 INT", source2},
@@ -56,7 +56,7 @@ var (
 		{"ALTER TABLE %s.%s ADD COLUMN case9_flag INT;", source3},
 	}
 
-	// ALL ADD COLUMN, ALL DROP COLUMN
+	// ALL ADD COLUMN, ALL DROP COLUMN.
 	case1 = Case{
 		{{"ALTER TABLE %s.%s ADD COLUMN case1 INT;", source1}},
 		{{"ALTER TABLE %s.%s ADD COLUMN case1 INT;", source2}},
@@ -65,12 +65,12 @@ var (
 		{{"ALTER TABLE %s.%s DROP COLUMN case1;", source2}},
 		{{"ALTER TABLE %s.%s DROP COLUMN case1;", source3}},
 	}
-	// ADD COLUMN, DROP COLUMN for one source
+	// ADD COLUMN, DROP COLUMN for one source.
 	case2 = Case{
 		{{"ALTER TABLE %s.%s ADD COLUMN case2 INT;", source1}},
 		{{"ALTER TABLE %s.%s DROP COLUMN case2;", source1}},
 	}
-	// ADD columns out of order
+	// ADD columns out of order.
 	case3 = Case{
 		{
 			{"ALTER TABLE %s.%s DROP COLUMN case3_1;", source1},
@@ -93,7 +93,7 @@ var (
 		{{"ALTER TABLE %s.%s ADD COLUMN case3_1 INT AFTER case3_flag1;", source2}},
 		{{"ALTER TABLE %s.%s ADD COLUMN case3_2 INT AFTER case3_flag2;", source3}},
 	}
-	// MULTIPLE ADD COLUMN out of order
+	// MULTIPLE ADD COLUMN out of order.
 	case4 = Case{
 		{
 			{"ALTER TABLE %s.%s DROP COLUMN case4_1, DROP COLUMN case4_2, DROP COLUMN case4_3;", source1},
@@ -104,7 +104,7 @@ var (
 		{{"ALTER TABLE %s.%s ADD COLUMN case4_2 INT AFTER case4_flag2, ADD COLUMN case4_3 INT AFTER case4_flag3, ADD COLUMN case4_1 INT AFTER case4_flag1;", source2}},
 		{{"ALTER TABLE %s.%s ADD COLUMN case4_3 INT AFTER case4_flag3, ADD COLUMN case4_1 INT AFTER case4_flag1, ADD COLUMN case4_2 INT AFTER case4_flag2;", source3}},
 	}
-	// MULTIPLE ADD COLUMN vs ADD columns
+	// MULTIPLE ADD COLUMN vs ADD columns.
 	case5 = Case{
 		{
 			{"ALTER TABLE %s.%s DROP COLUMN case5_1;", source1},
@@ -123,7 +123,7 @@ var (
 		{{"ALTER TABLE %s.%s ADD COLUMN case5_3 INT AFTER case5_flag3;", source2}},
 		{{"ALTER TABLE %s.%s ADD COLUMN case5_2 INT AFTER case5_flag2, ADD COLUMN case5_3 INT AFTER case5_flag3;", source1}},
 	}
-	// ALL ADD INDEX, ALL DROP INDEX
+	// ALL ADD INDEX, ALL DROP INDEX.
 	case6 = Case{
 		{{"ALTER TABLE %s.%s ADD INDEX case6_idx(case3_flag1);", source1}},
 		{{"ALTER TABLE %s.%s ADD INDEX case6_idx(case3_flag1);", source2}},
@@ -132,12 +132,12 @@ var (
 		{{"ALTER TABLE %s.%s DROP INDEX case6_idx;", source2}},
 		{{"ALTER TABLE %s.%s DROP INDEX case6_idx;", source3}},
 	}
-	// ADD INDEX, DROP INDEX for one source
+	// ADD INDEX, DROP INDEX for one source.
 	case7 = Case{
 		{{"ALTER TABLE %s.%s ADD INDEX case7_idx(uuid);", source1}},
 		{{"ALTER TABLE %s.%s DROP INDEX case7_idx;", source1}},
 	}
-	// ADD MULTI-COLUMN INDEX
+	// ADD MULTI-COLUMN INDEX.
 	case8 = Case{
 		{
 			{"ALTER TABLE %s.%s DROP INDEX case8_idx;", source1},
@@ -148,7 +148,7 @@ var (
 		{{"ALTER TABLE %s.%s ADD INDEX case8_idx(case4_flag1, case4_flag2, case4_flag3);", source2}},
 		{{"ALTER TABLE %s.%s ADD INDEX case8_idx(case4_flag1, case4_flag2, case4_flag3);", source3}},
 	}
-	// ADD COLUMN AND INDEX
+	// ADD COLUMN AND INDEX.
 	case9 = Case{
 		{
 			{"ALTER TABLE %s.%s DROP INDEX case9_idx;", source1},
@@ -206,7 +206,7 @@ func NewCaseGenerator(shardMode string) *CaseGenerator {
 	return g
 }
 
-// Start starts to generate sqls case
+// Start starts to generate sqls case.
 func (g *CaseGenerator) Start(ctx context.Context, schema string, tables []string) {
 	g.schema = schema
 	g.tables = tables
@@ -238,12 +238,12 @@ func (g *CaseGenerator) genSQLs(ctx context.Context) {
 	}
 }
 
-// GetSQLs gets sql from CaseGenerator
+// GetSQLs gets sql from CaseGenerator.
 func (g *CaseGenerator) GetSQLs() SQLs {
 	return <-g.sqlsChan
 }
 
-// GetPreSQLs gets preSQLs from CaseGenerator
+// GetPreSQLs gets preSQLs from CaseGenerator.
 func (g *CaseGenerator) GetPreSQLs() SQLs {
 	testPreSQLs := make(SQLs, 0, len(g.testPreSQLs)*len(g.tables))
 	for _, table := range g.tables {