Run staticcheck explicitly in make lint

It is currently part of golangci-lint, but seems to have configuration issues. If we run is directly, we can control it better, including using the recommended `//lint:ignore` directives. This also fixes or ignores staticcheck issues in the code
cloudfoundry · Jul 6, 2023 · 46b1ad2 · 46b1ad2
1 parent d0f67a2
commit 46b1ad2
Show file tree

Hide file tree

Showing 14 changed files with 40 additions and 28 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -26,6 +26,9 @@ linters:
     - dupl
     - ginkgolinter
 
+  disable:
+    - staticcheck # we run this separately
+
 issues:
   exclude-rules:
     - path: _test\.go

diff --git a/Makefile b/Makefile
@@ -42,18 +42,20 @@ fmt: install-gofumpt install-shfmt
 vet: ## Run go vet against code.
 	go vet ./...
 
-lint: fmt vet gosec
+lint: fmt vet gosec staticcheck
 	golangci-lint run -v
 
 gosec: install-gosec
 	$(GOSEC) --exclude=G101,G304,G401,G404,G505 --exclude-dir=tests ./...
 
+staticcheck: install-staticcheck
+	$(STATICCHECK) ./...
+
 test: lint
 	@for comp in $(COMPONENTS); do make -C $$comp test; done
 	make test-tools
 	make test-e2e
 
-
 test-tools:
 	./scripts/run-tests.sh tools
 
@@ -79,5 +81,9 @@ GOSEC = $(shell go env GOPATH)/bin/gosec
 install-gosec:
 	go install github.com/securego/gosec/v2/cmd/gosec@latest
 
+STATICCHECK = $(shell go env GOPATH)/bin/staticcheck
+install-staticcheck:
+	go install honnef.co/go/tools/cmd/staticcheck@latest
+
 vendir-update-dependencies: install-vendir
 	$(VENDIR) sync --chdir tests
diff --git a/api/actions/manifest/normalizer.go b/api/actions/manifest/normalizer.go
@@ -43,19 +43,21 @@ func procValIfSet[T any](appVal, procVal *T) *T {
 
 func fixDeprecatedFields(appInfo *payloads.ManifestApplication) {
 	if appInfo.DiskQuota == nil {
-		//nolint:staticcheck
+		//lint:ignore SA1019 we have to deal with this deprecation
 		appInfo.DiskQuota = appInfo.AltDiskQuota
 	}
 
 	for i := range appInfo.Processes {
 		if appInfo.Processes[i].DiskQuota == nil {
-			//nolint:staticcheck
+			//lint:ignore SA1019 we have to deal with this deprecation
 			appInfo.Processes[i].DiskQuota = appInfo.Processes[i].AltDiskQuota
 		}
 	}
 
-	if hasBuildpackSet(appInfo.Buildpack) { // nolint: staticcheck
-		appInfo.Buildpacks = append(appInfo.Buildpacks, appInfo.Buildpack) // nolint: staticcheck
+	//lint:ignore SA1019 we have to deal with this deprecation
+	if hasBuildpackSet(appInfo.Buildpack) {
+		//lint:ignore SA1019 we have to deal with this deprecation
+		appInfo.Buildpacks = append(appInfo.Buildpacks, appInfo.Buildpack)
 	}
 }
 

diff --git a/api/actions/manifest/normalizer_test.go b/api/actions/manifest/normalizer_test.go
@@ -80,7 +80,8 @@ var _ = Describe("Normalizer", func() {
 
 		When("deprecated 'buildpack' is specified", func() {
 			BeforeEach(func() {
-				appInfo.Buildpack = "deprecated-buildpack" // nolint: staticcheck
+				//lint:ignore SA1019 we have to deal with this deprecation
+				appInfo.Buildpack = "deprecated-buildpack"
 			})
 
 			It("adds it to the buildpacks list", func() {
@@ -91,7 +92,8 @@ var _ = Describe("Normalizer", func() {
 
 			When("set to 'default'", func() {
 				BeforeEach(func() {
-					appInfo.Buildpack = "default" // nolint: staticcheck
+					//lint:ignore SA1019 we have to deal with this deprecation
+					appInfo.Buildpack = "default"
 				})
 
 				It("ignores it", func() {
@@ -103,7 +105,8 @@ var _ = Describe("Normalizer", func() {
 
 			When("set to 'null'", func() {
 				BeforeEach(func() {
-					appInfo.Buildpack = "null" // nolint: staticcheck
+					//lint:ignore SA1019 we have to deal with this deprecation
+					appInfo.Buildpack = "null"
 				})
 
 				It("ignores it", func() {
@@ -370,7 +373,7 @@ var _ = Describe("Normalizer", func() {
 
 		When("disk-quota is set on app", func() {
 			BeforeEach(func() {
-				//nolint:staticcheck
+				//lint:ignore SA1019 we have to deal with this deprecation
 				appInfo.AltDiskQuota = tools.PtrTo("123M")
 			})
 

diff --git a/api/authorization/testhelpers/client_cert.go b/api/authorization/testhelpers/client_cert.go
@@ -1,7 +1,7 @@
 package testhelpers
 
 import (
-	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega" //lint:ignore ST1001 this is a test file
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 )
 

diff --git a/api/config/config.go b/api/config/config.go
@@ -108,7 +108,7 @@ func (c *APIConfig) validate() error {
 
 	if c.UserCertificateExpirationWarningDuration != "" {
 		if _, err := time.ParseDuration(c.UserCertificateExpirationWarningDuration); err != nil {
-			return errors.New(`Invalid duration format for userCertificateExpirationWarningDuration. Use a format like "48h"`)
+			return errors.New(`invalid duration format for userCertificateExpirationWarningDuration. Use a format like "48h"`)
 		}
 	}
 

diff --git a/api/config/config_test.go b/api/config/config_test.go
@@ -173,7 +173,7 @@ var _ = Describe("Config", func() {
 		})
 
 		It("returns an error", func() {
-			Expect(loadErr).To(MatchError(ContainSubstring("Invalid duration format")))
+			Expect(loadErr).To(MatchError(ContainSubstring("invalid duration format")))
 		})
 	})
 

diff --git a/api/errors/errors.go b/api/errors/errors.go
@@ -338,12 +338,12 @@ type RollingDeployNotSupportedError struct {
 	apiError
 }
 
-func NewRollingDeployNotSupportedError(cause error) RollingDeployNotSupportedError {
+func NewRollingDeployNotSupportedError(runnerName string) RollingDeployNotSupportedError {
+	detail := fmt.Sprintf("The configured runner '%s' does not support rolling deploys", runnerName)
 	return RollingDeployNotSupportedError{
 		apiError: apiError{
-			cause:      cause,
 			title:      "CF-RollingDeployNotSupported",
-			detail:     cause.Error(),
+			detail:     detail,
 			code:       42000,
 			httpStatus: http.StatusBadRequest,
 		},

diff --git a/api/handlers/deployment.go b/api/handlers/deployment.go
@@ -3,7 +3,6 @@ package handlers
 import (
 	"context"
 	"errors"
-	"fmt"
 	"net/http"
 	"net/url"
 
@@ -74,15 +73,13 @@ func (h *Deployment) create(r *http.Request) (*routing.Response, error) {
 		var notFoundErr apierrors.NotFoundError
 		if errors.As(err, &notFoundErr) {
 			logger.Info("Could not find RunnerInfo", "runner", h.runnerName, "error", err)
-			ret := fmt.Errorf("The configured runner '%s' does not support rolling deploys", h.runnerName)
-			return nil, apierrors.NewRollingDeployNotSupportedError(ret)
+			return nil, apierrors.NewRollingDeployNotSupportedError(h.runnerName)
 		}
 		return nil, apierrors.LogAndReturn(logger, apierrors.ForbiddenAsNotFound(err), "Error getting runner info in repository")
 	}
 
 	if !runnerInfo.Capabilities.RollingDeploy {
-		err = fmt.Errorf("The configured runner '%s' does not support rolling deploys", h.runnerName)
-		return nil, apierrors.LogAndReturn(logger, apierrors.NewRollingDeployNotSupportedError(err), "runner does not support rolling deploys", "name", h.runnerName)
+		return nil, apierrors.LogAndReturn(logger, apierrors.NewRollingDeployNotSupportedError(h.runnerName), "runner does not support rolling deploys", "name", h.runnerName)
 	}
 
 	deploymentCreateMessage := payload.ToMessage()

diff --git a/scripts/helmdoc/main.go b/scripts/helmdoc/main.go
@@ -8,6 +8,8 @@ import (
 	"strings"
 
 	"golang.org/x/exp/maps"
+	"golang.org/x/text/cases"
+	"golang.org/x/text/language"
 )
 
 func printDocForSchema(schema map[string]any, indentLevel int) {
@@ -34,8 +36,7 @@ func printDocForSchema(schema map[string]any, indentLevel int) {
 		if typeStr == "object" {
 			typeStr = ""
 		} else {
-			// nolint:staticcheck
-			typeStr = " (_" + strings.Title(typeStr) + "_)"
+			typeStr = " (_" + cases.Title(language.AmericanEnglish).String(typeStr) + "_)"
 		}
 
 		fmt.Printf("%s- `%s`%s:%s\n", indentStr, name, typeStr, desc)

diff --git a/tests/helpers/cache_syncing_client.go b/tests/helpers/cache_syncing_client.go
@@ -3,8 +3,8 @@ package helpers
 import (
 	"context"
 
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
+	. "github.com/onsi/ginkgo/v2" //lint:ignore ST1001 this is a test file
+	. "github.com/onsi/gomega"    //lint:ignore ST1001 this is a test file
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )

diff --git a/tests/helpers/cert_auth_header.go b/tests/helpers/cert_auth_header.go
@@ -10,7 +10,7 @@ import (
 	"math/big"
 	"time"
 
-	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega" //lint:ignore ST1001 this is a test file
 )
 
 func CreateCertificatePEM() []byte {

diff --git a/tests/helpers/eventually_should_hold.go b/tests/helpers/eventually_should_hold.go
@@ -1,7 +1,7 @@
 package helpers
 
 import (
-	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega" //lint:ignore ST1001 this is a test file
 )
 
 func EventuallyShouldHold(condition func(g Gomega)) {

diff --git a/tests/matchers/wraps_error.go b/tests/matchers/wraps_error.go
@@ -4,7 +4,7 @@ import (
 	"errors"
 	"fmt"
 
-	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega" //lint:ignore ST1001 this is a test file
 	"github.com/onsi/gomega/format"
 	"github.com/onsi/gomega/types"
 )