build: add static analysis tools (#269)

* feat: add linting

* fix: handle trailing ; in findParams

* fix(examples): vet issue

* chore: move linting to unit-tests workflow

* feat: check for consistent formatting

* feat: add staticcheck

* fix(tests): check for unused errors SA400*

* fix(tests): check errors on close (SA5001)

* fix(tests): avoid calling rand.Seed

* fix: don't use deprecated pb types

* fix: avoid deprecated grpc.WithInsecure

* fix: avoid deprecated grpc.Dial

* fix(testutil): avoid using deprecated rand funcs

* fix: remove deprecated spanner options

* fix: some staticcheck warnings

* fix: ignore field in test instead of removing

* fix: improve error output

* fix(testutil): remove unnecessary nil check

* fix(examples): don't use deprecated type

* fix(examples): additional staticcheck fixes

* fix(examples/dml-batches): use short style if statement

* fix: remove previously ineffective code

Author: egonelbre

.github/workflows/unit-tests.yml

@@ -20,3 +20,49 @@ jobs:
         uses: actions/checkout@v4
       - name: Run unit tests
         run: go test -race -short
+
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Install Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: 1.23.x
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: vet .
+        run: go vet ./...
+
+      - name: vet ./examples
+        working-directory: ./examples
+        run: go vet ./...
+
+      - name: vet ./benchmarks
+        working-directory: ./benchmarks
+        run: go vet ./...
+
+      - name: gofmt
+        run: |
+          OUT=$(gofmt -s -d .)
+          if [ -n "$OUT" ]; then
+            echo -e "$OUT"
+            echo "Code unformatted, please run 'gofmt -w -s .'"
+            exit 1
+          fi
+
+      - name: staticcheck .
+        uses: dominikh/staticcheck-action@v1.3.1
+        with:
+          version: "latest"
+          install-go: false
+          checks: "inherit,-U1000"
+          working-directory: "."
+
+      - name: staticcheck ./examples
+        uses: dominikh/staticcheck-action@v1.3.1
+        with:
+          version: "latest"
+          install-go: false
+          checks: "inherit,-U1000"
+          working-directory: "./examples"

checksum_row_iterator_test.go

@@ -196,6 +196,9 @@ func TestUpdateChecksumForNullValues(t *testing.T) {
 	enc2 := gob.NewEncoder(buffer2)
 	initial2 := new([32]byte)
 	checksum2, err := updateChecksum(enc2, buffer2, initial2, row)
+	if err != nil {
+		t.Fatalf("failed to update checksum: %v", err)
+	}
 	if *checksum != *checksum2 {
 		t.Fatalf("recalculated checksum does not match the initial calculation")
 	}

client_side_statement.go

@@ -25,7 +25,7 @@ import (
 	"time"
 
 	"cloud.google.com/go/spanner"
-	sppb "google.golang.org/genproto/googleapis/spanner/v1"
+	"cloud.google.com/go/spanner/apiv1/spannerpb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
@@ -232,33 +232,33 @@ func matchesToMap(re *regexp.Regexp, s string) map[string]string {
 // one row. This is used for client side statements that return a result set
 // containing a BOOL value.
 func createBooleanIterator(column string, value bool) (*clientSideIterator, error) {
-	return createSingleValueIterator(column, value, sppb.TypeCode_BOOL)
+	return createSingleValueIterator(column, value, spannerpb.TypeCode_BOOL)
 }
 
 // createStringIterator creates a row iterator with a single STRING column with
 // one row. This is used for client side statements that return a result set
 // containing a STRING value.
 func createStringIterator(column string, value string) (*clientSideIterator, error) {
-	return createSingleValueIterator(column, value, sppb.TypeCode_STRING)
+	return createSingleValueIterator(column, value, spannerpb.TypeCode_STRING)
 }
 
 // createTimestampIterator creates a row iterator with a single TIMESTAMP column with
 // one row. This is used for client side statements that return a result set
 // containing a TIMESTAMP value.
 func createTimestampIterator(column string, value *time.Time) (*clientSideIterator, error) {
-	return createSingleValueIterator(column, value, sppb.TypeCode_TIMESTAMP)
+	return createSingleValueIterator(column, value, spannerpb.TypeCode_TIMESTAMP)
 }
 
-func createSingleValueIterator(column string, value interface{}, code sppb.TypeCode) (*clientSideIterator, error) {
+func createSingleValueIterator(column string, value interface{}, code spannerpb.TypeCode) (*clientSideIterator, error) {
 	row, err := spanner.NewRow([]string{column}, []interface{}{value})
 	if err != nil {
 		return nil, err
 	}
 	return &clientSideIterator{
-		metadata: &sppb.ResultSetMetadata{
-			RowType: &sppb.StructType{
-				Fields: []*sppb.StructType_Field{
-					{Name: column, Type: &sppb.Type{Code: code}},
+		metadata: &spannerpb.ResultSetMetadata{
+			RowType: &spannerpb.StructType{
+				Fields: []*spannerpb.StructType_Field{
+					{Name: column, Type: &spannerpb.Type{Code: code}},
 				},
 			},
 		},
@@ -270,7 +270,7 @@ func createSingleValueIterator(column string, value interface{}, code sppb.TypeC
 // statements. All values are created and kept in memory, and this struct
 // should only be used for small result sets.
 type clientSideIterator struct {
-	metadata *sppb.ResultSetMetadata
+	metadata *spannerpb.ResultSetMetadata
 	rows     []*spanner.Row
 	index    int
 	stopped  bool
@@ -291,6 +291,6 @@ func (t *clientSideIterator) Stop() {
 	t.metadata = nil
 }
 
-func (t *clientSideIterator) Metadata() *sppb.ResultSetMetadata {
+func (t *clientSideIterator) Metadata() *spannerpb.ResultSetMetadata {
 	return t.metadata
 }

driver.go

@@ -238,7 +238,7 @@ func newConnector(d *Driver, dsn string) (*connector, error) {
 	}
 	if strval, ok := connectorConfig.params["numchannels"]; ok {
 		if val, err := strconv.Atoi(strval); err == nil && val > 0 {
-			config.NumChannels = val
+			opts = append(opts, option.WithGRPCConnectionPool(val))
 		}
 	}
 	if strval, ok := connectorConfig.params["rpcpriority"]; ok {

driver_test.go

@@ -183,12 +183,11 @@ func TestExtractDnsParts(t *testing.T) {
 					WriteSessions:                     0.2,
 					HealthCheckInterval:               spanner.DefaultSessionPoolConfig.HealthCheckInterval,
 					HealthCheckWorkers:                spanner.DefaultSessionPoolConfig.HealthCheckWorkers,
-					MaxBurst:                          spanner.DefaultSessionPoolConfig.MaxBurst,
+					MaxBurst:                          spanner.DefaultSessionPoolConfig.MaxBurst, //lint:ignore SA1019 because it's a spanner default.
 					MaxIdle:                           spanner.DefaultSessionPoolConfig.MaxIdle,
 					TrackSessionHandles:               spanner.DefaultSessionPoolConfig.TrackSessionHandles,
 					InactiveTransactionRemovalOptions: spanner.DefaultSessionPoolConfig.InactiveTransactionRemovalOptions,
 				},
-				NumChannels:           10,
 				UserAgent:             userAgent,
 				DisableRouteToLeader:  true,
 				EnableEndToEndTracing: true,
@@ -216,15 +215,15 @@ func TestExtractDnsParts(t *testing.T) {
 				if tc.wantErr {
 					t.Error("did not encounter expected error")
 				}
-				if !cmp.Equal(config, tc.wantConnectorConfig, cmp.AllowUnexported(connectorConfig{})) {
-					t.Errorf("connector config mismatch for %q\ngot: %v\nwant %v", tc.input, config, tc.wantConnectorConfig)
+				if diff := cmp.Diff(config, tc.wantConnectorConfig, cmp.AllowUnexported(connectorConfig{})); diff != "" {
+					t.Errorf("connector config mismatch for %q\n%v", tc.input, diff)
 				}
 				conn, err := newConnector(&Driver{connectors: make(map[string]*connector)}, tc.input)
 				if err != nil {
 					t.Errorf("failed to get connector for %q: %v", tc.input, err)
 				}
-				if !cmp.Equal(conn.spannerClientConfig, tc.wantSpannerConfig, cmpopts.IgnoreUnexported(spanner.ClientConfig{}, spanner.SessionPoolConfig{}, spanner.InactiveTransactionRemovalOptions{}, spannerpb.ExecuteSqlRequest_QueryOptions{})) {
-					t.Errorf("connector Spanner client config mismatch for %q\n Got: %v\nWant: %v", tc.input, conn.spannerClientConfig, tc.wantSpannerConfig)
+				if diff := cmp.Diff(conn.spannerClientConfig, tc.wantSpannerConfig, cmpopts.IgnoreUnexported(spanner.ClientConfig{}, spanner.SessionPoolConfig{}, spanner.InactiveTransactionRemovalOptions{}, spannerpb.ExecuteSqlRequest_QueryOptions{})); diff != "" {
+					t.Errorf("connector Spanner client config mismatch for %q\n%v", tc.input, diff)
 				}
 			}
 		})

driver_with_mockserver_test.go

@@ -1502,6 +1502,9 @@ func TestDdlBatch(t *testing.T) {
 
 	statements := []string{"CREATE TABLE FOO", "CREATE TABLE BAR"}
 	conn, err := db.Conn(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
 	defer conn.Close()
 
 	if _, err = conn.ExecContext(ctx, "START BATCH DDL"); err != nil {
@@ -1543,6 +1546,9 @@ func TestAbortDdlBatch(t *testing.T) {
 
 	statements := []string{"CREATE TABLE FOO", "CREATE TABLE BAR"}
 	c, err := db.Conn(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
 	defer c.Close()
 
 	if _, err = c.ExecContext(ctx, "START BATCH DDL"); err != nil {
@@ -1912,7 +1918,11 @@ func TestCommitTimestamp(t *testing.T) {
 	defer teardown()
 
 	conn, err := db.Conn(ctx)
-	defer conn.Close()
+	defer func() {
+		if err := conn.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}()
 	if err != nil {
 		t.Fatalf("failed to get a connection: %v", err)
 	}
@@ -1947,7 +1957,11 @@ func TestCommitTimestampAutocommit(t *testing.T) {
 	defer teardown()
 
 	conn, err := db.Conn(ctx)
-	defer conn.Close()
+	defer func() {
+		if err := conn.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}()
 	if err != nil {
 		t.Fatalf("failed to get a connection: %v", err)
 	}
@@ -1978,7 +1992,11 @@ func TestCommitTimestampFailsAfterRollback(t *testing.T) {
 	defer teardown()
 
 	conn, err := db.Conn(ctx)
-	defer conn.Close()
+	defer func() {
+		if err := conn.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}()
 	if err != nil {
 		t.Fatalf("failed to get a connection: %v", err)
 	}
@@ -2007,7 +2025,11 @@ func TestCommitTimestampFailsAfterAutocommitQuery(t *testing.T) {
 	defer teardown()
 
 	conn, err := db.Conn(ctx)
-	defer conn.Close()
+	defer func() {
+		if err := conn.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}()
 	if err != nil {
 		t.Fatalf("failed to get a connection: %v", err)
 	}
@@ -2034,7 +2056,11 @@ func TestShowVariableCommitTimestamp(t *testing.T) {
 	defer teardown()
 
 	conn, err := db.Conn(ctx)
-	defer conn.Close()
+	defer func() {
+		if err := conn.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}()
 	if err != nil {
 		t.Fatalf("failed to get a connection: %v", err)
 	}
@@ -2199,7 +2225,8 @@ func TestStressClientReuse(t *testing.T) {
 	_, server, teardown := setupTestDBConnection(t)
 	defer teardown()
 
-	rand.Seed(time.Now().UnixNano())
+	rng := rand.New(rand.NewSource(time.Now().UnixNano()))
+
 	numSessions := 10
 	numClients := 5
 	numParallel := 50
@@ -2215,14 +2242,16 @@ func TestStressClientReuse(t *testing.T) {
 		}
 		// Execute random operations in parallel on the database.
 		for i := 0; i < numParallel; i++ {
+			doUpdate := rng.Int()%2 == 0
+
 			wg.Add(1)
 			go func() {
 				defer wg.Done()
 				conn, err := db.Conn(ctx)
 				if err != nil {
 					t.Errorf("failed to get a connection: %v", err)
 				}
-				if rand.Int()%2 == 0 {
+				if doUpdate {
 					if _, err := conn.ExecContext(ctx, testutil.UpdateBarSetFoo); err != nil {
 						t.Errorf("failed to execute update on connection: %v", err)
 					}
@@ -2262,7 +2291,11 @@ func TestExcludeTxnFromChangeStreams_AutoCommitUpdate(t *testing.T) {
 	defer teardown()
 
 	conn, err := db.Conn(ctx)
-	defer conn.Close()
+	defer func() {
+		if err := conn.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}()
 	if err != nil {
 		t.Fatalf("failed to get a connection: %v", err)
 	}
@@ -2306,7 +2339,11 @@ func TestExcludeTxnFromChangeStreams_AutoCommitBatchDml(t *testing.T) {
 	defer teardown()
 
 	conn, err := db.Conn(ctx)
-	defer conn.Close()
+	defer func() {
+		if err := conn.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}()
 	if err != nil {
 		t.Fatalf("failed to get a connection: %v", err)
 	}
@@ -2341,7 +2378,11 @@ func TestExcludeTxnFromChangeStreams_PartitionedDml(t *testing.T) {
 	defer teardown()
 
 	conn, err := db.Conn(ctx)
-	defer conn.Close()
+	defer func() {
+		if err := conn.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}()
 	if err != nil {
 		t.Fatalf("failed to get a connection: %v", err)
 	}
@@ -2368,7 +2409,11 @@ func TestExcludeTxnFromChangeStreams_Transaction(t *testing.T) {
 	defer teardown()
 
 	conn, err := db.Conn(ctx)
-	defer conn.Close()
+	defer func() {
+		if err := conn.Close(); err != nil {
+			t.Fatal(err)
+		}
+	}()
 	if err != nil {
 		t.Fatalf("failed to get a connection: %v", err)
 	}

examples/commit-timestamp/main.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"time"
 
-	_ "github.com/googleapis/go-sql-spanner"
 	spannerdriver "github.com/googleapis/go-sql-spanner"
 	"github.com/googleapis/go-sql-spanner/examples"
 )

examples/data-types/main.go

@@ -59,7 +59,7 @@ func dataTypes(projectId, instanceId, databaseId string) error {
 	ctx := context.Background()
 	db, err := sql.Open("spanner", fmt.Sprintf("projects/%s/instances/%s/databases/%s", projectId, instanceId, databaseId))
 	if err != nil {
-		return fmt.Errorf("failed to open database connection: %v\n", err)
+		return fmt.Errorf("failed to open database connection: %v", err)
 	}
 	defer db.Close()
 
@@ -75,7 +75,7 @@ func dataTypes(projectId, instanceId, databaseId string) error {
 		1, true, "string", []byte("bytes"), 100, float32(3.14), 3.14, *big.NewRat(1, 1), civil.DateOf(time.Now()), time.Now(),
 		[]bool{true, false}, []string{"s1", "s2"}, [][]byte{[]byte("b1"), []byte("b2")}, []int64{1, 2},
 		[]float32{1.1, 2.2}, []float64{1.1, 2.2}, []big.Rat{*big.NewRat(1, 2), *big.NewRat(1, 3)},
-		[]civil.Date{{2021, 10, 12}, {2021, 10, 13}},
+		[]civil.Date{{Year: 2021, Month: 10, Day: 12}, {Year: 2021, Month: 10, Day: 13}},
 		[]time.Time{time.Now(), time.Now().Add(24 * time.Hour)}); err != nil {
 		return fmt.Errorf("failed to insert a record with all non-null values using DML: %v", err)
 	}

examples/ddl-batches/main.go

@@ -19,7 +19,6 @@ import (
 	"database/sql"
 	"fmt"
 
-	_ "github.com/googleapis/go-sql-spanner"
 	spannerdriver "github.com/googleapis/go-sql-spanner"
 	"github.com/googleapis/go-sql-spanner/examples"
 )
@@ -40,7 +39,7 @@ func ddlBatches(projectId, instanceId, databaseId string) error {
 	ctx := context.Background()
 	db, err := sql.Open("spanner", fmt.Sprintf("projects/%s/instances/%s/databases/%s", projectId, instanceId, databaseId))
 	if err != nil {
-		return fmt.Errorf("failed to open database connection: %v\n", err)
+		return fmt.Errorf("failed to open database connection: %v", err)
 	}
 	defer db.Close()