Merge pull request #5405 from restic/dependabot/github_actions/golangci/golangci-lint-action-8

build(deps): bump golangci/golangci-lint-action from 6 to 8

.github/workflows/tests.yml

@@ -250,10 +250,10 @@ jobs:
           go-version: ${{ env.latest_go }}
 
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v6
+        uses: golangci/golangci-lint-action@v8
         with:
           # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
-          version: v1.64.8
+          version: v2.4.0
           args: --verbose --timeout 5m
 
         # only run golangci-lint for pull requests, otherwise ALL hints get

.golangci.yml

@@ -1,70 +1,79 @@
-# This is the configuration for golangci-lint for the restic project.
-#
-# A sample config with all settings is here:
-# https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml
-
+version: "2"
 linters:
   # only enable the linters listed below
-  disable-all: true
+  default: none
   enable:
+    - asciicheck
+    # ensure that http response bodies are closed
+    - bodyclose
+    - copyloopvar
     # make sure all errors returned by functions are handled
     - errcheck
-
-    # show how code can be simplified
-    - gosimple
-
-    # make sure code is formatted
-    - gofmt
-
     # examine code and report suspicious constructs, such as Printf calls whose
     # arguments do not align with the format string
     - govet
-
-    # make sure names and comments are used according to the conventions
-    - revive
-
+    # consistent imports
+    - importas
     # detect when assignments to existing variables are not used
     - ineffassign
-
+    - nolintlint
+    # make sure names and comments are used according to the conventions
+    - revive
     # run static analysis and find errors
     - staticcheck
-
     # find unused variables, functions, structs, types, etc.
     - unused
-
-    # parse and typecheck code
-    - typecheck
-
-    # ensure that http response bodies are closed
-    - bodyclose
-
-    - importas
-
-issues:
-  # don't use the default exclude rules, this hides (among others) ignored
-  # errors from Close() calls
-  exclude-use-default: false
-
-  # list of things to not warn about
-  exclude:
-    # revive: do not warn about missing comments for exported stuff
-    - exported (function|method|var|type|const) .* should have comment or be unexported
-    # revive: ignore constants in all caps
-    - don't use ALL_CAPS in Go names; use CamelCase
-    # revive: lots of packages don't have such a comment
-    - "package-comments: should have a package comment"
-    # staticcheck: there's no easy way to replace these packages
-    - "SA1019: \"golang.org/x/crypto/poly1305\" is deprecated"
-    - "SA1019: \"golang.org/x/crypto/openpgp\" is deprecated"
-    - "redefines-builtin-id:"
-
-  exclude-rules:
-    # revive: ignore unused parameters in tests
-    - path: (_test\.go|testing\.go|backend/.*/tests\.go)
-      text: "unused-parameter:"
-
-linters-settings:
-  importas:
-    alias:
-      - pkg: github.com/restic/restic/internal/test
-        alias: rtest
+  settings:
+    importas:
+      alias:
+        - pkg: github.com/restic/restic/internal/test
+          alias: rtest
+    staticcheck:
+      checks:
+        # default
+        - "all"
+        - "-ST1000"
+        - "-ST1003"
+        - "-ST1016"
+        - "-ST1020"
+        - "-ST1021"
+        - "-ST1022"
+        # extra disables
+        - "-QF1008" # don't warn about specifing name of embedded field on access
+  exclusions:
+    rules:
+      # revive: ignore unused parameters in tests
+      - path: (_test\.go|testing\.go|backend/.*/tests\.go)
+        text: "unused-parameter:"
+      # revive: do not warn about missing comments for exported stuff
+      - path: (.+)\.go$
+        text: exported (function|method|var|type|const) .* should have comment or be unexported
+      # revive: ignore constants in all caps
+      - path: (.+)\.go$
+        text: don't use ALL_CAPS in Go names; use CamelCase
+      # revive: lots of packages don't have such a comment
+      - path: (.+)\.go$
+        text: "package-comments: should have a package comment"
+      # staticcheck: there's no easy way to replace these packages
+      - path: (.+)\.go$
+        text: 'SA1019: "golang.org/x/crypto/poly1305" is deprecated'
+      - path: (.+)\.go$
+        text: 'SA1019: "golang.org/x/crypto/openpgp" is deprecated'
+      - path: (.+)\.go$
+        text: "redefines-builtin-id:"
+      # revive: collection of helpers to implement a backend, more descriptive names would be too repetitive
+      - path: internal/backend/util/.*.go$
+        text: "var-naming: avoid meaningless package names"
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+formatters:
+  enable:
+    # make sure code is formatted
+    - gofmt
+  exclusions:
+    paths:
+      - third_party$
+      - builtin$
+      - examples$

cmd/restic/cmd_backup_integration_test.go

@@ -461,7 +461,6 @@ func TestIncrementalBackup(t *testing.T) {
 	t.Logf("repository grown by %d bytes", stat3.size-stat2.size)
 }
 
-// nolint: staticcheck // false positive nil pointer dereference check
 func TestBackupTags(t *testing.T) {
 	env, cleanup := withTestEnvironment(t)
 	defer cleanup()
@@ -497,7 +496,6 @@ func TestBackupTags(t *testing.T) {
 		"expected parent to be %v, got %v", parent.ID, newest.Parent)
 }
 
-// nolint: staticcheck // false positive nil pointer dereference check
 func TestBackupProgramVersion(t *testing.T) {
 	env, cleanup := withTestEnvironment(t)
 	defer cleanup()

cmd/restic/cmd_find.go

@@ -168,7 +168,7 @@ func (s *statefulOutput) PrintPatternJSON(path string, node *restic.Node) {
 	}
 	if s.newsn != s.oldsn {
 		if s.oldsn != nil {
-			_, _ = s.stdout.Write([]byte(fmt.Sprintf("],\"hits\":%d,\"snapshot\":%q},", s.hits, s.oldsn.ID())))
+			_, _ = fmt.Fprintf(s.stdout, "],\"hits\":%d,\"snapshot\":%q},", s.hits, s.oldsn.ID())
 		}
 		_, _ = s.stdout.Write([]byte(`{"matches":[`))
 		s.oldsn = s.newsn
@@ -255,7 +255,7 @@ func (s *statefulOutput) Finish() {
 	if s.JSON {
 		// do some finishing up
 		if s.oldsn != nil {
-			_, _ = s.stdout.Write([]byte(fmt.Sprintf("],\"hits\":%d,\"snapshot\":%q}", s.hits, s.oldsn.ID())))
+			_, _ = fmt.Fprintf(s.stdout, "],\"hits\":%d,\"snapshot\":%q}", s.hits, s.oldsn.ID())
 		}
 		if s.inuse {
 			_, _ = s.stdout.Write([]byte("]\n"))

cmd/restic/cmd_init.go

@@ -65,17 +65,19 @@ func runInit(ctx context.Context, opts InitOptions, gopts GlobalOptions, args []
 	printer := newTerminalProgressPrinter(gopts.JSON, gopts.verbosity, term)
 
 	var version uint
-	if opts.RepositoryVersion == "latest" || opts.RepositoryVersion == "" {
+	switch opts.RepositoryVersion {
+	case "latest", "":
 		version = restic.MaxRepoVersion
-	} else if opts.RepositoryVersion == "stable" {
+	case "stable":
 		version = restic.StableRepoVersion
-	} else {
+	default:
 		v, err := strconv.ParseUint(opts.RepositoryVersion, 10, 32)
 		if err != nil {
 			return errors.Fatal("invalid repository version")
 		}
 		version = uint(v)
 	}
+
 	if version < restic.MinRepoVersion || version > restic.MaxRepoVersion {
 		return errors.Fatalf("only repository versions between %v and %v are allowed", restic.MinRepoVersion, restic.MaxRepoVersion)
 	}

cmd/restic/cmd_restore_integration_test.go

@@ -398,7 +398,7 @@ func TestRestoreNoMetadataOnIgnoredIntermediateDirs(t *testing.T) {
 	fi, err := os.Stat(f2)
 	rtest.OK(t, err)
 
-	rtest.Assert(t, fi.ModTime() == time.Unix(0, 0),
+	rtest.Assert(t, fi.ModTime().Equal(time.Unix(0, 0)),
 		"meta data of intermediate directory hasn't been restore")
 }
 

cmd/restic/cmd_tag_integration_test.go

@@ -12,7 +12,6 @@ func testRunTag(t testing.TB, opts TagOptions, gopts GlobalOptions) {
 	rtest.OK(t, runTag(context.TODO(), opts, gopts, nil, []string{}))
 }
 
-// nolint: staticcheck // false positive nil pointer dereference check
 func TestTag(t *testing.T) {
 	env, cleanup := withTestEnvironment(t)
 	defer cleanup()

cmd/restic/global.go

@@ -498,6 +498,7 @@ func innerOpen(ctx context.Context, s string, gopts GlobalOptions, opts options.
 	}
 
 	if errors.Is(err, backend.ErrNoRepository) {
+		//nolint:staticcheck // capitalized error string is intentional
 		return nil, fmt.Errorf("Fatal: %w at %v: %v", ErrNoRepository, location.StripPassword(gopts.backends, s), err)
 	}
 	if err != nil {
@@ -552,6 +553,7 @@ func open(ctx context.Context, s string, gopts GlobalOptions, opts options.Optio
 	// check if config is there
 	fi, err := be.Stat(ctx, backend.Handle{Type: restic.ConfigFile})
 	if be.IsNotExist(err) {
+		//nolint:staticcheck // capitalized error string is intentional
 		return nil, fmt.Errorf("Fatal: %w: unable to open config file: %v\nIs there a repository at the following location?\n%v", ErrNoRepository, err, location.StripPassword(gopts.backends, s))
 	}
 	if err != nil {

cmd/restic/main.go

@@ -173,9 +173,10 @@ func main() {
 	ctx := createGlobalContext()
 	err = newRootCommand().ExecuteContext(ctx)
 
-	if err == nil {
+	switch err {
+	case nil:
 		err = ctx.Err()
-	} else if err == ErrOK {
+	case ErrOK:
 		// ErrOK overwrites context cancellation errors
 		err = nil
 	}

internal/backend/http_transport.go

@@ -45,10 +45,7 @@ func readPEMCertKey(filename string) (certs []byte, key []byte, err error) {
 	}
 
 	var block *pem.Block
-	for {
-		if len(data) == 0 {
-			break
-		}
+	for len(data) > 0 {
 		block, data = pem.Decode(data)
 		if block == nil {
 			break

internal/backend/limiter/static_limiter_test.go

@@ -118,7 +118,7 @@ func TestRoundTripperReader(t *testing.T) {
 	test.Assert(t, bytes.Equal(data, out.Bytes()), "data ping-pong failed")
 }
 
-// nolint:bodyclose // the http response is just a mock
+//nolint:bodyclose // the http response is just a mock
 func TestRoundTripperCornerCases(t *testing.T) {
 	limiter := NewStaticLimiter(Limits{42 * 1024, 42 * 1024})
 

internal/backend/location/location.go

@@ -34,6 +34,7 @@ func isPath(s string) bool {
 
 	// check for drive paths
 	drive := s[0]
+	//nolint:staticcheck // de morgan's law makes this harder to read
 	if !(drive >= 'a' && drive <= 'z') && !(drive >= 'A' && drive <= 'Z') {
 		return false
 	}

internal/backend/rest/rest_int_test.go

@@ -79,8 +79,8 @@ func TestListAPI(t *testing.T) {
 				t.Logf("req %v %v, accept: %v", req.Method, req.URL.Path, req.Header["Accept"])
 
 				var err error
-				switch {
-				case req.Method == "GET":
+				switch req.Method {
+				case "GET":
 					// list files in data/
 					res.Header().Set("Content-Type", test.ContentType)
 					_, err = res.Write([]byte(test.Data))
@@ -89,7 +89,7 @@ func TestListAPI(t *testing.T) {
 						t.Fatal(err)
 					}
 					return
-				case req.Method == "HEAD":
+				case "HEAD":
 					// stat file in data/, use the first two bytes in the name
 					// of the file as the size :)
 					filename := req.URL.Path[6:]

internal/backend/sftp/sftp.go

@@ -176,7 +176,6 @@ func (r *SFTP) mkdirAllDataSubdirs(ctx context.Context, nconn uint) error {
 	g.SetLimit(int(nconn))
 
 	for _, d := range r.Paths() {
-		d := d
 		g.Go(func() error {
 			// First try Mkdir. For most directories in Paths, this takes one
 			// round trip, not counting duplicate parent creations causes by

internal/checker/checker_test.go

@@ -669,7 +669,6 @@ func benchmarkSnapshotScaling(t *testing.B, newSnapshots int) {
 func BenchmarkCheckerSnapshotScaling(b *testing.B) {
 	counts := []int{50, 100, 200}
 	for _, count := range counts {
-		count := count
 		b.Run(strconv.Itoa(count), func(b *testing.B) {
 			benchmarkSnapshotScaling(b, count)
 		})

internal/debug/debug.go

@@ -64,10 +64,11 @@ func parseFilter(envname string, pad func(string) string) map[string]bool {
 	for _, fn := range strings.Split(env, ",") {
 		t := pad(strings.TrimSpace(fn))
 		val := true
-		if t[0] == '-' {
+		switch t[0] {
+		case '-':
 			val = false
 			t = t[1:]
-		} else if t[0] == '+' {
+		case '+':
 			val = true
 			t = t[1:]
 		}

internal/filter/filter_test.go

@@ -116,13 +116,13 @@ func TestMatch(t *testing.T) {
 
 		// Test with native path separator
 		if filepath.Separator != '/' {
-			pattern := strings.Replace(test.pattern, "/", string(filepath.Separator), -1)
+			pattern := strings.ReplaceAll(test.pattern, "/", string(filepath.Separator))
 			// Test with pattern as native
 			t.Run("pattern-native", func(t *testing.T) {
 				testpattern(t, pattern, test.path, test.match)
 			})
 
-			path := strings.Replace(test.path, "/", string(filepath.Separator), -1)
+			path := strings.ReplaceAll(test.path, "/", string(filepath.Separator))
 			t.Run("path-native", func(t *testing.T) {
 				// Test with path as native
 				testpattern(t, test.pattern, path, test.match)
@@ -206,13 +206,13 @@ func TestChildMatch(t *testing.T) {
 
 		// Test with native path separator
 		if filepath.Separator != '/' {
-			pattern := strings.Replace(test.pattern, "/", string(filepath.Separator), -1)
+			pattern := strings.ReplaceAll(test.pattern, "/", string(filepath.Separator))
 			// Test with pattern as native
 			t.Run("pattern-native", func(t *testing.T) {
 				testchildpattern(t, pattern, test.path, test.match)
 			})
 
-			path := strings.Replace(test.path, "/", string(filepath.Separator), -1)
+			path := strings.ReplaceAll(test.path, "/", string(filepath.Separator))
 			t.Run("path-native", func(t *testing.T) {
 				// Test with path as native
 				testchildpattern(t, test.pattern, path, test.match)

internal/fs/fs_reader_test.go

@@ -253,11 +253,7 @@ func TestFSReaderDir(t *testing.T) {
 			})
 			test.OK(t, err)
 			dir := path.Dir(tst.filename)
-			for {
-				if dir == "/" || dir == "." {
-					break
-				}
-
+			for dir != "/" && dir != "." {
 				fi, err := fs.Lstat(dir)
 				test.OK(t, err)
 

internal/fs/node_test.go

@@ -13,7 +13,6 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/restic/restic/internal/errors"
 	"github.com/restic/restic/internal/restic"
-	"github.com/restic/restic/internal/test"
 	rtest "github.com/restic/restic/internal/test"
 )
 
@@ -296,5 +295,5 @@ func TestNodeRestoreMetadataError(t *testing.T) {
 	// This will fail because the target file does not exist
 	err := NodeRestoreMetadata(node, nodePath, func(msg string) { rtest.OK(t, fmt.Errorf("Warning triggered for path: %s: %s", nodePath, msg)) },
 		func(_ string) bool { return true })
-	test.Assert(t, errors.Is(err, os.ErrNotExist), "failed for an unexpected reason")
+	rtest.Assert(t, errors.Is(err, os.ErrNotExist), "failed for an unexpected reason")
 }

internal/fs/stat.go

@@ -25,7 +25,7 @@ type ExtendedFileInfo struct {
 	ModTime    time.Time // last (content) modification time stamp
 	ChangeTime time.Time // last status change time stamp
 
-	// nolint:unused // only used on Windows
+	//nolint:unused // only used on Windows
 	sys any // Value returned by os.FileInfo.Sys()
 }
 

internal/repository/index/index_test.go

@@ -297,11 +297,12 @@ func TestIndexUnserialize(t *testing.T) {
 			rtest.Equals(t, test.tpe, blob.Type)
 			rtest.Equals(t, test.offset, blob.Offset)
 			rtest.Equals(t, test.length, blob.Length)
-			if task.version == 1 {
+			switch task.version {
+			case 1:
 				rtest.Equals(t, uint(0), blob.UncompressedLength)
-			} else if task.version == 2 {
+			case 2:
 				rtest.Equals(t, test.uncompressedLength, blob.UncompressedLength)
-			} else {
+			default:
 				t.Fatal("Invalid index version")
 			}
 		}

internal/repository/pack/pack.go

@@ -87,7 +87,7 @@ func (p *Packer) Finalize() error {
 	encryptedHeader = binary.LittleEndian.AppendUint32(encryptedHeader, uint32(len(encryptedHeader)))
 
 	if err := verifyHeader(p.k, encryptedHeader, p.blobs); err != nil {
-		//nolint:revive // ignore linter warnings about error message spelling
+		//nolint:revive,staticcheck // ignore linter warnings about error message spelling
 		return fmt.Errorf("Detected data corruption while writing pack-file header: %w\nCorrupted data is either caused by hardware issues or software bugs. Please open an issue at https://github.com/restic/restic/issues/new/choose for further troubleshooting.", err)
 	}
 

internal/repository/repository.go

@@ -395,7 +395,7 @@ func (r *Repository) saveAndEncrypt(ctx context.Context, t restic.BlobType, data
 	ciphertext = r.key.Seal(ciphertext, nonce, data, nil)
 
 	if err := r.verifyCiphertext(ciphertext, uncompressedLength, id); err != nil {
-		//nolint:revive // ignore linter warnings about error message spelling
+		//nolint:revive,staticcheck // ignore linter warnings about error message spelling
 		return 0, fmt.Errorf("Detected data corruption while saving blob %v: %w\nCorrupted blobs are either caused by hardware issues or software bugs. Please open an issue at https://github.com/restic/restic/issues/new/choose for further troubleshooting.", id, err)
 	}
 
@@ -500,7 +500,7 @@ func (r *Repository) saveUnpacked(ctx context.Context, t restic.FileType, buf []
 	ciphertext = r.key.Seal(ciphertext, nonce, p, nil)
 
 	if err := r.verifyUnpacked(ciphertext, t, buf); err != nil {
-		//nolint:revive // ignore linter warnings about error message spelling
+		//nolint:revive,staticcheck // ignore linter warnings about error message spelling
 		return restic.ID{}, fmt.Errorf("Detected data corruption while saving file of type %v: %w\nCorrupted data is either caused by hardware issues or software bugs. Please open an issue at https://github.com/restic/restic/issues/new/choose for further troubleshooting.", t, err)
 	}
 

internal/restic/node.go

@@ -413,7 +413,6 @@ func handleUnknownGenericAttributeFound(genericAttributeType GenericAttributeTyp
 
 // HandleAllUnknownGenericAttributesFound performs validations for all generic attributes of a node.
 // This is not used on windows currently because windows has handling for generic attributes.
-// nolint:unused
 func HandleAllUnknownGenericAttributesFound(attributes map[GenericAttributeType]json.RawMessage, warn func(msg string)) error {
 	for name := range attributes {
 		handleUnknownGenericAttributeFound(name, warn)

internal/restorer/restorer.go

@@ -593,10 +593,11 @@ func shouldOverwrite(overwrite OverwriteBehavior, node *restic.Node, destination
 		return false, err
 	}
 
-	if overwrite == OverwriteIfNewer {
+	switch overwrite {
+	case OverwriteIfNewer:
 		// return if node is newer
 		return node.ModTime.After(fi.ModTime()), nil
-	} else if overwrite == OverwriteNever {
+	case OverwriteNever:
 		// file exists
 		return false, nil
 	}

internal/ui/backup/rate_estimator_test.go

@@ -64,7 +64,7 @@ func TestBucketWidth(t *testing.T) {
 
 	b := e.buckets.Back().Value.(*rateBucket)
 	rtest.Assert(t, b.totalBytes == 2, "b.totalBytes is %d, want 2", b.totalBytes)
-	rtest.Assert(t, b.end == when.Add(bucketWidth), "b.end is %v, want %v", b.end, when.Add(bucketWidth))
+	rtest.Assert(t, b.end.Equal(when.Add(bucketWidth)), "b.end is %v, want %v", b.end, when.Add(bucketWidth))
 
 	// Recording a byte outside the bucket width causes another bucket.
 	e.recordBytes(when.Add(bucketWidth), 1)
@@ -72,7 +72,7 @@ func TestBucketWidth(t *testing.T) {
 
 	b = e.buckets.Back().Value.(*rateBucket)
 	rtest.Assert(t, b.totalBytes == 1, "b.totalBytes is %d, want 1", b.totalBytes)
-	rtest.Assert(t, b.end == when.Add(2*bucketWidth), "b.end is %v, want %v", b.end, when.Add(bucketWidth))
+	rtest.Assert(t, b.end.Equal(when.Add(2*bucketWidth)), "b.end is %v, want %v", b.end, when.Add(bucketWidth))
 
 	// Recording a byte after a longer delay creates a sparse bucket list.
 	e.recordBytes(when.Add(time.Hour+time.Millisecond), 7)

internal/ui/signals/signals.go

@@ -8,7 +8,7 @@ import (
 // GetProgressChannel returns a channel with which a single listener
 // receives each incoming signal.
 func GetProgressChannel() <-chan os.Signal {
-	signals.Once.Do(func() {
+	signals.once.Do(func() {
 		signals.ch = make(chan os.Signal, 1)
 		setupSignals()
 	})
@@ -19,6 +19,6 @@ func GetProgressChannel() <-chan os.Signal {
 // XXX The fact that signals is a single global variable means that only one
 // listener receives each incoming signal.
 var signals struct {
-	ch chan os.Signal
-	sync.Once
+	ch   chan os.Signal
+	once sync.Once
 }