From e4fa520a1884409771be8cb373cd4798b171007b Mon Sep 17 00:00:00 2001 From: Joshua Hoblitt Date: Wed, 26 Mar 2025 15:52:51 -0700 Subject: [PATCH] core: replace gofmt with golangci-lint This has the effect of running gofmt and gofumpt at the same time. Signed-off-by: Joshua Hoblitt (cherry picked from commit c5f59db8595cd0f4bb42fa04d075df61261b1fb6) --- .github/workflows/golangci-lint.yaml | 15 ++----- .golangci.yaml | 58 ++++++++++++++++++++++++++++ Makefile | 8 ++-- build/makelib/golang.mk | 31 ++++++++------- 4 files changed, 80 insertions(+), 32 deletions(-) create mode 100644 .golangci.yaml diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml index 0fda31ad8..6a9ab8b99 100644 --- a/.github/workflows/golangci-lint.yaml +++ b/.github/workflows/golangci-lint.yaml @@ -33,21 +33,12 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@55c2c1448f86e01eaae002a5a3a9624417608d84 # v6.5.2 with: - # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. - version: v1.62 - - # Optional: working directory, useful for monorepos - # working-directory: somedir - - # Optional: golangci-lint command line arguments. - args: -E gosec -E gofmt -E gofumpt --timeout=10m - + # `version` is parsed by build/makelib/golang.mk + # Do not change the structure of this job without updating the parsing logic. + version: v2.0.2 # actions/setup-go already handles caching skip-cache: true - # Optional: show only new issues if it's a pull request. The default value is `false`. - # only-new-issues: true - govulncheck: name: govulncheck runs-on: ubuntu-latest diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 000000000..fb9a7cb2b --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,58 @@ +--- +version: "2" +linters: + default: none + enable: + - errcheck + - govet + - gosec + - ineffassign + - staticcheck + - unused + exclusions: + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + rules: + - linters: + - staticcheck + text: "QF1003:" # could use tagged switch on errCode (staticcheck) + - linters: + - staticcheck + text: "ST1005:" # error strings should not be capitalized (staticcheck) + - linters: + - staticcheck + text: "QF1008:" # could remove embedded field "Connection" from selector (staticcheck) + - linters: + - staticcheck + text: "QF1007:" # could merge conditional assignment into variable declaration (staticcheck) + - linters: + - staticcheck + text: "ST1023:" # should omit type string from declaration; it will be inferred from the right-hand side (staticcheck) + - linters: + - staticcheck + text: "QF1001:" # could apply De Morgan's law (staticcheck) + - linters: + - staticcheck + text: "QF1002:" # could use tagged switch on args[0] (staticcheck) + - linters: + - staticcheck + text: "QF1011:" # could omit type string from declaration; it will be inferred from the right-hand side (staticcheck) + - linters: + - staticcheck + text: "QF1004:" # could use strings.ReplaceAll instead (staticcheck) + - linters: + - staticcheck + text: "ST1008:" # error should be returned as the last argument (staticcheck) + - linters: + - staticcheck + text: "ST1019" # package "github.com/rook/rook/pkg/daemon/ceph/client" is being imported more than once (staticcheck) + - linters: + - staticcheck + text: "QF1006:" # could lift into loop condition (staticcheck) +formatters: + enable: + - gofmt + - gofumpt diff --git a/Makefile b/Makefile index 21ff74a4d..67c565ed7 100644 --- a/Makefile +++ b/Makefile @@ -154,9 +154,11 @@ vet: ## Runs lint checks on go sources. @$(MAKE) go.vet fmt: ## Check formatting of go sources. - @$(MAKE) go.init @$(MAKE) go.fmt +fmt-fix: ## Reformatting of go sources. + @$(MAKE) go.fmt-fix + .PHONY: markdownlint markdownlint: ## Check formatting of documentation sources markdownlint-cli2 "Documentation/**/**.md" "#Documentation/Helm-Charts/**" --config .markdownlint-cli2.cjs @@ -165,10 +167,6 @@ markdownlint: ## Check formatting of documentation sources yamllint: yamllint -c .yamllint deploy/examples/ --no-warnings -.PHONY: golangci-lint -golangci-lint: - go run github.com/golangci/golangci-lint/cmd/golangci-lint@latest run - .PHONY: lint lint: yamllint pylint shellcheck vet markdownlint golangci-lint ## Run various linters diff --git a/build/makelib/golang.mk b/build/makelib/golang.mk index 7e4397eb4..72d4acf9c 100644 --- a/build/makelib/golang.mk +++ b/build/makelib/golang.mk @@ -71,12 +71,8 @@ GOHOST := GOOS=$(GOHOSTOS) GOARCH=$(GOHOSTARCH) go GO_VERSION := $(shell $(GO) version | sed -ne 's/[^0-9]*\(\([0-9]\.\)\{0,4\}[0-9][^.]\).*/\1/p') GO_FULL_VERSION := $(shell $(GO) version) -GOFMT_VERSION := $(GO_VERSION) -ifneq ($(findstring $(GOFMT_VERSION),$(GO_VERSION)),) -GOFMT := $(shell which gofmt) -else -GOFMT := $(TOOLS_HOST_DIR)/gofmt$(GOFMT_VERSION) -endif +GOLANGCI_LINT_VERSION := $(strip $(shell yq .jobs.golangci.steps[2].with.version .github/workflows/golangci-lint.yaml)) +GOLANGCI_LINT := $(TOOLS_HOST_DIR)/golangci-lint-$(GOLANGCI_LINT_VERSION) GO_OUT_DIR := $(abspath $(OUTPUT_DIR)/bin/$(PLATFORM)) GO_TEST_OUTPUT := $(abspath $(OUTPUT_DIR)/tests/$(PLATFORM)) @@ -149,11 +145,16 @@ go.vet: CGO_ENABLED=$(CGO_ENABLED_VALUE) $(GOHOST) vet $(GO_COMMON_FLAGS) $(GO_PACKAGES) $(GO_INTEGRATION_TEST_PACKAGES) .PHONY: go.fmt -# ignore deepcopy generated files since the tool hardcoded the header with a "// +build" which in Golang 1.17 makes it fail gofmt since "////go:build" is preferred -# see: https://github.com/kubernetes/gengo/blob/master/examples/deepcopy-gen/generators/deepcopy.go#L136 -# https://github.com/kubernetes/gengo/pull/210 -go.fmt: $(GOFMT) - @gofmt_out=$$(find $(GO_SUBDIRS) -name "*.go" -not -name "*.deepcopy.go" | xargs $(GOFMT) -s -d -e 2>&1) && [ -z "$${gofmt_out}" ] || (echo "$${gofmt_out}" 1>&2; exit 1) +go.fmt: $(GOLANGCI_LINT) + @$(GOLANGCI_LINT) fmt -d + +.PHONY: go.fmt-fix +go.fmt-fix: $(GOLANGCI_LINT) + @$(GOLANGCI_LINT) fmt + +.PHONY: golangci-lint +golangci-lint: $(GOLANGCI_LINT) + @$(GOLANGCI_LINT) run go.validate: go.vet go.fmt @@ -179,11 +180,11 @@ go.mod.clean: @sudo rm -fr $(WORK_DIR)/cross_pkg @$(GOHOST) clean -modcache -$(GOFMT): - @echo === installing gofmt$(GOFMT_VERSION) +$(GOLANGCI_LINT): + @echo === installing golangci-lint-$(GOLANGCI_LINT_VERSION) @mkdir -p $(TOOLS_HOST_DIR)/tmp - @curl -sL https://dl.google.com/go/go$(GOFMT_VERSION).$(shell go env GOHOSTOS)-$(GOHOSTARCH).tar.gz | tar -xz -C $(TOOLS_HOST_DIR)/tmp - @mv $(TOOLS_HOST_DIR)/tmp/go/bin/gofmt $(GOFMT) + @curl -sL https://github.com/golangci/golangci-lint/releases/download/$(GOLANGCI_LINT_VERSION)/golangci-lint-$(patsubst v%,%,$(GOLANGCI_LINT_VERSION))-$(shell go env GOHOSTOS)-$(GOHOSTARCH).tar.gz | tar -xz -C $(TOOLS_HOST_DIR)/tmp + @mv $(TOOLS_HOST_DIR)/tmp/golangci-lint-$(patsubst v%,%,$(GOLANGCI_LINT_VERSION))-$(shell go env GOHOSTOS)-$(GOHOSTARCH)/golangci-lint $(GOLANGCI_LINT) @rm -fr $(TOOLS_HOST_DIR)/tmp $(GOJUNIT):