diff --git a/.golangci.yml b/.golangci.yml index aef536177b4a..ee349a283c25 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -32,12 +32,11 @@ linters: - govet # reports suspicious constructs. It is roughly the same as 'go vet' (replaced vet and vetshadow) - ineffassign # detects when assignments to existing variables are not used - misspell # finds commonly misspelled English words. - #- nilerr # Finds the code that returns nil even if it checks that the error is not nil. - #- nlreturn # Nlreturn checks for a new line before return and branch statements to increase code clarity. + - nilerr # Finds code that returns nil after it checks that the error is not nil. - prealloc # finds slice declarations that could potentially be pre-allocated. - predeclared # find code that shadows one of Go's predeclared identifiers. - reassign # checks that package variables are not reassigned. - #- revive #Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. + - tenv #detects using os.Setenv instead of t.Setenv since Go1.17. - tagalign # checks struct tags that do not align with the specified column in struct definitions. - staticcheck # checks rules from staticcheck. It's not the same thing as the staticcheck binary. - unused # checks Go code for unused constants, variables, functions and types. @@ -45,24 +44,20 @@ linters: - unparam # reports unused function parameters. - wastedassign # finds wasted assignment statements - whitespace # checks for unnecessary newlines at the start and end of functions, if, for, etc. ( - #- wsl # add or remove empty lines. - ##### need to confirm these are valid and not false positives ##### + ##### need to confirm these are valid and adding t.Parallel() to unit tests would be beneficial / integration tests would not be affected #- paralleltest # detects missing usage of t.Parallel() method in your Go test. #- tparallel # Tparallel detects inappropriate usage of t.Parallel() method in your Go test codes. - ###### DISABLED because : integer overflow conversion int -> int32 + ###### DISABLED because : the number of possible integer overflow conversions from int -> int32. it's not an incorrect callout? # - gosec # Gosec is a security linter for Go source code - ##### DISABLED as i was fixing them with other linters, and not realizing how many there are - disabling for now even thou it is valid + ##### DISABLED as it (correctly) flags fmt.Errorf("constant") to be replaced with errors.New("constant") and there are ~1500 instances of this in the codebase #- perfsprint # Checks that fmt.Sprintf can be replaced with a faster alternative. - #### VALID but DISABLED as we have a lot of these in the codebase to switch over + #### DISABLED but valid as relying on output variable names is less than idea, have a lot of these in the codebase to switch over #- nakedret # Checks that functions with naked returns are not longer than a maximum size - #### DISABLED TO DO IN ITS OWN PR - should be enabled and done ##### - #- tenv #detects using os.Setenv instead of t.Setenv since Go1.17. - #### bunch of hits, need to confirm if errors or not ##### #- copyloopvar #Detects range loop variables that are overwritten in the loop body @@ -92,9 +87,4 @@ linters-settings: - tfschema - computed predeclared: - ignore: new,min,max # should we use newer, minimum, and maximum instead? - revive: - rules: - - name: var-naming - disabled: true - + ignore: new,min,max # should we use newer, minimum, and maximum instead? \ No newline at end of file diff --git a/internal/provider/framework/config_test.go b/internal/provider/framework/config_test.go index 1bee7a45c710..70a8cd6c0499 100644 --- a/internal/provider/framework/config_test.go +++ b/internal/provider/framework/config_test.go @@ -27,7 +27,7 @@ func TestProviderConfig_LoadDefault(t *testing.T) { } // Skip enhanced validation - _ = os.Setenv("ARM_PROVIDER_ENHANCED_VALIDATION", "false") + t.Setenv("ARM_PROVIDER_ENHANCED_VALIDATION", "false") testData := &ProviderModel{ ResourceProviderRegistrations: types.StringValue("none"), diff --git a/internal/provider/framework/helpers_test.go b/internal/provider/framework/helpers_test.go index 4f980e2684e4..98ba67ff4822 100644 --- a/internal/provider/framework/helpers_test.go +++ b/internal/provider/framework/helpers_test.go @@ -4,7 +4,6 @@ package framework import ( - "os" "strings" "testing" @@ -70,10 +69,7 @@ func Test_getOidcTokenExpectMismatch(t *testing.T) { func Test_getOidcTokenAKSWorkload(t *testing.T) { expectedString := "testOidcFromFile" - err := os.Setenv("AZURE_FEDERATED_TOKEN_FILE", "./testdata/oidc_test_input.txt") - if err != nil { - t.Fatalf("could not set env var (`AZURE_FEDERATED_TOKEN_FILE`) for test: %+v", err) - } + t.Setenv("AZURE_FEDERATED_TOKEN_FILE", "./testdata/oidc_test_input.txt") p := &ProviderModel{ UseAKSWorkloadIdentity: basetypes.NewBoolValue(true), @@ -91,17 +87,14 @@ func Test_getOidcTokenAKSWorkload(t *testing.T) { func Test_getOidcTokenAKSWorkloadExpectMismatch(t *testing.T) { configuredString := "testOidc" - err := os.Setenv("AZURE_FEDERATED_TOKEN_FILE", "./testdata/oidc_test_input.txt") - if err != nil { - t.Fatalf("could not set env var (`AZURE_FEDERATED_TOKEN_FILE`) for test: %+v", err) - } + t.Setenv("AZURE_FEDERATED_TOKEN_FILE", "./testdata/oidc_test_input.txt") p := &ProviderModel{ OIDCToken: basetypes.NewStringValue(configuredString), UseAKSWorkloadIdentity: basetypes.NewBoolValue(true), } - _, err = getOidcToken(p) + _, err := getOidcToken(p) if err == nil { t.Fatal("expected an error but did not get one") } @@ -147,7 +140,7 @@ func Test_getClientSecretExpectMismatch(t *testing.T) { } func Test_getClientSecretFromFile(t *testing.T) { - os.Setenv("ARM_CLIENT_SECRET", "") + t.Setenv("ARM_CLIENT_SECRET", "") expectedString := "testClientSecretFromFile" p := &ProviderModel{ ClientSecretFilePath: basetypes.NewStringValue("./testdata/client_secret_test_input.txt"), @@ -166,7 +159,7 @@ func Test_getClientSecretFromFile(t *testing.T) { } func Test_getClientSecretFromFileMismatch(t *testing.T) { - os.Setenv("ARM_CLIENT_SECRET", "foo") + t.Setenv("ARM_CLIENT_SECRET", "foo") p := &ProviderModel{ ClientSecretFilePath: basetypes.NewStringValue("./testdata/client_secret_test_input.txt"), } @@ -233,14 +226,12 @@ func Test_getClientIDFromFile(t *testing.T) { func Test_getClientIDAKSWorkload(t *testing.T) { expectedString := "testClientIDAKSWorkload" - err := os.Setenv("ARM_CLIENT_ID", expectedString) - if err != nil { - t.Fatalf("failed to set environment variable ARM_CLIENT_ID: %v", err) - } + t.Setenv("ARM_CLIENT_ID", expectedString) p := &ProviderModel{ UseAKSWorkloadIdentity: basetypes.NewBoolValue(true), } + result, err := getClientId(p) if err != nil { t.Fatalf("getClientID returned unexpected error %v", err) @@ -255,16 +246,14 @@ func Test_getClientIDAKSWorkload(t *testing.T) { func Test_getClientIDAKSWorkloadExpectMismatch(t *testing.T) { configuredString := "testClientIDAKSWorkload" - err := os.Setenv("ARM_CLIENT_ID", "testClientID") - if err != nil { - t.Fatalf("failed to set environment variable ARM_CLIENT_ID: %v", err) - } + t.Setenv("ARM_CLIENT_ID", "testClientID") + p := &ProviderModel{ ClientId: basetypes.NewStringValue(configuredString), UseAKSWorkloadIdentity: basetypes.NewBoolValue(true), } - _, err = getClientId(p) + _, err := getClientId(p) if err == nil { t.Fatalf("expected an error but did not get one") } diff --git a/internal/sdk/plugin_sdk_test.go b/internal/sdk/plugin_sdk_test.go index 99d2ce272f00..f7022e27410c 100644 --- a/internal/sdk/plugin_sdk_test.go +++ b/internal/sdk/plugin_sdk_test.go @@ -15,7 +15,7 @@ import ( ) func TestAccPluginSDKAndDecoder(t *testing.T) { - os.Setenv("TF_ACC", "1") + os.Setenv("TF_ACC", "1") // nolint:tenv // plugin testing harness prevents this type NestedType struct { Key string `tfschema:"key"` @@ -64,7 +64,7 @@ func TestAccPluginSDKAndDecoder(t *testing.T) { } // lintignore:AT001 - resource.ParallelTest(t, resource.TestCase{ + resource.Test(t, resource.TestCase{ ProviderFactories: map[string]func() (*schema.Provider, error){ "validator": func() (*schema.Provider, error) { //nolint:unparam return &schema.Provider{ @@ -216,7 +216,7 @@ func TestAccPluginSDKAndDecoder(t *testing.T) { } func TestAccPluginSDKAndDecoderOptionalComputed(t *testing.T) { - os.Setenv("TF_ACC", "1") + os.Setenv("TF_ACC", "1") // nolint:tenv // plugin testing harness prevents this type MyType struct { Hello string `tfschema:"hello"` @@ -264,7 +264,7 @@ func TestAccPluginSDKAndDecoderOptionalComputed(t *testing.T) { } // lintignore:AT001 - resource.ParallelTest(t, resource.TestCase{ + resource.Test(t, resource.TestCase{ ProviderFactories: map[string]func() (*schema.Provider, error){ "validator": func() (*schema.Provider, error) { //nolint:unparam return &schema.Provider{ @@ -340,7 +340,7 @@ resource "validator_decoder_unspecified" "test" {} } func TestAccPluginSDKAndDecoderOptionalComputedOverride(t *testing.T) { - os.Setenv("TF_ACC", "1") + os.Setenv("TF_ACC", "1") // nolint:tenv // plugin testing harness prevents this type MyType struct { Hello string `tfschema:"hello"` @@ -350,7 +350,7 @@ func TestAccPluginSDKAndDecoderOptionalComputedOverride(t *testing.T) { } // lintignore:AT001 - resource.ParallelTest(t, resource.TestCase{ + resource.Test(t, resource.TestCase{ ProviderFactories: map[string]func() (*schema.Provider, error){ "validator": func() (*schema.Provider, error) { //nolint:unparam return &schema.Provider{ @@ -444,7 +444,7 @@ resource "validator_decoder_override" "test" { } func TestAccPluginSDKAndDecoderSets(t *testing.T) { - os.Setenv("TF_ACC", "1") + os.Setenv("TF_ACC", "1") // nolint:tenv // plugin testing harness prevents this type MyType struct { SetOfStrings []string `tfschema:"set_of_strings"` @@ -456,7 +456,7 @@ func TestAccPluginSDKAndDecoderSets(t *testing.T) { } // lintignore:AT001 - resource.ParallelTest(t, resource.TestCase{ + resource.Test(t, resource.TestCase{ ProviderFactories: map[string]func() (*schema.Provider, error){ "validator": func() (*schema.Provider, error) { //nolint:unparam return &schema.Provider{ @@ -624,7 +624,7 @@ func TestAccPluginSDKAndDecoderSets(t *testing.T) { } func TestAccPluginSDKAndEncoder(t *testing.T) { - os.Setenv("TF_ACC", "1") + os.Setenv("TF_ACC", "1") // nolint:tenv // plugin testing harness prevents this type NestedType struct { Key string `tfschema:"key"` @@ -649,7 +649,7 @@ func TestAccPluginSDKAndEncoder(t *testing.T) { } // lintignore:AT001 - resource.ParallelTest(t, resource.TestCase{ + resource.Test(t, resource.TestCase{ ProviderFactories: map[string]func() (*schema.Provider, error){ "validator": func() (*schema.Provider, error) { //nolint:unparam return &schema.Provider{ @@ -863,7 +863,7 @@ func TestAccPluginSDKAndEncoder(t *testing.T) { } func TestAccPluginSDKReturnsComputedFields(t *testing.T) { - os.Setenv("TF_ACC", "1") + os.Setenv("TF_ACC", "1") // nolint:tenv // plugin sdk always is resourceName := "validator_computed.test" // lintignore:AT001 diff --git a/internal/services/apimanagement/migration/policy.go b/internal/services/apimanagement/migration/policy.go index 03477bb5f0df..8a54109e7223 100644 --- a/internal/services/apimanagement/migration/policy.go +++ b/internal/services/apimanagement/migration/policy.go @@ -22,7 +22,7 @@ func (ApiManagementPolicyV0ToV1) UpgradeFunc() pluginsdk.StateUpgraderFunc { return func(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) { apiMgmtId, err := policy.ParseServiceID(rawState["id"].(string)) if err != nil { - return rawState, nil + return rawState, nil // lint:ignore nilerr this is not an error as we just want to skip the upgrade } id := policy.NewServiceID(apiMgmtId.SubscriptionId, apiMgmtId.ResourceGroupName, apiMgmtId.ServiceName) rawState["id"] = id.ID() diff --git a/internal/services/compute/capacity_reservation_group_resource.go b/internal/services/compute/capacity_reservation_group_resource.go index a3dd355949cd..c0c41ec46cf9 100644 --- a/internal/services/compute/capacity_reservation_group_resource.go +++ b/internal/services/compute/capacity_reservation_group_resource.go @@ -172,7 +172,7 @@ func resourceCapacityReservationGroupDelete(d *pluginsdk.ResourceData, meta inte Refresh: func() (interface{}, string, error) { res, err := client.Delete(ctx, *id) if err != nil { - return res, "Deleting", nil + return res, "Deleting", nil // lint:ignore nilerr Returning nil error is intentional as we will retry the delete operation } return res, "Deleted", nil }, diff --git a/internal/services/compute/no_downtime_resize.go b/internal/services/compute/no_downtime_resize.go index faef57f50257..231e8f57f38b 100644 --- a/internal/services/compute/no_downtime_resize.go +++ b/internal/services/compute/no_downtime_resize.go @@ -73,7 +73,7 @@ func determineIfVirtualMachineSkuSupportsNoDowntimeResize(ctx context.Context, v virtualMachineId, err := virtualmachines.ParseVirtualMachineIDInsensitively(*virtualMachineIdRaw) if err != nil { log.Printf("[DEBUG] unable to parse Virtual Machine ID %q that the Managed Disk is attached too - skipping no-downtime-resize since we can't guarantee that's available", *virtualMachineIdRaw) - return pointer.To(false), nil + return pointer.To(false), nil // lint:ignore nilerr this is not an error as we just want to skip the check in this situation since we can't guarantee it's available } log.Printf("[DEBUG] Retrieving %s..", *virtualMachineId) diff --git a/internal/services/storage/storage_account_static_website_data_plane_resource.go b/internal/services/storage/storage_account_static_website_data_plane_resource.go index 88ff3a7d5c65..ad286ddd2581 100644 --- a/internal/services/storage/storage_account_static_website_data_plane_resource.go +++ b/internal/services/storage/storage_account_static_website_data_plane_resource.go @@ -195,8 +195,7 @@ func (a AccountStaticWebsiteResource) Delete() sdk.ResourceFunc { accountDetails, err := storageClient.FindAccount(ctx, id.SubscriptionId, id.StorageAccountName) if err != nil { - // If we don't find the account we can safely assume we don't need to remove the website since it must already be deleted - return nil + return nil // lint:ignore nilerr If we don't find the account we can safely assume we don't need to remove the website since it must already be deleted } properties := accounts.StorageServiceProperties{