Staticcheck cleanup (cadence-workflow#4751)

* unnecessary use of fmt.Sprintf (S1039)

* could eliminate this type assertion (S1034)

* package is being imported more than once (ST1019)

* redundant return statement (S1023)

* should use make(...) instead (S1019)

* should omit nil check; len() for nil slices is defined as zero (S1009)

* should merge variable declaration with assignment on next line (S1021)

* should use fmt.Fprintf instead of fmt.Fprint(fmt.Sprintf(...)) (S1038)

* hould replace this if statement with an unconditional strings.TrimPrefix (S1017)

* should use bytes.Equal(data, data2) instead (S1004)

* should use 'return X' instead of 'if X { return true }; return false' (S1008)

* should omit comparison to bool constant, can be simplified to trees[br.TreeID] (S1002)

* should replace loop with ancestors = append(ancestors, branchInfo.Ancestors...) (S1011)

* should use a simple channel send/receive instead of select with a single case (S1000)

* value of type int cannot be used with binary.Write (SA1003)

* do not pass a nil Context, even if a function permits it; pass context.TODO if you are unsure about which Context to use (SA1012)

* Using a deprecated function, variable, constant or field (SA1019)

* should not use built-in type string as key for value; define your own type to avoid collisions (SA1029)

* removed unused code (U1000)

* error strings should not be capitalized (ST1005)

* don't use unit-specific suffix Seconds (ST1011)

* should use time.Since instead of time.Now().Sub (S1012)

* should use time.Until instead of t.Sub(time.Now()) (S1024)

* this value of ... is never used (SA4006)

* Fixing integration test

* Fix TestDNSSRVMode

* Fix TestExecutionFixerActivity_Success

* Use int64 for binary.Write

* Update service/frontend/clusterRedirectionHandler_test.go

Co-authored-by: Steven L <[email protected]>

Co-authored-by: Steven L <[email protected]>

bench/load/concurrentexec/batchWorkflow.go

@@ -143,7 +143,7 @@ func concurrentActivity(
 ) (time.Duration, error) {
 	var latency time.Duration
 	if activity.GetInfo(ctx).Attempt == 0 {
-		latency = time.Now().Sub(time.Unix(0, scheduledTimeNanos))
+		latency = time.Since(time.Unix(0, scheduledTimeNanos))
 	}
 
 	time.Sleep(time.Duration(rand.Intn(maxSleepTimeInSeconds)) * time.Second)

canary/common.go

@@ -53,7 +53,7 @@ func int32Ptr(v int32) *int32 {
 
 // getContextValue retrieves and returns the value corresponding
 // to the given key - panics if the key does not exist
-func getContextValue(ctx context.Context, key string) interface{} {
+func getContextValue(ctx context.Context, key contextKey) interface{} {
 	value := ctx.Value(key)
 	if value == nil {
 		panic("ctx.Value(" + key + ") returned nil")

canary/const.go

@@ -35,11 +35,11 @@ const (
 	activityTaskTimeout           = 3 * time.Minute
 	childWorkflowTimeout          = 6 * time.Minute
 	taskListName                  = "canary-task-queue"
-	ctxKeyActivityRuntime         = "runtime"
-	ctxKeyActivityArchivalRuntime = "runtime-archival"
-	ctxKeyActivitySystemClient    = "system-client"
-	ctxKeyActivityBatcherClient   = "batcher-client"
-	ctxKeyConfig                  = "runtime-config"
+	ctxKeyActivityRuntime         = contextKey("runtime")
+	ctxKeyActivityArchivalRuntime = contextKey("runtime-archival")
+	ctxKeyActivitySystemClient    = contextKey("system-client")
+	ctxKeyActivityBatcherClient   = contextKey("batcher-client")
+	ctxKeyConfig                  = contextKey("runtime-config")
 	archivalDomain                = "canary-archival-domain"
 	archivalTaskListName          = "canary-archival-task-queue"
 )

canary/historyArchival.go

@@ -172,5 +172,5 @@ func archivalExternalWorkflow(ctx workflow.Context, scheduledTimeNanos int64) er
 }
 
 func largeResultActivity() ([]byte, error) {
-	return make([]byte, resultSize, resultSize), nil
+	return make([]byte, resultSize), nil
 }

canary/retry.go

@@ -32,9 +32,9 @@ import (
 )
 
 var (
-	errRetryableActivityError = errors.New("Retry me")
-	errUnexpectedProgress     = errors.New("Unexpected progress")
-	errUnexpectedResult       = errors.New("Unexpected result")
+	errRetryableActivityError = errors.New("retry me")
+	errUnexpectedProgress     = errors.New("unexpected progress")
+	errUnexpectedResult       = errors.New("unexpected result")
 )
 
 func init() {

canary/timeout.go

@@ -82,9 +82,7 @@ func timeoutActivity(ctx context.Context, scheduledTimeNanos int64) error {
 	defer recordActivityEnd(scope, sw, err)
 
 	timer := time.NewTimer(activityDelay)
-	select {
-	case <-timer.C:
-	}
+	<-timer.C
 	timer.Stop()
 
 	return nil

cmd/server/cadence/cadence.go

@@ -87,16 +87,12 @@ func startHandler(c *cli.Context) {
 		server.Start()
 	}
 
-	select {
-	case <-sigc:
-		{
-			log.Println("Received SIGTERM signal, initiating shutdown.")
-			for _, daemon := range daemons {
-				daemon.Stop()
-			}
-			os.Exit(0)
-		}
+	<-sigc
+	log.Println("Received SIGTERM signal, initiating shutdown.")
+	for _, daemon := range daemons {
+		daemon.Stop()
 	}
+	os.Exit(0)
 }
 
 func getEnvironment(c *cli.Context) string {

common/archiver/filestore/queryParser.go

@@ -95,13 +95,13 @@ func (p *queryParser) convertWhereExpr(expr sqlparser.Expr, parsedQuery *parsedQ
 		return errors.New("where expression is nil")
 	}
 
-	switch expr.(type) {
+	switch expr := expr.(type) {
 	case *sqlparser.ComparisonExpr:
-		return p.convertComparisonExpr(expr.(*sqlparser.ComparisonExpr), parsedQuery)
+		return p.convertComparisonExpr(expr, parsedQuery)
 	case *sqlparser.AndExpr:
-		return p.convertAndExpr(expr.(*sqlparser.AndExpr), parsedQuery)
+		return p.convertAndExpr(expr, parsedQuery)
 	case *sqlparser.ParenExpr:
-		return p.convertParenExpr(expr.(*sqlparser.ParenExpr), parsedQuery)
+		return p.convertParenExpr(expr, parsedQuery)
 	default:
 		return errors.New("only comparison and \"and\" expression is supported")
 	}

common/archiver/gcloud/connector/client.go

@@ -27,7 +27,6 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"regexp"
 
 	"cloud.google.com/go/storage"
 	"google.golang.org/api/iterator"
@@ -36,15 +35,10 @@ import (
 	"github.com/uber/cadence/common/config"
 )
 
-const (
-	bucketNameRegExpRaw = "^gs:\\/\\/[^:\\/\n?]+"
-)
-
 var (
 	// ErrBucketNotFound is non retriable error that is thrown when the bucket doesn't exist
 	ErrBucketNotFound = errors.New("bucket not found")
 	errObjectNotFound = errors.New("object not found")
-	bucketNameRegExp  = regexp.MustCompile(bucketNameRegExpRaw)
 )
 
 type (
@@ -109,7 +103,6 @@ func (s *storageWrapper) Upload(ctx context.Context, URI archiver.URI, fileName
 // Exist check if a bucket or an object exist
 // If fileName is empty, then 'Exist' function will only check if the given bucket exist.
 func (s *storageWrapper) Exist(ctx context.Context, URI archiver.URI, fileName string) (exists bool, err error) {
-	err = ErrBucketNotFound
 	bucket := s.client.Bucket(URI.Hostname())
 	if _, err := bucket.Attrs(ctx); err != nil {
 		return false, err

common/archiver/gcloud/connector/clientDelegate.go

@@ -23,7 +23,6 @@ package connector
 import (
 	"context"
 	"io/ioutil"
-	"os"
 
 	"cloud.google.com/go/storage"
 	"golang.org/x/oauth2/google"
@@ -97,20 +96,8 @@ type (
 	ObjectIteratorWrapper interface {
 		Next() (*storage.ObjectAttrs, error)
 	}
-
-	objectIteratorDelegate struct {
-		iterator *storage.ObjectIterator
-	}
 )
 
-func newClientDelegate() (*clientDelegate, error) {
-	ctx := context.Background()
-	if credentialsPath := os.Getenv("GOOGLE_APPLICATION_CREDENTIALS"); credentialsPath != "" {
-		return newClientDelegateWithCredentials(ctx, credentialsPath)
-	}
-	return newDefaultClientDelegate(ctx)
-}
-
 func newDefaultClientDelegate(ctx context.Context) (*clientDelegate, error) {
 	nativeClient, err := storage.NewClient(ctx)
 	return &clientDelegate{nativeClient: nativeClient}, err
@@ -164,17 +151,6 @@ func (b *bucketDelegate) Attrs(ctx context.Context) (*storage.BucketAttrs, error
 	return b.bucket.Attrs(ctx)
 }
 
-// Next returns the next result. Its second return value is iterator.Done if
-// there are no more results. Once Next returns iterator.Done, all subsequent
-// calls will return iterator.Done.
-//
-// If Query.Delimiter is non-empty, some of the ObjectAttrs returned by Next will
-// have a non-empty Prefix field, and a zero value for all other fields. These
-// represent prefixes.
-func (o *objectIteratorDelegate) Next() (*storage.ObjectAttrs, error) {
-	return o.iterator.Next()
-}
-
 // NewWriter returns a storage Writer that writes to the GCS object
 // associated with this ObjectHandle.
 //

common/archiver/gcloud/connector/client_test.go

@@ -84,6 +84,7 @@ func (s *clientSuite) TestUpload() {
 	mockWriter.On("Close").Return(nil).Times(1)
 
 	URI, err := archiver.NewURI("gs://my-bucket-cad/cadence_archival/development")
+	s.Require().NoError(err)
 	err = storageWrapper.Upload(ctx, URI, "myfile.history", []byte("{}"))
 	s.Require().NoError(err)
 }
@@ -105,6 +106,7 @@ func (s *clientSuite) TestUploadWriterCloseError() {
 	mockWriter.On("Close").Return(errors.New("Not Found")).Times(1)
 
 	URI, err := archiver.NewURI("gs://my-bucket-cad/cadence_archival/development")
+	s.Require().NoError(err)
 	err = storageWrapper.Upload(ctx, URI, "myfile.history", []byte("{}"))
 	s.Require().EqualError(err, "Not Found")
 }
@@ -211,6 +213,7 @@ func (s *clientSuite) TestGet() {
 	mockReader.On("Close").Return(nil).Times(1)
 
 	URI, err := archiver.NewURI("gs://my-bucket-cad/cadence_archival/development")
+	s.Require().NoError(err)
 	_, err = storageWrapper.Get(ctx, URI, "myfile.history")
 	s.Require().NoError(err)
 }
@@ -253,6 +256,7 @@ func (s *clientSuite) TestQuery() {
 
 	var fileNames []string
 	URI, err := archiver.NewURI("gs://my-bucket-cad/cadence_archival/development")
+	s.Require().NoError(err)
 	fileNames, err = storageWrapper.Query(ctx, URI, "7478875943689868082123907395549832634615673687049942026838")
 	s.Require().NoError(err)
 	s.Equal(strings.Join(fileNames, ", "), "fileName_01")
@@ -299,6 +303,7 @@ func (s *clientSuite) TestQueryWithFilter() {
 
 	var fileNames []string
 	URI, err := archiver.NewURI("gs://my-bucket-cad/cadence_archival/development")
+	s.Require().NoError(err)
 	fileNames, _, _, err = storageWrapper.QueryWithFilters(ctx, URI, "closeTimeout_2020-02-27T09:42:28Z", 0, 0, []connector.Precondition{newWorkflowIDPrecondition("4418294404690464320")})
 
 	s.Require().NoError(err)

common/archiver/gcloud/historyArchiver_test.go

@@ -552,6 +552,7 @@ func (h *historyArchiverSuite) TestGet_Success_FromToken() {
 	ctx := context.Background()
 	mockCtrl := gomock.NewController(h.T())
 	URI, err := archiver.NewURI("gs://my-bucket-cad/cadence_archival/development")
+	h.Require().NoError(err)
 	storageWrapper := &mocks.Client{}
 	storageWrapper.On("Exist", ctx, URI, "").Return(true, nil).Times(1)
 	storageWrapper.On("Query", ctx, URI, "71817125141568232911739672280485489488911532452831150339470").Return([]string{"905702227796330300141628222723188294514017512010591354159_-24_0.history", "905702227796330300141628222723188294514017512010591354159_-24_1.history", "905702227796330300141628222723188294514017512010591354159_-24_2.history", "905702227796330300141628222723188294514017512010591354159_-24_3.history", "905702227796330300141628222723188294514017512010591354159_-25_0.history"}, nil).Times(1)

common/archiver/gcloud/queryParser.go

@@ -26,13 +26,11 @@ import (
 	"errors"
 	"fmt"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/xwb1989/sqlparser"
 
 	"github.com/uber/cadence/common"
-	"github.com/uber/cadence/common/types"
 )
 
 type (
@@ -96,7 +94,7 @@ func (p *queryParser) Parse(query string) (*parsedQuery, error) {
 	}
 
 	if (parsedQuery.closeTime == 0 && parsedQuery.startTime == 0) || (parsedQuery.closeTime != 0 && parsedQuery.startTime != 0) {
-		return nil, errors.New("Requires a StartTime or CloseTime")
+		return nil, errors.New("requires a StartTime or CloseTime")
 	}
 
 	if parsedQuery.searchPrecision == nil {
@@ -111,13 +109,13 @@ func (p *queryParser) convertWhereExpr(expr sqlparser.Expr, parsedQuery *parsedQ
 		return errors.New("where expression is nil")
 	}
 
-	switch expr.(type) {
+	switch expr := expr.(type) {
 	case *sqlparser.ComparisonExpr:
-		return p.convertComparisonExpr(expr.(*sqlparser.ComparisonExpr), parsedQuery)
+		return p.convertComparisonExpr(expr, parsedQuery)
 	case *sqlparser.AndExpr:
-		return p.convertAndExpr(expr.(*sqlparser.AndExpr), parsedQuery)
+		return p.convertAndExpr(expr, parsedQuery)
 	case *sqlparser.ParenExpr:
-		return p.convertParenExpr(expr.(*sqlparser.ParenExpr), parsedQuery)
+		return p.convertParenExpr(expr, parsedQuery)
 	default:
 		return errors.New("only comparison and \"and\" expression is supported")
 	}
@@ -233,21 +231,6 @@ func (p *queryParser) convertComparisonExpr(compExpr *sqlparser.ComparisonExpr,
 	return nil
 }
 
-func (p *queryParser) convertCloseTime(timestamp int64, op string, parsedQuery *parsedQuery) error {
-	switch op {
-	case "=":
-		if err := p.convertCloseTime(timestamp, ">=", parsedQuery); err != nil {
-			return err
-		}
-		if err := p.convertCloseTime(timestamp, "<=", parsedQuery); err != nil {
-			return err
-		}
-	default:
-		return fmt.Errorf("operator %s is not supported for close time", op)
-	}
-	return nil
-}
-
 func convertToTimestamp(timeStr string) (int64, error) {
 	timestamp, err := strconv.ParseInt(timeStr, 10, 64)
 	if err == nil {
@@ -264,24 +247,6 @@ func convertToTimestamp(timeStr string) (int64, error) {
 	return parsedTime.UnixNano(), nil
 }
 
-func convertStatusStr(statusStr string) (types.WorkflowExecutionCloseStatus, error) {
-	statusStr = strings.ToLower(statusStr)
-	switch statusStr {
-	case "completed":
-		return types.WorkflowExecutionCloseStatusCompleted, nil
-	case "failed":
-		return types.WorkflowExecutionCloseStatusFailed, nil
-	case "canceled":
-		return types.WorkflowExecutionCloseStatusCanceled, nil
-	case "continuedasnew":
-		return types.WorkflowExecutionCloseStatusContinuedAsNew, nil
-	case "timedout":
-		return types.WorkflowExecutionCloseStatusTimedOut, nil
-	default:
-		return 0, fmt.Errorf("unknown workflow close status: %s", statusStr)
-	}
-}
-
 func extractStringValue(s string) (string, error) {
 	if len(s) >= 2 && s[0] == '\'' && s[len(s)-1] == '\'' {
 		return s[1 : len(s)-1], nil

common/archiver/gcloud/util.go

@@ -50,11 +50,6 @@ func decodeHistoryBatches(data []byte) ([]*types.History, error) {
 	return historyBatches, nil
 }
 
-func constructHistoryFilename(domainID, workflowID, runID string, version int64) string {
-	combinedHash := constructHistoryFilenamePrefix(domainID, workflowID, runID)
-	return fmt.Sprintf("%s_%v.history", combinedHash, version)
-}
-
 func constructHistoryFilenameMultipart(domainID, workflowID, runID string, version int64, partNumber int) string {
 	combinedHash := constructHistoryFilenamePrefix(domainID, workflowID, runID)
 	return fmt.Sprintf("%s_%v_%v.history", combinedHash, version, partNumber)

common/archiver/s3store/historyArchiver.go

@@ -68,7 +68,6 @@ type (
 		s3cli     s3iface.S3API
 		// only set in test code
 		historyIterator archiver.HistoryIterator
-		config          *config.S3Archiver
 	}
 
 	getHistoryToken struct {

common/archiver/s3store/historyArchiver_test.go

@@ -46,7 +46,6 @@ import (
 	"github.com/uber/cadence/common"
 	"github.com/uber/cadence/common/archiver"
 	"github.com/uber/cadence/common/archiver/s3store/mocks"
-	"github.com/uber/cadence/common/log"
 	"github.com/uber/cadence/common/log/loggerimpl"
 	"github.com/uber/cadence/common/metrics"
 	"github.com/uber/cadence/common/types"
@@ -73,7 +72,6 @@ type historyArchiverSuite struct {
 	suite.Suite
 	s3cli              *mocks.S3API
 	container          *archiver.HistoryBootstrapContainer
-	logger             log.Logger
 	testArchivalURI    archiver.URI
 	historyBatchesV1   []*archiver.HistoryBlob
 	historyBatchesV100 []*archiver.HistoryBlob

common/archiver/s3store/queryParser.go

@@ -110,13 +110,13 @@ func (p *queryParser) convertWhereExpr(expr sqlparser.Expr, parsedQuery *parsedQ
 		return errors.New("where expression is nil")
 	}
 
-	switch expr.(type) {
+	switch expr := expr.(type) {
 	case *sqlparser.ComparisonExpr:
-		return p.convertComparisonExpr(expr.(*sqlparser.ComparisonExpr), parsedQuery)
+		return p.convertComparisonExpr(expr, parsedQuery)
 	case *sqlparser.AndExpr:
-		return p.convertAndExpr(expr.(*sqlparser.AndExpr), parsedQuery)
+		return p.convertAndExpr(expr, parsedQuery)
 	case *sqlparser.ParenExpr:
-		return p.convertParenExpr(expr.(*sqlparser.ParenExpr), parsedQuery)
+		return p.convertParenExpr(expr, parsedQuery)
 	default:
 		return errors.New("only comparison and \"and\" expression is supported")
 	}

common/archiver/s3store/visibilityArchiver_test.go

@@ -40,7 +40,6 @@ import (
 	"github.com/uber/cadence/common"
 	"github.com/uber/cadence/common/archiver"
 	"github.com/uber/cadence/common/archiver/s3store/mocks"
-	"github.com/uber/cadence/common/log"
 	"github.com/uber/cadence/common/log/loggerimpl"
 	"github.com/uber/cadence/common/metrics"
 	"github.com/uber/cadence/common/types"
@@ -52,7 +51,6 @@ type visibilityArchiverSuite struct {
 	s3cli *mocks.S3API
 
 	container         *archiver.VisibilityBootstrapContainer
-	logger            log.Logger
 	visibilityRecords []*visibilityRecord
 
 	controller      *gomock.Controller

common/archiver/util.go

@@ -30,7 +30,7 @@ import (
 
 var (
 	errEmptyDomainID         = errors.New("DomainID is empty")
-	errEmptyDomainName       = errors.New("Domain name is empty")
+	errEmptyDomainName       = errors.New("DomainName is empty")
 	errEmptyWorkflowID       = errors.New("WorkflowID is empty")
 	errEmptyRunID            = errors.New("RunID is empty")
 	errInvalidPageSize       = errors.New("PageSize should be greater than 0")