Reformatting most things for go 1.19, rebuilding go.mod tools after c…

…lean, warning about different go versions (cadence-workflow#5019)

The .bin folder isn't cleaned and doesn't version itself based on all possible influences...
... so it's kinda just turning into too much of a headache when it misbehaves.

In particular, Go versions sometimes change formatting, and that tends to be baked into
formatting tools, rather than executing `go` externally.  So when e.g. `goimports` is built
with a different version of Go than you are currently using, things can get confusing.
These are now in the .build folder, so `make clean` will force a rebuild.

Downloaded bins like `buf` and `protoc` are pre-built and should generally be resistant to
this kind of leakage... and forcing a re-download on every `make clean` seems like definitely
a bad idea in some scenarios.  So the .bin folder remains, but its use is now much smaller.

---

For gofmt purposes, formatting with the latest go version _tends_ to be stable...
but unfortunately 1.19 and 1.17 disagree on protoc's code block indentation in comments.

As CI currently runs 1.17, we need to be stable with 1.17's output.
So this commit does a 1.19 format, then a 1.17 format.  This appears stable with our
hand-crafted comments, and is likely an improvement over just 1.17.

It does, unfortunately, mean that you truly must use <1.19 or your commits may not be stable.
We will likely be upgrading to 1.19 soon, but that involves a number of changes, so it's not
done here.  Stability first, upgrades second.

---

The warning is printed immediately for all commands, but does not stop you from using that version:
```
❯ make
Makefile:41: Caution: you are not using CI's go version. Expected: go1.17, current: go version go1.18 darwin/amd64
help                 Prints a help message showing any specially-commented targets
...
```

Makefile

@@ -19,9 +19,27 @@ default: help
 
 # temporary build products and book-keeping targets that are always good to / safe to clean.
 BUILD := .build
-# less-than-temporary build products, e.g. tools.
+# bins that are `make clean` friendly, i.e. they build quickly and do not require new downloads.
+# in particular this should include goimports, as it changes based on which version of go compiles it,
+# and few know to do more than `make clean`.
+BIN := $(BUILD)/bin
+# relatively stable build products, e.g. tools.
 # usually unnecessary to clean, and may require downloads to restore, so this folder is not automatically cleaned.
-BIN := .bin
+STABLE_BIN := .bin
+
+
+# current (when committed) version of Go used in CI, and ideally also our docker images.
+# this generally does not matter, but can impact goimports output.
+# for maximum stability, make sure you use the same version as CI uses.
+#
+# this can _likely_ remain a major version, as fmt output does not tend to change in minor versions,
+# which will allow findstring to match any minor version.
+EXPECTED_GO_VERSION := go1.17
+CURRENT_GO_VERSION := $(shell go version)
+ifeq (,$(findstring $(EXPECTED_GO_VERSION),$(CURRENT_GO_VERSION)))
+# if you are seeing this warning: consider using https://github.com/travis-ci/gimme to pin your version
+$(warning Caution: you are not using CI's go version. Expected: $(EXPECTED_GO_VERSION), current: $(CURRENT_GO_VERSION))
+endif
 
 # ====================================
 # book-keeping files that are used to control sequencing.
@@ -88,7 +106,7 @@ PROJECT_ROOT = github.com/uber/cadence
 
 # helper for executing bins that need other bins, just `$(BIN_PATH) the_command ...`
 # I'd recommend not exporting this in general, to reduce the chance of accidentally using non-versioned tools.
-BIN_PATH := PATH="$(abspath $(BIN)):$$PATH"
+BIN_PATH := PATH="$(abspath $(BIN)):$(abspath $(STABLE_BIN)):$$PATH"
 
 # version, git sha, etc flags.
 # reasonable to make a :=, but it's only used in one place, so just leave it lazy or do it inline.
@@ -144,7 +162,7 @@ endef
 
 # utility target.
 # use as an order-only prerequisite for targets that do not implicitly create these folders.
-$(BIN) $(BUILD):
+$(BIN) $(BUILD) $(STABLE_BIN):
 	$Q mkdir -p $@
 
 $(BIN)/thriftrw: go.mod
@@ -200,7 +218,7 @@ BUF_URL = https://github.com/bufbuild/buf/releases/download/v$(BUF_VERSION)/buf-
 # use BUF_VERSION_BIN as a bin prerequisite, not "buf", so the correct version will be used.
 # otherwise this must be a .PHONY rule, or the buf bin / symlink could become out of date.
 BUF_VERSION_BIN = buf-$(BUF_VERSION)
-$(BIN)/$(BUF_VERSION_BIN): | $(BIN)
+$(STABLE_BIN)/$(BUF_VERSION_BIN): | $(STABLE_BIN)
 	$Q echo "downloading buf $(BUF_VERSION)"
 	$Q curl -sSL $(BUF_URL) -o $@
 	$Q chmod +x $@
@@ -211,17 +229,17 @@ $(BIN)/$(BUF_VERSION_BIN): | $(BIN)
 PROTOC_VERSION = 3.14.0
 PROTOC_URL = https://github.com/protocolbuffers/protobuf/releases/download/v$(PROTOC_VERSION)/protoc-$(PROTOC_VERSION)-$(subst Darwin,osx,$(OS))-$(ARCH).zip
 # the zip contains an /include folder that we need to use to learn the well-known types
-PROTOC_UNZIP_DIR = $(BIN)/protoc-$(PROTOC_VERSION)-zip
+PROTOC_UNZIP_DIR = $(STABLE_BIN)/protoc-$(PROTOC_VERSION)-zip
 # use PROTOC_VERSION_BIN as a bin prerequisite, not "protoc", so the correct version will be used.
 # otherwise this must be a .PHONY rule, or the buf bin / symlink could become out of date.
 PROTOC_VERSION_BIN = protoc-$(PROTOC_VERSION)
-$(BIN)/$(PROTOC_VERSION_BIN): | $(BIN)
+$(STABLE_BIN)/$(PROTOC_VERSION_BIN): | $(STABLE_BIN)
 	$Q echo "downloading protoc $(PROTOC_VERSION): $(PROTOC_URL)"
 	$Q # recover from partial success
-	$Q rm -rf $(BIN)/protoc.zip $(PROTOC_UNZIP_DIR)
+	$Q rm -rf $(STABLE_BIN)/protoc.zip $(PROTOC_UNZIP_DIR)
 	$Q # download, unzip, copy to a normal location
-	$Q curl -sSL $(PROTOC_URL) -o $(BIN)/protoc.zip
-	$Q unzip -q $(BIN)/protoc.zip -d $(PROTOC_UNZIP_DIR)
+	$Q curl -sSL $(PROTOC_URL) -o $(STABLE_BIN)/protoc.zip
+	$Q unzip -q $(STABLE_BIN)/protoc.zip -d $(PROTOC_UNZIP_DIR)
 	$Q cp $(PROTOC_UNZIP_DIR)/bin/protoc $@
 
 # ====================================
@@ -276,12 +294,12 @@ PROTO_DIRS = $(sort $(dir $(PROTO_FILES)))
 # import paths due to multiple packages being compiled at once.
 #
 # After compilation files are moved to final location, as plugins adds additional path based on proto package.
-$(BUILD)/protoc: $(PROTO_FILES) $(BIN)/$(PROTOC_VERSION_BIN) $(BIN)/protoc-gen-gogofast $(BIN)/protoc-gen-yarpc-go | $(BUILD)
+$(BUILD)/protoc: $(PROTO_FILES) $(STABLE_BIN)/$(PROTOC_VERSION_BIN) $(BIN)/protoc-gen-gogofast $(BIN)/protoc-gen-yarpc-go | $(BUILD)
 	$(call ensure_idl_submodule)
 	$Q mkdir -p $(PROTO_OUT)
 	$Q echo "protoc..."
-	$Q chmod +x $(BIN)/$(PROTOC_VERSION_BIN)
-	$Q $(foreach PROTO_DIR,$(PROTO_DIRS),$(EMULATE_X86) $(BIN)/$(PROTOC_VERSION_BIN) \
+	$Q chmod +x $(STABLE_BIN)/$(PROTOC_VERSION_BIN)
+	$Q $(foreach PROTO_DIR,$(PROTO_DIRS),$(EMULATE_X86) $(STABLE_BIN)/$(PROTOC_VERSION_BIN) \
 		--plugin $(BIN)/protoc-gen-gogofast \
 		--plugin $(BIN)/protoc-gen-yarpc-go \
 		-I=$(PROTO_ROOT)/public \
@@ -310,15 +328,15 @@ $(BUILD)/protoc: $(PROTO_FILES) $(BIN)/$(PROTOC_VERSION_BIN) $(BIN)/protoc-gen-g
 .fake-codegen: .fake-protoc .fake-thrift
 	$(warning build-tool binaries have been faked, you will need to delete the $(BIN) folder if you wish to build real ones)
 	$Q # touch a marker-file for a `make clean` warning.  this does not impact behavior.
-	touch $(BIN)/fake-codegen
+	touch $(STABLE_BIN)/fake-codegen
 
 # "build" fake binaries, and touch the book-keeping files, so Make thinks codegen has been run.
 # order matters, as e.g. a $(BIN) newer than a $(BUILD) implies Make should run the $(BIN).
-.fake-protoc: | $(BIN) $(BUILD)
-	touch $(BIN)/$(PROTOC_VERSION_BIN) $(BIN)/protoc-gen-gogofast $(BIN)/protoc-gen-yarpc-go
+.fake-protoc: | $(STABLE_BIN) $(BUILD) $(BIN)
+	touch $(STABLE_BIN)/$(PROTOC_VERSION_BIN) $(BIN)/protoc-gen-gogofast $(BIN)/protoc-gen-yarpc-go
 	touch $(BUILD)/protoc
 
-.fake-thrift: | $(BIN) $(BUILD)
+.fake-thrift: | $(BUILD) $(BIN)
 	touch $(BIN)/thriftrw $(BIN)/thriftrw-plugin-yarpc
 	$Q # if the submodule exists, touch thrift_gen markers to fake their generation.
 	$Q # if it does not, do nothing - there are none.
@@ -329,8 +347,8 @@ $(BUILD)/protoc: $(PROTO_FILES) $(BIN)/$(PROTOC_VERSION_BIN) $(BIN)/protoc-gen-g
 # other intermediates
 # ====================================
 
-$(BUILD)/proto-lint: $(PROTO_FILES) $(BIN)/$(BUF_VERSION_BIN) | $(BUILD)
-	$Q cd $(PROTO_ROOT) && ../$(BIN)/$(BUF_VERSION_BIN) lint
+$(BUILD)/proto-lint: $(PROTO_FILES) $(STABLE_BIN)/$(BUF_VERSION_BIN) | $(BUILD)
+	$Q cd $(PROTO_ROOT) && ../$(STABLE_BIN)/$(BUF_VERSION_BIN) lint
 	$Q touch $@
 
 # note that LINT_SRC is fairly fake as a prerequisite.

canary/canary.go

@@ -223,6 +223,7 @@ func (c *canaryImpl) getCrossClusterTargetDomain() string {
 }
 
 // Override worker options to create large number of pollers to improve the chances of activities getting sync matched
+//
 //nolint:unused
 func overrideWorkerOptions(ctx context.Context) context.Context {
 	optionsOverride := make(map[string]map[string]string)

cmd/tools/copyright/licensegen.go

@@ -72,7 +72,7 @@ var (
 // command line utility that adds license header
 // to the source files. Usage as follows:
 //
-//  ./cmd/tools/copyright/licensegen.go
+//	./cmd/tools/copyright/licensegen.go
 func main() {
 
 	var cfg config

common/archiver/gcloud/connector/clientDelegate.go

@@ -125,7 +125,8 @@ func newClientDelegateWithCredentials(ctx context.Context, credentialsPath strin
 // The supplied name must contain only lowercase letters, numbers, dashes,
 // underscores, and dots. The full specification for valid bucket names can be
 // found at:
-//   https://cloud.google.com/storage/docs/bucket-naming
+//
+//	https://cloud.google.com/storage/docs/bucket-naming
 func (c *clientDelegate) Bucket(bucketName string) BucketHandleWrapper {
 	return &bucketDelegate{bucket: c.nativeClient.Bucket(bucketName)}
 }
@@ -135,7 +136,8 @@ func (c *clientDelegate) Bucket(bucketName string) BucketHandleWrapper {
 //
 // name must consist entirely of valid UTF-8-encoded runes. The full specification
 // for valid object names can be found at:
-//   https://cloud.google.com/storage/docs/bucket-naming
+//
+//	https://cloud.google.com/storage/docs/bucket-naming
 func (b *bucketDelegate) Object(name string) ObjectHandleWrapper {
 	return &objectDelegate{object: b.bucket.Object(name)}
 }

common/collection/concurrent_tx_map.go

@@ -68,9 +68,12 @@ type (
 // map during iterator can cause a dead lock.
 //
 // @param initialSz
-//		The initial size for the map
+//
+//	The initial size for the map
+//
 // @param hashfn
-// 		The hash function to use for sharding
+//
+//	The hash function to use for sharding
 func NewShardedConcurrentTxMap(initialCap int, hashfn HashFunc) ConcurrentTxMap {
 	cmap := new(ShardedConcurrentTxMap)
 	cmap.hashfn = hashfn

common/config/loader.go

@@ -58,10 +58,9 @@ const (
 //
 // The hierarchy is as follows from lowest to highest
 //
-//   base.yaml
-//       env.yaml   -- environment is one of the input params ex-development
-//         env_az.yaml -- zone is another input param
-//
+//	base.yaml
+//	    env.yaml   -- environment is one of the input params ex-development
+//	      env_az.yaml -- zone is another input param
 func Load(env string, configDir string, zone string, config interface{}) error {
 
 	if len(env) == 0 {

common/elasticsearch/esql/esql.go

@@ -92,16 +92,16 @@ func (e *ESql) SetBucketNum(bucketNumArg int) {
 // Transform sql to elasticsearch dsl, and prettify the output json
 //
 // usage:
-//  - dsl, sortField, err := e.ConvertPretty(sql, pageParam1, pageParam2, ...)
+//   - dsl, sortField, err := e.ConvertPretty(sql, pageParam1, pageParam2, ...)
 //
 // arguments:
-//  - sql: the sql query needs conversion in string format
-//  - pagination: variadic arguments that indicates es search_after for pagination
+//   - sql: the sql query needs conversion in string format
+//   - pagination: variadic arguments that indicates es search_after for pagination
 //
 // return values:
-//  - dsl: the elasticsearch dsl json style string
-//  - sortField: string array that contains all column names used for sorting. useful for pagination.
-//  - err: contains err information
+//   - dsl: the elasticsearch dsl json style string
+//   - sortField: string array that contains all column names used for sorting. useful for pagination.
+//   - err: contains err information
 func (e *ESql) ConvertPretty(sql string, pagination ...interface{}) (dsl string, sortField []string, err error) {
 	dsl, sortField, err = e.Convert(sql, pagination...)
 	if err != nil {
@@ -120,16 +120,16 @@ func (e *ESql) ConvertPretty(sql string, pagination ...interface{}) (dsl string,
 // Transform sql to elasticsearch dsl string
 //
 // usage:
-//  - dsl, sortField, err := e.Convert(sql, pageParam1, pageParam2, ...)
+//   - dsl, sortField, err := e.Convert(sql, pageParam1, pageParam2, ...)
 //
 // arguments:
-//  - sql: the sql query needs conversion in string format
-//  - pagination: variadic arguments that indicates es search_after
+//   - sql: the sql query needs conversion in string format
+//   - pagination: variadic arguments that indicates es search_after
 //
 // return values:
-//	- dsl: the elasticsearch dsl json style string
-//	- sortField: string array that contains all column names used for sorting. useful for pagination.
-//  - err: contains err information
+//   - dsl: the elasticsearch dsl json style string
+//   - sortField: string array that contains all column names used for sorting. useful for pagination.
+//   - err: contains err information
 func (e *ESql) Convert(sql string, pagination ...interface{}) (dsl string, sortField []string, err error) {
 	stmt, err := sqlparser.Parse(sql)
 	if err != nil {

common/log/interface.go

@@ -26,19 +26,20 @@ import (
 
 // Logger is our abstraction for logging
 // Usage examples:
-//  import "github.com/uber/cadence/common/log/tag"
-//  1) logger = logger.WithTags(
-//          tag.WorkflowNextEventID( 123),
-//          tag.WorkflowActionWorkflowStarted,
-//          tag.WorkflowDomainID("test-domain-id"))
-//     logger.Info("hello world")
-//  2) logger.Info("hello world",
-//          tag.WorkflowNextEventID( 123),
-//          tag.WorkflowActionWorkflowStarted,
-//          tag.WorkflowDomainID("test-domain-id"))
-//	   )
-//  Note: msg should be static, it is not recommended to use fmt.Sprintf() for msg.
-//        Anything dynamic should be tagged.
+//
+//	 import "github.com/uber/cadence/common/log/tag"
+//	 1) logger = logger.WithTags(
+//	         tag.WorkflowNextEventID( 123),
+//	         tag.WorkflowActionWorkflowStarted,
+//	         tag.WorkflowDomainID("test-domain-id"))
+//	    logger.Info("hello world")
+//	 2) logger.Info("hello world",
+//	         tag.WorkflowNextEventID( 123),
+//	         tag.WorkflowActionWorkflowStarted,
+//	         tag.WorkflowDomainID("test-domain-id"))
+//		   )
+//	 Note: msg should be static, it is not recommended to use fmt.Sprintf() for msg.
+//	       Anything dynamic should be tagged.
 type Logger interface {
 	Debug(msg string, tags ...tag.Tag)
 	Info(msg string, tags ...tag.Tag)

common/log/tag/tags.go

@@ -185,7 +185,7 @@ func WorkflowDomainIDs(domainIDs interface{}) Tag {
 	return newObjectTag("wf-domain-ids", domainIDs)
 }
 
-//	OperationName returns tag for OperationName
+// OperationName returns tag for OperationName
 func OperationName(operationName string) Tag {
 	return newStringTag("operation-name", operationName)
 }

common/persistence/configStoreManager.go

@@ -40,7 +40,7 @@ type (
 
 var _ ConfigStoreManager = (*configStoreManagerImpl)(nil)
 
-//NewConfigStoreManagerImpl returns new ConfigStoreManager
+// NewConfigStoreManagerImpl returns new ConfigStoreManager
 func NewConfigStoreManagerImpl(persistence ConfigStore, logger log.Logger) ConfigStoreManager {
 	return &configStoreManagerImpl{
 		serializer:  NewPayloadSerializer(),

common/persistence/domainManager.go

@@ -41,7 +41,7 @@ type (
 
 var _ DomainManager = (*domainManagerImpl)(nil)
 
-//NewDomainManagerImpl returns new DomainManager
+// NewDomainManagerImpl returns new DomainManager
 func NewDomainManagerImpl(persistence DomainStore, logger log.Logger) DomainManager {
 	return &domainManagerImpl{
 		serializer:  NewPayloadSerializer(),

common/persistence/elasticsearch/decodeBench_test.go

@@ -57,7 +57,7 @@ BenchmarkJSONDecodeToType-8       200000              9321 ns/op
 BenchmarkJSONDecodeToMap-8        100000             12878 ns/op
 */
 
-//nolint
+// nolint
 func BenchmarkJSONDecodeToType(b *testing.B) {
 	bytes := (*json.RawMessage)(&data)
 	for i := 0; i < b.N; i++ {
@@ -82,7 +82,7 @@ func BenchmarkJSONDecodeToType(b *testing.B) {
 	}
 }
 
-//nolint
+// nolint
 func BenchmarkJSONDecodeToMap(b *testing.B) {
 	for i := 0; i < b.N; i++ {
 		var source map[string]interface{}

common/persistence/elasticsearch/esVisibilityStore_test.go

@@ -586,7 +586,7 @@ func (s *ESVisibilitySuite) TestShouldSearchAfter() {
 	s.True(es.ShouldSearchAfter(token))
 }
 
-//nolint
+// nolint
 func (s *ESVisibilitySuite) TestGetESQueryDSL() {
 	request := &p.ListWorkflowExecutionsByQueryRequest{
 		DomainUUID: testDomainID,