Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce inlining functions with a single error return in require.NoError #1500

Merged
merged 71 commits into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
a2771ba
ban `require.Equal` when testing for `0`
dhrubabasu May 12, 2023
88f6841
Merge branch 'dev' into ban-require-equal-zero
dhrubabasu May 12, 2023
b065a98
ban usage of `require.Zero` when testing for length `0`
dhrubabasu May 12, 2023
0ccf063
Merge branch 'ban-require-equal-zero' into ban-require-zero-len
dhrubabasu May 12, 2023
fd714b1
add failing test
dhrubabasu May 12, 2023
d4d925b
fix
dhrubabasu May 12, 2023
e961d37
nit
dhrubabasu May 12, 2023
7aab521
Merge branch 'ban-require-zero-len' into ban-require-equal-len
dhrubabasu May 12, 2023
cfd7ca0
push
dhrubabasu May 12, 2023
946588a
ban usage of `nil` in require functions
dhrubabasu May 12, 2023
5fcf0f0
nit
dhrubabasu May 12, 2023
10dff95
fix regex
dhrubabasu May 12, 2023
9fdc7c5
nit
dhrubabasu May 12, 2023
c343bc9
Merge branch 'dev' into ban-require-equal-zero
StephenButtolph May 12, 2023
3dbdab1
merged
dhrubabasu May 12, 2023
5530818
Merge branch 'ban-require-zero-len' into ban-require-equal-len
dhrubabasu May 12, 2023
6b2a170
Merge branch 'ban-require-equal-len' into ban-require-nil
dhrubabasu May 12, 2023
421b72d
add comment
dhrubabasu May 12, 2023
0c919fd
match uint(0)
dhrubabasu May 12, 2023
f8c4359
Update scripts/lint.sh
dhrubabasu May 12, 2023
df6b971
Merge branch 'ban-require-equal-zero' into ban-require-zero-len
dhrubabasu May 12, 2023
4a37e01
merged
dhrubabasu May 12, 2023
7e40d38
nit
dhrubabasu May 12, 2023
fb10a1a
pr review
dhrubabasu May 12, 2023
7aadaf4
revert
dhrubabasu May 12, 2023
af4382d
nit
dhrubabasu May 12, 2023
5301a7d
nits
dhrubabasu May 12, 2023
e249d2b
reduce diff
dhrubabasu May 12, 2023
be82305
reduce diff
dhrubabasu May 12, 2023
8aa012d
reduce diff
dhrubabasu May 12, 2023
24fb051
nit
dhrubabasu May 12, 2023
7de1858
Merge branch 'ban-require-equal-len' into ban-require-nil
dhrubabasu May 12, 2023
dfeb84b
more nil checks
dhrubabasu May 12, 2023
7901762
wording
dhrubabasu May 12, 2023
2f1ba2d
regex nit
dhrubabasu May 12, 2023
90b376e
Merge branch 'ban-require-equal-len' into ban-require-nil
dhrubabasu May 12, 2023
4680df5
add linter
dhrubabasu May 13, 2023
e967771
fix
dhrubabasu May 13, 2023
ff94591
fix regex
dhrubabasu May 13, 2023
0aa2d36
moar
dhrubabasu May 13, 2023
720bfb0
nit
dhrubabasu May 13, 2023
b71ff1c
include multi-line functions
dhrubabasu May 13, 2023
31e4a37
should be good now
dhrubabasu May 13, 2023
c7226cf
moar
dhrubabasu May 13, 2023
e48fbb5
nit
dhrubabasu May 13, 2023
488d1a8
nit
dhrubabasu May 13, 2023
f17e674
fix
dhrubabasu May 13, 2023
a96a2d1
fix regex
dhrubabasu May 13, 2023
31fa556
merged
dhrubabasu May 15, 2023
5e3745a
Merge branch 'ban-require-zero-len' into ban-require-equal-len
StephenButtolph May 15, 2023
6709b1c
Update scripts/lint.sh
dhrubabasu May 16, 2023
5d2f1e2
Update scripts/lint.sh
dhrubabasu May 16, 2023
0457148
Merge branch 'dev' into ban-require-zero-len
dhrubabasu May 16, 2023
f4f0c15
nit
dhrubabasu May 16, 2023
62d168e
nit
dhrubabasu May 16, 2023
e7cf371
regex is easy they said
dhrubabasu May 16, 2023
b9d80b3
revert regex changes
dhrubabasu May 16, 2023
d1d0fdc
revert window change
dhrubabasu May 16, 2023
38e5742
nit
dhrubabasu May 16, 2023
a4bca48
Merge branch 'dev' into ban-require-zero-len
dhrubabasu May 16, 2023
f824d68
Merge branch 'ban-require-zero-len' into ban-require-equal-len
dhrubabasu May 16, 2023
adf3e39
Merge branch 'ban-require-equal-len' into ban-require-nil
dhrubabasu May 16, 2023
b58df9d
merged
dhrubabasu May 16, 2023
b93c523
reduce diff
dhrubabasu May 16, 2023
3402670
merged
dhrubabasu May 17, 2023
51b303b
Merge branch 'dev' into enforce-require-no-error-inline
dhrubabasu May 18, 2023
dc4c374
merged
StephenButtolph May 26, 2023
115dfcb
Merge branch 'dev' into enforce-require-no-error-inline
StephenButtolph May 27, 2023
c2a7868
add comment that confused me
StephenButtolph May 30, 2023
66217b4
nit
StephenButtolph May 30, 2023
6c18e3e
Merge branch 'dev' into enforce-require-no-error-inline
StephenButtolph May 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
ban usage of nil in require functions
  • Loading branch information
dhrubabasu committed May 12, 2023
commit 946588af6df14154af3527c3147fdf4b87831a5e
2 changes: 1 addition & 1 deletion api/keystore/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func TestServiceExportImport(t *testing.T) {
User: exportReply.User,
Encoding: encoding,
}, &api.EmptyReply{})
require.ErrorIs(err, nil)
require.NoError(err)
}

{
Expand Down
11 changes: 10 additions & 1 deletion scripts/lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -91,6 +91,15 @@ function test_require_equal_len {
fi
}

function test_require_nil {
if grep -R -zo -P 'require.+?nil\)\n' .; then
echo ""
echo "Use a require function to test for 'nil'"
echo ""
return 1
fi
}

# Ref: https://go.dev/doc/effective_go#blank_implements
function test_interface_compliance_nil {
if grep -R -o -P '_ .+? = &.+?\{\}' .; then
Expand Down
2 changes: 1 addition & 1 deletion snow/engine/snowman/syncer/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion vms/avm/txs/mempool/mempool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions vms/platformvm/blocks/builder/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
35 changes: 18 additions & 17 deletions vms/platformvm/txs/executor/create_subnet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
})
}
}
6 changes: 3 additions & 3 deletions vms/platformvm/txs/mempool/mempool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down