Skip to content

Commit 83e0fee

Browse files
LEILEI0628yaolei_cao
andauthored
Fix 137 golangci lint errors (#138)
* fix: resolve 26 errcheck linter errors (#137) - Fix implicit error ignores in tests - Add fail-fast for test prerequisites - Explicit ignore cleanup errors + add rationale comments - Handle non-critical log print errors * fix: resolve golangci-lint code quality issues (#137) - Simplify for-select patterns to direct channel operations - Replace if-else chains with switch statements - Eliminate redundant boolean expressions and loops - Fix Yoda conditions to natural comparison order - Remove unnecessary select statements - Clean up dead code and unused return statements No functional changes. All tests pass. * fix: resolve code quality and documentation issues (#137) - Prevent potential deadlock in timer wheel - Replace blocking channel sends with non-blocking select pattern - Add enqueueTimerAction helper method for fail-fast behavior - Update AddTimer, deleteTimer, and resetTimer to handle enqueue errors - Add missing docstrings for strings package - Add docstring for IsNil with deep nil checking explanation - Add docstring for RegSplit with parameters and usage example - Add comprehensive docstring for Slice with unsafe operation warnings Docstring coverage increased from 25% to 100% (4/4 functions). No functional changes. All tests pass. * fix: resolve code quality and safety issues (#137) - Fix IsNil panic for non-nilable types - Check reflect.Kind before calling IsNil to prevent panic - Add comprehensive test coverage for all type categories - Prevent timer wheel convenience API panics - Add nil checks in After, Sleep, AfterFunc, and Tick methods - Return nil/exit early when timer creation fails under load - Remove outdated commented code in After method - Add missing docstrings for strings package - Add docstring for RegSplit with parameters and usage example - Add comprehensive docstring for Slice with unsafe operation warnings --------- Co-authored-by: yaolei_cao <yaolei_cao@intsig.net>
1 parent f3035e7 commit 83e0fee

24 files changed

Lines changed: 307 additions & 198 deletions

bytes/buffer.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ func (b *Buffer) WriteNextEnd(n int) (int, error) {
219219
bufLen := len(b.buf)
220220
l := bufLen + n
221221
if l > peekBufLen {
222-
return 0, fmt.Errorf("U have not invoked @WriteNextBegin")
222+
return 0, fmt.Errorf("you have not invoked @WriteNextBegin")
223223
}
224224

225225
b.lastRead = opInvalid
@@ -331,7 +331,8 @@ func (b *Buffer) WriteByte(c byte) error {
331331
func (b *Buffer) WriteRune(r rune) (n int, err error) {
332332
// Compare as uint32 to correctly handle negative runes.
333333
if uint32(r) < utf8.RuneSelf {
334-
b.WriteByte(byte(r))
334+
// WriteByte returns nil for bytes.Buffer
335+
_ = b.WriteByte(byte(r))
335336
return 1, nil
336337
}
337338
b.lastRead = opInvalid

bytes/buffer_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ import (
3131

3232
func TestBufferWithPeek(t *testing.T) {
3333
var b Buffer
34-
b.WriteString("hello")
34+
_, err := b.WriteString("hello")
35+
assert.Nil(t, err)
3536

3637
b1 := b
3738
b1.WriteNextBegin(100)

bytes/bytes_pool.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (bp *BytesPool) findIndex(size int) int {
6565
func (bp *BytesPool) AcquireBytes(size int) *[]byte {
6666
idx := bp.findIndex(size)
6767
if idx >= bp.length {
68-
buf := make([]byte, size, size)
68+
buf := make([]byte, size)
6969
return &buf
7070
}
7171

container/chan/unbounded_chan_test.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func TestUnboundedChan(t *testing.T) {
3737
}
3838

3939
for i := 1; i < 60; i++ {
40-
v, _ := <-ch.Out()
40+
v := <-ch.Out()
4141
count += v.(int)
4242
}
4343

@@ -222,9 +222,7 @@ func BenchmarkUnboundedChan_Fixed(b *testing.B) {
222222

223223
b.RunParallel(func(pb *testing.PB) {
224224
for pb.Next() {
225-
select {
226-
case ch.In() <- 1:
227-
}
225+
ch.In() <- 1
228226

229227
<-ch.Out()
230228
}
@@ -238,9 +236,7 @@ func BenchmarkUnboundedChan_Extension(b *testing.B) {
238236

239237
b.RunParallel(func(pb *testing.PB) {
240238
for pb.Next() {
241-
select {
242-
case ch.In() <- 1:
243-
}
239+
ch.In() <- 1
244240

245241
<-ch.Out()
246242
}
@@ -254,9 +250,7 @@ func BenchmarkUnboundedChan_ExtensionUnlimited(b *testing.B) {
254250

255251
b.RunParallel(func(pb *testing.PB) {
256252
for pb.Next() {
257-
select {
258-
case ch.In() <- 1:
259-
}
253+
ch.In() <- 1
260254

261255
<-ch.Out()
262256
}

container/queue/queue_test.go

Lines changed: 62 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ import (
3131
func TestPut(t *testing.T) {
3232
q := New(10)
3333

34-
q.Put(`test`)
34+
err := q.Put(`test`)
35+
assert.Nil(t, err)
3536
assert.Equal(t, int64(1), q.Len())
3637

3738
results, err := q.Get(1)
@@ -41,7 +42,8 @@ func TestPut(t *testing.T) {
4142
assert.Equal(t, `test`, result)
4243
assert.True(t, q.Empty())
4344

44-
q.Put(`test2`)
45+
err = q.Put(`test2`)
46+
assert.Nil(t, err)
4547
assert.Equal(t, int64(1), q.Len())
4648

4749
results, err = q.Get(1)
@@ -55,7 +57,8 @@ func TestPut(t *testing.T) {
5557
func TestGet(t *testing.T) {
5658
q := New(10)
5759

58-
q.Put(`test`)
60+
err := q.Put(`test`)
61+
assert.Nil(t, err)
5962
result, err := q.Get(2)
6063
if !assert.Nil(t, err) {
6164
return
@@ -65,8 +68,10 @@ func TestGet(t *testing.T) {
6568
assert.Equal(t, `test`, result[0])
6669
assert.Equal(t, int64(0), q.Len())
6770

68-
q.Put(`1`)
69-
q.Put(`2`)
71+
err = q.Put(`1`)
72+
assert.Nil(t, err)
73+
err = q.Put(`2`)
74+
assert.Nil(t, err)
7075

7176
result, err = q.Get(1)
7277
if !assert.Nil(t, err) {
@@ -89,9 +94,11 @@ func TestPoll(t *testing.T) {
8994
q := New(10)
9095

9196
// should be able to Poll() before anything is present, without breaking future Puts
92-
q.Poll(1, time.Millisecond)
97+
// testing timeout behavior on empty queue
98+
_, _ = q.Poll(1, time.Millisecond)
9399

94-
q.Put(`test`)
100+
err := q.Put(`test`)
101+
assert.Nil(t, err)
95102
result, err := q.Poll(2, 0)
96103
if !assert.Nil(t, err) {
97104
return
@@ -101,8 +108,10 @@ func TestPoll(t *testing.T) {
101108
assert.Equal(t, `test`, result[0])
102109
assert.Equal(t, int64(0), q.Len())
103110

104-
q.Put(`1`)
105-
q.Put(`2`)
111+
err = q.Put(`1`)
112+
assert.Nil(t, err)
113+
err = q.Put(`2`)
114+
assert.Nil(t, err)
106115

107116
result, err = q.Poll(1, time.Millisecond)
108117
if !assert.Nil(t, err) {
@@ -135,15 +144,17 @@ func TestPollNoMemoryLeak(t *testing.T) {
135144

136145
for i := 0; i < 10; i++ {
137146
// Poll() should cleanup waiters after timeout
138-
q.Poll(1, time.Nanosecond)
147+
// testing waiter cleanup mechanism
148+
_, _ = q.Poll(1, time.Nanosecond)
139149
assert.Len(t, q.waiters, 0)
140150
}
141151
}
142152

143153
func TestAddEmptyPut(t *testing.T) {
144154
q := New(10)
145155

146-
q.Put()
156+
// testing empty parameters behavior
157+
_ = q.Put()
147158

148159
if q.Len() != 0 {
149160
t.Errorf(`Expected len: %d, received: %d`, 0, q.Len())
@@ -153,7 +164,8 @@ func TestAddEmptyPut(t *testing.T) {
153164
func TestGetNonPositiveNumber(t *testing.T) {
154165
q := New(10)
155166

156-
q.Put(`test`)
167+
err := q.Put(`test`)
168+
assert.Nil(t, err)
157169
result, err := q.Get(0)
158170
if !assert.Nil(t, err) {
159171
return
@@ -171,7 +183,8 @@ func TestEmpty(t *testing.T) {
171183
t.Errorf(`Expected empty queue.`)
172184
}
173185

174-
q.Put(`test`)
186+
err := q.Put(`test`)
187+
assert.Nil(t, err)
175188
if q.Empty() {
176189
t.Errorf(`Expected non-empty queue.`)
177190
}
@@ -182,7 +195,8 @@ func TestGetEmpty(t *testing.T) {
182195

183196
go func() {
184197
time.Sleep(time.Second)
185-
q.Put(`a`)
198+
// test setup, not test target
199+
_ = q.Put(`a`)
186200
}()
187201

188202
result, err := q.Get(2)
@@ -219,7 +233,8 @@ func TestMultipleGetEmpty(t *testing.T) {
219233
wg.Wait()
220234
wg.Add(2)
221235

222-
q.Put(`a`, `b`, `c`)
236+
err := q.Put(`a`, `b`, `c`)
237+
assert.Nil(t, err)
223238
wg.Wait()
224239

225240
if assert.Len(t, results[0], 1) && assert.Len(t, results[1], 1) {
@@ -238,7 +253,8 @@ func TestDispose(t *testing.T) {
238253

239254
// when the queue is not empty
240255
q = New(10)
241-
q.Put(`1`)
256+
err := q.Put(`1`)
257+
assert.Nil(t, err)
242258
itemsDisposed = q.Dispose()
243259

244260
expected := []interface{}{`1`}
@@ -305,7 +321,8 @@ func BenchmarkQueue(b *testing.B) {
305321

306322
go func() {
307323
for {
308-
q.Get(1)
324+
// benchmark focuses on throughput
325+
_, _ = q.Get(1)
309326
i++
310327
if i == b.N {
311328
wg.Done()
@@ -315,7 +332,8 @@ func BenchmarkQueue(b *testing.B) {
315332
}()
316333

317334
for i := 0; i < b.N; i++ {
318-
q.Put(`a`)
335+
// benchmark focuses on throughput
336+
_ = q.Put(`a`)
319337
}
320338

321339
wg.Wait()
@@ -347,9 +365,12 @@ func BenchmarkChannel(b *testing.B) {
347365

348366
func TestPeek(t *testing.T) {
349367
q := New(10)
350-
q.Put(`a`)
351-
q.Put(`b`)
352-
q.Put(`c`)
368+
err := q.Put(`a`)
369+
assert.Nil(t, err)
370+
err = q.Put(`b`)
371+
assert.Nil(t, err)
372+
err = q.Put(`c`)
373+
assert.Nil(t, err)
353374
peekResult, err := q.Peek()
354375
peekExpected := `a`
355376
assert.Nil(t, err)
@@ -373,7 +394,8 @@ func TestPeekOnDisposedQueue(t *testing.T) {
373394

374395
func TestGetUntil(t *testing.T) {
375396
q := New(10)
376-
q.Put(`a`, `b`, `c`)
397+
err := q.Put(`a`, `b`, `c`)
398+
assert.Nil(t, err)
377399
result, err := q.GetUntil(func(item interface{}) bool {
378400
return item != `c`
379401
})
@@ -402,7 +424,8 @@ func TestGetUntilEmptyQueue(t *testing.T) {
402424

403425
func TestGetUntilThenGet(t *testing.T) {
404426
q := New(10)
405-
q.Put(`a`, `b`, `c`)
427+
err := q.Put(`a`, `b`, `c`)
428+
assert.Nil(t, err)
406429
takeItems, _ := q.GetUntil(func(item interface{}) bool {
407430
return item != `c`
408431
})
@@ -414,7 +437,8 @@ func TestGetUntilThenGet(t *testing.T) {
414437

415438
func TestGetUntilNoMatches(t *testing.T) {
416439
q := New(10)
417-
q.Put(`a`, `b`, `c`)
440+
err := q.Put(`a`, `b`, `c`)
441+
assert.Nil(t, err)
418442
takeItems, _ := q.GetUntil(func(item interface{}) bool {
419443
return item != `a`
420444
})
@@ -500,7 +524,8 @@ func TestWaiters(t *testing.T) {
500524
func TestExecuteInParallel(t *testing.T) {
501525
q := New(10)
502526
for i := 0; i < 10; i++ {
503-
q.Put(i)
527+
err := q.Put(i)
528+
assert.Nil(t, err)
504529
}
505530

506531
numCalls := uint64(0)
@@ -537,7 +562,8 @@ func BenchmarkQueuePut(b *testing.B) {
537562
for i := 0; i < b.N; i++ {
538563
q := qs[i]
539564
for j := int64(0); j < numItems; j++ {
540-
q.Put(j)
565+
// benchmark focuses on throughput
566+
_ = q.Put(j)
541567
}
542568
}
543569
}
@@ -550,7 +576,8 @@ func BenchmarkQueueGet(b *testing.B) {
550576
for i := 0; i < b.N; i++ {
551577
q := New(numItems)
552578
for j := int64(0); j < numItems; j++ {
553-
q.Put(j)
579+
// benchmark setup
580+
_ = q.Put(j)
554581
}
555582
qs = append(qs, q)
556583
}
@@ -560,7 +587,8 @@ func BenchmarkQueueGet(b *testing.B) {
560587
for i := 0; i < b.N; i++ {
561588
q := qs[i]
562589
for j := int64(0); j < numItems; j++ {
563-
q.Get(1)
590+
// benchmark focuses on throughput
591+
_, _ = q.Get(1)
564592
}
565593
}
566594
}
@@ -573,7 +601,8 @@ func BenchmarkQueuePoll(b *testing.B) {
573601
for i := 0; i < b.N; i++ {
574602
q := New(numItems)
575603
for j := int64(0); j < numItems; j++ {
576-
q.Put(j)
604+
// benchmark setup
605+
_ = q.Put(j)
577606
}
578607
qs = append(qs, q)
579608
}
@@ -582,7 +611,8 @@ func BenchmarkQueuePoll(b *testing.B) {
582611

583612
for _, q := range qs {
584613
for j := int64(0); j < numItems; j++ {
585-
q.Poll(1, time.Millisecond)
614+
// benchmark focuses on throughput
615+
_, _ = q.Poll(1, time.Millisecond)
586616
}
587617
}
588618
}
@@ -595,7 +625,8 @@ func BenchmarkExecuteInParallel(b *testing.B) {
595625
for i := 0; i < b.N; i++ {
596626
q := New(numItems)
597627
for j := int64(0); j < numItems; j++ {
598-
q.Put(j)
628+
// benchmark setup
629+
_ = q.Put(j)
599630
}
600631
qs = append(qs, q)
601632
}

database/kv/etcd/v3/client.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,8 @@ func NewClientWithOptions(ctx context.Context, opts *Options) (*Client, error) {
175175
// NOTICE: need to get the lock before calling this method
176176
func (c *Client) clean() {
177177
// close raw client
178-
c.rawClient.Close()
178+
// cleanup prioritizes resource release
179+
_ = c.rawClient.Close()
179180

180181
// cancel ctx for raw client
181182
c.cancel()
@@ -456,7 +457,8 @@ func (c *Client) keepAliveKV(k string, v string) error {
456457

457458
keepAlive, err := rawClient.KeepAlive(c.ctx, lease.ID)
458459
if err != nil || keepAlive == nil {
459-
rawClient.Revoke(c.ctx, lease.ID)
460+
// prioritize returning keepalive failure
461+
_, _ = rawClient.Revoke(c.ctx, lease.ID)
460462
if err != nil {
461463
return perrors.WithMessage(err, "keep alive lease")
462464
}

database/kv/etcd/v3/client_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ func (suite *ClientTestSuite) SetupSuite() {
111111
}
112112

113113
suite.etcd = e
114-
return
115114
}
116115

117116
// stop etcd server
@@ -136,9 +135,10 @@ func (suite *ClientTestSuite) setUpClient() *Client {
136135
// set up a client for suite
137136
func (suite *ClientTestSuite) SetupTest() {
138137
c := suite.setUpClient()
139-
c.CleanKV()
138+
if err := c.CleanKV(); err != nil {
139+
suite.T().Fatal("CleanKV failed:", err)
140+
}
140141
suite.client = c
141-
return
142142
}
143143

144144
func (suite *ClientTestSuite) TestClientClose() {

0 commit comments

Comments
 (0)