Merge pull request #34 from henrybear327/feat/add_golangci-lint

Add make verify target for running static analysis checks

.github/workflows/static-analysis.yaml

@@ -14,3 +14,22 @@ jobs:
         uses: golangci/golangci-lint-action@v4
         with:
           version: v1.57.1
+          args: --config tools/.golangci.yaml
+      - run: |
+          set -euo pipefail
+
+          make verify
+      - run: |
+          set -euo pipefail
+
+          make fmt
+
+          DIFF=$(git status --porcelain)
+
+          if [ -n "$DIFF" ]; then
+            echo "These files were modified:"
+            echo
+            echo "$DIFF"
+            echo
+            exit 1
+          fi

Makefile

@@ -19,6 +19,19 @@ GO_VERSION ?= 1.21.8
 GOOS ?= linux
 GOARCH ?= amd64
 TEMP_DIR := $(shell mktemp -d)
+GOFILES = $(shell find . -name \*.go)
+
+.PHONY: fmt
+fmt:
+	@echo "Verifying gofmt, failures can be fixed with ./scripts/fix.sh"
+	@!(gofmt -l -s -d ${GOFILES} | grep '[a-z]')
+
+	@echo "Verifying goimports, failures can be fixed with ./scripts/fix.sh"
+	@!(go run golang.org/x/tools/cmd/goimports@latest -l -d ${GOFILES} | grep '[a-z]')
+
+.PHONY: verify
+verify:
+	golangci-lint run --config tools/.golangci.yaml ./...
 
 # Local development build
 build:

cmd/analyze.go

@@ -36,7 +36,7 @@ type analyzeOptions struct {
 	filename string
 }
 
-var analyzeOpts *analyzeOptions = &analyzeOptions{}
+var analyzeOpts = &analyzeOptions{}
 
 func init() {
 	RootCmd.AddCommand(analyzeCmd)
@@ -52,7 +52,7 @@ func analyzeValidateAndRun() error {
 	objectCounts := map[string]uint{}
 	for _, s := range summaries {
 		if s.TypeMeta != nil {
-			objectCounts[fmt.Sprintf("%s/%s", s.TypeMeta.APIVersion, s.TypeMeta.Kind)] += 1
+			objectCounts[fmt.Sprintf("%s/%s", s.TypeMeta.APIVersion, s.TypeMeta.Kind)]++
 		}
 	}
 

cmd/checksum.go

@@ -36,7 +36,7 @@ type checksumOptions struct {
 	revision int64
 }
 
-var checksumOpts *checksumOptions = &checksumOptions{}
+var checksumOpts = &checksumOptions{}
 
 func init() {
 	RootCmd.AddCommand(checksumCmd)

cmd/decode.go

@@ -73,7 +73,7 @@ type decodeOptions struct {
 	batchProcess  bool // special flag to handle incoming etcd-dump-logs output
 }
 
-var options *decodeOptions = &decodeOptions{}
+var options = &decodeOptions{}
 
 func init() {
 	RootCmd.AddCommand(decodeCmd)
@@ -124,7 +124,7 @@ func runInBatchMode(metaOnly bool, outMediaType string, out io.Writer) (err erro
 			return nil
 		}
 		if err != nil {
-			return fmt.Errorf("error reading --batch-process input: %v\n", err)
+			return fmt.Errorf("error reading --batch-process input: %v", err)
 		}
 
 		input = stripNewline(input)
@@ -210,7 +210,6 @@ func readInput(inputFilename string) ([]byte, error) {
 func stripNewline(d []byte) []byte {
 	if len(d) > 0 && d[len(d)-1] == '\n' {
 		return d[:len(d)-1]
-	} else {
-		return d
 	}
+	return d
 }

cmd/encode.go

@@ -51,7 +51,7 @@ type encodeOptions struct {
 	inputFilename string
 }
 
-var encodeOpts *encodeOptions = &encodeOptions{}
+var encodeOpts = &encodeOptions{}
 
 func init() {
 	RootCmd.AddCommand(encodeCmd)

cmd/extract.go

@@ -90,7 +90,7 @@ type extractOptions struct {
 	filter       string
 }
 
-var opts *extractOptions = &extractOptions{}
+var opts = &extractOptions{}
 
 func init() {
 	RootCmd.AddCommand(extractCmd)
@@ -250,7 +250,7 @@ func printLeafItemValue(kv *mvccpb.KeyValue, outMediaType string, out io.Writer)
 // printKeySummaries prints all keys in the db file with the given key prefix.
 func printKeySummaries(filename string, keyPrefix string, revision int64, fields []string, out io.Writer) error {
 	if len(fields) == 0 {
-		return fmt.Errorf("no fields provided, nothing to output.")
+		return fmt.Errorf("no fields provided, nothing to output")
 	}
 
 	var hasKey bool
@@ -288,7 +288,7 @@ func printTemplateSummaries(filename string, keyPrefix string, revision int64, t
 	}
 
 	if len(templatestr) == 0 {
-		return fmt.Errorf("no template provided, nothing to output.")
+		return fmt.Errorf("no template provided, nothing to output")
 	}
 
 	filters := []data.Filter{}

pkg/data/data.go

@@ -37,7 +37,7 @@ var (
 	keyBucket  = []byte("key")
 	metaBucket = []byte("meta")
 
-	finishedCompactKeyName  = []byte("finishedCompactRev")
+	finishedCompactKeyName = []byte("finishedCompactRev")
 )
 
 // KeySummary represents a kubernetes object stored in etcd.
@@ -272,7 +272,7 @@ func ListKeySummaries(filename string, filters []Filter, proj *KeySummaryProject
 					ks.Version = kv.ModRevision
 					ks.Stats.ValueSize = len(kv.Value)
 				}
-				ks.Stats.VersionCount += 1
+				ks.Stats.VersionCount++
 				ks.Stats.AllVersionsKeySize += len(kv.Key)
 				ks.Stats.AllVersionsValueSize += len(kv.Value)
 			}

pkg/data/data_test.go

@@ -114,22 +114,22 @@ func TestParseFilters(t *testing.T) {
 		{
 			name:      "namespace-equals",
 			rawFilter: ".Value.metadata.namespace=default",
-			expected:  []*FieldConstraint{&FieldConstraint{lhs: ".Value.metadata.namespace", op: Equals, rhs: "default"}},
+			expected:  []*FieldConstraint{{lhs: ".Value.metadata.namespace", op: Equals, rhs: "default"}},
 		},
 		{
 			name:      "2-filters",
 			rawFilter: ".Value.metadata.namespace=default,.Value.metadata.name=example",
 			expected: []*FieldConstraint{
-				&FieldConstraint{lhs: ".Value.metadata.namespace", op: Equals, rhs: "default"},
-				&FieldConstraint{lhs: ".Value.metadata.name", op: Equals, rhs: "example"},
+				{lhs: ".Value.metadata.namespace", op: Equals, rhs: "default"},
+				{lhs: ".Value.metadata.name", op: Equals, rhs: "example"},
 			},
 		},
 		{
 			name:      "whitespace",
 			rawFilter: " .Value.metadata.namespace=default\t, .Value.metadata.name=example\n",
 			expected: []*FieldConstraint{
-				&FieldConstraint{lhs: ".Value.metadata.namespace", op: Equals, rhs: "default"},
-				&FieldConstraint{lhs: ".Value.metadata.name", op: Equals, rhs: "example"},
+				{lhs: ".Value.metadata.namespace", op: Equals, rhs: "default"},
+				{lhs: ".Value.metadata.name", op: Equals, rhs: "example"},
 			},
 		},
 	}

pkg/encoding/scheme.go

@@ -87,17 +87,18 @@ func init() {
 // AddToScheme adds all types of this clientset into the given scheme. This allows composition
 // of clientsets, like in:
 //
-//   import (
-//     "k8s.io/client-go/kubernetes"
-//     clientsetscheme "k8s.io/client-go/kuberentes/scheme"
-//     aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme"
-//   )
+//	import (
+//	  "k8s.io/client-go/kubernetes"
+//	  clientsetscheme "k8s.io/client-go/kuberentes/scheme"
+//	  aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme"
+//	)
 //
-//   kclientset, _ := kubernetes.NewForConfig(c)
-//   aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme)
+//	kclientset, _ := kubernetes.NewForConfig(c)
+//	aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme)
 //
 // After this, RawExtensions in Kubernetes types will serialize kube-aggregator types
 // correctly.
+//
 //nolint:errcheck
 func AddToScheme(scheme *runtime.Scheme) {
 	admissionv1beta1.AddToScheme(scheme)

scripts/fix.sh

@@ -0,0 +1,10 @@
+GO_CMD="go"
+
+GOFILES=$(${GO_CMD} list  --f "{{with \$d:=.}}{{range .GoFiles}}{{\$d.Dir}}/{{.}}{{\"\n\"}}{{end}}{{end}}" ./...)
+TESTGOFILES=$(${GO_CMD} list  --f "{{with \$d:=.}}{{range .TestGoFiles}}{{\$d.Dir}}/{{.}}{{\"\n\"}}{{end}}{{end}}" ./...)
+XTESTGOFILES=$(${GO_CMD} list  --f "{{with \$d:=.}}{{range .XTestGoFiles}}{{\$d.Dir}}/{{.}}{{\"\n\"}}{{end}}{{end}}" ./...)
+
+echo "${GOFILES}" "${TESTGOFILES}" "${XTESTGOFILES}"| xargs -n 100 go run golang.org/x/tools/cmd/goimports@latest -w -local go.etcd.io
+
+go fmt ./...
+go mod tidy

tools/.golangci.yaml

@@ -0,0 +1,103 @@
+---
+run:
+  timeout: 30m
+  issues.exclude-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:
+  disable-all: true
+  enable: # please keep this alphabetized
+    # Don't use soon to deprecated[1] linters that lead to false
+    # https://github.com/golangci/golangci-lint/issues/1841
+    # - deadcode
+    # - structcheck
+    # - varcheck
+    - goimports
+    - ineffassign
+    - nakedret
+    - revive
+    - staticcheck
+    - stylecheck
+    - unconvert # Remove unnecessary type conversions
+    - unparam
+    - unused
+linters-settings: # please keep this alphabetized
+  goimports:
+    local-prefixes: go.etcd.io # Put imports beginning with prefix after 3rd-party packages.
+  nakedret:
+    # Align with https://github.com/alexkohler/nakedret/blob/v1.0.2/cmd/nakedret/main.go#L10
+    max-func-lines: 5
+  revive:
+    ignore-generated-header: false
+    severity: error
+    confidence: 0.8
+    enable-all-rules: false
+    rules:
+      - name: blank-imports
+        severity: error
+        disabled: false
+      - name: context-as-argument
+        severity: error
+        disabled: false
+      - name: dot-imports
+        severity: error
+        disabled: false
+      - name: error-return
+        severity: error
+        disabled: false
+      - name: error-naming
+        severity: error
+        disabled: false
+      - name: if-return
+        severity: error
+        disabled: false
+      - name: increment-decrement
+        severity: error
+        disabled: false
+      - name: var-declaration
+        severity: error
+        disabled: false
+      - name: package-comments
+        severity: error
+        disabled: false
+      - name: range
+        severity: error
+        disabled: false
+      - name: receiver-naming
+        severity: error
+        disabled: false
+      - name: time-naming
+        severity: error
+        disabled: false
+      - name: indent-error-flow
+        severity: error
+        disabled: false
+      - name: errorf
+        severity: error
+        disabled: false
+      - name: context-keys-type
+        severity: error
+        disabled: false
+      - name: error-strings
+        severity: error
+        disabled: false
+      # TODO: enable the following rules
+      - name: var-naming
+        disabled: true
+      - name: exported
+        disabled: true
+      - name: unexported-return
+        disabled: true
+  staticcheck:
+    checks:
+      - all
+      - -SA1019 # TODO(fix) Using a deprecated function, variable, constant or field
+      - -SA2002 # TODO(fix) Called testing.T.FailNow or SkipNow in a goroutine, which isn’t allowed
+  stylecheck:
+    checks:
+      - ST1019 # Importing the same package multiple times.