*: enable and fix unparam lint

[fuweid]

Part of #15549

Highlight of changes:

1. transport/listener_test.go:44:32: fakeCertificateParserFunc - cert always receives tls.Certificate{} (nil)

fakeCertificateParserFunc returns tls.Certificate{} by default.

2. retry_interceptor.go:381:22: withRetryPolicy - rp always receives repeatable (0) (unparam) func withRetryPolicy(rp retryPolicy) retryOption {

Uses the following code to fix

func withRepeatablePolicy() retryOption {
	return retryOption{applyFunc: func(o *options) {
		o.retryPolicy = repeatable
	}}
}

3. history.go:349:22: staleGetRequest - key always receives "key" (unparam) func staleGetRequest(key string, revision int64) EtcdRequest {

Changed one model operation's key from key to key1.

4. member_test.go:84:40: newTestMemberAsLearner - peerURLs always receives nil (unparam) func newTestMemberAsLearner(id uint64, peerURLs []string, name string, clientURLs []string) *Member {

Changed nil to []string{} to fix the lint issue.

diff --git a/server/etcdserver/api/membership/cluster_test.go b/server/etcdserver/api/membership/cluster_test.go
index a3ad87013db..b06d3302475 100644
--- a/server/etcdserver/api/membership/cluster_test.go
+++ b/server/etcdserver/api/membership/cluster_test.go
@@ -544,7 +544,7 @@ func TestClusterAddMemberAsLearner(t *testing.T) {
 	st := mockstore.NewRecorder()
 	c := newTestCluster(t, nil)
 	c.SetStore(st)
-	c.AddMember(newTestMemberAsLearner(1, nil, "node1", nil), true)
+	c.AddMember(newTestMemberAsLearner(1, []string{}, "node1", []string{"http://node1"}), true)
 
 	wactions := []testutil.Action{
 		{
@@ -552,7 +552,7 @@ func TestClusterAddMemberAsLearner(t *testing.T) {
 			Params: []any{
 				path.Join(StoreMembersPrefix, "1", "raftAttributes"),
 				false,
-				`{"peerURLs":null,"isLearner":true}`,
+				`{"peerURLs":[],"isLearner":true}`,
 				false,
 				v2store.TTLOptionSet{ExpireTime: v2store.Permanent},
 			},

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

[serathius]

Note, this is new check that was not enabled before, so this is not part of #16610 and should start from community discussion whether we want to adopt unparam check at all.

[fuweid]

OK. Let me fix all the existing lint issues and then discuss this one.

[fuweid]

ping @ahrtr @serathius @jmhbnz

[serathius]

This is not the same, it changes the exit code and the output.

[ahrtr]

+1 let's try not to change anything unrelated to this PR.

[fuweid]

lg.Fatal exits with code 1. Yes, I should not update the output.
Updated with removing the lg *zap.Logger since no function uses lg.

[ahrtr]

LGTM

thx @fuweid

[jmhbnz]

LGTM - Great to tidy and tighten this up, thanks @fuweid.

[fuweid]

Is it ready to move on?

[fuweid]

ping @serathius @ahrtr

[ahrtr]

ping @serathius @ahrtr

Sorry for the delay. I already approved the PR, and leave it to @serathius to take another look and merge, since he raised comment/concern #16622 (comment) previously

[fuweid]

Thanks for the comment.

As far as I know, the test script has been refactored several times and I believe we miss some of them. The unparam_pass isn't introduced in this pr. The pr just enables it.

The lint did find some unparam issues. It has strict rule, especially when we write test code. But it could help us to find some issues in test code.

In my opinion, based on the fact that it's not new lint in the repo, we can enable it and evaluate it for a while. If it's annoying, we can consider to disable it.

[fuweid]

ping @serathius ~

[fuweid]

Thanks! @serathius