golangci-lint: support stricter checking in new code

It is useful to check new code with a stricter configuration because we want it
to be of higher quality. Code reviews also become easier when reviewers don't
need to point out inefficient code manually.

What exactly should be enabled is up for debate. The current config uses the
golangci-lint defaults plus everything that is enabled explicitly by the normal
golangci.yaml, just to be on the safe side.
This commit is contained in:
Patrick Ohly 2022-04-29 20:39:45 +02:00
parent 0e9ad242bd
commit 78a3430606
2 changed files with 81 additions and 5 deletions

38
hack/golangci-strict.yaml Normal file
View File

@ -0,0 +1,38 @@
# This file configures checks that all new code for Kubernetes is meant to
# pass, in contrast to .golangci.yaml which defines checks that also the
# existing code passes.
run:
timeout: 30m
skip-files:
- "^zz_generated.*"
issues:
max-same-issues: 0
# Excluding configuration per-path, per-linter, per-text and per-source
exclude-rules:
# exclude ineffassing linter for generated files for conversion
- path: conversion\.go
linters:
- ineffassign
linters:
enable: # please keep this alphabetized
- gocritic
- govet
- ineffassign
- logcheck
- staticcheck
- unused
linters-settings: # please keep this alphabetized
custom:
logcheck:
# Installed there by hack/verify-golangci-lint.sh.
path: ../_output/local/bin/logcheck.so
description: structured logging checker
original-url: k8s.io/logtools/logcheck
staticcheck:
checks: [
"all",
]

View File

@ -14,19 +14,24 @@
# See the License for the specific language governing permissions and
# limitations under the License.
# This script checks coding style for go language files in each
# Kubernetes package by golint.
# This script checks the coding style for the Go language files using
# golangci-lint. Which checks are enabled depends on command line flags. The
# default is a minimal set of checks that all existing code passes without
# issues.
usage () {
cat <<EOF >&2
Usage: $0 [-- <golangci-lint run flags>] [packages]"
Usage: $0 [-r <revision>|-a] [-s] [-c none|<config>] [-- <golangci-lint run flags>] [packages]"
-r <revision>: only report issues in code added since that revision
-a: automatically select the common base of origin/master and HEAD
as revision
-s: select a strict configuration for new code
-c <config|"none">: use the specified configuration or none instead of the default hack/golangci.yaml
[packages]: check specific packages or directories instead of everything
EOF
exit 1
}
set -o errexit
set -o nounset
set -o pipefail
@ -50,8 +55,26 @@ invocation=(./hack/verify-golangci-lint.sh "$@")
# _output/local/bin/golangci-lint cache clean
golangci=(env LOGCHECK_CONFIG="${KUBE_ROOT}/hack/logcheck.conf" "${GOBIN}/golangci-lint" run)
golangci_config="${KUBE_ROOT}/hack/golangci.yaml"
while getopts "c:" o; do
base=
strict=
githubactions=
while getopts "ar:sc:" o; do
case "${o}" in
a)
base="$(git merge-base origin/master HEAD)"
;;
r)
base="${OPTARG}"
if [ ! "$base" ]; then
echo "ERROR: -c needs a non-empty parameter" >&2
echo >&2
usage
fi
;;
s)
golangci_config="${KUBE_ROOT}/hack/golangci-strict.yaml"
strict=1
;;
c)
if [ "${OPTARG}" = "none" ]; then
golangci_config=""
@ -69,6 +92,16 @@ if [ "${golangci_config}" ]; then
golangci+=(--config="${golangci_config}")
fi
if [ "$base" ]; then
# Must be a something that git can resolve to a commit.
# "git rev-parse --verify" checks that and prints a detailed
# error.
base="$(git rev-parse --verify "$base")"
golangci+=(--new --new-from-rev="$base")
fi
kube::golang::verify_go_version
# Filter out arguments that start with "-" and move them to the run flags.
shift $((OPTIND-1))
targets=()
@ -122,6 +155,11 @@ else
echo "Please review the above warnings. You can test via \"${invocation[*]}\""
echo 'If the above warnings do not make sense, you can exempt this warning with a comment'
echo ' (if your reviewer is okay with it).'
if [ "$strict" ]; then
echo 'The more strict golangci-strict.yaml was used. If you feel that this warns about issues'
echo 'that should be ignored by default, then please discuss with your reviewer and propose'
echo 'a change for hack/golangci-strict.yaml as part of your PR.'
fi
echo 'In general please prefer to fix the error, we have already disabled specific lints'
echo ' that the project chooses to ignore.'
echo 'See: https://golangci-lint.run/usage/false-positives/'