From aa9459925468364f83caffeb504406066961f8d8 Mon Sep 17 00:00:00 2001 From: petoju Date: Tue, 7 May 2024 21:31:02 +0200 Subject: [PATCH] Fix issues related to error comparison (#150) Mostly related to static code analysis --- mysql/provider.go | 4 ++-- mysql/resource_database.go | 3 ++- mysql/resource_default_roles_test.go | 16 ++++++++-------- mysql/resource_global_variable.go | 3 ++- mysql/resource_global_variable_test.go | 3 ++- mysql/resource_grant_test.go | 6 +++--- mysql/resource_ti_config_variable_test.go | 2 +- mysql/resource_user_test.go | 10 ++++++---- 8 files changed, 26 insertions(+), 21 deletions(-) diff --git a/mysql/provider.go b/mysql/provider.go index 9507910f..5feee7b9 100644 --- a/mysql/provider.go +++ b/mysql/provider.go @@ -26,10 +26,10 @@ import ( "golang.org/x/net/proxy" "golang.org/x/oauth2" - cloudsqlconn "cloud.google.com/go/cloudsqlconn" + "cloud.google.com/go/cloudsqlconn" cloudsql "cloud.google.com/go/cloudsqlconn/mysql/mysql" "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" - azidentity "github.com/Azure/azure-sdk-for-go/sdk/azidentity" + "github.com/Azure/azure-sdk-for-go/sdk/azidentity" ) const ( diff --git a/mysql/resource_database.go b/mysql/resource_database.go index 12a286e3..1c98dfb3 100644 --- a/mysql/resource_database.go +++ b/mysql/resource_database.go @@ -3,6 +3,7 @@ package mysql import ( "context" "database/sql" + "errors" "fmt" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "log" @@ -134,7 +135,7 @@ func ReadDatabase(ctx context.Context, d *schema.ResourceData, meta interface{}) } if res != nil { - if res == sql.ErrNoRows { + if errors.Is(res, sql.ErrNoRows) { return diag.Errorf("charset %s has no default collation", defaultCharset) } diff --git a/mysql/resource_default_roles_test.go b/mysql/resource_default_roles_test.go index 1f4ec7cc..4e6f38d5 100644 --- a/mysql/resource_default_roles_test.go +++ b/mysql/resource_default_roles_test.go @@ -21,7 +21,7 @@ func TestAccDefaultRoles_basic(t *testing.T) { CheckDestroy: testAccDefaultRolesCheckDestroy, Steps: []resource.TestStep{ { - Config: testAccDefaultRoles_basic, + Config: testAccDefaultRolesBasic, Check: resource.ComposeTestCheckFunc( testAccDefaultRoles("mysql_default_roles.test", "role1"), resource.TestCheckResourceAttr("mysql_default_roles.test", "roles.#", "1"), @@ -29,7 +29,7 @@ func TestAccDefaultRoles_basic(t *testing.T) { ), }, { - Config: testAccDefaultRoles_multiple, + Config: testAccDefaultRolesMultiple, Check: resource.ComposeTestCheckFunc( testAccDefaultRoles("mysql_default_roles.test", "role1", "role2"), resource.TestCheckResourceAttr("mysql_default_roles.test", "roles.#", "2"), @@ -38,21 +38,21 @@ func TestAccDefaultRoles_basic(t *testing.T) { ), }, { - Config: testAccDefaultRoles_none, + Config: testAccDefaultRolesNone, Check: resource.ComposeTestCheckFunc( testAccDefaultRoles("mysql_default_roles.test"), resource.TestCheckResourceAttr("mysql_default_roles.test", "roles.#", "0"), ), }, { - Config: testAccDefaultRoles_basic, + Config: testAccDefaultRolesBasic, ResourceName: "mysql_default_roles.test", ImportState: true, ImportStateVerify: true, ImportStateId: fmt.Sprintf("%v@%v", "jdoe", "%"), }, { - Config: testAccDefaultRoles_multiple, + Config: testAccDefaultRolesMultiple, ResourceName: "mysql_default_roles.test", ImportState: true, ImportStateVerify: true, @@ -142,7 +142,7 @@ func testAccDefaultRolesCheckDestroy(s *terraform.State) error { return nil } -const testAccDefaultRoles_basic = ` +const testAccDefaultRolesBasic = ` resource "mysql_role" "role1" { name = "role1" } @@ -166,7 +166,7 @@ resource "mysql_default_roles" "test" { } ` -const testAccDefaultRoles_multiple = ` +const testAccDefaultRolesMultiple = ` resource "mysql_role" "role1" { name = "role1" } @@ -194,7 +194,7 @@ resource "mysql_default_roles" "test" { } ` -const testAccDefaultRoles_none = ` +const testAccDefaultRolesNone = ` resource "mysql_user" "test" { user = "jdoe" host = "%" diff --git a/mysql/resource_global_variable.go b/mysql/resource_global_variable.go index 428fa78f..0ba8def1 100644 --- a/mysql/resource_global_variable.go +++ b/mysql/resource_global_variable.go @@ -3,6 +3,7 @@ package mysql import ( "context" "database/sql" + "errors" "fmt" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "log" @@ -88,7 +89,7 @@ func ReadGlobalVariable(ctx context.Context, d *schema.ResourceData, meta interf var name, value string err = stmt.QueryRow(d.Id()).Scan(&name, &value) - if err != nil && err != sql.ErrNoRows { + if err != nil && !errors.Is(err, sql.ErrNoRows) { d.SetId("") return diag.Errorf("error during show global variables: %s", err) } diff --git a/mysql/resource_global_variable_test.go b/mysql/resource_global_variable_test.go index ea129f3e..668b239d 100644 --- a/mysql/resource_global_variable_test.go +++ b/mysql/resource_global_variable_test.go @@ -3,6 +3,7 @@ package mysql import ( "context" "database/sql" + "errors" "fmt" "regexp" "testing" @@ -153,7 +154,7 @@ func testAccGetGlobalVar(varName string, db *sql.DB) (string, error) { var value string err = stmt.QueryRow(varName).Scan(&name, &value) - if err != nil && err != sql.ErrNoRows { + if err != nil && !errors.Is(err, sql.ErrNoRows) { return "", err } diff --git a/mysql/resource_grant_test.go b/mysql/resource_grant_test.go index 015e8efe..7dab0322 100644 --- a/mysql/resource_grant_test.go +++ b/mysql/resource_grant_test.go @@ -855,7 +855,7 @@ func prepareProcedure(dbname string, procedureName string) resource.TestCheckFun _, err = db.ExecContext(ctx, fmt.Sprintf("USE `%s`", dbname)) log.Printf("[DEBUG] SQL: %s", dbname) if err != nil { - return fmt.Errorf("Error selecting database %s: %s", dbname, err) + return fmt.Errorf("error selecting database %s: %s", dbname, err) } // Check if the procedure exists @@ -868,7 +868,7 @@ WHERE ROUTINE_SCHEMA = ? AND ROUTINE_NAME = ? AND ROUTINE_TYPE = 'PROCEDURE' log.Printf("[DEBUG] SQL: %s", checkExistenceSQL) err = db.QueryRowContext(ctx, checkExistenceSQL, dbname, procedureName).Scan(&exists) if err != nil { - return fmt.Errorf("Error checking existence of procedure %s: %s", procedureName, err) + return fmt.Errorf("error checking existence of procedure %s: %s", procedureName, err) } if exists > 0 { @@ -1041,7 +1041,7 @@ func testAccCheckProcedureGrant(resourceName, userName, hostName, procedureName // Compare the result with the expected outcome if found != expected { - return fmt.Errorf("Grant for procedure %s does not match expected state: %v", procedureName, expected) + return fmt.Errorf("grant for procedure %s does not match expected state: %v", procedureName, expected) } return nil diff --git a/mysql/resource_ti_config_variable_test.go b/mysql/resource_ti_config_variable_test.go index 204a3805..7440045c 100644 --- a/mysql/resource_ti_config_variable_test.go +++ b/mysql/resource_ti_config_variable_test.go @@ -193,7 +193,7 @@ func testAccGetConfigVar(varName string, varType string, db *sql.DB) (string, st err = stmt.QueryRow(varName, varType).Scan(&resType, &resInstance, &resName, &resValue) - if err != nil && err != sql.ErrNoRows { + if err != nil && !errors.Is(err, sql.ErrNoRows) { return "nil", "nil", err } diff --git a/mysql/resource_user_test.go b/mysql/resource_user_test.go index d6aef0d7..71b5113b 100644 --- a/mysql/resource_user_test.go +++ b/mysql/resource_user_test.go @@ -3,6 +3,7 @@ package mysql import ( "context" "database/sql" + "errors" "fmt" "log" "regexp" @@ -214,7 +215,7 @@ func testAccUserExists(rn string) resource.TestCheckFunc { var count int err = db.QueryRow(stmtSQL).Scan(&count) if err != nil { - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return fmt.Errorf("expected 1 row reading user but got no rows") } return fmt.Errorf("error reading user: %s", err) @@ -246,7 +247,7 @@ func testAccUserAuthExists(rn string) resource.TestCheckFunc { var count int err = db.QueryRow(stmtSQL).Scan(&count) if err != nil { - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return fmt.Errorf("expected 1 row reading user but got no rows") } return fmt.Errorf("error reading user: %s", err) @@ -292,8 +293,9 @@ func testAccUserCheckDestroy(s *terraform.State) error { if err != nil { return fmt.Errorf("error issuing query: %s", err) } - defer rows.Close() - if rows.Next() { + haveNext := rows.Next() + rows.Close() + if haveNext { return fmt.Errorf("user still exists after destroy") } }