More complicated linting

Checked linter warnings using:

     CGO_ENABLED=0 golangci-lint run --enable-all --disable testpackage,paralleltest,funlen,dupl,exhaustive,nakedret,noctx,nlreturn,gomnd,gochecknoglobals,gochecknoinits,interfacer,exhaustivestruct,wsl,lll,nestif,godox,gocyclo,gocognit,gomodguard,maligned,stylecheck,gosec,forbidigo,goconst,wrapcheck,staticcheck ./...

- Got rid of dynamic errors (errors.New())
- Using errors.Is() / errors.As() to test errors
- Added t.Helper() to test helpers
- Fixed InMemoryCache (use pointer receiver, don't copy or locks break)
- Simplified code in various places
- Aligned closer to standards

binder_test.go

@@ -23,7 +23,7 @@ type A struct {
 	ID   int
 	Name string
 	B    B
-	//nolint:unused
+	//nolint:unused,structcheck
 	private int
 }
 
@@ -386,6 +386,8 @@ func TestUnbinder(t *testing.T) {
 // Helpers
 
 func valEq(t *testing.T, name string, actual, expected reflect.Value) {
+	t.Helper()
+
 	switch expected.Kind() {
 	case reflect.Slice:
 		// Check the type/length/element type

cache/cache.go

@@ -5,7 +5,6 @@
 package cache
 
 import (
-	"errors"
 	"time"
 )
 
@@ -118,12 +117,20 @@ type Cache interface {
 	Flush() error
 }
 
-var (
-	Instance Cache
+var Instance Cache
 
-	ErrCacheMiss    = errors.New("revel/cache: key not found")
-	ErrNotStored    = errors.New("revel/cache: not stored")
-	ErrInvalidValue = errors.New("revel/cache: invalid value")
+// Error is used for constant errors.
+type Error string
+
+// Error implements the error interface.
+func (e Error) Error() string {
+	return string(e)
+}
+
+const (
+	ErrCacheMiss    Error = "revel/cache: key not found"
+	ErrNotStored    Error = "revel/cache: not stored"
+	ErrInvalidValue Error = "revel/cache: invalid value"
 )
 
 // The package implements the Cache interface (as sugar).

cache/cache_test.go

@@ -5,6 +5,7 @@
 package cache
 
 import (
+	"errors"
 	"math"
 	"testing"
 	"time"
@@ -16,6 +17,8 @@ type cacheFactory func(*testing.T, time.Duration) Cache
 
 // Test typical cache interactions.
 func typicalGetSet(t *testing.T, newCache cacheFactory) {
+	t.Helper()
+
 	var err error
 	cache := newCache(t, time.Hour)
 
@@ -36,6 +39,8 @@ func typicalGetSet(t *testing.T, newCache cacheFactory) {
 
 // Test the increment-decrement cases.
 func incrDecr(t *testing.T, newCache cacheFactory) {
+	t.Helper()
+
 	var err error
 	cache := newCache(t, time.Hour)
 
@@ -79,6 +84,8 @@ func incrDecr(t *testing.T, newCache cacheFactory) {
 }
 
 func expiration(t *testing.T, newCache cacheFactory) {
+	t.Helper()
+
 	// memcached does not support expiration times less than 1 second.
 	var err error
 	cache := newCache(t, time.Second)
@@ -88,7 +95,7 @@ func expiration(t *testing.T, newCache cacheFactory) {
 		t.Errorf("Set failed: %s", err)
 	}
 	time.Sleep(2 * time.Second)
-	if err = cache.Get("int", &value); err != ErrCacheMiss {
+	if err = cache.Get("int", &value); !errors.Is(err, ErrCacheMiss) {
 		t.Errorf("Expected CacheMiss, but got: %s", err)
 	}
 
@@ -97,7 +104,7 @@ func expiration(t *testing.T, newCache cacheFactory) {
 		t.Errorf("Set failed: %s", err)
 	}
 	time.Sleep(2 * time.Second)
-	if err = cache.Get("int", &value); err != ErrCacheMiss {
+	if err = cache.Get("int", &value); !errors.Is(err, ErrCacheMiss) {
 		t.Errorf("Expected CacheMiss, but got: %s", err)
 	}
 
@@ -121,39 +128,43 @@ func expiration(t *testing.T, newCache cacheFactory) {
 }
 
 func emptyCache(t *testing.T, newCache cacheFactory) {
+	t.Helper()
+
 	var err error
 	cache := newCache(t, time.Hour)
 
 	err = cache.Get("notexist", 0)
 	if err == nil {
 		t.Errorf("Error expected for non-existent key")
 	}
-	if err != ErrCacheMiss {
+	if !errors.Is(err, ErrCacheMiss) {
 		t.Errorf("Expected ErrCacheMiss for non-existent key: %s", err)
 	}
 
 	err = cache.Delete("notexist")
-	if err != ErrCacheMiss {
+	if !errors.Is(err, ErrCacheMiss) {
 		t.Errorf("Expected ErrCacheMiss for non-existent key: %s", err)
 	}
 
 	_, err = cache.Increment("notexist", 1)
-	if err != ErrCacheMiss {
+	if !errors.Is(err, ErrCacheMiss) {
 		t.Errorf("Expected cache miss incrementing non-existent key: %s", err)
 	}
 
 	_, err = cache.Decrement("notexist", 1)
-	if err != ErrCacheMiss {
+	if !errors.Is(err, ErrCacheMiss) {
 		t.Errorf("Expected cache miss decrementing non-existent key: %s", err)
 	}
 }
 
 func testReplace(t *testing.T, newCache cacheFactory) {
+	t.Helper()
+
 	var err error
 	cache := newCache(t, time.Hour)
 
 	// Replace in an empty cache.
-	if err = cache.Replace("notexist", 1, ForEverNeverExpiry); err != ErrNotStored {
+	if err = cache.Replace("notexist", 1, ForEverNeverExpiry); !errors.Is(err, ErrNotStored) {
 		t.Errorf("Replace in empty cache: expected ErrNotStored, got: %s", err)
 	}
 
@@ -175,15 +186,17 @@ func testReplace(t *testing.T, newCache cacheFactory) {
 
 	// Wait for it to expire and replace with 3 (unsuccessfully).
 	time.Sleep(2 * time.Second)
-	if err = cache.Replace("int", 3, time.Second); err != ErrNotStored {
+	if err = cache.Replace("int", 3, time.Second); !errors.Is(err, ErrNotStored) {
 		t.Errorf("Expected ErrNotStored, got: %s", err)
 	}
-	if err = cache.Get("int", &i); err != ErrCacheMiss {
+	if err = cache.Get("int", &i); !errors.Is(err, ErrCacheMiss) {
 		t.Errorf("Expected cache miss, got: %s", err)
 	}
 }
 
 func testAdd(t *testing.T, newCache cacheFactory) {
+	t.Helper()
+
 	var err error
 	cache := newCache(t, time.Hour)
 	// Add to an empty cache.
@@ -192,10 +205,8 @@ func testAdd(t *testing.T, newCache cacheFactory) {
 	}
 
 	// Try to add again. (fail)
-	if err = cache.Add("int", 2, time.Second*3); err != nil {
-		if err != ErrNotStored {
-			t.Errorf("Expected ErrNotStored adding dupe to cache: %s", err)
-		}
+	if err = cache.Add("int", 2, time.Second*3); !errors.Is(err, ErrNotStored) {
+		t.Errorf("Expected ErrNotStored adding dupe to cache: %s", err)
 	}
 
 	// Wait for it to expire, and add again.
@@ -215,6 +226,8 @@ func testAdd(t *testing.T, newCache cacheFactory) {
 }
 
 func testGetMulti(t *testing.T, newCache cacheFactory) {
+	t.Helper()
+
 	cache := newCache(t, time.Hour)
 
 	m := map[string]interface{}{
@@ -223,7 +236,7 @@ func testGetMulti(t *testing.T, newCache cacheFactory) {
 		"foo": struct{ Bar string }{"baz"},
 	}
 
-	var keys []string
+	keys := make([]string, 0, len(m))
 	for key, value := range m {
 		keys = append(keys, key)
 		if err := cache.Set(key, value, time.Second*30); err != nil {

cache/inmemory.go

@@ -13,16 +13,18 @@ import (
 	"github.com/patrickmn/go-cache"
 )
 
+const ErrCannotSet Error = "revel/cache: cannot set/get"
+
 type InMemoryCache struct {
 	cache cache.Cache  // Only expose the methods we want to make available
 	mu    sync.RWMutex // For increment / decrement prevent reads and writes
 }
 
-func NewInMemoryCache(defaultExpiration time.Duration) InMemoryCache {
-	return InMemoryCache{cache: *cache.New(defaultExpiration, time.Minute), mu: sync.RWMutex{}}
+func NewInMemoryCache(defaultExpiration time.Duration) *InMemoryCache {
+	return &InMemoryCache{cache: *cache.New(defaultExpiration, time.Minute), mu: sync.RWMutex{}}
 }
 
-func (c InMemoryCache) Get(key string, ptrValue interface{}) error {
+func (c *InMemoryCache) Get(key string, ptrValue interface{}) error {
 	c.mu.RLock()
 	defer c.mu.RUnlock()
 
@@ -37,24 +39,24 @@ func (c InMemoryCache) Get(key string, ptrValue interface{}) error {
 		return nil
 	}
 
-	err := fmt.Errorf("revel/cache: attempt to get %s, but can not set value %v", key, v)
+	err := fmt.Errorf("%w, get %s, value %v", ErrCannotSet, key, v)
 	cacheLog.Error(err.Error())
 	return err
 }
 
-func (c InMemoryCache) GetMulti(keys ...string) (Getter, error) {
+func (c *InMemoryCache) GetMulti(keys ...string) (Getter, error) {
 	return c, nil
 }
 
-func (c InMemoryCache) Set(key string, value interface{}, expires time.Duration) error {
+func (c *InMemoryCache) Set(key string, value interface{}, expires time.Duration) error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	// NOTE: go-cache understands the values of DefaultExpiryTime and ForEverNeverExpiry
 	c.cache.Set(key, value, expires)
 	return nil
 }
 
-func (c InMemoryCache) Add(key string, value interface{}, expires time.Duration) error {
+func (c *InMemoryCache) Add(key string, value interface{}, expires time.Duration) error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	err := c.cache.Add(key, value, expires)
@@ -64,7 +66,7 @@ func (c InMemoryCache) Add(key string, value interface{}, expires time.Duration)
 	return err
 }
 
-func (c InMemoryCache) Replace(key string, value interface{}, expires time.Duration) error {
+func (c *InMemoryCache) Replace(key string, value interface{}, expires time.Duration) error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	if err := c.cache.Replace(key, value, expires); err != nil {
@@ -73,7 +75,7 @@ func (c InMemoryCache) Replace(key string, value interface{}, expires time.Durat
 	return nil
 }
 
-func (c InMemoryCache) Delete(key string) error {
+func (c *InMemoryCache) Delete(key string) error {
 	c.mu.RLock()
 	defer c.mu.RUnlock()
 	if _, found := c.cache.Get(key); !found {
@@ -83,7 +85,7 @@ func (c InMemoryCache) Delete(key string) error {
 	return nil
 }
 
-func (c InMemoryCache) Increment(key string, n uint64) (newValue uint64, err error) {
+func (c *InMemoryCache) Increment(key string, n uint64) (newValue uint64, err error) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	if _, found := c.cache.Get(key); !found {
@@ -96,25 +98,28 @@ func (c InMemoryCache) Increment(key string, n uint64) (newValue uint64, err err
 	return c.convertTypeToUint64(key)
 }
 
-func (c InMemoryCache) Decrement(key string, n uint64) (newValue uint64, err error) {
+func (c *InMemoryCache) Decrement(key string, n uint64) (newValue uint64, err error) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	if nv, err := c.convertTypeToUint64(key); err != nil {
+
+	nv, err := c.convertTypeToUint64(key)
+	if err != nil {
 		return 0, err
-	} else {
-		// Stop from going below zero
-		if n > nv {
-			n = nv
-		}
 	}
+
+	// Stop from going below zero
+	if n > nv {
+		n = nv
+	}
+
 	if err = c.cache.Decrement(key, int64(n)); err != nil {
 		return
 	}
 
 	return c.convertTypeToUint64(key)
 }
 
-func (c InMemoryCache) Flush() error {
+func (c *InMemoryCache) Flush() error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
@@ -123,7 +128,7 @@ func (c InMemoryCache) Flush() error {
 }
 
 // Fetches and returns the converted type to a uint64.
-func (c InMemoryCache) convertTypeToUint64(key string) (newValue uint64, err error) {
+func (c *InMemoryCache) convertTypeToUint64(key string) (newValue uint64, err error) {
 	v, found := c.cache.Get(key)
 	if !found {
 		return newValue, ErrCacheMiss

cache/inmemory_test.go

@@ -9,7 +9,9 @@ import (
 	"time"
 )
 
-var newInMemoryCache = func(_ *testing.T, defaultExpiration time.Duration) Cache {
+var newInMemoryCache = func(t *testing.T, defaultExpiration time.Duration) Cache {
+	t.Helper()
+
 	return NewInMemoryCache(defaultExpiration)
 }
 

cache/memcached.go

@@ -12,6 +12,8 @@ import (
 	"github.com/revel/revel/logger"
 )
 
+const ErrCannotFlush Error = "flush: can not flush memcached"
+
 // MemcachedCache wraps the Memcached client to meet the Cache interface.
 type MemcachedCache struct {
 	*memcache.Client
@@ -65,7 +67,7 @@ func (c MemcachedCache) Decrement(key string, delta uint64) (newValue uint64, er
 }
 
 func (c MemcachedCache) Flush() error {
-	err := errors.New("Flush: can not flush memcached")
+	err := ErrCannotFlush
 	cacheLog.Error(err.Error())
 	return err
 }
@@ -103,12 +105,15 @@ func (g ItemMapGetter) Get(key string, ptrValue interface{}) error {
 }
 
 func convertMemcacheError(err error) error {
-	switch err {
-	case nil:
+	if err == nil {
 		return nil
-	case memcache.ErrCacheMiss:
+	}
+
+	if errors.Is(err, memcache.ErrCacheMiss) {
 		return ErrCacheMiss
-	case memcache.ErrNotStored:
+	}
+
+	if errors.Is(err, memcache.ErrNotStored) {
 		return ErrNotStored
 	}