linting: added errcheck, gocritic, goimpors, golint, govet & megacheck

added linters using golangci. fix linted errors.

.golangci.yml

@@ -0,0 +1,48 @@
+linters:
+  enable:
+  - errcheck
+  - gocritic
+  - goimports
+  - golint
+  - govet
+  - megacheck
+
+  disable-all: true
+
+linters-settings:
+  # Enable-checks are based on the gocritic check list tagged as "stable" check here:
+  # https://github.com/go-critic/go-critic/blob/e92184cb98471e662585b9b49e9b133cab72e20a/checkers/checkers_test.go#L63
+  gocritic:
+    enabled-checks:
+    - appendAssign
+    - appendCombine
+    - assignOp
+    - builtinShadow
+    - captLocal
+    - caseOrder
+    - defaultCaseOrder
+    - dupArg
+    - dupBranchBody
+    - dupCase
+    - elseif
+    - flagDeref
+    - ifElseChain
+    - importShadow
+    - indexAlloc
+    - paramTypeCombine
+    - rangeExprCopy
+    - rangeValCopy
+    - regexpMust
+    - singleCaseSwitch
+    - sloppyLen
+    - switchTrue
+    - typeSwitchVar
+    - typeUnparen
+    - underef
+    - unlambda
+    - unslice
+    - dupSubExpr
+    - hugeParam
+
+issues:
+  exclude-use-default: false

Makefile

@@ -0,0 +1,7 @@
+.PHONY: sanity-check
+sanity-check: golangci-lint
+
+.PHONY: golangci-lint
+golangci-lint:
+	@echo "Running golangci-lint..."
+	golangci-lint run ./...

README.md

@@ -207,5 +207,15 @@ Phoenix driver, which uses the scheme to determine if we should connect over `ht
 project is structured, it was also almost impossible to add support for embeddable migration files without major
 changes.
 
+## Contributing
+We run automatically some sanitizations to check code quality before merging it. For this purpose,
+we use the [golangci-lint](https://github.com/golangci/golangci-lint) tool which is run via a [Makefile](Makefile) 
+target. 
+
+It is a recommended practice to run and ensure passing all the checks locally before submitting a pull request.
+
+After [installing golangci-lint](https://github.com/golangci/golangci-lint) running a `make sanity-check` locally 
+would trigger this tests.
+
 ## License
 This library is licensed under the Apache 2 License.

driver/golang/golang.go

@@ -9,12 +9,13 @@ import (
 	m "github.com/Boostport/migration"
 )
 
+// Source implements migration.Source
 type Source struct {
 	sync.Mutex
 	migrations map[string]func() error
 }
 
-// NewGolangSource creates a source for storing Go functions as migrations.
+// NewSource creates a source for storing Go functions as migrations.
 func NewSource() *Source {
 	return &Source{
 		migrations: map[string]func() error{},
@@ -73,17 +74,20 @@ func (s *Source) GetMigrationFile(file string) (io.Reader, error) {
 	return strings.NewReader(""), nil
 }
 
+// Driver is the golang migration.Driver implementation
 type Driver struct {
 	source        *Source
 	updateVersion UpdateVersion
 	applied       AppliedVersions
 }
 
+// UpdateVersion takes an id and a direction and returns an error if something fails
 type UpdateVersion func(id string, direction m.Direction) error
 
+// AppliedVersions returns a list of applied versions and an error if something fails
 type AppliedVersions func() ([]string, error)
 
-// NewGolang creates a new Go migration driver. It requires a source a function for saving the executed migration version, a function for deleting a version
+// New creates a new Go migration driver. It requires a source a function for saving the executed migration version, a function for deleting a version
 // that was migrated downwards, a function for listing all applied migrations and optionally a configuration.
 func New(source *Source, updateVersion UpdateVersion, applied AppliedVersions) (m.Driver, error) {
 	return &Driver{
@@ -93,12 +97,13 @@ func New(source *Source, updateVersion UpdateVersion, applied AppliedVersions) (
 	}, nil
 }
 
+// Close is the migration.Driver implementation of io.Closer
 func (g *Driver) Close() error {
 	return nil
 }
 
+// Migrate executes a planned migration
 func (g *Driver) Migrate(migration *m.PlannedMigration) error {
-
 	file := migration.ID
 
 	if migration.Direction == m.Up {
@@ -124,7 +129,7 @@ func (g *Driver) Migrate(migration *m.PlannedMigration) error {
 	return nil
 }
 
-// Version returns all applied migration versions
+// Versions returns all applied migration versions
 func (g *Driver) Versions() ([]string, error) {
 	return g.applied()
 }

driver/mysql/mysql.go

@@ -11,6 +11,7 @@ import (
 	"github.com/go-sql-driver/mysql"
 )
 
+// Driver is the mysql migration.Driver implementation
 type Driver struct {
 	db *sql.DB
 }
@@ -50,8 +51,8 @@ func New(dsn string) (m.Driver, error) {
 	return d, nil
 }
 
+// NewFromDB returns a mysql driver from a sql.DB
 func NewFromDB(db *sql.DB) (m.Driver, error) {
-
 	if _, ok := db.Driver().(*mysql.MySQLDriver); !ok {
 		return nil, errors.New("database instance is not using the MySQL driver")
 	}
@@ -132,7 +133,9 @@ func (driver *Driver) Versions() ([]string, error) {
 		return versions, err
 	}
 
-	defer rows.Close()
+	defer func() {
+		_ = rows.Close()
+	}()
 
 	for rows.Next() {
 		var version string

driver/mysql/mysql_test.go

@@ -12,7 +12,6 @@ import (
 )
 
 func TestMySQLDriver(t *testing.T) {
-
 	mysqlHost := os.Getenv("MYSQL_HOST")
 
 	database := "migrationtest"
@@ -24,7 +23,12 @@ func TestMySQLDriver(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	defer connection.Close()
+	defer func() {
+		err := connection.Close()
+		if err != nil {
+			t.Errorf("unexpected error while closing the mysql connection: %v", err)
+		}
+	}()
 
 	_, err = connection.Exec("CREATE DATABASE IF NOT EXISTS " + database)
 
@@ -41,13 +45,21 @@ func TestMySQLDriver(t *testing.T) {
 	driver, err := New("root:@tcp(" + mysqlHost + ")/" + database + "?multiStatements=true")
 
 	if err != nil {
-		t.Errorf("Unable to open connection to mysql server: %s", err)
+		t.Errorf("unable to open connection to mysql server: %s", err)
 	}
 
-	defer driver.Close()
+	defer func() {
+		err := driver.Close()
+		if err != nil {
+			t.Errorf("unexpected error while closing the mysql driver: %v", err)
+		}
+	}()
 
 	defer func() {
-		connection.Exec("DROP DATABASE IF EXISTS " + database)
+		_, err := connection.Exec("DROP DATABASE IF EXISTS " + database)
+		if err != nil {
+			t.Errorf("unexpected error while dropping the mysql database %s: %v", database, err)
+		}
 	}()
 
 	migrations := []*migration.PlannedMigration{
@@ -100,82 +112,81 @@ func TestMySQLDriver(t *testing.T) {
 	err = driver.Migrate(migrations[0])
 
 	if err != nil {
-		t.Errorf("Unexpected error while running migration: %s", err)
+		t.Errorf("unexpected error while running migration: %s", err)
 	}
 
 	_, err = connection.Exec("INSERT INTO test_table1 (id) values (1)")
 
 	if err != nil {
-		t.Errorf("Unexpected error while testing if migration succeeded: %s", err)
+		t.Errorf("unexpected error while testing if migration succeeded: %s", err)
 	}
 
 	_, err = connection.Exec("INSERT INTO test_table2 (id) values (1)")
 
 	if err != nil {
-		t.Errorf("Unexpected error while testing if migration succeeded: %s", err)
+		t.Errorf("unexpected error while testing if migration succeeded: %s", err)
 	}
 
 	err = driver.Migrate(migrations[1])
 
 	if err != nil {
-		t.Errorf("Unexpected error while running migration: %s", err)
+		t.Errorf("unexpected error while running migration: %s", err)
 	}
 
 	if _, err = connection.Exec("INSERT INTO test_table2 (id) values (1)"); err != nil {
 		if err.(*mysql.MySQLError).Number != 1146 {
-			t.Errorf("Received an error while inserting into a non-existent table, but it was not a table_undefined error: %s", err)
+			t.Errorf("received an error while inserting into a non-existent table, but it was not a table_undefined error: %s", err)
 		}
 	} else {
-		t.Error("Expected an error while inserting into non-existent table, but did not receive any.")
+		t.Error("expected an error while inserting into non-existent table, but did not receive any.")
 	}
 
 	err = driver.Migrate(migrations[2])
 
 	if err == nil {
-		t.Error("Expected an error while executing invalid statement, but did not receive any.")
+		t.Error("expected an error while executing invalid statement, but did not receive any.")
 	}
 
 	versions, err := driver.Versions()
 
 	if err != nil {
-		t.Errorf("Unexpected error while retriving version information: %s", err)
+		t.Errorf("unexpected error while retriving version information: %s", err)
 	}
 
 	if len(versions) != 2 {
-		t.Errorf("Expected %d versions to be applied, %d was actually applied.", 2, len(versions))
+		t.Errorf("expected %d versions to be applied, %d was actually applied.", 2, len(versions))
 	}
 
 	migrations[1].Direction = migration.Down
 
 	err = driver.Migrate(migrations[1])
 
 	if err != nil {
-		t.Errorf("Unexpected error while running migration: %s", err)
+		t.Errorf("unexpected error while running migration: %s", err)
 	}
 
 	versions, err = driver.Versions()
 
 	if err != nil {
-		t.Errorf("Unexpected error while retriving version information: %s", err)
+		t.Errorf("unexpected error while retriving version information: %s", err)
 	}
 
 	if len(versions) != 1 {
-		t.Errorf("Expected %d versions to be applied, %d was actually applied.", 2, len(versions))
+		t.Errorf("expected %d versions to be applied, %d was actually applied.", 2, len(versions))
 	}
 }
 
 func TestCreateDriverUsingInvalidDBInstance(t *testing.T) {
-
 	db, _, err := sqlmock.New()
 
 	if err != nil {
-		t.Fatalf("Error opening stub database connection: %s", err)
+		t.Fatalf("error opening stub database connection: %s", err)
 	}
 
 	_, err = NewFromDB(db)
 
 	if err == nil {
-		t.Error("Expected error when creating MySQL driver with a non-MySQL database instance, but there was no error")
+		t.Error("expected error when creating MySQL driver with a non-MySQL database instance, but there was no error")
 	}
 }
 
@@ -197,23 +208,36 @@ func TestCreateDriverUsingDBInstance(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	defer connection.Close()
+	defer func() {
+		err := connection.Close()
+		if err != nil {
+			t.Errorf("unexpected error while closing the mysql connection: %v", err)
+		}
+	}()
 
 	defer func() {
-		connection.Exec("DROP DATABASE IF EXISTS " + database)
+		_, err := connection.Exec("DROP DATABASE IF EXISTS " + database)
+		if err != nil {
+			t.Errorf("unexpected error while dropping the mysql database %s: %v", database, err)
+		}
 	}()
 
 	db, err := sql.Open("mysql", "root:@tcp("+mysqlHost+")/"+database+"?multiStatements=true")
 
 	if err != nil {
-		t.Fatalf("Could not open MySQL connection: %s", err)
+		t.Fatalf("could not open MySQL connection: %s", err)
 	}
 
 	driver, err := NewFromDB(db)
 
 	if err != nil {
-		t.Errorf("Unable to create MySQL driver: %s", err)
+		t.Errorf("unable to create MySQL driver: %s", err)
 	}
 
-	defer driver.Close()
+	defer func() {
+		err := driver.Close()
+		if err != nil {
+			t.Errorf("unexpected error while closing the mysql driver: %v", err)
+		}
+	}()
 }