From eb529947af531eb529020ba979a7a887338904d1 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Mon, 11 May 2020 18:09:09 +0200 Subject: [PATCH] Code quality improvements (#67) * add golangci-lint configuration, fix discovered issues * add GitHub Actions test and build and linter run --- .github/workflows/ci.yml | 32 ++++++++++++++++++++++++++++++++ .golangci.yml | 33 +++++++++++++++++++++++++++++++++ 2q.go | 3 +-- arc.go | 1 - lru.go | 4 ++-- lru_test.go | 4 ++-- simplelru/lru.go | 6 +++--- simplelru/lru_interface.go | 5 +++-- simplelru/lru_test.go | 4 ++-- 9 files changed, 78 insertions(+), 14 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 .golangci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..e56894b --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,32 @@ +name: build + +on: + push: + branches: + tags: + pull_request: + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - name: set up go 1.14 + uses: actions/setup-go@v1 + with: + go-version: 1.14 + id: go + + - name: checkout + uses: actions/checkout@v2 + + - name: build and test + run: | + go test -timeout=60s -race + go build -race + + - name: install golangci-lint + run: curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $GITHUB_WORKSPACE v1.26.0 + + - name: run golangci-lint + run: $GITHUB_WORKSPACE/golangci-lint run --out-format=github-actions diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..97aac99 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,33 @@ +linters: + enable: + - megacheck + - golint + - govet + - unconvert + - megacheck + - structcheck + - gas + - gocyclo + - dupl + - misspell + - unparam + - varcheck + - deadcode + - typecheck + - ineffassign + - varcheck + - stylecheck + - scopelint + - gocritic + - nakedret + - gosimple + - prealloc + fast: false + disable-all: true + +issues: + exclude-rules: + - path: _test\.go + linters: + - dupl + exclude-use-default: false diff --git a/2q.go b/2q.go index e474cd0..15fcad0 100644 --- a/2q.go +++ b/2q.go @@ -44,7 +44,7 @@ func New2Q(size int) (*TwoQueueCache, error) { // New2QParams creates a new TwoQueueCache using the provided // parameter values. -func New2QParams(size int, recentRatio float64, ghostRatio float64) (*TwoQueueCache, error) { +func New2QParams(size int, recentRatio, ghostRatio float64) (*TwoQueueCache, error) { if size <= 0 { return nil, fmt.Errorf("invalid size") } @@ -138,7 +138,6 @@ func (c *TwoQueueCache) Add(key, value interface{}) { // Add to the recently seen list c.ensureSpace(false) c.recent.Add(key, value) - return } // ensureSpace is used to ensure we have space in the cache diff --git a/arc.go b/arc.go index 555225a..e396f84 100644 --- a/arc.go +++ b/arc.go @@ -173,7 +173,6 @@ func (c *ARCCache) Add(key, value interface{}) { // Add to the recently seen list c.t1.Add(key, value) - return } // replace is used to adaptively evict from either T1 or T2 diff --git a/lru.go b/lru.go index 4e5e9d8..aa52433 100644 --- a/lru.go +++ b/lru.go @@ -118,7 +118,7 @@ func (c *Cache) Resize(size int) (evicted int) { } // RemoveOldest removes the oldest item from the cache. -func (c *Cache) RemoveOldest() (key interface{}, value interface{}, ok bool) { +func (c *Cache) RemoveOldest() (key, value interface{}, ok bool) { c.lock.Lock() key, value, ok = c.lru.RemoveOldest() c.lock.Unlock() @@ -126,7 +126,7 @@ func (c *Cache) RemoveOldest() (key interface{}, value interface{}, ok bool) { } // GetOldest returns the oldest entry -func (c *Cache) GetOldest() (key interface{}, value interface{}, ok bool) { +func (c *Cache) GetOldest() (key, value interface{}, ok bool) { c.lock.Lock() key, value, ok = c.lru.GetOldest() c.lock.Unlock() diff --git a/lru_test.go b/lru_test.go index b52eafd..e4750d9 100644 --- a/lru_test.go +++ b/lru_test.go @@ -267,7 +267,7 @@ func TestLRUResize(t *testing.T) { // Downsize l.Add(1, 1) l.Add(2, 2) - evicted := l.Resize(1); + evicted := l.Resize(1) if evicted != 1 { t.Errorf("1 element should have been evicted: %v", evicted) } @@ -281,7 +281,7 @@ func TestLRUResize(t *testing.T) { } // Upsize - evicted = l.Resize(2); + evicted = l.Resize(2) if evicted != 0 { t.Errorf("0 elements should have been evicted: %v", evicted) } diff --git a/simplelru/lru.go b/simplelru/lru.go index a86c853..9233583 100644 --- a/simplelru/lru.go +++ b/simplelru/lru.go @@ -25,7 +25,7 @@ type entry struct { // NewLRU constructs an LRU of the given size func NewLRU(size int, onEvict EvictCallback) (*LRU, error) { if size <= 0 { - return nil, errors.New("Must provide a positive size") + return nil, errors.New("must provide a positive size") } c := &LRU{ size: size, @@ -109,7 +109,7 @@ func (c *LRU) Remove(key interface{}) (present bool) { } // RemoveOldest removes the oldest item from the cache. -func (c *LRU) RemoveOldest() (key interface{}, value interface{}, ok bool) { +func (c *LRU) RemoveOldest() (key, value interface{}, ok bool) { ent := c.evictList.Back() if ent != nil { c.removeElement(ent) @@ -120,7 +120,7 @@ func (c *LRU) RemoveOldest() (key interface{}, value interface{}, ok bool) { } // GetOldest returns the oldest entry -func (c *LRU) GetOldest() (key interface{}, value interface{}, ok bool) { +func (c *LRU) GetOldest() (key, value interface{}, ok bool) { ent := c.evictList.Back() if ent != nil { kv := ent.Value.(*entry) diff --git a/simplelru/lru_interface.go b/simplelru/lru_interface.go index 92d7093..cb7f8ca 100644 --- a/simplelru/lru_interface.go +++ b/simplelru/lru_interface.go @@ -1,3 +1,4 @@ +// Package simplelru provides simple LRU implementation based on build-in container/list. package simplelru // LRUCache is the interface for simple LRU cache. @@ -34,6 +35,6 @@ type LRUCache interface { // Clears all cache entries. Purge() - // Resizes cache, returning number evicted - Resize(int) int + // Resizes cache, returning number evicted + Resize(int) int } diff --git a/simplelru/lru_test.go b/simplelru/lru_test.go index bc7f696..a9ffcd5 100644 --- a/simplelru/lru_test.go +++ b/simplelru/lru_test.go @@ -180,7 +180,7 @@ func TestLRU_Resize(t *testing.T) { // Downsize l.Add(1, 1) l.Add(2, 2) - evicted := l.Resize(1); + evicted := l.Resize(1) if evicted != 1 { t.Errorf("1 element should have been evicted: %v", evicted) } @@ -194,7 +194,7 @@ func TestLRU_Resize(t *testing.T) { } // Upsize - evicted = l.Resize(2); + evicted = l.Resize(2) if evicted != 0 { t.Errorf("0 elements should have been evicted: %v", evicted) }