Fix lint errors (#100)

pglass · web-flow · commit 728518dbeee7 · 2025-06-24T12:09:53.000-05:00
* Fix all lint errors
* Enable CI lint check
* Use 'golanci-lint fmt' and gci to format
* Re-run fmt and apply changes
diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml
@@ -13,9 +13,9 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  build:
+  lint:
     runs-on: ubuntu-latest
-    name: sanity checks - pull request
+    name: lint - pull request
     steps:
       - name: Checkout
         uses: actions/checkout@v4
@@ -33,12 +33,29 @@ jobs:
           go-version-file: 'go.mod'
           cache: ${{ github.ref == 'refs/heads/main' }} # only update the cache in main.
 
+      - name: Run golangci-lint
+        uses: golangci/golangci-lint-action@v8
+        with:
+          version: v2.1
+
+  build:
+    runs-on: ubuntu-latest
+    name: build and unittest - pull request
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - name: Install go
+        uses: actions/setup-go@v4
+        with:
+          go-version-file: 'go.mod'
+          cache: ${{ github.ref == 'refs/heads/main' }} # only update the cache in main.
+
       - name: Run go build
         run: go build ./...
 
-      # - name: Run lint
-      #   run: make lint
-
       - name: Run go unittest
         run: make test
 
diff --git a/.gitignore b/.gitignore
@@ -25,6 +25,7 @@ go.work
 go.work.sum
 
 # misc file
+!.golangci.yml
 /*.yaml
 /*.yml
 *.pem
diff --git a/.golangci.yml b/.golangci.yml
@@ -0,0 +1,18 @@
+# https://golangci-lint.run/usage/configuration/
+
+version: "2"
+
+linters:
+  default: standard
+  settings: {}
+
+formatters:
+  enable:
+    - gci
+  settings:
+    gci:
+      sections:
+        - standard
+        - default
+        - localmodule
+      custom-order: true
diff --git a/Makefile b/Makefile
@@ -30,15 +30,16 @@ s2s-proxy: $(ALL_SRC)
 	@printf $(COLOR) "Build s2s-proxy with CGO_ENABLED=$(CGO_ENABLED) for $(GOOS)/$(GOARCH)...\n"
 	GOOS=$(GOOS) GOARCH=$(GOARCH) CGO_ENABLED=$(CGO_ENABLED) go build -o ./bins/s2s-proxy ./cmd/proxy
 
-fmt:
-	$(GO_TOOL) goimports -local github.com/temporalio/s2s-proxy -w .
-
 update-tools:
-	$(GO_GET_TOOL) golang.org/x/tools/cmd/goimports@latest
-	$(GO_GET_TOOL) github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest
+# When changing the golangci-lint version, update the version in .github/workflows/pull-request.yml
+	$(GO_GET_TOOL) github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1
 	go mod tidy --modfile=$(TOOLS_MOD_FILE)
 
-# Lint target
+# Refer to .golangci.yml for configuration options
+fmt:
+	$(GO_TOOL) golangci-lint fmt
+
+# Refer to .golangci.yml for configuration options
 lint:
 	@printf $(COLOR) "Running golangci-lint...\n"
 	$(GO_TOOL) golangci-lint run
diff --git a/client/temporal_client.go b/client/temporal_client.go
@@ -4,12 +4,12 @@ import (
 	"fmt"
 	"sync"
 
-	"github.com/temporalio/s2s-proxy/config"
-	"github.com/temporalio/s2s-proxy/transport"
-
 	"go.temporal.io/api/workflowservice/v1"
 	"go.temporal.io/server/api/adminservice/v1"
 	"go.temporal.io/server/common/log"
+
+	"github.com/temporalio/s2s-proxy/config"
+	"github.com/temporalio/s2s-proxy/transport"
 )
 
 type (
diff --git a/cmd/proxy/main.go b/cmd/proxy/main.go
@@ -7,15 +7,15 @@ import (
 	"os/signal"
 	"syscall"
 
-	"github.com/temporalio/s2s-proxy/client"
-	"github.com/temporalio/s2s-proxy/config"
-	"github.com/temporalio/s2s-proxy/proxy"
-	"github.com/temporalio/s2s-proxy/transport"
-
 	"github.com/urfave/cli/v2"
 	"go.temporal.io/server/common/log"
 	"go.temporal.io/server/common/log/tag"
 	"go.uber.org/fx"
+
+	"github.com/temporalio/s2s-proxy/client"
+	"github.com/temporalio/s2s-proxy/config"
+	"github.com/temporalio/s2s-proxy/proxy"
+	"github.com/temporalio/s2s-proxy/transport"
 )
 
 const (
diff --git a/cmd/tools/genrpcwrappers/main.go b/cmd/tools/genrpcwrappers/main.go
@@ -39,15 +39,14 @@ import (
 
 	taskqueuepb "go.temporal.io/api/taskqueue/v1"
 	"go.temporal.io/api/workflowservice/v1"
+	"go.temporal.io/server/api/adminservice/v1"
+	"go.temporal.io/server/api/historyservice/v1"
+	"go.temporal.io/server/api/matchingservice/v1"
 	"golang.org/x/text/cases"
 	"golang.org/x/text/language"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/reflect/protoregistry"
-
-	"go.temporal.io/server/api/adminservice/v1"
-	"go.temporal.io/server/api/historyservice/v1"
-	"go.temporal.io/server/api/matchingservice/v1"
 )
 
 type (
@@ -443,13 +442,14 @@ func writeTemplatedMethod(w io.Writer, service service, impl string, m reflect.M
 		fields["WithLargeTimeout"] = "WithLargeTimeout"
 	}
 	if impl == "client" {
-		if service.name == "history" {
+		switch service.name {
+		case "history":
 			routingOptions := historyRoutingOptions(reqType)
 			if routingOptions.Custom {
 				return
 			}
 			fields["GetClient"] = makeGetHistoryClient(reqType, routingOptions)
-		} else if service.name == "matching" {
+		case "matching":
 			fields["GetClient"] = makeGetMatchingClient(reqType)
 		}
 	}
diff --git a/develop/tools.mod b/develop/tools.mod
@@ -2,10 +2,7 @@ module tools
 
 go 1.24.4
 
-tool (
-	github.com/golangci/golangci-lint/v2/cmd/golangci-lint
-	golang.org/x/tools/cmd/goimports
-)
+tool github.com/golangci/golangci-lint/v2/cmd/golangci-lint
 
 require (
 	github.com/gogo/status v1.1.1
diff --git a/encryption/tls.go b/encryption/tls.go
@@ -156,7 +156,7 @@ func fetchCACert(pathOrUrl string) (caPool *x509.CertPool, err error) {
 		if err != nil {
 			return nil, err
 		}
-		defer resp.Body.Close()
+		defer func() { _ = resp.Body.Close() }()
 		caBytes, err = io.ReadAll(resp.Body)
 		if err != nil {
 			return nil, err
diff --git a/interceptor/reflection.go b/interceptor/reflection.go
@@ -130,7 +130,9 @@ func visitSearchAttributes(obj any, match stringMatcher) (bool, error) {
 			case map[string]*common.Payload:
 				attrs, changed = translateIndexedFields(attrs, match)
 				if changed {
-					visit.Assign(vwp, reflect.ValueOf(attrs))
+					if err := visit.Assign(vwp, reflect.ValueOf(attrs)); err != nil {
+						return visit.Stop, err
+					}
 				}
 			default:
 				return visit.Stop, fmt.Errorf("unhandled search attribute type: %T", attrs)
diff --git a/interceptor/reflection_test.go b/interceptor/reflection_test.go
@@ -33,7 +33,7 @@ func BenchmarkVisitNamespace(b *testing.B) {
 						input := c.makeType(variant.inputNSName)
 
 						b.StartTimer()
-						visitNamespace(input, translator)
+						_, _ = visitNamespace(input, translator)
 					}
 				})
 			}
diff --git a/proxy/adminservice.go b/proxy/adminservice.go
@@ -3,15 +3,9 @@ package proxy
 import (
 	"context"
 	"fmt"
-	"github.com/temporalio/s2s-proxy/metrics"
 	"io"
 	"sync"
 
-	"github.com/temporalio/s2s-proxy/client"
-	adminclient "github.com/temporalio/s2s-proxy/client/admin"
-	"github.com/temporalio/s2s-proxy/common"
-	"github.com/temporalio/s2s-proxy/config"
-
 	"go.temporal.io/api/serviceerror"
 	"go.temporal.io/server/api/adminservice/v1"
 	"go.temporal.io/server/client/history"
@@ -20,6 +14,12 @@ import (
 	"go.temporal.io/server/common/log"
 	"go.temporal.io/server/common/log/tag"
 	"google.golang.org/grpc/metadata"
+
+	"github.com/temporalio/s2s-proxy/client"
+	adminclient "github.com/temporalio/s2s-proxy/client/admin"
+	"github.com/temporalio/s2s-proxy/common"
+	"github.com/temporalio/s2s-proxy/config"
+	"github.com/temporalio/s2s-proxy/metrics"
 )
 
 type (
diff --git a/proxy/proxy.go b/proxy/proxy.go
@@ -274,7 +274,7 @@ func NewProxy(
 
 func (s *Proxy) startHealthCheckHandler(cfg config.HealthCheckConfig) error {
 	if cfg.Protocol != config.HTTP {
-		return fmt.Errorf("Not supported health check protocol %s", cfg.Protocol)
+		return fmt.Errorf("unsupported health check protocol %s", cfg.Protocol)
 	}
 
 	// Set up the handler. Avoid the global ServeMux so that we can create N of these in unit test suites
diff --git a/proxy/test/echo_proxy_test.go b/proxy/test/echo_proxy_test.go
@@ -641,6 +641,6 @@ func (s *proxyTestSuite) Test_ForceStopSourceServer() {
 	// This should fail because StreamWorkflowReplicationMessages handler stopped.
 	s.ErrorContains(err, "EOF")
 
-	stream.CloseSend()
+	_ = stream.CloseSend()
 	echoClient.stop()
 }
diff --git a/proxy/test/echo_server.go b/proxy/test/echo_server.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/gogo/status"
 	"go.temporal.io/api/workflowservice/v1"
 	"go.temporal.io/server/api/adminservice/v1"
 	replicationpb "go.temporal.io/server/api/replication/v1"
@@ -15,8 +16,6 @@ import (
 	"google.golang.org/grpc/metadata"
 	"google.golang.org/protobuf/types/known/timestamppb"
 
-	"github.com/gogo/status"
-
 	"github.com/temporalio/s2s-proxy/client"
 	"github.com/temporalio/s2s-proxy/common"
 	"github.com/temporalio/s2s-proxy/config"
@@ -311,9 +310,12 @@ func (s *echoServer) SendAndRecv(sequence []int64) (map[int64]bool, error) {
 
 	s.logger.Info("==== SendAndRecv starting ====")
 	echoed, err = sendRecv(stream, sequence)
+	if err != nil {
+		s.logger.Error("sendRecv", tag.NewErrorTag("error", err))
+	}
 	_ = stream.CloseSend()
 	s.logger.Info("==== SendAndRecv completed ====")
-	return echoed, nil
+	return echoed, err
 }
 
 // Test workflowservice by making some request.
diff --git a/proxy/workflowservice.go b/proxy/workflowservice.go
@@ -3,14 +3,14 @@ package proxy
 import (
 	"context"
 
+	"go.temporal.io/api/workflowservice/v1"
+	"go.temporal.io/server/common/log"
+
 	"github.com/temporalio/s2s-proxy/auth"
 	"github.com/temporalio/s2s-proxy/client"
 	feclient "github.com/temporalio/s2s-proxy/client/frontend"
 	"github.com/temporalio/s2s-proxy/common"
 	"github.com/temporalio/s2s-proxy/config"
-
-	"go.temporal.io/api/workflowservice/v1"
-	"go.temporal.io/server/common/log"
 )
 
 type (
diff --git a/transport/grpc.go b/transport/grpc.go
@@ -64,6 +64,7 @@ func dial(hostName string, tlsConfig *tls.Config, dialer func(ctx context.Contex
 			grpc.WithContextDialer(dialer))
 	}
 
+	// nolint:staticcheck // SA1019 grpc.Dial is deprecated. Need to figure out how to use grpc.NewClient.
 	return grpc.Dial(
 		hostName,
 		dialOptions...,
diff --git a/transport/mux_connection_manager.go b/transport/mux_connection_manager.go
@@ -3,7 +3,6 @@ package transport
 import (
 	"crypto/tls"
 	"fmt"
-	"github.com/temporalio/s2s-proxy/metrics"
 	"net"
 	"sync"
 	"sync/atomic"
@@ -16,6 +15,7 @@ import (
 
 	"github.com/temporalio/s2s-proxy/config"
 	"github.com/temporalio/s2s-proxy/encryption"
+	"github.com/temporalio/s2s-proxy/metrics"
 )
 
 type status int32
@@ -62,7 +62,7 @@ func newMuxConnectManager(cfg config.MuxTransportConfig, logger log.Logger) *mux
 
 func (m *muxConnectMananger) open() (MuxTransport, error) {
 	if !m.isStarted() {
-		return nil, fmt.Errorf("Connection manager is not running.")
+		return nil, fmt.Errorf("connection manager is not running")
 	}
 
 	// Wait for transport to be connected
@@ -85,15 +85,15 @@ func (m *muxConnectMananger) open() (MuxTransport, error) {
 	select {
 	case <-m.shutdownCh:
 		m.mu.Unlock()
-		return nil, fmt.Errorf("Connection manager is not running.")
+		return nil, fmt.Errorf("connection manager is not running")
 
 	case <-m.connectedCh:
 		muxTransport = m.muxTransport
 	}
 	m.mu.Unlock()
 
 	if muxTransport.session.IsClosed() {
-		return nil, fmt.Errorf("Session is closed")
+		return nil, fmt.Errorf("session is closed")
 	}
 	return muxTransport, nil
 }
@@ -125,7 +125,7 @@ func (m *muxConnectMananger) serverLoop(setting config.TCPServerSetting) error {
 	m.wg.Add(1)
 	go func() {
 		defer func() {
-			listener.Close()
+			_ = listener.Close()
 			m.wg.Done()
 		}()
 
@@ -170,7 +170,7 @@ func (m *muxConnectMananger) serverLoop(setting config.TCPServerSetting) error {
 	go func() {
 		// wait for shutdown
 		<-m.shutdownCh
-		listener.Close() // this will cause listener.Accept to fail.
+		_ = listener.Close() // this will cause listener.Accept to fail.
 	}()
 
 	return nil
@@ -271,7 +271,7 @@ func (m *muxConnectMananger) start() error {
 		int32(statusInitialized),
 		int32(statusStarted),
 	) {
-		return fmt.Errorf("Connection manager can't be started. status: %d", m.getStatus())
+		return fmt.Errorf("connection manager can't be started. status: %d", m.getStatus())
 	}
 
 	m.shutdownCh = make(chan struct{})
@@ -290,7 +290,7 @@ func (m *muxConnectMananger) start() error {
 		}
 
 	default:
-		return fmt.Errorf("invalid multiplexed transport mode: name %s, mode %s.", m.config.Name, m.config.Mode)
+		return fmt.Errorf("invalid multiplexed transport mode: name %s, mode %s", m.config.Name, m.config.Mode)
 	}
 
 	return nil
diff --git a/transport/mux_connection_manager_test.go b/transport/mux_connection_manager_test.go
@@ -86,7 +86,7 @@ func testClient(t *testing.T, clientTs ClientTransport) {
 	require.NoError(t, err)
 	require.Equal(t, clusterName, res.ClusterName)
 
-	conn.Close()
+	require.NoError(t, conn.Close())
 }
 
 func testMuxConnection(t *testing.T, muxClientCfg config.MuxTransportConfig, muxServerCfg config.MuxTransportConfig, repeat int) {
diff --git a/transport/mux_transport.go b/transport/mux_transport.go
diff --git a/transport/transport.go b/transport/transport.go

Original file line number	Diff line number	Diff line change
`@@ -156,7 +156,7 @@ func fetchCACert(pathOrUrl string) (caPool *x509.CertPool, err error) {`
`156`	`156`	`if err != nil {`
`157`	`157`	`return nil, err`
`158`	`158`	`}`
`159`		`- defer resp.Body.Close()`
	`159`	`+ defer func() { _ = resp.Body.Close() }()`
`160`	`160`	`caBytes, err = io.ReadAll(resp.Body)`
`161`	`161`	`if err != nil {`
`162`	`162`	`return nil, err`