From a9bec98fcde6c56641652f189400cc1737cad51b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Sun, 1 Dec 2024 14:46:46 +0100
Subject: [PATCH] Fix linter warnings

Fix most linter warnings reported by 'staticcheck'. This doesn't fix all
of them, some unused functions are still there, and some generated code
also still triggers some warnings. Most issues are fixed, though.

No functional changes, except for the captialisation of some error
messages.
---
 internal/manager/api_impl/dummy/shaman.go     |  1 +
 .../manager/task_logs/log_rotation_test.go    | 17 ++++++++--------
 internal/manager/task_logs/task_logs.go       |  6 +++---
 internal/manager/task_logs/task_logs_test.go  |  7 +++----
 .../task_state_machine/task_statuses.go       |  8 --------
 internal/stresser/stresser.go                 |  1 +
 internal/worker/autodiscovery.go              |  2 +-
 internal/worker/cli_runner/cli_runner.go      |  1 -
 internal/worker/command_file_mgmt.go          | 20 +++++++++----------
 internal/worker/command_file_mgmt_test.go     |  4 ++--
 internal/worker/state_awake.go                |  1 -
 pkg/shaman/checkout/manager.go                |  2 +-
 pkg/shaman/fileserver/receivefile_test.go     |  4 ++--
 pkg/shaman/fileserver/receivelistener.go      |  4 +---
 pkg/shaman/filestore/filestore_test.go        |  3 +--
 pkg/shaman/filestore/testing.go               |  3 +--
 pkg/shaman/touch/touch_test.go                |  3 +--
 17 files changed, 36 insertions(+), 51 deletions(-)

diff --git a/internal/manager/api_impl/dummy/shaman.go b/internal/manager/api_impl/dummy/shaman.go
index cd249405..0425cfda 100644
--- a/internal/manager/api_impl/dummy/shaman.go
+++ b/internal/manager/api_impl/dummy/shaman.go
@@ -16,6 +16,7 @@ type DummyShaman struct{}
 
 var _ api_impl.Shaman = (*DummyShaman)(nil)
 
+//lint:ignore ST1005 Shaman is a name that's always capitalised, so also in errors.
 var ErrDummyShaman = errors.New("Shaman storage component is inactive, configure Flamenco first")
 
 func (ds *DummyShaman) IsEnabled() bool {
diff --git a/internal/manager/task_logs/log_rotation_test.go b/internal/manager/task_logs/log_rotation_test.go
index 67ce7b67..88b9cdae 100644
--- a/internal/manager/task_logs/log_rotation_test.go
+++ b/internal/manager/task_logs/log_rotation_test.go
@@ -5,7 +5,6 @@ package task_logs
 import (
 	"errors"
 	"io/fs"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
@@ -16,7 +15,7 @@ import (
 )
 
 func setUpTest(t *testing.T) string {
-	temppath, err := ioutil.TempDir("", "testlogs")
+	temppath, err := os.MkdirTemp("", "testlogs")
 	require.NoError(t, err)
 	return temppath
 }
@@ -78,12 +77,12 @@ func TestMultipleFilesWithHoles(t *testing.T) {
 	defer tearDownTest(temppath)
 
 	filepath := filepath.Join(temppath, "existing.txt")
-	require.NoError(t, ioutil.WriteFile(filepath, []byte("thefile"), 0666))
-	require.NoError(t, ioutil.WriteFile(filepath+".1", []byte("file .1"), 0666))
-	require.NoError(t, ioutil.WriteFile(filepath+".2", []byte("file .2"), 0666))
-	require.NoError(t, ioutil.WriteFile(filepath+".3", []byte("file .3"), 0666))
-	require.NoError(t, ioutil.WriteFile(filepath+".5", []byte("file .5"), 0666))
-	require.NoError(t, ioutil.WriteFile(filepath+".7", []byte("file .7"), 0666))
+	require.NoError(t, os.WriteFile(filepath, []byte("thefile"), 0666))
+	require.NoError(t, os.WriteFile(filepath+".1", []byte("file .1"), 0666))
+	require.NoError(t, os.WriteFile(filepath+".2", []byte("file .2"), 0666))
+	require.NoError(t, os.WriteFile(filepath+".3", []byte("file .3"), 0666))
+	require.NoError(t, os.WriteFile(filepath+".5", []byte("file .5"), 0666))
+	require.NoError(t, os.WriteFile(filepath+".7", []byte("file .7"), 0666))
 
 	err := rotateLogFile(zerolog.Nop(), filepath)
 
@@ -100,7 +99,7 @@ func TestMultipleFilesWithHoles(t *testing.T) {
 	assert.False(t, fileExists(filepath+".9"))
 
 	read := func(filename string) string {
-		content, err := ioutil.ReadFile(filename)
+		content, err := os.ReadFile(filename)
 		require.NoError(t, err)
 		return string(content)
 	}
diff --git a/internal/manager/task_logs/task_logs.go b/internal/manager/task_logs/task_logs.go
index 5e65ba36..7762dd42 100644
--- a/internal/manager/task_logs/task_logs.go
+++ b/internal/manager/task_logs/task_logs.go
@@ -185,7 +185,7 @@ func (s *Storage) Tail(jobID, taskID string) (string, error) {
 		return "", fmt.Errorf("unable to open log file for reading: %w", err)
 	}
 
-	fileSize, err := file.Seek(0, os.SEEK_END)
+	fileSize, err := file.Seek(0, io.SeekEnd)
 	if err != nil {
 		return "", fmt.Errorf("unable to seek to end of log file: %w", err)
 	}
@@ -197,14 +197,14 @@ func (s *Storage) Tail(jobID, taskID string) (string, error) {
 	)
 	if fileSize <= tailSize {
 		// The file is small, just read all of it.
-		_, err = file.Seek(0, os.SEEK_SET)
+		_, err = file.Seek(0, io.SeekStart)
 		if err != nil {
 			return "", fmt.Errorf("unable to seek to start of log file: %w", err)
 		}
 		buffer, err = io.ReadAll(file)
 	} else {
 		// Read the last 'tailSize' number of bytes.
-		_, err = file.Seek(-tailSize, os.SEEK_END)
+		_, err = file.Seek(-tailSize, io.SeekEnd)
 		if err != nil {
 			return "", fmt.Errorf("unable to seek in log file: %w", err)
 		}
diff --git a/internal/manager/task_logs/task_logs_test.go b/internal/manager/task_logs/task_logs_test.go
index 8fc4f512..a37a2115 100644
--- a/internal/manager/task_logs/task_logs_test.go
+++ b/internal/manager/task_logs/task_logs_test.go
@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"io/fs"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -43,7 +42,7 @@ func TestLogWriting(t *testing.T) {
 	require.NoError(t, err)
 
 	filename := filepath.Join(jobDir, "task-20ff9d06-53ec-4019-9e2e-1774f05f170a.txt")
-	contents, err := ioutil.ReadFile(filename)
+	contents, err := os.ReadFile(filename)
 	require.NoError(t, err, "the log file should exist")
 	assert.Equal(t, "Ovo je priča\nIma dvije linije\n", string(contents))
 }
@@ -67,7 +66,7 @@ func TestLogRotation(t *testing.T) {
 	filename := filepath.Join(jobDir, "task-20ff9d06-53ec-4019-9e2e-1774f05f170a.txt")
 	rotatedFilename := filename + ".1"
 
-	contents, err := ioutil.ReadFile(rotatedFilename)
+	contents, err := os.ReadFile(rotatedFilename)
 	require.NoError(t, err, "the rotated log file should exist")
 	assert.Equal(t, "Ovo je priča\n", string(contents))
 
@@ -216,7 +215,7 @@ type TaskLogsMocks struct {
 func taskLogsTestFixtures(t *testing.T) (*Storage, func(), *TaskLogsMocks) {
 	mockCtrl := gomock.NewController(t)
 
-	temppath, err := ioutil.TempDir("", "testlogs")
+	temppath, err := os.MkdirTemp("", "testlogs")
 	require.NoError(t, err)
 
 	mocks := &TaskLogsMocks{
diff --git a/internal/manager/task_state_machine/task_statuses.go b/internal/manager/task_state_machine/task_statuses.go
index 4e14a189..66e41d34 100644
--- a/internal/manager/task_state_machine/task_statuses.go
+++ b/internal/manager/task_state_machine/task_statuses.go
@@ -5,14 +5,6 @@ package task_state_machine
 import "projects.blender.org/studio/flamenco/pkg/api"
 
 var (
-	// Task statuses that always get requeued when the job is requeueing.
-	nonCompletedStatuses = []api.TaskStatus{
-		api.TaskStatusCanceled,
-		api.TaskStatusFailed,
-		api.TaskStatusPaused,
-		api.TaskStatusSoftFailed,
-	}
-
 	// Workers are allowed to keep running tasks when they are in this status.
 	// 'queued', 'claimed-by-manager', and 'soft-failed' aren't considered runnable,
 	// as those statuses indicate the task wasn't assigned to a Worker by the scheduler.
diff --git a/internal/stresser/stresser.go b/internal/stresser/stresser.go
index 12ab4710..e299f275 100644
--- a/internal/stresser/stresser.go
+++ b/internal/stresser/stresser.go
@@ -79,6 +79,7 @@ func stressByRequestingTask(ctx context.Context, client worker.FlamencoClient) {
 	}
 }
 
+//lint:ignore U1000 stressBySendingTaskUpdate is currently unused, but someone may find it useful for different kinds of stess testing.
 func stressBySendingTaskUpdate(ctx context.Context, client worker.FlamencoClient, task *api.AssignedTask) {
 	logLine := "This is a log-line for stress testing. It will be repeated more than once.\n"
 	logToSend := strings.Repeat(logLine, 5)
diff --git a/internal/worker/autodiscovery.go b/internal/worker/autodiscovery.go
index 5145eda8..5c756ce7 100644
--- a/internal/worker/autodiscovery.go
+++ b/internal/worker/autodiscovery.go
@@ -46,7 +46,7 @@ func AutodiscoverManager(ctx context.Context) (string, error) {
 
 	logger := log.Logger
 	if deadline, ok := ctx.Deadline(); ok {
-		timeout := deadline.Sub(time.Now()).Round(1 * time.Second)
+		timeout := time.Until(deadline).Round(1 * time.Second)
 		logger = logger.With().Str("timeout", timeout.String()).Logger()
 	}
 	logger.Info().Msg("auto-discovering Manager via UPnP/SSDP")
diff --git a/internal/worker/cli_runner/cli_runner.go b/internal/worker/cli_runner/cli_runner.go
index 9369c3cf..67b4d781 100644
--- a/internal/worker/cli_runner/cli_runner.go
+++ b/internal/worker/cli_runner/cli_runner.go
@@ -101,7 +101,6 @@ readloop:
 		// Prepend any leftovers from the previous line to the received bytes.
 		if len(leftovers) > 0 {
 			lineBytes = append(leftovers, lineBytes...)
-			leftovers = []byte{}
 		}
 		// Make sure long lines are broken on character boundaries.
 		lineBytes, leftovers = splitOnCharacterBoundary(lineBytes)
diff --git a/internal/worker/command_file_mgmt.go b/internal/worker/command_file_mgmt.go
index 5a4b4263..08cfae47 100644
--- a/internal/worker/command_file_mgmt.go
+++ b/internal/worker/command_file_mgmt.go
@@ -133,7 +133,7 @@ func (ce *CommandExecutor) cmdCopyFile(ctx context.Context, logger zerolog.Logge
 		return err
 	}
 
-	err, logMsg := fileCopy(src, dest)
+	logMsg, err := fileCopy(src, dest)
 	return ce.errorLogProcess(ctx, logger, cmd, taskID, err, logMsg)
 }
 
@@ -171,48 +171,48 @@ func (ce *CommandExecutor) moveAndLog(ctx context.Context, taskID, cmdName, src,
 	return nil
 }
 
-func fileCopy(src, dest string) (error, string) {
+func fileCopy(src, dest string) (string, error) {
 	src_file, err := os.Open(src)
 	if err != nil {
 		msg := fmt.Sprintf("failed to open source file %q: %v", src, err)
-		return err, msg
+		return msg, err
 	}
 	defer src_file.Close()
 
 	src_file_stat, err := src_file.Stat()
 	if err != nil {
 		msg := fmt.Sprintf("failed to stat source file %q: %v", src, err)
-		return err, msg
+		return msg, err
 	}
 
 	if !src_file_stat.Mode().IsRegular() {
-		err := &os.PathError{Op: "stat", Path: src, Err: errors.New("Not a regular file")}
+		err := &os.PathError{Op: "stat", Path: src, Err: errors.New("not a regular file")}
 		msg := fmt.Sprintf("invalid source file %q: %v", src, err)
-		return err, msg
+		return msg, err
 	}
 
 	dest_dirpath := filepath.Dir(dest)
 	if !fileExists(dest_dirpath) {
 		if err := os.MkdirAll(dest_dirpath, 0750); err != nil {
 			msg := fmt.Sprintf("failed to create directories %q: %v", dest_dirpath, err)
-			return err, msg
+			return msg, err
 		}
 	}
 
 	dest_file, err := os.Create(dest)
 	if err != nil {
 		msg := fmt.Sprintf("failed to create destination file %q: %v", dest, err)
-		return err, msg
+		return msg, err
 	}
 	defer dest_file.Close()
 
 	if _, err := io.Copy(dest_file, src_file); err != nil {
 		msg := fmt.Sprintf("failed to copy %q to %q: %v", src, dest, err)
-		return err, msg
+		return msg, err
 	}
 
 	msg := fmt.Sprintf("copied %q to %q", src, dest)
-	return nil, msg
+	return msg, nil
 }
 
 func fileExists(filename string) bool {
diff --git a/internal/worker/command_file_mgmt_test.go b/internal/worker/command_file_mgmt_test.go
index c0ae29a9..d6ab9a78 100644
--- a/internal/worker/command_file_mgmt_test.go
+++ b/internal/worker/command_file_mgmt_test.go
@@ -163,7 +163,7 @@ func TestTimestampedPathFile(t *testing.T) {
 
 	fileCreateEmpty("somefile.txt")
 	if err := os.Chtimes("somefile.txt", mtime, mtime); err != nil {
-		t.Fatalf(err.Error())
+		t.Fatal(err.Error())
 	}
 
 	newpath, err := timestampedPath("somefile.txt")
@@ -365,7 +365,7 @@ func TestCmdCopyFileSourceIsDir(t *testing.T) {
 		fmt.Sprintf("copy-file: copying %q to %q", f.absolute_src_path, f.absolute_dest_path))
 
 	f.mocks.listener.EXPECT().LogProduced(gomock.Any(), taskID,
-		fmt.Sprintf("copy-file: invalid source file %q: stat %s: Not a regular file",
+		fmt.Sprintf("copy-file: invalid source file %q: stat %s: not a regular file",
 			f.absolute_src_path, f.absolute_src_path))
 
 	assert.Error(t, f.run())
diff --git a/internal/worker/state_awake.go b/internal/worker/state_awake.go
index fad9cdcb..9d47aeb2 100644
--- a/internal/worker/state_awake.go
+++ b/internal/worker/state_awake.go
@@ -185,7 +185,6 @@ checkloop:
 			break checkloop
 		case <-time.After(mayKeepRunningPeriod):
 			// Time to do another check.
-			break
 		}
 
 		mkr := w.mayIKeepRunning(taskCtx, task.Uuid)
diff --git a/pkg/shaman/checkout/manager.go b/pkg/shaman/checkout/manager.go
index 33128321..91904a38 100644
--- a/pkg/shaman/checkout/manager.go
+++ b/pkg/shaman/checkout/manager.go
@@ -73,7 +73,7 @@ func (err ErrInvalidCheckoutPath) Error() string {
 
 // Errors returned by the Checkout Manager.
 var (
-	ErrCheckoutAlreadyExists = errors.New("A checkout with this ID already exists")
+	ErrCheckoutAlreadyExists = errors.New("a checkout with this ID already exists")
 )
 
 // NewManager creates and returns a new Checkout Manager.
diff --git a/pkg/shaman/fileserver/receivefile_test.go b/pkg/shaman/fileserver/receivefile_test.go
index be51f11e..ee4787f6 100644
--- a/pkg/shaman/fileserver/receivefile_test.go
+++ b/pkg/shaman/fileserver/receivefile_test.go
@@ -26,7 +26,7 @@ import (
 	"bytes"
 	"context"
 	"io"
-	"io/ioutil"
+	"os"
 	"testing"
 
 	"projects.blender.org/studio/flamenco/pkg/shaman/config"
@@ -86,7 +86,7 @@ func TestStoreFile(t *testing.T) {
 	assert.Equal(t, filestore.StatusStored, status)
 	assert.FileExists(t, path)
 
-	savedContent, err := ioutil.ReadFile(path)
+	savedContent, err := os.ReadFile(path)
 	require.NoError(t, err)
 	assert.EqualValues(t, payload, savedContent, "The file should be saved uncompressed")
 }
diff --git a/pkg/shaman/fileserver/receivelistener.go b/pkg/shaman/fileserver/receivelistener.go
index e4639043..6a9734ac 100644
--- a/pkg/shaman/fileserver/receivelistener.go
+++ b/pkg/shaman/fileserver/receivelistener.go
@@ -47,9 +47,7 @@ func (fs *FileServer) receiveListenerFor(checksum string, filesize int64) chan s
 
 	go func() {
 		// Wait until the channel closes.
-		select {
-		case <-channel:
-		}
+		<-channel
 
 		fs.receiverMutex.Lock()
 		defer fs.receiverMutex.Unlock()
diff --git a/pkg/shaman/filestore/filestore_test.go b/pkg/shaman/filestore/filestore_test.go
index d9ef274d..ea8c9a4e 100644
--- a/pkg/shaman/filestore/filestore_test.go
+++ b/pkg/shaman/filestore/filestore_test.go
@@ -23,7 +23,6 @@
 package filestore
 
 import (
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
@@ -115,7 +114,7 @@ func TestOpenForUpload(t *testing.T) {
 	assert.Equal(t, file.Name(), foundPath)
 	assert.Equal(t, StatusUploading, status)
 
-	readContents, err := ioutil.ReadFile(foundPath)
+	readContents, err := os.ReadFile(foundPath)
 	require.NoError(t, err)
 	assert.EqualValues(t, contents, readContents)
 }
diff --git a/pkg/shaman/filestore/testing.go b/pkg/shaman/filestore/testing.go
index 9caecfbe..5379ec94 100644
--- a/pkg/shaman/filestore/testing.go
+++ b/pkg/shaman/filestore/testing.go
@@ -25,7 +25,6 @@ package filestore
 import (
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -36,7 +35,7 @@ import (
 
 // CreateTestStore returns a Store that can be used for unit testing.
 func CreateTestStore() *Store {
-	tempDir, err := ioutil.TempDir("", "shaman-filestore-test-")
+	tempDir, err := os.MkdirTemp("", "shaman-filestore-test-")
 	if err != nil {
 		panic(err)
 	}
diff --git a/pkg/shaman/touch/touch_test.go b/pkg/shaman/touch/touch_test.go
index 75c41c59..aa6294fa 100644
--- a/pkg/shaman/touch/touch_test.go
+++ b/pkg/shaman/touch/touch_test.go
@@ -23,7 +23,6 @@
 package touch
 
 import (
-	"io/ioutil"
 	"os"
 	"testing"
 	"time"
@@ -36,7 +35,7 @@ func TestTouch(t *testing.T) {
 	testPath := "_touch_test.txt"
 
 	// Create a file
-	assert.Nil(t, ioutil.WriteFile(testPath, []byte("just a test"), 0644))
+	assert.Nil(t, os.WriteFile(testPath, []byte("just a test"), 0644))
 	defer os.Remove(testPath)
 
 	// Make it old