chore: Bump golangci-lint to v1.52

Updated golangci-lint to v1.52 and also changed the linter config such that all linters are initially enabled and selected linters are disabled. This ensures that linters are not overlooked.

Code changes were required to resolve problems detected by the newly enabled linters.

Signed-off-by: Bob Stasyszyn <Bob.Stasyszyn@securekey.com>

.golangci.yml

@@ -12,6 +12,7 @@ run:
   tests: true
   build-tags: [""]
   skip-dirs:  [""]
+  go: '1.19'
 
 output:
   format: colored-line-number
@@ -80,61 +81,60 @@ linters-settings:
     line-length: 140
   misspell:
     locale: US
+    ignore-words:
+      - marshaller
+      - marshalling
+      - unmarshalling
   nolintlint:
     allow-unused: false # report any unused nolint directives
     require-explanation: false # don't require an explanation for nolint directives
     require-specific: false # don't require nolint directives to be specific about which linter is being skipped
     machine: false # don't require //nolint instead of // nolint
     explain: false # don't require //nolint // my explanation instead of just //nolint
+  revive:
+    rules:
+      - name: exported
+        arguments:
+          - disableStutteringCheck
 
 linters:
-  disable-all: true
-  enable:
-    - bodyclose
-    - depguard
-    - dogsled
-    - dupl
-    - errcheck
-    - exportloopref
-    - funlen
-    - gochecknoinits
-    - goconst
-    - gocritic
-    - gocyclo
-    - goimports
-    - gomnd
-    - goprintffuncname
-    - gosec
-    - gosimple
-    - govet
-    - ineffassign
-    - lll
-    - nakedret
-    - noctx
-    - nolintlint
-    - staticcheck
-    - stylecheck
-    - typecheck
-    - unconvert
-    - unparam
-    - unused
-    - whitespace
-
-  # don't enable:
-  # - asciicheck
-  # - scopelint
-  # - gochecknoglobals
-  # - gocognit
-  # - godot
-  # - godox
-  # - goerr113
-  # - interfacer
-  # - maligned
-  # - nestif
-  # - prealloc
-  # - testpackage
-  # - revive
-  # - wsl
+  enable-all: true
+  disable:
+   - asciicheck
+   - gochecknoglobals
+   - gocognit
+   - godot
+   - godox
+   - goerr113
+   - prealloc
+   - testpackage
+   - wsl
+   - exhaustive
+   - exhaustruct
+   - gomoddirectives
+   - cyclop
+   - ireturn
+   - nonamedreturns
+   - paralleltest
+   - varnamelen
+   - wrapcheck
+   - tagliatelle
+   - maintidx
+   - gci
+   - containedctx
+   - forbidigo
+   - musttag
+   # The following linters are deprecated:
+   - scopelint
+   - varcheck
+   - deadcode
+   - exhaustivestruct
+   - maligned
+   - structcheck
+   - ifshort
+   - nosnakecase
+   - interfacer
+   - golint
 
 issues:
   # Excluding configuration per-path, per-linter, per-text and per-source
@@ -173,3 +173,7 @@ issues:
     -  at least one file in a package should have a package comment
     # Allow package logger variables (for now)
     - logger is a global variable
+
+  include:
+    - EXC0012  # EXC0012 revive: Annoying issue about not having a comment. The rare codebase has such comments
+    - EXC0014  # EXC0014 revive: Annoying issue about not having a comment. The rare codebase has such comments

cmd/orb-cli/common/activitypubclient.go

@@ -131,7 +131,8 @@ type requestSender func(req []byte, method, endpointURL string) ([]byte, error)
 type targetOverrideFunc func(targetURI string) string
 
 func newAPCollIterator(cmd *cobra.Command, collURI string, sendRequest requestSender,
-	getTargetOverride targetOverrideFunc) (*ActivityPubCollectionIterator, error) {
+	getTargetOverride targetOverrideFunc,
+) (*ActivityPubCollectionIterator, error) {
 	resp, err := sendRequest(nil, http.MethodGet, getTargetOverride(collURI))
 	if err != nil {
 		return nil, fmt.Errorf("failed to send http request: %w", err)

cmd/orb-cli/common/common.go

@@ -179,7 +179,8 @@ func ParsePrivateKey(der []byte) (crypto.PrivateKey, error) {
 
 // GetPublicKeyFromKMS get publickey from kms.
 func GetPublicKeyFromKMS(cmd *cobra.Command, keyIDFlagName, keyIDEnvKey string,
-	webKmsClient kms.KeyManager) (interface{}, error) {
+	webKmsClient kms.KeyManager,
+) (interface{}, error) {
 	keyID, err := cmdutil.GetUserSetVarFromString(cmd, keyIDFlagName,
 		keyIDEnvKey, false)
 	if err != nil {
@@ -222,7 +223,8 @@ func GetPublicKeyFromKMS(cmd *cobra.Command, keyIDFlagName, keyIDEnvKey string,
 
 // GetKey get key.
 func GetKey(cmd *cobra.Command, keyFlagName, keyEnvKey, keyFileFlagName, keyFileEnvKey string,
-	password []byte, privateKey bool) (interface{}, error) {
+	password []byte, privateKey bool,
+) (interface{}, error) {
 	keyString := cmdutil.GetUserSetOptionalVarFromString(cmd, keyFlagName,
 		keyEnvKey)
 
@@ -335,7 +337,8 @@ func GetServices(serviceFilePath string) ([]docdid.Service, error) {
 
 // SendRequest send http request.
 func SendRequest(httpClient *http.Client, req []byte, headers map[string]string, method,
-	endpointURL string) ([]byte, error) {
+	endpointURL string,
+) ([]byte, error) {
 	var httpReq *http.Request
 
 	var err error
@@ -446,7 +449,8 @@ func newAuthTokenHeader(cmd *cobra.Command) map[string]string {
 
 // GetDuration get duration.
 func GetDuration(cmd *cobra.Command, flagName, envKey string,
-	defaultDuration time.Duration) (time.Duration, error) {
+	defaultDuration time.Duration,
+) (time.Duration, error) {
 	timeoutStr, err := cmdutil.GetUserSetVarFromString(cmd, flagName, envKey, true)
 	if err != nil {
 		return -1, err
@@ -489,7 +493,8 @@ var headerAlgorithm = map[string]string{
 
 // NewSigner return new signer.
 func NewSigner(signingkey crypto.PrivateKey, signingKeyID string, webKmsCryptoClient webcrypto.Crypto,
-	signingKeyPK crypto.PublicKey) *Signer {
+	signingKeyPK crypto.PublicKey,
+) *Signer {
 	if webKmsCryptoClient == nil {
 		switch key := signingkey.(type) {
 		case *ecdsa.PrivateKey:

cmd/orb-cli/common/common_test.go

@@ -468,7 +468,7 @@ func TestGetVDRPublicKeys(t *testing.T) {
 		file, err := os.CreateTemp("", "*.json")
 		require.NoError(t, err)
 
-		_, err = file.WriteString(fmt.Sprintf(publicKeyData, jwk1File.Name(), jwk2File.Name()))
+		_, err = fmt.Fprintf(file, publicKeyData, jwk1File.Name(), jwk2File.Name())
 		require.NoError(t, err)
 
 		defer func() {
@@ -515,7 +515,7 @@ func TestGetVDRPublicKeys(t *testing.T) {
 		file, err := os.CreateTemp("", "*.json")
 		require.NoError(t, err)
 
-		_, err = file.WriteString(fmt.Sprintf(publicKeyData, jwk1File.Name(), jwk2File.Name()))
+		_, err = fmt.Fprintf(file, publicKeyData, jwk1File.Name(), jwk2File.Name())
 		require.NoError(t, err)
 
 		defer func() {

cmd/orb-cli/createdidcmd/createdid.go

@@ -266,9 +266,7 @@ func getServices(cmd *cobra.Command) ([]did.Service, error) {
 			return nil, fmt.Errorf("failed to get services from file %w", err)
 		}
 
-		for i := range services {
-			svc = append(svc, services[i])
-		}
+		svc = append(svc, services...)
 	}
 
 	return svc, nil

cmd/orb-cli/createdidcmd/createdid_test.go

@@ -194,7 +194,7 @@ func TestCreateDID(t *testing.T) {
 	publicKeyFile, err := os.CreateTemp("", "*.json")
 	require.NoError(t, err)
 
-	_, err = publicKeyFile.WriteString(fmt.Sprintf(publickeyData, jwk1File.Name(), jwk2File.Name()))
+	_, err = fmt.Fprintf(publicKeyFile, publickeyData, jwk1File.Name(), jwk2File.Name())
 	require.NoError(t, err)
 
 	defer func() { require.NoError(t, os.Remove(publicKeyFile.Name())) }()

cmd/orb-cli/ipnshostmetauploadcmd/ipnshostmetauploadcmd.go

@@ -43,6 +43,7 @@ const (
 	timeout = 240
 )
 
+//nolint:musttag
 type object struct {
 	Hash string
 }

cmd/orb-cli/main_test.go

@@ -10,7 +10,7 @@ import (
 	"testing"
 )
 
-// Correct behaviour is for main to finish with exit code 0.
+// Correct behavior is for main to finish with exit code 0.
 // This test fails otherwise. However, this can't be checked by the unit test framework. The *testing.T argument is
 // only there so that this test gets picked up by the framework but otherwise we don't need it.
 func TestWithoutUserAgs(_ *testing.T) {

cmd/orb-cli/recoverdidcmd/recoverdid.go

@@ -346,9 +346,7 @@ func getServices(cmd *cobra.Command) ([]ariesdid.Service, error) {
 			return nil, fmt.Errorf("failed to get services from file %w", err)
 		}
 
-		for i := range services {
-			svc = append(svc, services[i])
-		}
+		svc = append(svc, services...)
 	}
 
 	return svc, nil

cmd/orb-cli/recoverdidcmd/recoverdid_test.go

@@ -153,7 +153,7 @@ func TestRecoverDID(t *testing.T) {
 	publicKeyFile, err := os.CreateTemp("", "*.json")
 	require.NoError(t, err)
 
-	_, err = publicKeyFile.WriteString(fmt.Sprintf(publickeyData, jwk1File.Name(), jwk2File.Name()))
+	_, err = fmt.Fprintf(publicKeyFile, publickeyData, jwk1File.Name(), jwk2File.Name())
 	require.NoError(t, err)
 
 	defer func() { require.NoError(t, os.Remove(publicKeyFile.Name())) }()

cmd/orb-cli/updatedidcmd/updatedid.go

@@ -301,9 +301,7 @@ func getServices(cmd *cobra.Command) ([]ariesdid.Service, error) {
 			return nil, fmt.Errorf("failed to get services from file %w", err)
 		}
 
-		for i := range services {
-			svc = append(svc, services[i])
-		}
+		svc = append(svc, services...)
 	}
 
 	return svc, nil

cmd/orb-cli/updatedidcmd/updatedid_test.go

@@ -243,7 +243,7 @@ func TestUpdateDID(t *testing.T) {
 	file, err := os.CreateTemp("", "*.json")
 	require.NoError(t, err)
 
-	_, err = file.WriteString(fmt.Sprintf(publickeyData, jwk1File.Name(), jwk2File.Name()))
+	_, err = fmt.Fprintf(file, publickeyData, jwk1File.Name(), jwk2File.Name())
 	require.NoError(t, err)
 
 	defer func() { require.NoError(t, os.Remove(file.Name())) }()

cmd/orb-cli/vctcmd/verifycmd.go

@@ -213,7 +213,8 @@ func getVerifyArgs(cmd *cobra.Command) (casURL, anchorHash, authToken string, ve
 }
 
 func getVC(cmd *cobra.Command, anchorHash, casURL string, docLoader jsonld.DocumentLoader,
-	verbose bool) (*verifiable.Credential, error) {
+	verbose bool,
+) (*verifiable.Credential, error) {
 	anchorLinksetBytes, err := common.SendHTTPRequest(cmd, nil, http.MethodGet,
 		fmt.Sprintf("%s/%s", casURL, anchorHash))
 	if err != nil {
@@ -304,7 +305,7 @@ func getVCParameters(proof verifiable.Proof) (domain string, created time.Time,
 		return "", time.Time{}, fmt.Errorf("parse 'created': %w", err)
 	}
 
-	return d.(string), createdTime, nil
+	return d.(string), createdTime, nil //nolint:forcetypeassert
 }
 
 type ldStoreProvider struct {

cmd/orb-cli/vctcmd/verifycmd_test.go

@@ -222,7 +222,8 @@ func (m *mockVCTClient) GetSTH(ctx context.Context) (*command.GetSTHResponse, er
 }
 
 func (m *mockVCTClient) GetProofByHash(ctx context.Context, hash string,
-	treeSize uint64) (*command.GetProofByHashResponse, error) {
+	treeSize uint64,
+) (*command.GetProofByHashResponse, error) {
 	return m.getProofByHashResponse, m.getProofByHashErr
 }
 

cmd/orb-driver/main_test.go

@@ -10,7 +10,7 @@ import (
 	"testing"
 )
 
-// Correct behaviour is for main to finish with exit code 0.
+// Correct behavior is for main to finish with exit code 0.
 // This test fails otherwise. However, this can't be checked by the unit test framework. The *testing.T argument is
 // only there so that this test gets picked up by the framework but otherwise we don't need it.
 func TestWithoutUserAgs(_ *testing.T) {

internal/pkg/log/fields_test.go

@@ -305,8 +305,8 @@ func TestStandardFields(t *testing.T) {
 }
 
 type mockObject struct {
-	Field1 string
-	Field2 int
+	Field1 string `json:"field1"`
+	Field2 int    `json:"field2"`
 }
 
 type logData struct {

internal/pkg/tlsutil/certpool_test.go

@@ -148,7 +148,8 @@ func TestTLSCAConfigWithMultipleCerts(t *testing.T) {
 }
 
 func verifyCertPoolInstance(t *testing.T, pool *x509.CertPool, tlsCertPool *CertPool, numberOfCertsInPool,
-	numberOfCerts, numberOfCertsByName, numberOfSubjects int, dirty int32) {
+	numberOfCerts, numberOfCertsByName, numberOfSubjects int, dirty int32,
+) {
 	t.Helper()
 
 	assert.NotNil(t, tlsCertPool)

pkg/activitypub/client/client.go

@@ -525,8 +525,10 @@ type activityIterator struct {
 	appendActivity appendFunc
 }
 
-func newActivityIterator(ctx context.Context, items []*vocab.ActivityType, currentPage, nextPage *url.URL, totalItems int,
-	get getFunc, getNext getNextIRIFunc, appendActivity appendFunc) *activityIterator {
+func newActivityIterator(ctx context.Context,
+	items []*vocab.ActivityType, currentPage, nextPage *url.URL, totalItems int,
+	get getFunc, getNext getNextIRIFunc, appendActivity appendFunc,
+) *activityIterator {
 	return &activityIterator{
 		ctx:            ctx,
 		currentItems:   items,
@@ -640,8 +642,10 @@ func (it *activityIterator) getNextPage() error {
 	return nil
 }
 
-func newForwardActivityIterator(ctx context.Context, items []*vocab.ActivityType, currentPage, nextPage *url.URL,
-	totalItems int, retrieve getFunc) *activityIterator {
+func newForwardActivityIterator(ctx context.Context,
+	items []*vocab.ActivityType, currentPage, nextPage *url.URL,
+	totalItems int, retrieve getFunc,
+) *activityIterator {
 	return newActivityIterator(ctx, items, currentPage, nextPage, totalItems, retrieve,
 		func(next, _ *url.URL) *url.URL {
 			return next
@@ -652,8 +656,10 @@ func newForwardActivityIterator(ctx context.Context, items []*vocab.ActivityType
 	)
 }
 
-func newReverseActivityIterator(ctx context.Context, items []*vocab.ActivityType, currentPage, nextPage *url.URL,
-	totalItems int, retrieve getFunc) *activityIterator {
+func newReverseActivityIterator(ctx context.Context,
+	items []*vocab.ActivityType, currentPage, nextPage *url.URL,
+	totalItems int, retrieve getFunc,
+) *activityIterator {
 	return newActivityIterator(ctx, reverseSort(items), currentPage, nextPage, totalItems, retrieve,
 		func(_, prev *url.URL) *url.URL {
 			return prev
@@ -665,8 +671,9 @@ func newReverseActivityIterator(ctx context.Context, items []*vocab.ActivityType
 	)
 }
 
-func unmarshalCollection(respBytes []byte) (items []*vocab.ObjectProperty, firstPage, lastPage *url.URL,
-	totalCount int, err error) {
+func unmarshalCollection(respBytes []byte) (items []*vocab.ObjectProperty, firstPage,
+	lastPage *url.URL, totalCount int, err error,
+) {
 	obj := &vocab.ObjectType{}
 
 	if err := json.Unmarshal(respBytes, &obj); err != nil {