update golangci-lint config, fix new lints

5 files changed, 161 insertions, 126 deletions

diff --git a/.golangci.yaml b/.golangci.yaml
index ce734c8..dd2ff32 100644
--- a/.golangci.yaml
+++ b/.golangci.yaml
@@ -1,159 +1,190 @@
-################################################################################
-# This file is AUTOGENERATED with <https://github.com/sapcc/go-makefile-maker> #
-# Edit Makefile.maker.yaml instead.                                            #
-################################################################################
+# SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company
+# SPDX-License-Identifier: Apache-2.0
 
+version: "2"
 run:
-  timeout: 3m # 1m by default
-  modules-download-mode: readonly
+  timeout: 3m0s # none by default in v2
 
-output:
-  # Do not print lines of code with issue.
-  print-issued-lines: false
+formatters:
+  enable:
+    - gofmt
+    - goimports
+  settings:
+    goimports:
+      # Put local imports after 3rd-party packages
+      local-prefixes:
+        - go.xyrillian.de/schwift/v2
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
 
 issues:
-  exclude:
-    # It is idiomatic Go to reuse the name 'err' with ':=' for subsequent errors.
-    # Ref: https://go.dev/doc/effective_go#redeclaration
-    - 'declaration of "err" shadows declaration at'
-  exclude-rules:
-    - path: _test\.go
-      linters:
-        - bodyclose
-        - dupl
-  # '0' disables the following options.
+  # '0' disables the following options
   max-issues-per-linter: 0
   max-same-issues: 0
 
-linters-settings:
-  dupl:
-    # Tokens count to trigger issue, 150 by default.
-    threshold: 100
-  errcheck:
-    # Report about assignment of errors to blank identifier.
-    check-blank: true
-    # Report about not checking of errors in type assertions.
-    check-type-assertions: true
-  forbidigo:
-    analyze-types: true # required for pkg:
-    forbid:
-      # ioutil package has been deprecated: https://github.com/golang/go/issues/42026
-      - ^ioutil\..*$
-      # Using http.DefaultServeMux is discouraged because it's a global variable that some packages silently and magically add handlers to (esp. net/http/pprof).
-      # Applications wishing to use http.ServeMux should obtain local instances through http.NewServeMux() instead of using the global default instance.
-      - ^http\.DefaultServeMux$
-      - ^http\.Handle(?:Func)?$
-      # Forbid usage of old and archived square/go-jose
-      - pkg: ^gopkg\.in/square/go-jose\.v2$
-        msg: "gopk.in/square/go-jose is arcived and has CVEs. Replace it with gopkg.in/go-jose/go-jose.v2"
-      - pkg: ^github.com/coreos/go-oidc$
-        msg: "github.com/coreos/go-oidc depends on gopkg.in/square/go-jose which has CVEs. Replace it with github.com/coreos/go-oidc/v3"
-
-      - pkg: ^github.com/howeyc/gopass$
-        msg: "github.com/howeyc/gopass is archived, use golang.org/x/term instead"
-  goconst:
-    ignore-tests: true
-    min-occurrences: 5
-  gocritic:
-    enabled-checks:
-      - boolExprSimplify
-      - builtinShadow
-      - emptyStringTest
-      - evalOrder
-      - httpNoBody
-      - importShadow
-      - initClause
-      - methodExprCall
-      - paramTypeCombine
-      - preferFilepathJoin
-      - ptrToRefParam
-      - redundantSprint
-      - returnAfterHttpError
-      - stringConcatSimplify
-      - timeExprSimplify
-      - truncateCmp
-      - typeAssertChain
-      - typeUnparen
-      - unnamedResult
-      - unnecessaryBlock
-      - unnecessaryDefer
-      - weakCond
-      - yodaStyleExpr
-  goimports:
-    # Put local imports after 3rd-party packages.
-    local-prefixes: github.com/majewsky/schwift
-  gosec:
-    excludes:
-      # gosec wants us to set a short ReadHeaderTimeout to avoid Slowloris attacks, but doing so would expose us to Keep-Alive race conditions (see https://iximiuz.com/en/posts/reverse-proxy-http-keep-alive-and-502s/)
-      - G112
-      # created file permissions are restricted by umask if necessary
-      - G306
-  govet:
-    enable-all: true
-    disable:
-      - fieldalignment
-  nolintlint:
-    require-specific: true
-  stylecheck:
-    dot-import-whitelist:
-      - github.com/onsi/ginkgo/v2
-      - github.com/onsi/gomega
-  usestdlibvars:
-    constant-kind: true
-    crypto-hash: true
-    default-rpc-path: true
-    http-method: true
-    http-status-code: true
-    sql-isolation-level: true
-    time-layout: true
-    time-month: true
-    time-weekday: true
-    tls-signature-scheme: true
-  whitespace:
-    # Enforce newlines (or comments) after multi-line function signatures.
-    multi-func: true
-
 linters:
-  # We use 'disable-all' and enable linters explicitly so that a newer version
-  # does not introduce new linters unexpectedly.
-  disable-all: true
+  # Disable all pre-enabled linters and enable them explicitly so that a newer version does not introduce new linters unexpectedly
+  default: none
   enable:
     - bodyclose
     - containedctx
     - copyloopvar
-    - dupl
     - dupword
     - durationcheck
     - errcheck
     - errname
     - errorlint
+    - exptostd
     - forbidigo
     - ginkgolinter
     - gocheckcompilerdirectives
     - goconst
     - gocritic
-    - gofmt
-    - goimports
-    - gosec
-    - gosimple
+    - gomoddirectives
+    # - gosec # TODO: consider enabling
     - govet
     - ineffassign
     - intrange
-    - misspell
+    - iotamixing
+    - modernize
     - nilerr
-    - noctx
     - nolintlint
     - nosprintfhostport
     - perfsprint
     - predeclared
     - rowserrcheck
+    - revive
     - sqlclosecheck
     - staticcheck
-    - stylecheck
-    - tenv
-    - typecheck
     - unconvert
     - unparam
     - unused
     - usestdlibvars
+    - usetesting
     - whitespace
+  settings:
+    dupword:
+      # Do not choke on SQL statements like `INSERT INTO things (foo, bar, baz) VALUES (TRUE, TRUE, TRUE)`.
+      ignore: [ "TRUE", "FALSE", "NULL" ]
+    errcheck:
+      check-type-assertions: false
+      # Report about assignment of errors to blank identifier.
+      check-blank: true
+      # Do not report about not checking of errors in type assertions.
+      # This is not as dangerous as skipping error values because an unchecked type assertion just immediately panics.
+      # We disable this because it makes a ton of useless noise esp. in test code.
+    forbidigo:
+      analyze-types: true # required for pkg:
+      forbid:
+        # ioutil package has been deprecated: https://github.com/golang/go/issues/42026
+        - pattern: ^ioutil\..*$
+        # Using http.DefaultServeMux is discouraged because it's a global variable that some packages silently and magically add handlers to (esp. net/http/pprof).
+        # Applications wishing to use http.ServeMux should obtain local instances through http.NewServeMux() instead of using the global default instance.
+        - pattern: ^http\.DefaultServeMux$
+        - pattern: ^http\.Handle(?:Func)?$
+        - pkg: ^gopkg\.in/square/go-jose\.v2$
+          msg: gopk.in/square/go-jose is archived and has CVEs. Replace it with gopkg.in/go-jose/go-jose.v2
+        - pkg: ^github.com/coreos/go-oidc$
+          msg: github.com/coreos/go-oidc depends on gopkg.in/square/go-jose which has CVEs. Replace it with github.com/coreos/go-oidc/v3
+        - pkg: ^github.com/howeyc/gopass$
+          msg: github.com/howeyc/gopass is archived, use golang.org/x/term instead
+    goconst:
+      min-occurrences: 5
+    gocritic:
+      enabled-checks:
+        - boolExprSimplify
+        - builtinShadow
+        - emptyStringTest
+        - evalOrder
+        - httpNoBody
+        - importShadow
+        - initClause
+        - methodExprCall
+        - paramTypeCombine
+        - preferFilepathJoin
+        - ptrToRefParam
+        - redundantSprint
+        - returnAfterHttpError
+        - stringConcatSimplify
+        - timeExprSimplify
+        - truncateCmp
+        - typeAssertChain
+        - typeUnparen
+        - unnamedResult
+        - unnecessaryBlock
+        - unnecessaryDefer
+        - weakCond
+        - yodaStyleExpr
+    gomoddirectives:
+      toolchain-forbidden: true
+      go-version-pattern: 1\.\d+(\.0)?$
+    gosec:
+      excludes:
+        # useless noise in tests
+        - G602
+    govet:
+      disable:
+        - fieldalignment
+      enable-all: true
+    nolintlint:
+      require-specific: true
+    perfsprint:
+      # modernize generates nicer fix code
+      concat-loop: false
+    revive:
+      rules:
+        - name: exported
+          arguments:
+            - checkPrivateReceivers
+            - disableChecksOnConstants
+    staticcheck:
+      dot-import-whitelist:
+        - go.xyrillian.de/gg/option
+    usestdlibvars:
+      http-method: true
+      http-status-code: true
+      time-weekday: true
+      time-month: true
+      time-layout: true
+      crypto-hash: true
+      default-rpc-path: true
+      sql-isolation-level: true
+      tls-signature-scheme: true
+      constant-kind: true
+    usetesting:
+      os-temp-dir: true
+    whitespace:
+      # Enforce newlines (or comments) after multi-line function signatures.
+      multi-func: true
+  exclusions:
+    generated: lax
+    presets:
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    rules:
+      - linters:
+          - bodyclose
+          - revive
+        path: _test\.go
+      # It is idiomatic Go to reuse the name 'err' with ':=' for subsequent errors.
+      # Ref: https://go.dev/doc/effective_go#redeclaration
+      - path: (.+)\.go$
+        text: declaration of "err" shadows declaration at
+      - linters:
+          - goconst
+        path: (.+)_test\.go
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+
+output:
+  formats:
+    text:
+      # Do not print lines of code with issue.
+      print-issued-lines: false
diff --git a/gopherschwift/package.go b/gopherschwift/package.go
index d6b4e7b..834d0b7 100644
--- a/gopherschwift/package.go
+++ b/gopherschwift/package.go
@@ -74,10 +74,12 @@ type backend struct {
 	userAgent string
 }
 
+// EndpointURL implements the [schwift.Backend] interface.
 func (g *backend) EndpointURL() string {
 	return g.c.Endpoint
 }
 
+// Clone implements the [schwift.Backend] interface.
 func (g *backend) Clone(newEndpointURL string) schwift.Backend {
 	clonedClient := *g.c
 	clonedClient.Endpoint = newEndpointURL
@@ -87,6 +89,7 @@ func (g *backend) Clone(newEndpointURL string) schwift.Backend {
 	}
 }
 
+// Do implements the [schwift.Backend] interface.
 func (g *backend) Do(req *http.Request) (*http.Response, error) {
 	return g.do(req, false)
 }
diff --git a/headers.go b/headers.go
index a4fda78..90574db 100644
--- a/headers.go
+++ b/headers.go
@@ -106,13 +106,13 @@ func headersFromHTTP(src http.Header) Headers {
 // IsDynamicLargeObject returns true if this set of headers belongs to a Dynamic
 // Large Object (DLO).
 func (h ObjectHeaders) IsDynamicLargeObject() bool {
-	return h.Headers.Get("X-Object-Manifest") != ""
+	return h.Get("X-Object-Manifest") != ""
 }
 
 // IsStaticLargeObject returns true if this set of headers belongs to a Static
 // Large Object (SLO).
 func (h ObjectHeaders) IsStaticLargeObject() bool {
-	return h.Headers.Get("X-Static-Large-Object") == "True"
+	return h.Get("X-Static-Large-Object") == "True"
 }
 
 // IsLargeObject returns true if this set of headers belongs to a large object
diff --git a/largeobject.go b/largeobject.go
index 2feac26..648ef46 100644
--- a/largeobject.go
+++ b/largeobject.go
@@ -712,6 +712,7 @@ type segmentingReader struct {
 	SegmentSizeBytes int64 // must be >0 // TODO: in Schwift 3, change field type to uint64 and clamp values to math.MaxInt64 internally
 }
 
+// NextSegment returns an io.Reader covering just the next segment.
 func (sr *segmentingReader) NextSegment() io.Reader {
 	// peek if there is more content in the backing reader
 	buf := make([]byte, 1)
diff --git a/tests/shared_test.go b/tests/shared_test.go
index 7cae4a5..4bef612 100644
--- a/tests/shared_test.go
+++ b/tests/shared_test.go
@@ -46,25 +46,25 @@ func testWithAccount(t *testing.T, testCode func(a *schwift.Account)) {
 		// option 1: Keystone authentication
 		provider, err := clientconfig.AuthenticatedClient(context.TODO(), nil)
 		if err != nil {
-			t.Errorf("clientconfig.AuthenticatedClient returned: " + err.Error())
+			t.Error("clientconfig.AuthenticatedClient returned: " + err.Error())
 			t.Error("probably missing Swift credentials (need either ST_AUTH, ST_USER, ST_KEY or OS_* variables)")
 			return
 		}
 		client, err = openstack.NewObjectStorageV1(provider, gophercloud.EndpointOpts{})
 		if err != nil {
-			t.Errorf("openstack.NewObjectStorageV1 returned: " + err.Error())
+			t.Error("openstack.NewObjectStorageV1 returned: " + err.Error())
 			return
 		}
 	} else {
 		// option 2: Swift authentication v1
 		provider, err := openstack.NewClient(stAuth)
 		if err != nil {
-			t.Errorf("openstack.NewClient returned: " + err.Error())
+			t.Error("openstack.NewClient returned: " + err.Error())
 			return
 		}
 		client, err = swauth.NewObjectStorageV1(context.TODO(), provider, swauth.AuthOpts{User: stUser, Key: stKey})
 		if err != nil {
-			t.Errorf("swauth.NewObjectStorageV1 returned: " + err.Error())
+			t.Error("swauth.NewObjectStorageV1 returned: " + err.Error())
 			return
 		}
 	}