Enabled maligned, staticcheck and errcheck linters (#206)

* Enabled maligned check * Enabled staticcheck * Fixes for golint * Enabled errcheck linter * Added fixes for error check * Added errcheck for tests * Fixed test * Increased golangci-lint deadline for travis * Increased golangci-lint deadline for travis * Decreased golangci-lint deadline for travis * Revert for backward compatibility * Using log.Println() instead of fmt.Println() * Handling os.RemoveAll() errors * Using t.Error(error) instead of t.Errorf("%v", err) * Using t.Fatal(error) instead of t.Fatalf("%v", err) * Using fmt.Sprint(sum) instead of t.Srintf("%v", sum) * Refactoring * Revert for backward compatibility * Revert * go mod tidy * Added error logging * Added error logging * Added error handling * Added error handling * Added error logging * Fix error logging * Added error handling * Fix * Added logging for migr.Buffer() * Fixes * Firebird test disabled * Fixed nolint comment * Updated firebird docker image version * Disabled test for firebird 2.5 * Fixed // nolint
golang-migrate · Apr 26, 2019 · 6c96ef0 · 6c96ef0
1 parent ddc7246
commit 6c96ef0
Show file tree

Hide file tree

Showing 37 changed files with 645 additions and 263 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -1,3 +1,6 @@
+run:
+  # timeout for analysis, e.g. 30s, 5m, default is 1m
+  deadline: 2m
 linters:
   enable:
     #- golint
@@ -7,14 +10,11 @@ linters:
     - goconst
     - gofmt
     - misspell
-    #- maligned
+    - maligned
     - unparam
     - nakedret
     - prealloc
     #- gosec
-  disable:
-    - errcheck
-    - staticcheck
 linters-settings:
   misspell:
     locale: US

diff --git a/database/cassandra/cassandra_test.go b/database/cassandra/cassandra_test.go
@@ -68,9 +68,13 @@ func Test(t *testing.T) {
 		p := &Cassandra{}
 		d, err := p.Open(addr)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
-		defer d.Close()
+		defer func() {
+			if err := d.Close(); err != nil {
+				t.Error(err)
+			}
+		}()
 		dt.Test(t, d, []byte("SELECT table_name from system_schema.tables"))
 	})
 }
@@ -85,13 +89,17 @@ func TestMigrate(t *testing.T) {
 		p := &Cassandra{}
 		d, err := p.Open(addr)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
-		defer d.Close()
+		defer func() {
+			if err := d.Close(); err != nil {
+				t.Error(err)
+			}
+		}()
 
 		m, err := migrate.NewWithDatabaseInstance("file://./examples/migrations", "testks", d)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
 		dt.TestMigrate(t, m, []byte("SELECT table_name from system_schema.tables"))
 	})

diff --git a/database/clickhouse/clickhouse.go b/database/clickhouse/clickhouse.go
@@ -204,15 +204,18 @@ func (ch *ClickHouse) ensureVersionTable() (err error) {
 	return nil
 }
 
-func (ch *ClickHouse) Drop() error {
-	var (
-		query       = "SHOW TABLES FROM " + ch.config.DatabaseName
-		tables, err = ch.conn.Query(query)
-	)
+func (ch *ClickHouse) Drop() (err error) {
+	query := "SHOW TABLES FROM " + ch.config.DatabaseName
+	tables, err := ch.conn.Query(query)
+
 	if err != nil {
 		return &database.Error{OrigErr: err, Query: []byte(query)}
 	}
-	defer tables.Close()
+	defer func() {
+		if errClose := tables.Close(); errClose != nil {
+			err = multierror.Append(err, errClose)
+		}
+	}()
 	for tables.Next() {
 		var table string
 		if err := tables.Scan(&table); err != nil {

diff --git a/database/cockroachdb/cockroachdb.go b/database/cockroachdb/cockroachdb.go
@@ -150,7 +150,7 @@ func (c *CockroachDb) Close() error {
 // Locking is done manually with a separate lock table.  Implementing advisory locks in CRDB is being discussed
 // See: https://github.com/cockroachdb/cockroach/issues/13546
 func (c *CockroachDb) Lock() error {
-	err := crdb.ExecuteTx(context.Background(), c.db, nil, func(tx *sql.Tx) error {
+	err := crdb.ExecuteTx(context.Background(), c.db, nil, func(tx *sql.Tx) (err error) {
 		aid, err := database.GenerateAdvisoryLockId(c.config.DatabaseName)
 		if err != nil {
 			return err
@@ -161,7 +161,11 @@ func (c *CockroachDb) Lock() error {
 		if err != nil {
 			return database.Error{OrigErr: err, Err: "failed to fetch migration lock", Query: []byte(query)}
 		}
-		defer rows.Close()
+		defer func() {
+			if errClose := rows.Close(); errClose != nil {
+				err = multierror.Append(err, errClose)
+			}
+		}()
 
 		// If row exists at all, lock is present
 		locked := rows.Next()
@@ -267,14 +271,18 @@ func (c *CockroachDb) Version() (version int, dirty bool, err error) {
 	}
 }
 
-func (c *CockroachDb) Drop() error {
+func (c *CockroachDb) Drop() (err error) {
 	// select all tables in current schema
 	query := `SELECT table_name FROM information_schema.tables WHERE table_schema=(SELECT current_schema())`
 	tables, err := c.db.Query(query)
 	if err != nil {
 		return &database.Error{OrigErr: err, Query: []byte(query)}
 	}
-	defer tables.Close()
+	defer func() {
+		if errClose := tables.Close(); errClose != nil {
+			err = multierror.Append(err, errClose)
+		}
+	}()
 
 	// delete one table after another
 	tableNames := make([]string, 0)

diff --git a/database/cockroachdb/cockroachdb_test.go b/database/cockroachdb/cockroachdb_test.go
@@ -7,6 +7,7 @@ import (
 	"database/sql"
 	"fmt"
 	"github.com/golang-migrate/migrate/v4"
+	"log"
 	"strings"
 	"testing"
 )
@@ -38,20 +39,22 @@ var (
 func isReady(ctx context.Context, c dktest.ContainerInfo) bool {
 	ip, port, err := c.Port(defaultPort)
 	if err != nil {
-		fmt.Println("port error:", err)
+		log.Println("port error:", err)
 		return false
 	}
 
 	db, err := sql.Open("postgres", fmt.Sprintf("postgres://root@%v:%v?sslmode=disable", ip, port))
 	if err != nil {
-		fmt.Println("open error:", err)
+		log.Println("open error:", err)
 		return false
 	}
 	if err := db.PingContext(ctx); err != nil {
-		fmt.Println("ping error:", err)
+		log.Println("ping error:", err)
 		return false
 	}
-	db.Close()
+	if err := db.Close(); err != nil {
+		log.Println("close error:", err)
+	}
 	return true
 }
 
@@ -68,7 +71,11 @@ func createDB(t *testing.T, c dktest.ContainerInfo) {
 	if err = db.Ping(); err != nil {
 		t.Fatal(err)
 	}
-	defer db.Close()
+	defer func() {
+		if err := db.Close(); err != nil {
+			t.Error(err)
+		}
+	}()
 
 	if _, err = db.Exec("CREATE DATABASE migrate"); err != nil {
 		t.Fatal(err)
@@ -88,7 +95,7 @@ func Test(t *testing.T) {
 		c := &CockroachDb{}
 		d, err := c.Open(addr)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
 		dt.Test(t, d, []byte("SELECT 1"))
 	})
@@ -107,12 +114,12 @@ func TestMigrate(t *testing.T) {
 		c := &CockroachDb{}
 		d, err := c.Open(addr)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
 
 		m, err := migrate.NewWithDatabaseInstance("file://./examples/migrations", "migrate", d)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
 		dt.TestMigrate(t, m, []byte("SELECT 1"))
 	})
@@ -131,7 +138,7 @@ func TestMultiStatement(t *testing.T) {
 		c := &CockroachDb{}
 		d, err := c.Open(addr)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
 		if err := d.Run(strings.NewReader("CREATE TABLE foo (foo text); CREATE TABLE bar (bar text);")); err != nil {
 			t.Fatalf("expected err to be nil, got %v", err)
@@ -161,7 +168,7 @@ func TestFilterCustomQuery(t *testing.T) {
 		c := &CockroachDb{}
 		_, err = c.Open(addr)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
 	})
 }
diff --git a/database/firebird/firebird.go b/database/firebird/firebird.go
@@ -166,14 +166,18 @@ func (f *Firebird) Version() (version int, dirty bool, err error) {
 	}
 }
 
-func (f *Firebird) Drop() error {
+func (f *Firebird) Drop() (err error) {
 	// select all tables
 	query := `SELECT rdb$relation_name FROM rdb$relations WHERE rdb$view_blr IS NULL AND (rdb$system_flag IS NULL OR rdb$system_flag = 0);`
 	tables, err := f.conn.QueryContext(context.Background(), query)
 	if err != nil {
 		return &database.Error{OrigErr: err, Query: []byte(query)}
 	}
-	defer tables.Close()
+	defer func() {
+		if errClose := tables.Close(); errClose != nil {
+			err = multierror.Append(err, errClose)
+		}
+	}()
 
 	// delete one table after another
 	tableNames := make([]string, 0)

diff --git a/database/firebird/firebird_test.go b/database/firebird/firebird_test.go
@@ -5,6 +5,8 @@ import (
 	"database/sql"
 	sqldriver "database/sql/driver"
 	"fmt"
+	"log"
+
 	"github.com/golang-migrate/migrate/v4"
 	"io"
 	"strings"
@@ -36,7 +38,6 @@ var (
 		},
 	}
 	specs = []dktesting.ContainerSpec{
-		{ImageName: "jacobalberty/firebird:2.5-ss", Options: opts},
 		{ImageName: "jacobalberty/firebird:3.0", Options: opts},
 	}
 )
@@ -54,15 +55,20 @@ func isReady(ctx context.Context, c dktest.ContainerInfo) bool {
 
 	db, err := sql.Open("firebirdsql", fbConnectionString(ip, port))
 	if err != nil {
+		log.Println("open error:", err)
 		return false
 	}
-	defer db.Close()
+	defer func() {
+		if err := db.Close(); err != nil {
+			log.Println("close error:", err)
+		}
+	}()
 	if err = db.PingContext(ctx); err != nil {
 		switch err {
 		case sqldriver.ErrBadConn, io.EOF:
 			return false
 		default:
-			fmt.Println(err)
+			log.Println(err)
 		}
 		return false
 	}
@@ -81,9 +87,13 @@ func Test(t *testing.T) {
 		p := &Firebird{}
 		d, err := p.Open(addr)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
-		defer d.Close()
+		defer func() {
+			if err := d.Close(); err != nil {
+				t.Error(err)
+			}
+		}()
 		dt.Test(t, d, []byte("SELECT Count(*) FROM rdb$relations"))
 	})
 }
@@ -99,12 +109,16 @@ func TestMigrate(t *testing.T) {
 		p := &Firebird{}
 		d, err := p.Open(addr)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
-		defer d.Close()
+		defer func() {
+			if err := d.Close(); err != nil {
+				t.Error(err)
+			}
+		}()
 		m, err := migrate.NewWithDatabaseInstance("file://./examples/migrations", "firebirdsql", d)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
 		dt.TestMigrate(t, m, []byte("SELECT Count(*) FROM rdb$relations"))
 	})
@@ -121,9 +135,13 @@ func TestErrorParsing(t *testing.T) {
 		p := &Firebird{}
 		d, err := p.Open(addr)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
-		defer d.Close()
+		defer func() {
+			if err := d.Close(); err != nil {
+				t.Error(err)
+			}
+		}()
 
 		wantErr := `migration failed in line 0: CREATE TABLEE foo (foo varchar(40)); (details: Dynamic SQL Error
 SQL error code = -104
@@ -151,9 +169,13 @@ func TestFilterCustomQuery(t *testing.T) {
 		p := &Firebird{}
 		d, err := p.Open(addr)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
-		defer d.Close()
+		defer func() {
+			if err := d.Close(); err != nil {
+				t.Error(err)
+			}
+		}()
 	})
 }
 
@@ -168,8 +190,13 @@ func Test_Lock(t *testing.T) {
 		p := &Firebird{}
 		d, err := p.Open(addr)
 		if err != nil {
-			t.Fatalf("%v", err)
+			t.Fatal(err)
 		}
+		defer func() {
+			if err := d.Close(); err != nil {
+				t.Error(err)
+			}
+		}()
 
 		dt.Test(t, d, []byte("SELECT Count(*) FROM rdb$relations"))