Update gocritic enabled/disabled checks to fix linter (#1084)

I believe `golangci` upgraded to `1.57.0` and it's giving us a bunch of
lint errors. I wanted to tackle most of them and disable some others but
it turns out you can't mix and match `enabled-checks` and
`disabled-checks`.

I ended up:
- enabling diagnostic preset
- enabling style preset
- disable `ifElseChain`
- disable  `exitAfterDefer`

edit:
- disabling all the things

I pretty much made sure not to disable something we previously had
enabled, and I enabled one or two options that I think make sense. Like
`deferInLoop` for example. Here is the list of options:
https://go-critic.com/overview#checks-overview

.github/workflows/test.yml

@@ -17,13 +17,6 @@ jobs:
       - name: Configure Windows
         if: matrix.os == 'windows-latest'
         run: git config --global core.autocrlf false # fixes go lint fmt error
-      - name: Configure MySQL
-        if: matrix.os == 'ubuntu-latest'
-        uses: mirromutth/[email protected]
-        with:
-          host port: 3800
-          mysql version: '8'
-          mysql root password: test
       - name: Checkout
         uses: actions/checkout@v3
       - name: Setup Go
@@ -43,6 +36,13 @@ jobs:
             autopilot
             bus bus/client
             worker worker/client
+      - name: Configure MySQL
+        if: matrix.os == 'ubuntu-latest'
+        uses: mirromutth/[email protected]
+        with:
+          host port: 3800
+          mysql version: '8'
+          mysql root password: test
       - name: Test
         uses: n8maninger/action-golang-test@v1
         with:

.golangci.yml

@@ -17,13 +17,6 @@ run:
   # list of build tags, all linters use it. Default is empty list.
   build-tags: []
 
-  # which dirs to skip: issues from them won't be reported;
-  # can use regexp here: generated.*, regexp is applied on full path;
-  # default value is empty list, but default dirs are skipped independently
-  # from this option's value (see skip-dirs-use-default).
-  skip-dirs:
-    - cover
-
   # default is true. Enables skipping of directories:
   #   vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
   skip-dirs-use-default: true
@@ -36,9 +29,6 @@ run:
 
 # output configuration options
 output:
-  # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number"
-  format: colored-line-number
-
   # print lines of code with issue, default is true
   print-issued-lines: true
 
@@ -61,23 +51,28 @@ linters-settings:
     # See https://go-critic.github.io/overview#checks-overview
     # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run`
     # By default list of stable checks is used.
-    enabled-checks:
-      - argOrder # Diagnostic options
-      - badCond
-      - caseOrder
-      - dupArg
-      - dupBranchBody
-      - dupCase
-      - dupSubExpr
-      - nilValReturn
-      - offBy1
-      - weakCond
-      - boolExprSimplify # Style options here and below.
-      - builtinShadow
-      - emptyFallthrough
-      - hexLiteral
-      - underef
-      - equalFold
+    enabled-tags:
+      - diagnostic
+      - style
+    disabled-checks:
+      # diagnostic
+      - commentedOutCode
+      - uncheckedInlineErr
+
+      # style
+      - exitAfterDefer
+      - ifElseChain
+      - importShadow
+      - octalLiteral
+      - paramTypeCombine
+      - ptrToRefParam
+      - stringsCompare
+      - tooManyResultsChecker
+      - typeDefFirst
+      - typeUnparen
+      - unlabelStmt
+      - unnamedResult
+      - whyNoLint
   revive:
     ignore-generated-header: true
     rules:

api/host.go

@@ -112,6 +112,6 @@ func (opts HostsForScanningOptions) Apply(values url.Values) {
 		values.Set("limit", fmt.Sprint(opts.Limit))
 	}
 	if !opts.MaxLastScan.IsZero() {
-		values.Set("lastScan", fmt.Sprint(TimeRFC3339(opts.MaxLastScan)))
+		values.Set("lastScan", TimeRFC3339(opts.MaxLastScan).String())
 	}
 }

api/object.go

@@ -367,8 +367,8 @@ func (opts SearchObjectOptions) Apply(values url.Values) {
 	}
 }
 
-func FormatETag(ETag string) string {
-	return fmt.Sprintf("\"%s\"", ETag)
+func FormatETag(eTag string) string {
+	return fmt.Sprintf("%q", eTag)
 }
 
 func ObjectPathEscape(path string) string {

api/param.go

@@ -105,7 +105,7 @@ func (t *TimeRFC3339) UnmarshalText(b []byte) error {
 
 // MarshalJSON implements json.Marshaler.
 func (t TimeRFC3339) MarshalJSON() ([]byte, error) {
-	return []byte(fmt.Sprintf(`"%s"`, (time.Time)(t).UTC().Format(time.RFC3339Nano))), nil
+	return []byte(fmt.Sprintf("%q", (time.Time)(t).UTC().Format(time.RFC3339Nano))), nil
 }
 
 // String implements fmt.Stringer.

api/setting.go

@@ -155,11 +155,11 @@ func (rs RedundancySettings) Validate() error {
 // valid.
 func (s3as S3AuthenticationSettings) Validate() error {
 	for accessKeyID, secretAccessKey := range s3as.V4Keypairs {
-		if len(accessKeyID) == 0 {
+		if accessKeyID == "" {
 			return fmt.Errorf("AccessKeyID cannot be empty")
 		} else if len(accessKeyID) < S3MinAccessKeyLen || len(accessKeyID) > S3MaxAccessKeyLen {
 			return fmt.Errorf("AccessKeyID must be between %d and %d characters long but was %d", S3MinAccessKeyLen, S3MaxAccessKeyLen, len(accessKeyID))
-		} else if len(secretAccessKey) == 0 {
+		} else if secretAccessKey == "" {
 			return fmt.Errorf("SecretAccessKey cannot be empty")
 		} else if len(secretAccessKey) != S3SecretKeyLen {
 			return fmt.Errorf("SecretAccessKey must be %d characters long but was %d", S3SecretKeyLen, len(secretAccessKey))

autopilot/hosts.go

@@ -33,7 +33,7 @@ func (hosts scoredHosts) randSelectByScore(n int) (selected []scoredHost) {
 			total += h.score
 		}
 		for i := range candidates {
-			candidates[i].score = candidates[i].score / total
+			candidates[i].score /= total
 		}
 
 		// select

bus/client/metrics.go

@@ -125,7 +125,7 @@ func (c *Client) PruneMetrics(ctx context.Context, metric string, cutoff time.Ti
 		panic(err)
 	}
 	u.RawQuery = values.Encode()
-	req, err := http.NewRequestWithContext(ctx, "DELETE", u.String(), nil)
+	req, err := http.NewRequestWithContext(ctx, "DELETE", u.String(), http.NoBody)
 	if err != nil {
 		panic(err)
 	}
@@ -180,7 +180,7 @@ func (c *Client) metric(ctx context.Context, key string, values url.Values, res
 		panic(err)
 	}
 	u.RawQuery = values.Encode()
-	req, err := http.NewRequestWithContext(ctx, "GET", u.String(), nil)
+	req, err := http.NewRequestWithContext(ctx, "GET", u.String(), http.NoBody)
 	if err != nil {
 		panic(err)
 	}

bus/client/slabs.go

@@ -63,7 +63,7 @@ func (c *Client) FetchPartialSlab(ctx context.Context, key object.EncryptionKey,
 		panic(err)
 	}
 	u.RawQuery = values.Encode()
-	req, err := http.NewRequestWithContext(ctx, "GET", u.String(), nil)
+	req, err := http.NewRequestWithContext(ctx, "GET", u.String(), http.NoBody)
 	if err != nil {
 		panic(err)
 	}

bus/client/wallet.go

@@ -149,7 +149,7 @@ func (c *Client) WalletTransactions(ctx context.Context, opts ...api.WalletTrans
 		panic(err)
 	}
 	u.RawQuery = values.Encode()
-	req, err := http.NewRequestWithContext(ctx, "GET", u.String(), nil)
+	req, err := http.NewRequestWithContext(ctx, "GET", u.String(), http.NoBody)
 	if err != nil {
 		panic(err)
 	}

cmd/renterd/config.go

@@ -376,7 +376,7 @@ func cmdBuildConfig() {
 
 	// write the config file
 	configPath := "renterd.yml"
-	if str := os.Getenv("RENTERD_CONFIG_FILE"); len(str) != 0 {
+	if str := os.Getenv("RENTERD_CONFIG_FILE"); str != "" {
 		configPath = str
 	}
 

cmd/renterd/main.go

@@ -139,7 +139,7 @@ func check(context string, err error) {
 }
 
 func mustLoadAPIPassword() {
-	if len(cfg.HTTP.Password) != 0 {
+	if cfg.HTTP.Password != "" {
 		return
 	}
 
@@ -192,7 +192,7 @@ func mustParseWorkers(workers, password string) {
 // loaded.
 func tryLoadConfig() {
 	configPath := "renterd.yml"
-	if str := os.Getenv("RENTERD_CONFIG_FILE"); len(str) != 0 {
+	if str := os.Getenv("RENTERD_CONFIG_FILE"); str != "" {
 		configPath = str
 	}
 

hostdb/hostdb.go

@@ -38,7 +38,7 @@ func ForEachAnnouncement(b types.Block, height uint64, fn func(types.PublicKey,
 			// verify signature
 			var hostKey types.PublicKey
 			copy(hostKey[:], ha.PublicKey.Key)
-			annHash := types.Hash256(crypto.HashObject(ha.HostAnnouncement)) // TODO
+			annHash := types.Hash256(crypto.HashObject(ha.HostAnnouncement))
 			if !hostKey.VerifyHash(annHash, ha.Signature) {
 				continue
 			}

internal/test/e2e/cluster_test.go

@@ -2062,6 +2062,9 @@ func TestMultipartUploads(t *testing.T) {
 	etag1 := putPart(1, 0, data1)
 	etag3 := putPart(3, len(data1)+len(data2), data3)
 	size := int64(len(data1) + len(data2) + len(data3))
+	expectedData := data1
+	expectedData = append(expectedData, data2...)
+	expectedData = append(expectedData, data3...)
 
 	// List parts
 	mup, err := b.MultipartUploadParts(context.Background(), api.DefaultBucketName, objPath, mpr.UploadID, 0, 0)
@@ -2118,7 +2121,7 @@ func TestMultipartUploads(t *testing.T) {
 		t.Fatal("unexpected size:", gor.Size)
 	} else if data, err := io.ReadAll(gor.Content); err != nil {
 		t.Fatal(err)
-	} else if expectedData := append(data1, append(data2, data3...)...); !bytes.Equal(data, expectedData) {
+	} else if !bytes.Equal(data, expectedData) {
 		t.Fatal("unexpected data:", cmp.Diff(data, expectedData))
 	}
 
@@ -2417,6 +2420,11 @@ func TestMultipartUploadWrappedByPartialSlabs(t *testing.T) {
 	})
 	tt.OK(err)
 
+	// combine all parts data
+	expectedData := part1Data
+	expectedData = append(expectedData, part2Data...)
+	expectedData = append(expectedData, part3Data...)
+
 	// finish the upload
 	tt.OKAll(b.CompleteMultipartUpload(context.Background(), api.DefaultBucketName, objPath, mpr.UploadID, []api.MultipartCompletedPart{
 		{
@@ -2436,7 +2444,6 @@ func TestMultipartUploadWrappedByPartialSlabs(t *testing.T) {
 	// download the object and verify its integrity
 	dst := new(bytes.Buffer)
 	tt.OK(w.DownloadObject(context.Background(), dst, api.DefaultBucketName, objPath, api.DownloadObjectOptions{}))
-	expectedData := append(part1Data, append(part2Data, part3Data...)...)
 	receivedData := dst.Bytes()
 	if len(receivedData) != len(expectedData) {
 		t.Fatalf("expected %v bytes, got %v", len(expectedData), len(receivedData))

stores/metadata_test.go

@@ -4538,7 +4538,7 @@ func TestTypeCurrency(t *testing.T) {
 		var result bool
 		query := fmt.Sprintf("SELECT ? %s ?", test.cmp)
 		if !isSQLite(ss.db) {
-			query = strings.Replace(query, "?", "HEX(?)", -1)
+			query = strings.ReplaceAll(query, "?", "HEX(?)")
 		}
 		if err := ss.db.Raw(query, test.a, test.b).Scan(&result).Error; err != nil {
 			t.Fatal(err)

worker/client/client.go

@@ -88,7 +88,7 @@ func (c *Client) HeadObject(ctx context.Context, bucket, path string, opts api.H
 	path += "?" + values.Encode()
 
 	// TODO: support HEAD in jape client
-	req, err := http.NewRequestWithContext(ctx, "HEAD", fmt.Sprintf("%s/objects/%s", c.c.BaseURL, path), nil)
+	req, err := http.NewRequestWithContext(ctx, "HEAD", fmt.Sprintf("%s/objects/%s", c.c.BaseURL, path), http.NoBody)
 	if err != nil {
 		panic(err)
 	}
@@ -271,7 +271,7 @@ func (c *Client) object(ctx context.Context, bucket, path string, opts api.Downl
 	path += "?" + values.Encode()
 
 	c.c.Custom("GET", fmt.Sprintf("/objects/%s", path), nil, (*[]api.ObjectMetadata)(nil))
-	req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("%s/objects/%s", c.c.BaseURL, path), nil)
+	req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("%s/objects/%s", c.c.BaseURL, path), http.NoBody)
 	if err != nil {
 		panic(err)
 	}

worker/mocks_test.go

@@ -375,7 +375,7 @@ func newObjectStoreMock(bucket string) *objectStoreMock {
 	return os
 }
 
-func (os *objectStoreMock) AddMultipartPart(ctx context.Context, bucket, path, contractSet, ETag, uploadID string, partNumber int, slices []object.SlabSlice) (err error) {
+func (os *objectStoreMock) AddMultipartPart(ctx context.Context, bucket, path, contractSet, eTag, uploadID string, partNumber int, slices []object.SlabSlice) (err error) {
 	return nil
 }
 

worker/rhpv2.go

@@ -217,8 +217,14 @@ func RPCFormContract(ctx context.Context, t *rhpv2.Transport, renterKey types.Pr
 		return rhpv2.ContractRevision{}, nil, err
 	}
 
-	txn.Signatures = append(renterContractSignatures, hostSigs.ContractSignatures...)
-	signedTxnSet := append(resp.Parents, append(parents, txn)...)
+	txn.Signatures = make([]types.TransactionSignature, 0, len(renterContractSignatures)+len(hostSigs.ContractSignatures))
+	txn.Signatures = append(txn.Signatures, renterContractSignatures...)
+	txn.Signatures = append(txn.Signatures, hostSigs.ContractSignatures...)
+
+	signedTxnSet := make([]types.Transaction, 0, len(resp.Parents)+len(parents)+1)
+	signedTxnSet = append(signedTxnSet, resp.Parents...)
+	signedTxnSet = append(signedTxnSet, parents...)
+	signedTxnSet = append(signedTxnSet, txn)
 	return rhpv2.ContractRevision{
 		Revision: initRevision,
 		Signatures: [2]types.TransactionSignature{

worker/rhpv3.go

@@ -511,7 +511,9 @@ func (a *accounts) deriveAccountKey(hostKey types.PublicKey) types.PrivateKey {
 	// Append the host for which to create it and the index to the
 	// corresponding sub-key.
 	subKey := a.key
-	data := append(subKey, hostKey[:]...)
+	data := make([]byte, 0, len(subKey)+len(hostKey)+1)
+	data = append(data, subKey[:]...)
+	data = append(data, hostKey[:]...)
 	data = append(data, index)
 
 	seed := types.HashBytes(data)
@@ -1078,7 +1080,8 @@ func RPCRenew(ctx context.Context, rrr api.RHPRenewRequest, bus Bus, t *transpor
 	txn.Signatures = append(txn.Signatures, hostSigs.TransactionSignatures...)
 
 	// Add the parents to get the full txnSet.
-	txnSet = append(parents, txn)
+	txnSet = parents
+	txnSet = append(txnSet, txn)
 
 	return rhpv2.ContractRevision{
 		Revision:   noOpRevision,

worker/worker.go

@@ -506,7 +506,8 @@ func (w *worker) rhpBroadcastHandler(jc jape.Context) {
 		return
 	}
 	// Broadcast the txn.
-	txnSet := append(parents, txn)
+	txnSet := parents
+	txnSet = append(txnSet, txn)
 	err = w.bus.BroadcastTransaction(ctx, txnSet)
 	if jc.Check("failed to broadcast transaction", err) != nil {
 		_ = w.bus.WalletDiscard(ctx, txn)