Ban usage of nil in require functions (ava-labs#1498)

Author: dhrubabasu

api/keystore/service_test.go

@@ -251,7 +251,7 @@ func TestServiceExportImport(t *testing.T) {
 				User:     exportReply.User,
 				Encoding: encoding,
 			}, &api.EmptyReply{})
-			require.ErrorIs(err, nil)
+			require.NoError(err)
 		}
 
 		{

database/test_database.go

@@ -921,7 +921,8 @@ func TestCompactNoPanic(t *testing.T, db Database) {
 
 	require.NoError(db.Compact(nil, nil))
 	require.NoError(db.Close())
-	require.Equal(ErrClosed, db.Compact(nil, nil))
+	err := db.Compact(nil, nil)
+	require.ErrorIs(err, ErrClosed)
 }
 
 // TestClear tests to make sure the deletion helper works as expected.

scripts/lint.sh

@@ -21,7 +21,7 @@ fi
 # by default, "./scripts/lint.sh" runs all lint tests
 # to run only "license_header" test
 # TESTS='license_header' ./scripts/lint.sh
-TESTS=${TESTS:-"golangci_lint license_header require_error_is_no_funcs_as_params single_import interface_compliance_nil require_equal_zero require_len_zero require_equal_len"}
+TESTS=${TESTS:-"golangci_lint license_header require_error_is_no_funcs_as_params single_import interface_compliance_nil require_equal_zero require_len_zero require_equal_len require_nil"}
 
 function test_golangci_lint {
   go install -v github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.2
@@ -102,6 +102,29 @@ function test_require_equal_len {
   fi
 }
 
+function test_require_nil {
+  if grep -R -o -P 'require\..+?!= nil' .; then
+    echo ""
+    echo "Use require.NotNil when testing for nil inequality."
+    echo ""
+    return 1
+  fi
+
+  if grep -R -o -P 'require\..+?== nil' .; then
+    echo ""
+    echo "Use require.Nil when testing for nil equality."
+    echo ""
+    return 1
+  fi
+
+  if grep -R -o -P 'require\.ErrorIs.+?nil\)' .; then
+    echo ""
+    echo "Use require.NoError instead of require.ErrorIs when testing for nil error."
+    echo ""
+    return 1
+  fi
+}
+
 # Ref: https://go.dev/doc/effective_go#blank_implements
 function test_interface_compliance_nil {
   if grep -R -o -P '_ .+? = &.+?\{\}' .; then

snow/engine/snowman/syncer/utils_test.go

@@ -91,7 +91,7 @@ func buildTestsObjects(t *testing.T, commonCfg *common.Config) (
 	})
 	require.IsType(t, &stateSyncer{}, commonSyncer)
 	syncer := commonSyncer.(*stateSyncer)
-	require.True(t, syncer.stateSyncVM != nil)
+	require.NotNil(t, syncer.stateSyncVM)
 
 	fullVM.GetOngoingSyncStateSummaryF = func(context.Context) (block.StateSummary, error) {
 		return nil, database.ErrNotFound

vms/avm/txs/mempool/mempool_test.go

@@ -83,7 +83,7 @@ func TestTxsInMempool(t *testing.T) {
 		require.True(mempool.Has(txID))
 
 		retrieved := mempool.Get(txID)
-		require.True(retrieved != nil)
+		require.NotNil(retrieved)
 		require.Equal(tx, retrieved)
 
 		// tx exists in mempool

vms/platformvm/blocks/builder/network_test.go

@@ -67,7 +67,7 @@ func TestMempoolValidGossipedTxIsAddedToMempool(t *testing.T) {
 	env.ctx.Lock.Lock()
 
 	// and gossiped if it has just been discovered
-	require.True(gossipedBytes != nil)
+	require.NotNil(gossipedBytes)
 
 	// show gossiped bytes can be decoded to the original tx
 	replyIntf, err := message.Parse(gossipedBytes)
@@ -129,7 +129,7 @@ func TestMempoolNewLocaTxIsGossiped(t *testing.T) {
 
 	err := env.Builder.AddUnverifiedTx(tx)
 	require.NoError(err)
-	require.True(gossipedBytes != nil)
+	require.NotNil(gossipedBytes)
 
 	// show gossiped bytes can be decoded to the original tx
 	replyIntf, err := message.Parse(gossipedBytes)

vms/platformvm/txs/executor/create_subnet_test.go

@@ -14,34 +14,35 @@ import (
 	"github.com/ava-labs/avalanchego/vms/components/avax"
 	"github.com/ava-labs/avalanchego/vms/platformvm/state"
 	"github.com/ava-labs/avalanchego/vms/platformvm/txs"
+	"github.com/ava-labs/avalanchego/vms/platformvm/utxo"
 	"github.com/ava-labs/avalanchego/vms/secp256k1fx"
 )
 
 func TestCreateSubnetTxAP3FeeChange(t *testing.T) {
 	ap3Time := defaultGenesisTime.Add(time.Hour)
 	tests := []struct {
-		name         string
-		time         time.Time
-		fee          uint64
-		expectsError bool
+		name        string
+		time        time.Time
+		fee         uint64
+		expectedErr error
 	}{
 		{
-			name:         "pre-fork - correctly priced",
-			time:         defaultGenesisTime,
-			fee:          0,
-			expectsError: false,
+			name:        "pre-fork - correctly priced",
+			time:        defaultGenesisTime,
+			fee:         0,
+			expectedErr: nil,
 		},
 		{
-			name:         "post-fork - incorrectly priced",
-			time:         ap3Time,
-			fee:          100*defaultTxFee - 1*units.NanoAvax,
-			expectsError: true,
+			name:        "post-fork - incorrectly priced",
+			time:        ap3Time,
+			fee:         100*defaultTxFee - 1*units.NanoAvax,
+			expectedErr: utxo.ErrInsufficientUnlockedFunds,
 		},
 		{
-			name:         "post-fork - correctly priced",
-			time:         ap3Time,
-			fee:          100 * defaultTxFee,
-			expectsError: false,
+			name:        "post-fork - correctly priced",
+			time:        ap3Time,
+			fee:         100 * defaultTxFee,
+			expectedErr: nil,
 		},
 	}
 	for _, test := range tests {
@@ -82,7 +83,7 @@ func TestCreateSubnetTxAP3FeeChange(t *testing.T) {
 				Tx:      tx,
 			}
 			err = tx.Unsigned.Visit(&executor)
-			require.Equal(test.expectsError, err != nil)
+			require.ErrorIs(err, test.expectedErr)
 		})
 	}
 }

vms/platformvm/txs/mempool/mempool_test.go

@@ -79,7 +79,7 @@ func TestDecisionTxsInMempool(t *testing.T) {
 		require.True(mpool.Has(tx.ID()))
 
 		retrieved := mpool.Get(tx.ID())
-		require.True(retrieved != nil)
+		require.NotNil(retrieved)
 		require.Equal(tx, retrieved)
 
 		// we can peek it
@@ -134,13 +134,13 @@ func TestProposalTxsInMempool(t *testing.T) {
 		require.True(mpool.Has(tx.ID()))
 
 		retrieved := mpool.Get(tx.ID())
-		require.True(retrieved != nil)
+		require.NotNil(retrieved)
 		require.Equal(tx, retrieved)
 
 		{
 			// we can peek it
 			peeked := mpool.PeekStakerTx()
-			require.True(peeked != nil)
+			require.NotNil(peeked)
 			require.Equal(tx, peeked)
 		}