Make staticcheck great again (#3103)

Makefile

@@ -228,7 +228,7 @@ goimports:
 
 staticcheck:
 	@printf $(COLOR) "Run staticcheck..."
-	@staticcheck -fail none ./...
+	@staticcheck ./...
 
 errcheck:
 	@printf $(COLOR) "Run errcheck..."

client/clientBean.go

@@ -42,8 +42,6 @@ import (
 	"go.temporal.io/server/common/cluster"
 )
 
-const clientBeanCallbackID = "clientBean"
-
 type (
 	// Bean is a collection of clients
 	Bean interface {
@@ -125,14 +123,10 @@ func (h *clientBeanImpl) registerClientEviction() {
 					continue
 				}
 				h.remoteAdminClientsLock.Lock()
-				if _, ok := h.remoteAdminClients[clusterName]; ok {
-					delete(h.remoteAdminClients, clusterName)
-				}
+				delete(h.remoteAdminClients, clusterName)
 				h.remoteAdminClientsLock.Unlock()
 				h.remoteFrontendClientsLock.Lock()
-				if _, ok := h.remoteFrontendClients[clusterName]; ok {
-					delete(h.remoteFrontendClients, clusterName)
-				}
+				delete(h.remoteFrontendClients, clusterName)
 				h.remoteFrontendClientsLock.Unlock()
 			}
 		})

cmd/server/main.go

@@ -158,6 +158,9 @@ func buildCLI() *cli.App {
 				authorizer, err := authorization.GetAuthorizerFromConfig(
 					&cfg.Global.Authorization,
 				)
+				if err != nil {
+					return cli.Exit(fmt.Sprintf("Unable to instantiate authorizer. Error: %v", err), 1)
+				}
 
 				claimMapper, err := authorization.GetClaimMapperFromConfig(&cfg.Global.Authorization, logger)
 				if err != nil {

cmd/tools/cassandra/main.go

@@ -31,5 +31,5 @@ import (
 )
 
 func main() {
-	cassandra.RunTool(os.Args) //nolint:errcheck
+	_ = cassandra.RunTool(os.Args)
 }

cmd/tools/sql/main.go

@@ -34,5 +34,5 @@ import (
 )
 
 func main() {
-	sql.RunTool(os.Args) //nolint:errcheck
+	_ = sql.RunTool(os.Args)
 }

common/archiver/filestore/queryParser.go

@@ -102,15 +102,15 @@ 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 comparsion and \"and\" expression is supported")
+		return errors.New("only comparison and \"and\" expression is supported")
 	}
 }
 

common/archiver/gcloud/connector/client.go

@@ -32,7 +32,6 @@ import (
 	"errors"
 	"io"
 	"os"
-	"regexp"
 
 	"cloud.google.com/go/storage"
 	"google.golang.org/api/iterator"
@@ -41,15 +40,10 @@ import (
 	"go.temporal.io/server/common/config"
 )
 
-const (
-	bucketNameRegExpRaw = "^gs:\\/\\/[^:\\/\n?]+"
-)
-
 var (
 	// ErrBucketNotFound is non retryable 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 (
@@ -114,7 +108,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

@@ -102,20 +102,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
@@ -169,17 +157,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

@@ -89,6 +89,7 @@ func (s *clientSuite) TestUpload() {
 	mockWriter.EXPECT().Close().Return(nil)
 
 	URI, err := archiver.NewURI("gs://my-bucket-cad/temporal_archival/development")
+	s.Require().NoError(err)
 	err = storageWrapper.Upload(ctx, URI, "myfile.history", []byte("{}"))
 	s.Require().NoError(err)
 }
@@ -110,6 +111,7 @@ func (s *clientSuite) TestUploadWriterCloseError() {
 	mockWriter.EXPECT().Close().Return(errors.New("Not Found"))
 
 	URI, err := archiver.NewURI("gs://my-bucket-cad/temporal_archival/development")
+	s.Require().NoError(err)
 	err = storageWrapper.Upload(ctx, URI, "myfile.history", []byte("{}"))
 	s.Require().EqualError(err, "Not Found")
 }
@@ -216,6 +218,7 @@ func (s *clientSuite) TestGet() {
 	mockReader.EXPECT().Close().Return(nil)
 
 	URI, err := archiver.NewURI("gs://my-bucket-cad/temporal_archival/development")
+	s.Require().NoError(err)
 	_, err = storageWrapper.Get(ctx, URI, "myfile.history")
 	s.Require().NoError(err)
 }
@@ -252,6 +255,7 @@ func (s *clientSuite) TestQuery() {
 
 	var fileNames []string
 	URI, err := archiver.NewURI("gs://my-bucket-cad/temporal_archival/development")
+	s.Require().NoError(err)
 	fileNames, err = storageWrapper.Query(ctx, URI, "7478875943689868082123907395549832634615673687049942026838")
 	s.Require().NoError(err)
 	s.Equal(strings.Join(fileNames, ", "), "fileName_01")
@@ -288,6 +292,7 @@ func (s *clientSuite) TestQueryWithFilter() {
 
 	var fileNames []string
 	URI, err := archiver.NewURI("gs://my-bucket-cad/temporal_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/queryParser.go

@@ -96,7 +96,7 @@ func (p *queryParser) Parse(query string) (*parsedQuery, error) {
 	}
 
 	if (parsedQuery.closeTime.IsZero() && parsedQuery.startTime.IsZero()) || (!parsedQuery.closeTime.IsZero() && !parsedQuery.startTime.IsZero()) {
-		return nil, errors.New("Requires a StartTime or CloseTime")
+		return nil, errors.New("requires a StartTime or CloseTime")
 	}
 
 	if parsedQuery.searchPrecision == nil {
@@ -111,15 +111,15 @@ 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 comparsion and \"and\" expression is supported")
+		return errors.New("only comparison and \"and\" expression is supported")
 	}
 }
 
@@ -233,21 +233,6 @@ func (p *queryParser) convertComparisonExpr(compExpr *sqlparser.ComparisonExpr,
 	return nil
 }
 
-func (p *queryParser) convertCloseTime(timestamp time.Time, 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 convertToTime(timeStr string) (time.Time, error) {
 	timestampStr, err := extractStringValue(timeStr)
 	if err != nil {

common/archiver/gcloud/util.go

@@ -50,11 +50,6 @@ func encode(message proto.Message) ([]byte, error) {
 	return encoder.Encode(message)
 }
 
-func constructHistoryFilename(namespaceID, workflowID, runID string, version int64) string {
-	combinedHash := constructHistoryFilenamePrefix(namespaceID, workflowID, runID)
-	return fmt.Sprintf("%s_%v.history", combinedHash, version)
-}
-
 func constructHistoryFilenameMultipart(namespaceID, workflowID, runID string, version int64, partNumber int) string {
 	combinedHash := constructHistoryFilenamePrefix(namespaceID, workflowID, runID)
 	return fmt.Sprintf("%s_%v_%v.history", combinedHash, version, partNumber)

common/archiver/s3store/historyArchiver.go

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

common/archiver/s3store/historyArchiver_test.go

@@ -77,7 +77,6 @@ type historyArchiverSuite struct {
 	suite.Suite
 	s3cli              *mocks.MockS3API
 	container          *archiver.HistoryBootstrapContainer
-	logger             log.Logger
 	testArchivalURI    archiver.URI
 	historyBatchesV1   []*archiverspb.HistoryBlob
 	historyBatchesV100 []*archiverspb.HistoryBlob

common/archiver/s3store/queryParser.go

@@ -115,15 +115,15 @@ 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 comparsion and \"and\" expression is supported")
+		return errors.New("only comparison and \"and\" expression is supported")
 	}
 }
 

common/archiver/s3store/visibilityArchiver_test.go

@@ -62,7 +62,6 @@ type visibilityArchiverSuite struct {
 	s3cli *mocks.MockS3API
 
 	container         *archiver.VisibilityBootstrapContainer
-	logger            log.Logger
 	visibilityRecords []*archiverspb.VisibilityRecord
 
 	controller      *gomock.Controller

common/archiver/util.go

@@ -33,15 +33,15 @@ import (
 )
 
 var (
-	errEmptyNamespaceID      = errors.New("NamespaceId is empty")
-	errEmptyNamespace        = errors.New("Namespace is empty")
-	errEmptyWorkflowID       = errors.New("WorkflowId is empty")
-	errEmptyRunID            = errors.New("RunId is empty")
-	errInvalidPageSize       = errors.New("PageSize should be greater than 0")
-	errEmptyWorkflowTypeName = errors.New("WorkflowTypeName is empty")
-	errEmptyStartTime        = errors.New("StartTime is empty")
-	errEmptyCloseTime        = errors.New("CloseTime is empty")
-	errEmptyQuery            = errors.New("Query string is empty")
+	errEmptyNamespaceID      = errors.New("field NamespaceId is empty")
+	errEmptyNamespace        = errors.New("field Namespace is empty")
+	errEmptyWorkflowID       = errors.New("field WorkflowId is empty")
+	errEmptyRunID            = errors.New("field RunId is empty")
+	errInvalidPageSize       = errors.New("field PageSize should be greater than 0")
+	errEmptyWorkflowTypeName = errors.New("field WorkflowTypeName is empty")
+	errEmptyStartTime        = errors.New("field StartTime is empty")
+	errEmptyCloseTime        = errors.New("field CloseTime is empty")
+	errEmptyQuery            = errors.New("field Query is empty")
 )
 
 // TagLoggerWithArchiveHistoryRequestAndURI tags logger with fields in the archive history request and the URI