chore(lint): check each //nolint and improve them (#2047)

- remove unneeded `//nolint` comments
- fix some of the lint-ignored errors
- specify a linter that should be ignored as `//nolint:lintername`
- add `nolintlint` linter to detect future unused `//nolint` comments
- move exclude rules from golangci.yml to inlined `//nolint` so `nolintlint` can pick them up.

.golangci.yml

@@ -87,6 +87,7 @@ linters:
     - megacheck
     - megacheck
     - misspell
+    - nolintlint
     - revive
     - staticcheck
     - unconvert
@@ -125,41 +126,30 @@ issues:
       linters:
         - govet
 
-    # Exclude known linters from partially hard-vendored code,
-    # which is impossible to exclude via "nolint" comments.
-    - path: internal/hmac/
-      text: "weak cryptographic primitive"
+    - text: "`finalized` is a misspelling of `finalised`"
       linters:
-        - gosec
+        - misspell
 
-    # Exclude some staticcheck messages
-    - linters:
-        - staticcheck
-      text: "SA9003:"
+    - text: "`finalize` is a misspelling of `finalise`"
+      linters:
+        - misspell
 
-    - linters:
-        - gosimple
-      text: "S1025:"
+    - text: "`initialize` is a misspelling of `initialise`"
+      linters:
+        - misspell
 
     - linters:
         - revive
       text: "package comment should be of the form"
 
     - linters:
         - revive
-      text: "don't use ALL_CAPS in Go names;"
-
-    - linters:
-        - revive
+      path: lib/runtime/life/
       text: "don't use underscores in Go names;"
 
     - linters:
-        - gocyclo
-      text: "cyclomatic complexity 36 of func " #runtime/imports.go: registerImports
-
-    - linters:
-        - gocyclo
-      text: "cyclomatic complexity 42 of func " # codec/decode.go: (*Decoder).DecodeTuple
+        - nolintlint
+      source: "^//nolint:revive"
 
     # Exclude lll issues for long lines with go:generate
     - linters:

cmd/gossamer/config.go

@@ -26,8 +26,8 @@ import (
 	"github.com/urfave/cli"
 )
 
-//nolint
 var (
+	// DefaultCfg is the default configuration for the node.
 	DefaultCfg                = dot.GssmrConfig
 	defaultGssmrConfigPath    = "./chain/gssmr/config.toml"
 	defaultKusamaConfigPath   = "./chain/kusama/config.toml"

cmd/gossamer/prune_test.go

@@ -12,32 +12,19 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func iterateDB(db *badger.DB, cb func(*badger.Item)) { //nolint
-	txn := db.NewTransaction(false)
-	itr := txn.NewIterator(badger.DefaultIteratorOptions)
-
-	for itr.Rewind(); itr.Valid(); itr.Next() {
-		cb(itr.Item())
-	}
-}
-
-func runPruneCmd(t *testing.T, configFile, prunedDBPath string) { //nolint
-	ctx, err := newTestContext(
-		"Test state trie offline pruning  --prune-state",
-		[]string{"config", "pruned-db-path", "bloom-size", "retain-blocks"},
-		[]interface{}{configFile, prunedDBPath, "256", int64(5)},
-	)
-	require.NoError(t, err)
-
-	command := pruningCommand
-	err = command.Run(ctx)
-	require.NoError(t, err)
-}
-
 func TestPruneState(t *testing.T) {
 	t.Skip() // this fails due to being unable to call blockState.GetHighestFinalisedHash() when initialising the blockstate
 	// need to regenerate the test database and/or move this to the state package (which would make sense)
 
+	iterateDB := func(db *badger.DB, cb func(*badger.Item)) {
+		txn := db.NewTransaction(false)
+		itr := txn.NewIterator(badger.DefaultIteratorOptions)
+
+		for itr.Rewind(); itr.Valid(); itr.Next() {
+			cb(itr.Item())
+		}
+	}
+
 	var (
 		inputDBPath   = "../../tests/data/db"
 		configFile    = "../../tests/data/db/config.toml"
@@ -67,7 +54,17 @@ func TestPruneState(t *testing.T) {
 	t.Log("Total keys in input DB", numStorageKeys+len(nonStorageKeys), "storage keys", numStorageKeys)
 	t.Log("pruned DB path", prunedDBPath)
 
-	runPruneCmd(t, configFile, prunedDBPath)
+	// Run Prune command
+	ctx, err := newTestContext(
+		"Test state trie offline pruning  --prune-state",
+		[]string{"config", "pruned-db-path", "bloom-size", "retain-blocks"},
+		[]interface{}{configFile, prunedDBPath, "256", int64(5)},
+	)
+	require.NoError(t, err)
+
+	command := pruningCommand
+	err = command.Run(ctx)
+	require.NoError(t, err)
 
 	prunedDB, err := badger.Open(badger.DefaultOptions(prunedDBPath))
 	require.NoError(t, err)

dot/core/messages.go

@@ -46,7 +46,7 @@ func (s *Service) HandleTransactionMessage(peerID peer.ID, msg *network.Transact
 			s.storageState.Lock()
 			defer s.storageState.Unlock()
 
-			ts, err := s.storageState.TrieState(&head.StateRoot) //nolint
+			ts, err := s.storageState.TrieState(&head.StateRoot)
 			if err != nil {
 				return err
 			}

dot/core/test_helpers.go

@@ -120,7 +120,7 @@ func NewTestService(t *testing.T, cfg *Config) *Service {
 	if cfg.CodeSubstitutes == nil {
 		cfg.CodeSubstitutes = make(map[common.Hash]string)
 
-		genesisData, err := cfg.CodeSubstitutedState.(*state.BaseState).LoadGenesisData() //nolint
+		genesisData, err := cfg.CodeSubstitutedState.(*state.BaseState).LoadGenesisData()
 		require.NoError(t, err)
 
 		for k, v := range genesisData.CodeSubstitutes {

dot/network/discovery_test.go

@@ -47,8 +47,7 @@ func newTestDiscovery(t *testing.T, num int) []*discovery {
 	return discs
 }
 
-// nolint
-func connectNoSync(t *testing.T, ctx context.Context, a, b *discovery) {
+func connectNoSync(ctx context.Context, t *testing.T, a, b *discovery) {
 	t.Helper()
 
 	idB := b.h.ID()
@@ -82,7 +81,7 @@ func TestKadDHT(t *testing.T) {
 	defer cancel()
 
 	// connects node 0 and node 2
-	connectNoSync(t, ctx, nodes[2], nodes[0])
+	connectNoSync(ctx, t, nodes[2], nodes[0])
 
 	time.Sleep(startDHTTimeout + 1)
 
@@ -91,7 +90,7 @@ func TestKadDHT(t *testing.T) {
 	require.ErrorIs(t, err, routing.ErrNotFound)
 
 	// connects node 1 and node 0
-	connectNoSync(t, ctx, nodes[1], nodes[0])
+	connectNoSync(ctx, t, nodes[1], nodes[0])
 
 	time.Sleep(startDHTTimeout + 1)
 

dot/network/gossip.go

@@ -24,7 +24,7 @@ func newGossip() *gossip {
 }
 
 // hasSeen broadcasts messages that have not been seen
-func (g *gossip) hasSeen(msg NotificationsMessage) bool { //nolint
+func (g *gossip) hasSeen(msg NotificationsMessage) bool {
 	// check if message has not been seen
 	if seen, ok := g.seen.Load(msg.Hash()); !ok || !seen.(bool) {
 		// set message to has been seen

dot/network/host.go

@@ -109,7 +109,7 @@ func newHost(ctx context.Context, cfg *Config) (*host, error) {
 
 	privateIPs := ma.NewFilters()
 	for _, cidr := range privateCIDRs {
-		_, ipnet, err := net.ParseCIDR(cidr) //nolint
+		_, ipnet, err := net.ParseCIDR(cidr)
 		if err != nil {
 			return nil, err
 		}

dot/network/host_test.go

@@ -32,7 +32,7 @@ func TestExternalAddrs(t *testing.T) {
 	addrInfo := node.host.addrInfo()
 	privateIPs := ma.NewFilters()
 	for _, cidr := range privateCIDRs {
-		_, ipnet, err := net.ParseCIDR(cidr) //nolint
+		_, ipnet, err := net.ParseCIDR(cidr)
 		require.NoError(t, err)
 		privateIPs.AddFilter(*ipnet, ma.ActionDeny)
 	}

dot/network/message.go

@@ -39,7 +39,7 @@ type NotificationsMessage interface {
 	IsHandshake() bool
 }
 
-// nolint
+//nolint:revive
 const (
 	RequestedDataHeader        = byte(1)
 	RequestedDataBody          = byte(2)

dot/network/service.go

@@ -94,7 +94,7 @@ type Service struct {
 
 // NewService creates a new network service from the configuration and message channels
 func NewService(cfg *Config) (*Service, error) {
-	ctx, cancel := context.WithCancel(context.Background()) //nolint
+	ctx, cancel := context.WithCancel(context.Background())
 
 	logger.Patch(log.SetLevel(cfg.LogLvl))
 	cfg.logger = logger
@@ -103,7 +103,7 @@ func NewService(cfg *Config) (*Service, error) {
 	err := cfg.build()
 	if err != nil {
 		cancel()
-		return nil, err //nolint
+		return nil, err
 	}
 
 	if cfg.MinPeers == 0 {
@@ -132,8 +132,8 @@ func NewService(cfg *Config) (*Service, error) {
 	}
 
 	// pre-allocate pool of buffers used to read from streams.
-	// initially allocate as many buffers as liekly necessary which is the number inbound streams we will have,
-	// which should equal average number of peers times the number of notifications protocols, which is currently 3.
+	// initially allocate as many buffers as likely necessary which is the number of inbound streams we will have,
+	// which should equal the average number of peers times the number of notifications protocols, which is currently 3.
 	preAllocateInPool := cfg.MinPeers * 3
 	poolSize := cfg.MaxPeers * 3
 	if cfg.noPreAllocate { // testing
@@ -407,17 +407,17 @@ func (s *Service) sentBlockIntervalTelemetry() {
 		}
 		bestHash := best.Hash()
 
-		finalized, err := s.blockState.GetHighestFinalisedHeader() //nolint
+		finalised, err := s.blockState.GetHighestFinalisedHeader()
 		if err != nil {
 			continue
 		}
-		finalizedHash := finalized.Hash()
+		finalizedHash := finalised.Hash()
 
 		err = telemetry.GetInstance().SendMessage(telemetry.NewBlockIntervalTM(
 			&bestHash,
 			best.Number,
 			&finalizedHash,
-			finalized.Number,
+			finalised.Number,
 			big.NewInt(int64(s.transactionHandler.TransactionsCount())),
 			big.NewInt(0), // TODO: (ed) determine where to get used_state_cache_size (#1501)
 		))

dot/network/utils.go

@@ -58,7 +58,7 @@ func generateKey(seed int64, fp string) (crypto.PrivKey, error) {
 	if seed == 0 {
 		r = crand.Reader
 	} else {
-		r = mrand.New(mrand.NewSource(seed)) //nolint
+		r = mrand.New(mrand.NewSource(seed)) //nolint:gosec
 	}
 	key, _, err := crypto.GenerateEd25519Key(r)
 	if err != nil {

dot/peerset/test_helpers.go

@@ -49,7 +49,7 @@ func newTestPeerSet(t *testing.T, in, out uint32, bootNodes, reservedPeers []pee
 	return handler
 }
 
-func newTestPeerState(t *testing.T, maxIn, maxOut uint32) *PeersState { //nolint
+func newTestPeerState(t *testing.T, maxIn, maxOut uint32) *PeersState {
 	t.Helper()
 	state, err := NewPeerState([]*config{
 		{

dot/rpc/modules/author_test.go

@@ -178,7 +178,6 @@ func TestAuthorModule_SubmitExtrinsic(t *testing.T) {
 	var testExt = common.MustHexToBytes("0x410284ffd43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d01f8efbe48487e57a22abf7e3acd491b7f3528a33a111b1298601554863d27eb129eaa4e718e1365414ff3d028b62bebc651194c6b5001e5c2839b982757e08a8c0000000600ff8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a480b00c465f14670")
 
 	// invalid transaction (above tx, with last byte changed)
-	//nolint
 	var testInvalidExt = []byte{1, 212, 53, 147, 199, 21, 253, 211, 28, 97, 20, 26, 189, 4, 169, 159, 214, 130, 44, 133, 88, 133, 76, 205, 227, 154, 86, 132, 231, 165, 109, 162, 125, 142, 175, 4, 21, 22, 135, 115, 99, 38, 201, 254, 161, 126, 37, 252, 82, 135, 97, 54, 147, 201, 18, 144, 156, 178, 38, 170, 71, 148, 242, 106, 72, 69, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 216, 5, 113, 87, 87, 40, 221, 120, 247, 252, 137, 201, 74, 231, 222, 101, 85, 108, 102, 39, 31, 190, 210, 14, 215, 124, 19, 160, 180, 203, 54, 110, 167, 163, 149, 45, 12, 108, 80, 221, 65, 238, 57, 237, 199, 16, 10, 33, 185, 8, 244, 184, 243, 139, 5, 87, 252, 245, 24, 225, 37, 154, 163, 143}
 
 	type fields struct {

dot/rpc/modules/state.go

@@ -173,7 +173,7 @@ func (sm *StateModule) GetPairs(_ *http.Request, req *StatePairRequest, res *Sta
 	}
 
 	if req.Prefix == nil || *req.Prefix == "" || *req.Prefix == "0x" {
-		pairs, err := sm.storageAPI.Entries(stateRootHash) //nolint
+		pairs, err := sm.storageAPI.Entries(stateRootHash)
 		if err != nil {
 			return err
 		}

dot/rpc/modules/system.go

@@ -259,7 +259,7 @@ func (sm *SystemModule) LocalListenAddresses(r *http.Request, req *EmptyRequest,
 }
 
 // LocalPeerId Returns the base58-encoded PeerId fo the node.
-func (sm *SystemModule) LocalPeerId(r *http.Request, req *EmptyRequest, res *string) error { //nolint
+func (sm *SystemModule) LocalPeerId(r *http.Request, req *EmptyRequest, res *string) error {
 	netstate := sm.networkAPI.NetworkState()
 	if netstate.PeerID == "" {
 		return errors.New("peer id cannot be empty")

dot/rpc/subscription/websocket.go

@@ -30,9 +30,6 @@ var errCannotReadFromWebsocket = errors.New("cannot read message from websocket"
 var errCannotUnmarshalMessage = errors.New("cannot unmarshal webasocket message data")
 var logger = log.NewFromGlobal(log.AddContext("pkg", "rpc/subscription"))
 
-// DEFAULT_BUFFER_SIZE buffer size for channels
-const DEFAULT_BUFFER_SIZE = 100
-
 // WSConn struct to hold WebSocket Connection references
 type WSConn struct {
 	UnsafeEnabled bool
@@ -97,7 +94,7 @@ func (c *WSConn) HandleComm() {
 				continue
 			}
 
-			listener, err := setupListener(reqid, params) //nolint
+			listener, err := setupListener(reqid, params)
 			if err != nil {
 				logger.Warnf("failed to create listener (method=%s): %s", method, err)
 				continue
@@ -107,7 +104,7 @@ func (c *WSConn) HandleComm() {
 			continue
 		}
 
-		listener, err := c.getUnsubListener(params) //nolint
+		listener, err := c.getUnsubListener(params)
 
 		if err != nil {
 			logger.Warnf("failed to get unsubscriber (method=%s): %s", method, err)

dot/services.go

@@ -88,7 +88,7 @@ func createRuntime(cfg *Config, ns runtime.NodeStorage, st *state.Service, ks *k
 
 	if !codeSubHash.IsEmpty() {
 		logger.Infof("🔄 detected runtime code substitution, upgrading to block hash %s...", codeSubHash)
-		genData, err := st.Base.LoadGenesisData() // nolint
+		genData, err := st.Base.LoadGenesisData()
 		if err != nil {
 			return nil, err
 		}

dot/state/block.go

@@ -611,7 +611,7 @@ func (bs *BlockState) HandleRuntimeChanges(newState *rtstorage.TrieState, rt run
 	codeSubBlockHash := bs.baseState.LoadCodeSubstitutedBlockHash()
 
 	if !codeSubBlockHash.Equal(common.Hash{}) {
-		newVersion, err := rt.CheckRuntimeVersion(code) //nolint
+		newVersion, err := rt.CheckRuntimeVersion(code)
 		if err != nil {
 			return err
 		}

dot/state/block_notify.go

@@ -13,26 +13,24 @@ import (
 	"github.com/google/uuid"
 )
 
-// DEFAULT_BUFFER_SIZE buffer size for channels
-const DEFAULT_BUFFER_SIZE = 128
+const defaultBufferSize = 128
 
 // GetImportedBlockNotifierChannel function to retrieve a imported block notifier channel
 func (bs *BlockState) GetImportedBlockNotifierChannel() chan *types.Block {
 	bs.importedLock.Lock()
 	defer bs.importedLock.Unlock()
 
-	ch := make(chan *types.Block, DEFAULT_BUFFER_SIZE)
+	ch := make(chan *types.Block, defaultBufferSize)
 	bs.imported[ch] = struct{}{}
 	return ch
 }
 
-//nolint
-// GetFinalisedNotifierChannel function to retrieve a finalized block notifier channel
+// GetFinalisedNotifierChannel function to retrieve a finalised block notifier channel
 func (bs *BlockState) GetFinalisedNotifierChannel() chan *types.FinalisationInfo {
 	bs.finalisedLock.Lock()
 	defer bs.finalisedLock.Unlock()
 
-	ch := make(chan *types.FinalisationInfo, DEFAULT_BUFFER_SIZE)
+	ch := make(chan *types.FinalisationInfo, defaultBufferSize)
 	bs.finalised[ch] = struct{}{}
 
 	return ch
@@ -45,8 +43,7 @@ func (bs *BlockState) FreeImportedBlockNotifierChannel(ch chan *types.Block) {
 	delete(bs.imported, ch)
 }
 
-//nolint
-// FreeFinalisedNotifierChannel to free finalized notifier channel
+// FreeFinalisedNotifierChannel to free finalised notifier channel
 func (bs *BlockState) FreeFinalisedNotifierChannel(ch chan *types.FinalisationInfo) {
 	bs.finalisedLock.Lock()
 	defer bs.finalisedLock.Unlock()