From 8a6af6068d3f4b850cdcca15f96ce56f49315f91 Mon Sep 17 00:00:00 2001 From: Chen Chen Date: Thu, 7 Sep 2023 01:06:53 +0800 Subject: [PATCH] Add golangci github action and replace the deprecated golint (#10187) * Add golangci github action and replace the deprecated golint Signed-off-by: z1cheng * Install if golangci-lint not exists Signed-off-by: z1cheng * Use -z operator Signed-off-by: z1cheng * Fix json tag for DatadogSampleRate field in config.go Signed-off-by: z1cheng * Add golangci linters Signed-off-by: z1cheng * Revert DatadogSampleRate fix Signed-off-by: z1cheng * Fix comments Signed-off-by: z1cheng * Add a new line Signed-off-by: z1cheng * fixup! Add a new line Signed-off-by: z1cheng * Add trigger condition Signed-off-by: z1cheng * Add golint-check entry in makefile Signed-off-by: Chen Chen * Run golint-check in a container Signed-off-by: Chen Chen --------- Signed-off-by: z1cheng Signed-off-by: Chen Chen --- .github/workflows/ci.yaml | 19 --- .github/workflows/golangci-lint.yml | 34 ++++ .golangci.yml | 241 ++++++++++++++++++++++++++++ Makefile | 6 + hack/verify-golint.sh | 22 +-- 5 files changed, 289 insertions(+), 33 deletions(-) create mode 100644 .github/workflows/golangci-lint.yml create mode 100644 .golangci.yml diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a5dd136ae6..e969933662 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -78,25 +78,6 @@ jobs: # G307 TODO: Deferring unsafe method "Close" args: -exclude=G109,G601,G104,G204,G304,G306,G307 -tests=false -exclude-dir=test -exclude-dir=images/ -exclude-dir=docs/ ./... - lint: - runs-on: ubuntu-latest - needs: changes - if: | - (needs.changes.outputs.go == 'true') - steps: - - name: Checkout - uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0 - - - name: Set up Go - id: go - uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # v4.1.0 - with: - go-version: '1.20' - check-latest: true - - - name: Run Lint - run: ./hack/verify-golint.sh - gofmt: runs-on: ubuntu-latest needs: changes diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 0000000000..d601d69dd5 --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,34 @@ +name: golangci-lint + +on: + pull_request: + push: + branches: + - main + paths-ignore: + - 'docs/**' + - 'deploy/**' + - '**.md' + +permissions: + contents: read + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + + - name: Set up Go + id: go + uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 + with: + go-version: '1.20' + check-latest: true + + - name: golangci-lint + uses: golangci/golangci-lint-action@639cd343e1d3b897ff35927a75193d57cfcba299 # v3.6.0 + with: + version: v1.53 diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000000..5da8f0399f --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,241 @@ +run: + timeout: 10m + allow-parallel-runners: true + + # Maximum issues count per one linter. Set to 0 to disable. Default is 50. + max-issues-per-linter: 0 + + # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. + max-same-issues: 0 +linters: + disable-all: true + enable: + - asasalint + - asciicheck + - bidichk + - bodyclose + - contextcheck + - decorder + - dogsled + - dupl + - durationcheck + - errcheck + - errchkjson + - errname + - execinquery + - ginkgolinter + - gocheckcompilerdirectives + - goconst + - gocritic + - gocyclo + - godox + - gofmt + - gofumpt + - goheader + - goimports + - gomoddirectives + - gomodguard + - goprintffuncname + - gosec + - gosimple + - govet + - grouper + - importas + - ineffassign + - loggercheck + - makezero + - misspell + - musttag + - nakedret + - nolintlint + - nosprintfhostport + - prealloc + - predeclared + - promlinter + - reassign + - revive + - rowserrcheck + - sqlclosecheck + - staticcheck + - stylecheck + - tenv + - testableexamples + - typecheck + - unconvert + - unparam + - unused + - usestdlibvars + - whitespace + # - containedctx + # - cyclop + # - dupword + # - errorlint + # - exhaustive + # - exhaustruct + # - exportloopref + # - forbidigo + # - forcetypeassert + # - funlen + # - gci + # - gochecknoglobals + # - gochecknoinits + # - gocognit + # - godot + # - goerr113 + # - gomnd + # - interfacebloat + # - ireturn + # - lll + # - maintidx + # - nestif + # - nilerr + # - nilnil + # - nlreturn + # - noctx + # - nonamedreturns + # - paralleltest + # - tagliatelle + # - testpackage + # - thelper + # - tparallel + # - varnamelen + # - wastedassign + # - wrapcheck + # - wsl +linters-settings: + gocyclo: + min-complexity: 40 + godox: + keywords: + - BUG + - FIXME + - HACK + errcheck: + check-type-assertions: true + check-blank: true + gocritic: + enabled-checks: + # Diagnostic + - appendAssign + - argOrder + - badCall + - badCond + - badLock + - badRegexp + - badSorting + - builtinShadowDecl + - caseOrder + - codegenComment + - commentedOutCode + - deferInLoop + - deprecatedComment + - dupArg + - dupBranchBody + - dupCase + - dupSubExpr + - dynamicFmtString + - emptyDecl + - evalOrder + - exitAfterDefer + - externalErrorReassign + - filepathJoin + - flagDeref + - flagName + - mapKey + - nilValReturn + - offBy1 + - regexpPattern + - returnAfterHttpError + - sloppyReassign + - sloppyTypeAssert + - sortSlice + - sprintfQuotedString + - sqlQuery + - syncMapLoadAndDelete + - truncateCmp + - unnecessaryDefer + - weakCond + + # Performance + - appendCombine + - equalFold + - hugeParam + - indexAlloc + - preferDecodeRune + - preferFprint + - preferStringWriter + - preferWriteByte + - rangeExprCopy + - rangeValCopy + - sliceClear + - stringXbytes + + # Style + - assignOp + - boolExprSimplify + - captLocal + - commentFormatting + - commentedOutImport + - defaultCaseOrder + - deferUnlambda + - docStub + - dupImport + - elseif + - emptyFallthrough + - emptyStringTest + - exposedSyncMutex + - hexLiteral + - httpNoBody + - ifElseChain + - methodExprCall + - newDeref + - octalLiteral + - preferFilepathJoin + - redundantSprint + - regexpMust + - regexpSimplify + - ruleguard + - singleCaseSwitch + - sloppyLen + - stringConcatSimplify + - stringsCompare + - switchTrue + - timeCmpSimplify + - timeExprSimplify + - todoCommentWithoutDetail + - tooManyResultsChecker + - typeAssertChain + - typeDefFirst + - typeSwitchVar + - underef + - unlabelStmt + - unlambda + - unslice + - valSwap + - whyNoLint + - wrapperFunc + - yodaStyleExpr + + # Opinionated + - builtinShadow + - importShadow + - initClause + - nestingReduce + - paramTypeCombine + - ptrToRefParam + - typeUnparen + - unnamedResult + - unnecessaryBlock + nolintlint: + # Enable to ensure that nolint directives are all used. Default is true. + allow-unused: false + # Disable to ensure that nolint directives don't have a leading space. Default is true. + # TODO(lint): Enforce machine-readable `nolint` directives + allow-leading-space: true + # Exclude following linters from requiring an explanation. Default is []. + allow-no-explanation: [] + # Enable to require an explanation of nonzero length after each nolint directive. Default is false. + # TODO(lint): Enforce explanations for `nolint` directives + require-explanation: false + # Enable to require nolint directives to mention the specific linter being suppressed. Default is false. + require-specific: true diff --git a/Makefile b/Makefile index 7b413141a0..792cd56a76 100644 --- a/Makefile +++ b/Makefile @@ -128,6 +128,12 @@ static-check: ## Run verification script for boilerplate, codegen, gofmt, golint MAC_OS=$(MAC_OS) \ hack/verify-all.sh +.PHONY: golint-check +golint-check: + @build/run-in-docker.sh \ + MAC_OS=$(MAC_OS) \ + hack/verify-golint.sh + ############################### # Tests for ingress-nginx ############################### diff --git a/hack/verify-golint.sh b/hack/verify-golint.sh index 7c92848809..17bcedd9fc 100755 --- a/hack/verify-golint.sh +++ b/hack/verify-golint.sh @@ -22,19 +22,13 @@ KUBE_ROOT=$(dirname "${BASH_SOURCE}")/.. cd "${KUBE_ROOT}" -GOLINT=${GOLINT:-"golint"} -PACKAGES=($(go list ./internal/... | grep -v /vendor/)) -bad_files=() -for package in "${PACKAGES[@]}"; do - out=$("${GOLINT}" -min_confidence=0.9 "${package}" | grep -v -E '(should not use dot imports|internal/file/bindata.go)' || :) - if [[ -n "${out}" ]]; then - bad_files+=("${out}") - fi -done -if [[ "${#bad_files[@]}" -ne 0 ]]; then - echo "!!! '$GOLINT' problems: " - echo "${bad_files[@]}" - exit 1 +LINT=${LINT:-golangci-lint} + +if [[ -z "$(command -v ${LINT})" ]]; then + echo "${LINT} is missing. Installing it now." + # See: https://golangci-lint.run/usage/install/#local-installation + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.53.3 + LINT=$(go env GOPATH)/bin/golangci-lint fi -# ex: ts=2 sw=2 et filetype=sh +${LINT} run