Merge pull request #1837 from jaffee/staticcheck-fixes

Staticcheck fixes (lint)

cluster.go

@@ -1792,7 +1792,7 @@ func (c *cluster) nodeLeave(nodeID string) error {
 	}
 
 	if c.state != ClusterStateNormal && c.state != ClusterStateDegraded {
-		return fmt.Errorf("Cluster must be in state %s to remove a node. Current state: %s",
+		return fmt.Errorf("cluster must be '%s' to remove a node but is '%s'",
 			ClusterStateNormal, c.state)
 	}
 
@@ -1803,7 +1803,7 @@ func (c *cluster) nodeLeave(nodeID string) error {
 
 	// Prevent removing the coordinator node (this node).
 	if nodeID == c.Node.ID {
-		return fmt.Errorf("coordinator cannot be removed; first, make a different node the new coordinator.")
+		return fmt.Errorf("coordinator cannot be removed; first, make a different node the new coordinator")
 	}
 
 	// See if resize job can be generated

ctl/import.go

@@ -151,11 +151,11 @@ func (cmd *ImportCommand) Run(ctx context.Context) error {
 func (cmd *ImportCommand) ensureSchema(ctx context.Context) error {
 	err := cmd.client.EnsureIndex(ctx, cmd.Index, cmd.IndexOptions)
 	if err != nil {
-		return fmt.Errorf("Error Creating Index: %s", err)
+		return errors.Wrap(err, "creating index")
 	}
 	err = cmd.client.EnsureFieldWithOptions(ctx, cmd.Index, cmd.Field, cmd.FieldOptions)
 	if err != nil {
-		return fmt.Errorf("Error Creating Field: %s", err)
+		return errors.Wrap(err, "creating field")
 	}
 	return nil
 }

diagnostics.go

@@ -137,11 +137,11 @@ func (d *diagnosticsCollector) compareVersion(value string) error {
 	localVersion := versionSegments(d.version)
 
 	if localVersion[0] < currentVersion[0] { //Major
-		return fmt.Errorf("Warning: You are running Pilosa %s. A newer version (%s) is available: https://github.com/pilosa/pilosa/releases", d.version, value)
+		return fmt.Errorf("you are running Pilosa %s, a newer version (%s) is available: https://github.com/pilosa/pilosa/releases", d.version, value)
 	} else if localVersion[1] < currentVersion[1] && localVersion[0] == currentVersion[0] { // Minor
-		return fmt.Errorf("Warning: You are running Pilosa %s. The latest Minor release is %s: https://github.com/pilosa/pilosa/releases", d.version, value)
+		return fmt.Errorf("you are running Pilosa %s, the latest minor release is %s: https://github.com/pilosa/pilosa/releases", d.version, value)
 	} else if localVersion[2] < currentVersion[2] && localVersion[0] == currentVersion[0] && localVersion[1] == currentVersion[1] { // Patch
-		return fmt.Errorf("There is a new patch release of Pilosa available: %s: https://github.com/pilosa/pilosa/releases", value)
+		return fmt.Errorf("there is a new patch release of Pilosa available: %s: https://github.com/pilosa/pilosa/releases", value)
 	}
 
 	return nil

diagnostics_internal_test.go

@@ -82,19 +82,19 @@ func TestDiagnosticsVersion_Compare(t *testing.T) {
 	d.SetVersion(version)
 
 	err := d.compareVersion("v1.7.0")
-	if !strings.Contains(err.Error(), "A newer version") {
+	if !strings.Contains(err.Error(), "a newer version") {
 		t.Fatalf("Expected a newer version is available, actual error: %s", err)
 	}
 	err = d.compareVersion("1.7.0")
-	if !strings.Contains(err.Error(), "A newer version") {
+	if !strings.Contains(err.Error(), "a newer version") {
 		t.Fatalf("Expected a newer version is available, actual error: %s", err)
 	}
 	err = d.compareVersion("0.7.0")
-	if !strings.Contains(err.Error(), "The latest Minor release is") {
+	if !strings.Contains(err.Error(), "the latest minor release is") {
 		t.Fatalf("Expected Minor Version Missmatch, actual error: %s", err)
 	}
 	err = d.compareVersion("0.1.2")
-	if !strings.Contains(err.Error(), "There is a new patch release of Pilosa") {
+	if !strings.Contains(err.Error(), "there is a new patch release of Pilosa") {
 		t.Fatalf("Expected Patch Version Missmatch, actual error: %s", err)
 	}
 	err = d.compareVersion("0.1.1")

executor.go

@@ -18,7 +18,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"log"
 	"sort"
 	"time"
 
@@ -806,7 +805,7 @@ func (e *executor) executeTopNShard(ctx context.Context, index string, c *pql.Ca
 		return nil, nil
 	}
 
-	if minThreshold <= 0 {
+	if minThreshold == 0 {
 		minThreshold = defaultMinThreshold
 	}
 
@@ -1188,7 +1187,7 @@ func (e *executor) executeRowShard(ctx context.Context, index string, c *pql.Cal
 	defer span.Finish()
 
 	if c.Name == "Range" {
-		log.Print("DEPRECATED: Range() is deprecated, please use Row() instead.")
+		e.Holder.Logger.Printf("DEPRECATED: Range() is deprecated, please use Row() instead.")
 	}
 
 	// Handle bsiGroup ranges differently.
@@ -1582,14 +1581,14 @@ func (e *executor) executeClearBit(ctx context.Context, index string, c *pql.Cal
 	if err != nil {
 		return false, fmt.Errorf("reading Clear() row: %v", err)
 	} else if !ok {
-		return false, fmt.Errorf("Clear() row argument '%v' required", rowLabel)
+		return false, fmt.Errorf("row=<row> argument required to Clear() call")
 	}
 
 	colID, ok, err := c.UintArg("_" + columnLabel)
 	if err != nil {
 		return false, fmt.Errorf("reading Clear() column: %v", err)
 	} else if !ok {
-		return false, fmt.Errorf("Clear() col argument '%v' required", columnLabel)
+		return false, fmt.Errorf("column argument to Clear(<COLUMN>, <FIELD>=<ROW>) required")
 	}
 
 	return e.executeClearBitField(ctx, index, c, f, colID, rowID, opt)
@@ -1713,14 +1712,14 @@ func (e *executor) executeSetRow(ctx context.Context, index string, c *pql.Call,
 	// Ensure the field type supports Store().
 	fieldName, err := c.FieldArg()
 	if err != nil {
-		return false, errors.New("Store() argument required: field")
+		return false, errors.New("field required for Store()")
 	}
 	field := e.Holder.Field(index, fieldName)
 	if field == nil {
 		return false, ErrFieldNotFound
 	}
 	if field.Type() != FieldTypeSet {
-		return false, fmt.Errorf("Store() is not supported on %s field types", field.Type())
+		return false, fmt.Errorf("can't Store() on a %s field", field.Type())
 	}
 
 	// Execute calls in bulk on each remote node and merge.
@@ -1753,7 +1752,7 @@ func (e *executor) executeSetRowShard(ctx context.Context, index string, c *pql.
 	if err != nil {
 		return false, fmt.Errorf("reading Store() row: %v", err)
 	} else if !ok {
-		return false, fmt.Errorf("Store() row argument '%v' required", rowLabel)
+		return false, fmt.Errorf("need the <FIELD>=<ROW> argument on Store()")
 	}
 
 	field := e.Holder.Field(index, fieldName)

fragment.go

@@ -1050,7 +1050,7 @@ func (f *fragment) top(opt topOptions) ([]Pair, error) {
 		rowID, cnt := pair.ID, pair.Count
 
 		// Ignore empty rows.
-		if cnt <= 0 {
+		if cnt == 0 {
 			continue
 		}
 

gossip/gossip.go

@@ -468,7 +468,7 @@ func newTransport(conf *memberlist.Config) (*memberlist.NetTransport, error) {
 
 	nt, err := makeNetRetry(limit)
 	if err != nil {
-		return nil, fmt.Errorf("Could not set up network transport: %v", err)
+		return nil, errors.Wrap(err, "could not set up network transport")
 	}
 
 	return nt, nil

http/handler.go

@@ -584,11 +584,11 @@ func validateOptions(data map[string]interface{}, validIndexOptions []string) er
 			}
 			for kk, vv := range options {
 				if !foundItem(validIndexOptions, kk) {
-					return fmt.Errorf("Unknown key: %v:%v", kk, vv)
+					return fmt.Errorf("unknown key: %v:%v", kk, vv)
 				}
 			}
 		default:
-			return fmt.Errorf("Unknown key: %v:%v", k, v)
+			return fmt.Errorf("unknown key: %v:%v", k, v)
 		}
 	}
 	return nil

http/handler_internal_test.go

@@ -35,8 +35,8 @@ func TestPostIndexRequestUnmarshalJSON(t *testing.T) {
 		{json: `{"options": {"trackExistence": false}}`, expected: postIndexRequest{Options: pilosa.IndexOptions{TrackExistence: false}}},
 		{json: `{"options": {"keys": true}}`, expected: postIndexRequest{Options: pilosa.IndexOptions{Keys: true, TrackExistence: true}}},
 		{json: `{"options": 4}`, err: "options is not map[string]interface{}"},
-		{json: `{"option": {}}`, err: "Unknown key: option:map[]"},
-		{json: `{"options": {"badKey": "test"}}`, err: "Unknown key: badKey:test"},
+		{json: `{"option": {}}`, err: "unknown key: option:map[]"},
+		{json: `{"options": {"badKey": "test"}}`, err: "unknown key: badKey:test"},
 	}
 	for _, test := range tests {
 		actual := &postIndexRequest{}

lru/lru.go

@@ -83,7 +83,7 @@ func (c *Cache) Get(key Key) (value interface{}, ok bool) {
 }
 
 // remove removes the provided key from the cache.
-func (c *Cache) remove(key Key) { // nolint: megacheck
+func (c *Cache) remove(key Key) { // nolint: staticcheck
 	if c.cache == nil {
 		return
 	}
@@ -121,7 +121,7 @@ func (c *Cache) Len() int {
 }
 
 // clear purges all stored items from the cache.
-func (c *Cache) clear() { // nolint: megacheck
+func (c *Cache) clear() { // nolint: staticcheck
 	if c.OnEvicted != nil {
 		for _, e := range c.cache {
 			kv := e.Value.(*entry)

pql/ast.go

@@ -259,7 +259,7 @@ func (c *Call) FieldArg() (string, error) {
 			return arg, nil
 		}
 	}
-	return "", fmt.Errorf("No field argument specified")
+	return "", fmt.Errorf("no field argument specified")
 }
 
 func IsReservedArg(name string) bool {

roaring/roaring.go

@@ -3868,7 +3868,7 @@ func readOfficialHeader(buf []byte) (size uint32, containerTyper func(index uint
 
 	header = pos
 	if size > (1 << 16) {
-		err = fmt.Errorf("It is logically impossible to have more than (1<<16) containers.")
+		err = fmt.Errorf("it is logically impossible to have more than (1<<16) containers")
 		return size, containerTyper, header, pos, haveRuns, err
 	}
 

roaring/roaring_internal_test.go

@@ -3224,11 +3224,11 @@ func TestContainerCombinations(t *testing.T) {
 
 //func getFunc(func(a, b *container) *container, m, n *container) *container {
 func runContainerFunc(f interface{}, c ...*Container) *Container {
-	switch f.(type) {
+	switch f := f.(type) {
 	case func(*Container) *Container:
-		return f.(func(*Container) *Container)(c[0])
+		return f(c[0])
 	case func(*Container, *Container) *Container:
-		return f.(func(a, b *Container) *Container)(c[0], c[1])
+		return f(c[0], c[1])
 	}
 	return nil
 }

server.go

@@ -487,7 +487,7 @@ func (s *Server) receiveMessage(m Message) error {
 	case *CreateShardMessage:
 		f := s.holder.Field(obj.Index, obj.Field)
 		if f == nil {
-			return fmt.Errorf("Local field not found: %s/%s", obj.Index, obj.Field)
+			return fmt.Errorf("local field not found: %s/%s", obj.Index, obj.Field)
 		}
 		if err := f.AddRemoteAvailableShards(roaring.NewBitmap(obj.Shard)); err != nil {
 			return errors.Wrap(err, "adding remote available shards")
@@ -505,7 +505,7 @@ func (s *Server) receiveMessage(m Message) error {
 	case *CreateFieldMessage:
 		idx := s.holder.Index(obj.Index)
 		if idx == nil {
-			return fmt.Errorf("Local Index not found: %s", obj.Index)
+			return fmt.Errorf("local index not found: %s", obj.Index)
 		}
 		opt := obj.Meta
 		_, err := idx.createField(obj.Field, *opt)
@@ -525,7 +525,7 @@ func (s *Server) receiveMessage(m Message) error {
 	case *CreateViewMessage:
 		f := s.holder.Field(obj.Index, obj.Field)
 		if f == nil {
-			return fmt.Errorf("Local Field not found: %s", obj.Field)
+			return fmt.Errorf("local field not found: %s", obj.Field)
 		}
 		_, _, err := f.createViewIfNotExistsBase(obj.View)
 		if err != nil {
@@ -534,7 +534,7 @@ func (s *Server) receiveMessage(m Message) error {
 	case *DeleteViewMessage:
 		f := s.holder.Field(obj.Index, obj.Field)
 		if f == nil {
-			return fmt.Errorf("Local Field not found: %s", obj.Field)
+			return fmt.Errorf("local field not found: %s", obj.Field)
 		}
 		err := f.deleteView(obj.View)
 		if err != nil {

server/cluster_test.go

@@ -399,7 +399,7 @@ func TestClusterResize_RemoveNode(t *testing.T) {
 		nodeID := mustNodeID(m0.URL())
 		resp := test.MustDo("POST", m0.URL()+fmt.Sprintf("/cluster/resize/remove-node"), fmt.Sprintf(`{"id": "%s"}`, nodeID))
 
-		expBody := "removing node: calling node leave: coordinator cannot be removed; first, make a different node the new coordinator."
+		expBody := "removing node: calling node leave: coordinator cannot be removed; first, make a different node the new coordinator"
 		if resp.StatusCode != http.StatusInternalServerError {
 			t.Fatalf("expected StatusCode %d but got %d", http.StatusInternalServerError, resp.StatusCode)
 		} else if strings.TrimSpace(resp.Body) != expBody {