Skip to content

Commit

Permalink
chore(*): enable revive.cyclomatic rules (#409)
Browse files Browse the repository at this point in the history
Signed-off-by: Soren Yang <[email protected]>
  • Loading branch information
lsytj0413 authored Dec 15, 2023
1 parent 0cd9f4f commit ac6b84a
Show file tree
Hide file tree
Showing 11 changed files with 191 additions and 114 deletions.
15 changes: 9 additions & 6 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ linters-settings:
- 8
- name: cyclomatic
severity: warning
disabled: true
disabled: false
arguments:
- 3
- 15
- name: function-length
severity: warning
disabled: true
Expand Down Expand Up @@ -128,8 +128,11 @@ linters-settings:

issues:
fix: true
max-same-issues: 0
max-issues-per-linter: 0
max-same-issues: 3
max-issues-per-linter: 50
# include:
# - "EXC0012"
# - "EXC0014"
exclude-rules:
- path: _test\.go
linters:
Expand All @@ -151,15 +154,15 @@ issues:
- path: _test\.go
linters:
- revive
text: "add-constant|cognitive-complexity|unused-parameter|unused-receiver|function-length|line-length-limit|unhandled-error|deep-exit|unchecked-type-assertion"
text: "add-constant|cognitive-complexity|unused-parameter|unused-receiver|function-length|line-length-limit|unhandled-error|deep-exit|unchecked-type-assertion|cyclomatic"
- path: \.go
linters:
- revive
text: "confusing-naming.*only by capitalization.*"
- path: maelstrom/
linters:
- revive
text: "unchecked-type-assertion|unhandled-error|unused-receiver|unused-parameter|add-constant"
text: "unchecked-type-assertion|unhandled-error|unused-receiver|unused-parameter|add-constant|cyclomatic"
- path: _grpc\.go
linters:
- revive
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ tla:
OxiaReplication.tla

license-check:
# go install github.com/palantir/go-license@latest
@command -v go-license > /dev/null || go install github.com/palantir/go-license@latest
find . -type f -name '*.go' | grep -v '.pb.go' | xargs go-license --config=.github/license.yml --verify

license-format:
# go install github.com/palantir/go-license@latest
@command -v go-license > /dev/null || go install github.com/palantir/go-license@latest
find . -type f -name '*.go' | grep -v '.pb.go' | xargs go-license --config=.github/license.yml
2 changes: 1 addition & 1 deletion common/batch/batcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (b *batcherImpl) failCall(call any, err error) {
batch.Fail(err)
}

func (b *batcherImpl) Run() {
func (b *batcherImpl) Run() { //nolint:revive
var batch Batch
var timer *time.Timer
var timeout <-chan time.Time
Expand Down
3 changes: 0 additions & 3 deletions common/batch/batcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,6 @@ func TestBatcher(t *testing.T) {
MaxRequestsPerBatch: item.maxSize,
}
batcher := factory.NewBatcher(batchFactory)

go batcher.Run()

batcher.Add(1)

if item.closeImmediately {
Expand Down
63 changes: 2 additions & 61 deletions server/kv/kv_pebble.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package kv

import (
"bytes"
"fmt"
"io"
"log/slog"
Expand All @@ -35,7 +34,7 @@ import (

var (
OxiaSlashSpanComparer = &pebble.Comparer{
Compare: CompareWithSlash,
Compare: compareWithSlash,
Equal: pebble.DefaultComparer.Equal,
AbbreviatedKey: pebble.DefaultComparer.AbbreviatedKey,
FormatKey: pebble.DefaultComparer.FormatKey,
Expand Down Expand Up @@ -206,7 +205,7 @@ func newKVPebble(factory *PebbleFactory, namespace string, shardId int64) (KV, e
},
FS: vfs.Default,
DisableWAL: true,
Logger: &PebbleLogger{
Logger: &pebbleLogger{
slog.With(
slog.String("component", "pebble"),
slog.Int64("shard", shardId),
Expand Down Expand Up @@ -549,64 +548,6 @@ func (p *PebbleReverseIterator) Value() ([]byte, error) {
return res, err
}

// Pebble logger wrapper

type PebbleLogger struct {
zl *slog.Logger
}

func (pl *PebbleLogger) Infof(format string, args ...any) {
pl.zl.Info(
fmt.Sprintf(format, args...),
)
}

func (pl *PebbleLogger) Errorf(format string, args ...any) {
pl.zl.Error(
fmt.Sprintf(format, args...),
)
}

func (pl *PebbleLogger) Fatalf(format string, args ...any) {
pl.zl.Warn(
fmt.Sprintf(format, args...),
)
os.Exit(1)
}

func CompareWithSlash(a, b []byte) int {
for len(a) > 0 && len(b) > 0 {
idxA, idxB := bytes.IndexByte(a, '/'), bytes.IndexByte(b, '/')
switch {
case idxA < 0 && idxB < 0:
return bytes.Compare(a, b)
case idxA < 0 && idxB >= 0:
return -1
case idxA >= 0 && idxB < 0:
return +1
}

// At this point, both slices have '/'
spanA, spanB := a[:idxA], b[:idxB]

spanRes := bytes.Compare(spanA, spanB)
if spanRes != 0 {
return spanRes
}

a, b = a[idxA+1:], b[idxB+1:]
}

switch {
case len(a) < len(b):
return -1
case len(a) > len(b):
return +1
}

return 0
}

type pebbleSnapshot struct {
path string
files []string
Expand Down
40 changes: 0 additions & 40 deletions server/kv/kv_pebble_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package kv

import (
"bytes"
"fmt"
"os"
"path/filepath"
Expand Down Expand Up @@ -220,45 +219,6 @@ func TestPebbleRangeScan(t *testing.T) {
assert.NoError(t, factory.Close())
}

var benchKeyA = []byte("/test/aaaaaaaaaaa/bbbbbbbbbbb/cccccccccccc/dddddddddddddd")
var benchKeyB = []byte("/test/aaaaaaaaaaa/bbbbbbbbbbb/ccccccccccccddddddddddddddd")

func BenchmarkStandardCompare(b *testing.B) {
for i := 0; i < b.N; i++ {
bytes.Compare(benchKeyA, benchKeyB)
}
}

func BenchmarkCompareWithSlash(b *testing.B) {
for i := 0; i < b.N; i++ {
CompareWithSlash(benchKeyA, benchKeyB)
}
}

func TestCompareWithSlash(t *testing.T) {
assert.Equal(t, 0, CompareWithSlash([]byte("aaaaa"), []byte("aaaaa")))
assert.Equal(t, -1, CompareWithSlash([]byte("aaaaa"), []byte("zzzzz")))
assert.Equal(t, +1, CompareWithSlash([]byte("bbbbb"), []byte("aaaaa")))

assert.Equal(t, +1, CompareWithSlash([]byte("aaaaa"), []byte("")))
assert.Equal(t, -1, CompareWithSlash([]byte(""), []byte("aaaaaa")))
assert.Equal(t, 0, CompareWithSlash([]byte(""), []byte("")))

assert.Equal(t, -1, CompareWithSlash([]byte("aaaaa"), []byte("aaaaaaaaaaa")))
assert.Equal(t, +1, CompareWithSlash([]byte("aaaaaaaaaaa"), []byte("aaa")))

assert.Equal(t, -1, CompareWithSlash([]byte("a"), []byte("/")))
assert.Equal(t, +1, CompareWithSlash([]byte("/"), []byte("a")))

assert.Equal(t, -1, CompareWithSlash([]byte("/aaaa"), []byte("/bbbbb")))
assert.Equal(t, -1, CompareWithSlash([]byte("/aaaa"), []byte("/aa/a")))
assert.Equal(t, -1, CompareWithSlash([]byte("/aaaa/a"), []byte("/aaaa/b")))
assert.Equal(t, +1, CompareWithSlash([]byte("/aaaa/a/a"), []byte("/bbbbbbbbbb")))
assert.Equal(t, +1, CompareWithSlash([]byte("/aaaa/a/a"), []byte("/aaaa/bbbbbbbbbb")))

assert.Equal(t, +1, CompareWithSlash([]byte("/a/b/a/a/a"), []byte("/a/b/a/b")))
}

func TestPebbleRangeScanWithSlashOrder(t *testing.T) {
keys := []string{
"/a/a/a/zzzzzz",
Expand Down
45 changes: 45 additions & 0 deletions server/kv/pebble_logger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright 2023 StreamNative, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package kv

import (
"fmt"
"log/slog"
"os"
)

// pebbleLogger is the wrapper of slog to implement pebbel's logger interface.
type pebbleLogger struct {
zl *slog.Logger
}

func (pl *pebbleLogger) Infof(format string, args ...any) {
pl.zl.Info(
fmt.Sprintf(format, args...),
)
}

func (pl *pebbleLogger) Errorf(format string, args ...any) {
pl.zl.Error(
fmt.Sprintf(format, args...),
)
}

func (pl *pebbleLogger) Fatalf(format string, args ...any) {
pl.zl.Warn(
fmt.Sprintf(format, args...),
)
os.Exit(1)
}
51 changes: 51 additions & 0 deletions server/kv/utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2023 StreamNative, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package kv

import (
"bytes"
)

func compareWithSlash(a, b []byte) int {
for len(a) > 0 && len(b) > 0 {
idxA, idxB := bytes.IndexByte(a, '/'), bytes.IndexByte(b, '/')
switch {
case idxA < 0 && idxB < 0:
return bytes.Compare(a, b)
case idxA < 0 && idxB >= 0:
return -1
case idxA >= 0 && idxB < 0:
return +1
}

// At this point, both slices have '/'
spanA, spanB := a[:idxA], b[:idxB]
spanRes := bytes.Compare(spanA, spanB)
if spanRes != 0 {
return spanRes
}

a, b = a[idxA+1:], b[idxB+1:]
}

switch {
case len(a) < len(b):
return -1
case len(a) > len(b):
return +1
}

return 0
}
35 changes: 35 additions & 0 deletions server/kv/utils_benchmark_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright 2023 StreamNative, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package kv

import (
"bytes"
"testing"
)

var benchKeyA = []byte("/test/aaaaaaaaaaa/bbbbbbbbbbb/cccccccccccc/dddddddddddddd")
var benchKeyB = []byte("/test/aaaaaaaaaaa/bbbbbbbbbbb/ccccccccccccddddddddddddddd")

func Benchmark_StandardCompare(b *testing.B) {
for i := 0; i < b.N; i++ {
bytes.Compare(benchKeyA, benchKeyB)
}
}

func Benchmark_CompareWithSlash(b *testing.B) {
for i := 0; i < b.N; i++ {
compareWithSlash(benchKeyA, benchKeyB)
}
}
45 changes: 45 additions & 0 deletions server/kv/utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright 2023 StreamNative, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package kv

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestCompareWithSlash(t *testing.T) {
assert.Equal(t, 0, compareWithSlash([]byte("aaaaa"), []byte("aaaaa")))
assert.Equal(t, -1, compareWithSlash([]byte("aaaaa"), []byte("zzzzz")))
assert.Equal(t, +1, compareWithSlash([]byte("bbbbb"), []byte("aaaaa")))

assert.Equal(t, +1, compareWithSlash([]byte("aaaaa"), []byte("")))
assert.Equal(t, -1, compareWithSlash([]byte(""), []byte("aaaaaa")))
assert.Equal(t, 0, compareWithSlash([]byte(""), []byte("")))

assert.Equal(t, -1, compareWithSlash([]byte("aaaaa"), []byte("aaaaaaaaaaa")))
assert.Equal(t, +1, compareWithSlash([]byte("aaaaaaaaaaa"), []byte("aaa")))

assert.Equal(t, -1, compareWithSlash([]byte("a"), []byte("/")))
assert.Equal(t, +1, compareWithSlash([]byte("/"), []byte("a")))

assert.Equal(t, -1, compareWithSlash([]byte("/aaaa"), []byte("/bbbbb")))
assert.Equal(t, -1, compareWithSlash([]byte("/aaaa"), []byte("/aa/a")))
assert.Equal(t, -1, compareWithSlash([]byte("/aaaa/a"), []byte("/aaaa/b")))
assert.Equal(t, +1, compareWithSlash([]byte("/aaaa/a/a"), []byte("/bbbbbbbbbb")))
assert.Equal(t, +1, compareWithSlash([]byte("/aaaa/a/a"), []byte("/aaaa/bbbbbbbbbb")))

assert.Equal(t, +1, compareWithSlash([]byte("/a/b/a/a/a"), []byte("/a/b/a/b")))
}
Loading

0 comments on commit ac6b84a

Please sign in to comment.