golangci - update to latest version, enable additional linters & fix …

…errors (hashicorp#27951)

* golangci - update config

* fix linting errors

* update golangci verison in workflow

* fix rest of linting errors

* fix rest of linting errors

* enable asasalint

* enable linters:dupword, durationcheck

* enable more linteres

* make generate

* fix tflint

* fix some whitespace

* remove extra nolint

* add ,nil

.github/labeler-issue-triage.yml

@@ -27,7 +27,7 @@ service/app-configuration:
   - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_app_configuration((.|\n)*)###'
 
 service/app-service:
-  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(app_service_environment_v3\W+|app_service_environment_v3\W+|app_service_source_control\W+|app_service_source_control_slot\W+|function_app_active_slot\W+|function_app_function\W+|function_app_hybrid_connection\W+|linux_function_app\W+|linux_function_app\W+|linux_function_app_slot\W+|linux_web_app\W+|linux_web_app\W+|linux_web_app_slot\W+|service_plan|source_control_token|static_web_app|web_app_|windows_function_app\W+|windows_function_app\W+|windows_function_app_slot\W+|windows_web_app\W+|windows_web_app\W+|windows_web_app_slot\W+)((.|\n)*)###'
+  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(app_service_environment_v3\W+|app_service_environment_v3\W+|app_service_source_control\W+|app_service_source_control_slot\W+|arc_kubernetes_cluster_extension\W+|arc_kubernetes_flux_configuration\W+|function_app_active_slot\W+|function_app_function\W+|function_app_hybrid_connection\W+|linux_function_app\W+|linux_function_app\W+|linux_function_app_slot\W+|linux_web_app\W+|linux_web_app\W+|linux_web_app_slot\W+|service_plan|source_control_token|static_web_app|web_app_|windows_function_app\W+|windows_function_app\W+|windows_function_app_slot\W+|windows_web_app\W+|windows_web_app\W+|windows_web_app_slot\W+)((.|\n)*)###'
 
 service/application-insights:
   - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_application_insights((.|\n)*)###'
@@ -69,7 +69,7 @@ service/cognitive-services:
   - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(ai_services|cognitive_)((.|\n)*)###'
 
 service/communication:
-  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(communication_service|email_communication_service)((.|\n)*)###'
+  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(communication_service|email_communication_service|gallery_application|orchestrated_virtual_machine_scale_set\W+|restore_point_collection|virtual_machine_gallery_application_assignment\W+|virtual_machine_implicit_data_disk_from_source\W+|virtual_machine_restore_point\W+|virtual_machine_restore_point_collection\W+|virtual_machine_run_command\W+)((.|\n)*)###'
 
 service/connections:
   - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(api_connection|managed_api)((.|\n)*)###'
@@ -240,13 +240,13 @@ service/netapp:
   - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_netapp_((.|\n)*)###'
 
 service/network:
-  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(application_gateway\W+|application_security_group\W+|bastion_host|custom_ip_prefix|express_route_|ip_group|local_network_gateway|nat_gateway|network_connection_monitor\W+|network_ddos_protection_plan\W+|network_interface\W+|network_interface_application_gateway_backend_address_pool_association\W+|network_interface_application_security_group_association\W+|network_interface_backend_address_pool_association\W+|network_interface_nat_rule_association\W+|network_interface_security_group_association\W+|network_manager\W+|network_manager\W+|network_manager_admin_rule\W+|network_manager_admin_rule_collection\W+|network_manager_connectivity_configuration\W+|network_manager_connectivity_configuration\W+|network_manager_deployment\W+|network_manager_management_group_connection\W+|network_manager_network_group\W+|network_manager_network_group\W+|network_manager_scope_connection\W+|network_manager_security_admin_configuration\W+|network_manager_static_member\W+|network_manager_subscription_connection\W+|network_packet_capture\W+|network_profile\W+|network_security_group\W+|network_security_rule\W+|network_service_tags\W+|network_watcher\W+|network_watcher_flow_log\W+|point_to_site_vpn_gateway|private_endpoint\W+|private_endpoint_application_security_group_association\W+|private_endpoint_connection\W+|private_link_service\W+|private_link_service_endpoint_connections\W+|public_ip|route|subnet|virtual_hub\W+|virtual_hub_bgp_connection\W+|virtual_hub_connection\W+|virtual_hub_ip\W+|virtual_hub_route_table\W+|virtual_hub_route_table_route\W+|virtual_hub_routing_intent\W+|virtual_hub_security_partner_provider\W+|virtual_machine_packet_capture\W+|virtual_machine_scale_set_packet_capture\W+|virtual_network\W+|virtual_network_dns_servers\W+|virtual_network_gateway\W+|virtual_network_gateway_connection\W+|virtual_network_gateway_nat_rule\W+|virtual_network_peering\W+|virtual_network_peering\W+|virtual_wan\W+|vpn_|web_application_firewall_policy)((.|\n)*)###'
+  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(application_gateway\W+|application_security_group\W+|bastion_host|custom_ip_prefix|express_route_|ip_group|local_network_gateway|nat_gateway|network_|point_to_site_vpn_gateway|private_endpoint\W+|private_endpoint_application_security_group_association\W+|private_endpoint_connection\W+|private_link_service\W+|private_link_service_endpoint_connections\W+|public_ip|route|subnet|virtual_hub\W+|virtual_hub_bgp_connection\W+|virtual_hub_connection\W+|virtual_hub_ip\W+|virtual_hub_route_table\W+|virtual_hub_route_table_route\W+|virtual_hub_routing_intent\W+|virtual_hub_security_partner_provider\W+|virtual_machine_packet_capture\W+|virtual_machine_scale_set_packet_capture\W+|virtual_network\W+|virtual_network_dns_servers\W+|virtual_network_gateway\W+|virtual_network_gateway_connection\W+|virtual_network_gateway_nat_rule\W+|virtual_network_peering\W+|virtual_network_peering\W+|virtual_wan\W+|vpn_|web_application_firewall_policy)((.|\n)*)###'
 
 service/network-function:
-  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_network_function_((.|\n)*)###'
+  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(network_function_azure_traffic_collector\W+|network_function_collector_policy\W+|new_relic_monitor\W+|new_relic_tag_rule\W+)((.|\n)*)###'
 
 service/nginx:
-  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_nginx_((.|\n)*)###'
+  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(nginx_|oracle_)((.|\n)*)###'
 
 service/notifications:
   - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_notification_hub((.|\n)*)###'
@@ -282,7 +282,7 @@ service/redhatopenshift:
   - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_redhat_openshift_cluster((.|\n)*)###'
 
 service/redis:
-  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_redis_((.|\n)*)###'
+  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(redis_cache\W+|redis_cache_access_policy\W+|redis_cache_access_policy_assignment\W+|redis_enterprise_cluster\W+|redis_enterprise_database\W+|redis_firewall_rule\W+|redis_linked_server\W+|resource_deployment_script_azure_cli\W+|resource_deployment_script_azure_power_shell\W+|resource_management_private_link\W+|resource_management_private_link_association\W+|resource_provider_registration\W+)((.|\n)*)###'
 
 service/relay:
   - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_relay_((.|\n)*)###'
@@ -318,7 +318,7 @@ service/spring:
   - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(spring_cloud_accelerator\W+|spring_cloud_active_deployment\W+|spring_cloud_api_portal\W+|spring_cloud_api_portal_custom_domain\W+|spring_cloud_app\W+|spring_cloud_app_cosmosdb_association\W+|spring_cloud_app_dynamics_application_performance_monitoring\W+|spring_cloud_app_mysql_association\W+|spring_cloud_app_redis_association\W+|spring_cloud_application_insights_application_performance_monitoring\W+|spring_cloud_application_live_view\W+|spring_cloud_build_deployment\W+|spring_cloud_build_pack_binding\W+|spring_cloud_builder\W+|spring_cloud_certificate\W+|spring_cloud_configuration_service\W+|spring_cloud_container_deployment\W+|spring_cloud_custom_domain\W+|spring_cloud_customized_accelerator\W+|spring_cloud_dev_tool_portal\W+|spring_cloud_dynatrace_application_performance_monitoring\W+|spring_cloud_elastic_application_performance_monitoring\W+|spring_cloud_gateway\W+|spring_cloud_gateway_custom_domain\W+|spring_cloud_gateway_route_config\W+|spring_cloud_java_deployment\W+|spring_cloud_new_relic_application_performance_monitoring\W+|spring_cloud_service\W+|spring_cloud_storage\W+)((.|\n)*)###'
 
 service/storage:
-  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(storage_account\W+|storage_account_blob_container_sas\W+|storage_account_customer_managed_key\W+|storage_account_local_user\W+|storage_account_network_rules\W+|storage_account_queue_properties\W+|storage_account_sas\W+|storage_account_static_website\W+|storage_blob\W+|storage_blob_inventory_policy\W+|storage_container\W+|storage_container_immutability_policy\W+|storage_containers\W+|storage_data_lake_gen2_filesystem\W+|storage_data_lake_gen2_path\W+|storage_encryption_scope\W+|storage_management_policy\W+|storage_object_replication\W+|storage_queue\W+|storage_share\W+|storage_share_directory\W+|storage_share_file\W+|storage_sync\W+|storage_sync_cloud_endpoint\W+|storage_sync_group\W+|storage_sync_server_endpoint\W+|storage_table\W+|storage_table\W+|storage_table_entities\W+|storage_table_entity\W+)((.|\n)*)###'
+  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(managed_lustre_file_system|storage_account\W+|storage_account_blob_container_sas\W+|storage_account_customer_managed_key\W+|storage_account_local_user\W+|storage_account_network_rules\W+|storage_account_queue_properties\W+|storage_account_sas\W+|storage_account_static_website\W+|storage_blob\W+|storage_blob_inventory_policy\W+|storage_container\W+|storage_container_immutability_policy\W+|storage_containers\W+|storage_data_lake_gen2_filesystem\W+|storage_data_lake_gen2_path\W+|storage_encryption_scope\W+|storage_management_policy\W+|storage_object_replication\W+|storage_queue\W+|storage_share\W+|storage_share_directory\W+|storage_share_file\W+|storage_sync\W+|storage_sync_cloud_endpoint\W+|storage_sync_group\W+|storage_sync_server_endpoint\W+|storage_table\W+|storage_table\W+|storage_table_entities\W+|storage_table_entity\W+)((.|\n)*)###'
 
 service/storagemover:
   - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_storage_mover((.|\n)*)###'
@@ -348,7 +348,7 @@ service/virtual-desktops:
   - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_virtual_desktop_((.|\n)*)###'
 
 service/vmware:
-  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_vmware_((.|\n)*)###'
+  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(vmware_cluster\W+|vmware_express_route_authorization\W+|vmware_netapp_volume_attachment\W+|vmware_private_cloud\W+|voice_services_communications_gateway\W+|voice_services_communications_gateway_test_line\W+)((.|\n)*)###'
 
 service/workloads:
-  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_workloads_sap_((.|\n)*)###'
+  - '### (|New or )Affected Resource\(s\)\/Data Source\(s\)((.|\n)*)azurerm_(chaos_studio_|container_connected_registry\W+|container_registry_cache_rule\W+|container_registry_cache_rule\W+|container_registry_task\W+|container_registry_task_schedule_run_now\W+|container_registry_token_password\W+|kubernetes_cluster_extension\W+|kubernetes_cluster_trusted_access_role_binding\W+|kubernetes_fleet_manager\W+|kubernetes_fleet_member\W+|kubernetes_fleet_update_run\W+|kubernetes_fleet_update_strategy\W+|kubernetes_flux_configuration\W+|kubernetes_node_pool_snapshot\W+|workloads_sap_)((.|\n)*)###'

.github/workflows/golint.yaml

@@ -26,7 +26,7 @@ jobs:
           go-version-file: ./.go-version
       - uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 # v6.1.1
         with:
-          version: 'v1.55.1'
+          version: 'v1.61.0'
           args: -v ./internal/...
   save-artifacts-on-fail:
     if: ${{ needs.golint.result }} == 'failure'

.golangci.yml

@@ -17,29 +17,60 @@ issues:
 linters:
   disable-all: true
   enable:
-    - asciicheck
-    - bidichk
-    - errcheck
-    - gocritic
-    - gofmt
-    - goimports
-    - gosimple
-    - govet
-    - ineffassign
-    #- nakedret
-    - misspell
-    #- nolintlint
-    #- nlreturn
-    - reassign
-    - staticcheck
-    - typecheck
-    - unused
-    - unconvert
-    - unparam
-    - vet
-    - vetshadow
-    # - wastedassign # disabled because of generics
-    # - whitespace # Disabled for performance reasons - Ignores cache and takes 12+ minutes to run on the repo for _any_ change
+    - asasalint # check for pass []any as any in variadic func(...any)
+    - asciicheck # code does not contain non-ASCII identifiers
+    - bidichk # Checks for dangerous unicode character sequences
+    - decorder # Check declaration order and count of types, constants, variables and functions.
+    - durationcheck # Check for common mistakes when working with time.Duration
+    - errcheck # checking for unchecked errors
+    - gocritic # Linter for Go source code that specializes in simplifying code
+    - gofmt # Gofmt checks whether code was gofmt-ed
+    #- gofumpt # Gofumpt is a stricter gofmt
+    - goimports # Check import statements are formatted according to the 'goimport' command
+    - gosimple # Linter for Go source code that specializes in simplifying code.
+    #- gosec # Gosec is a security linter for Go source code
+    - 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.
+    #- nakedret # Checks that functions with naked returns are not longer than a maximum size
+    #- 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.
+    #- paralleltest # Detects missing usage of t.Parallel() method in your Go test.
+    #- perfsprint # Checks that fmt.Sprintf can be replaced with a faster alternative.
+    #- 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.
+    - staticcheck # It's a set of rules from staticcheck. It's not the same thing as the staticcheck binary.
+    #- tparallel # Tparallel detects inappropriate usage of t.Parallel() method in your Go test codes.
+    - typecheck # doesn't exist?!>?! XXXXXXXXX
+    - unused # Checks Go code for unused constants, variables, functions and types.
+    - unconvert # Remove unnecessary type conversions.
+    - unparam # Reports unused function parameters.
+    - wastedassign # Finds wasted assignment statements
+    #- wsl Add or remove empty lines.
+
+    #### DISABLED as parsers generator has a bug with things like SSHAdmin and we get S S H Admin, will fix in a seperate PR #####
+    #- dupword #Check for duplicated words in comments
+
+    #### DISABLED TO DO IN ITS OWN PR - should be enabled and done #####
+    #- tenv #detects using os.Setenv instead of t.Setenv since Go1.17.
+
+    #### REQUIRES GO 1.22 #####
+    #- copyloopvar #Detects range loop variables that are overwritten in the loop body
+
+    #### DISABLED till %+v -> %w #####
+    #- err113 #Go linter to check the errors handling expressions. - disabled as it suggests (correctly?) to use %w in fmt.Errorf instead of %+v (1000s of usages in the codebase)
+    #- errorlint #### DISABLED till %+v -> %w ##### #Errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme used in the github.com/pkg/errors package.
+
+    # disabled as it may be useful but there are a lot of switch statements in the codebase with unhandled inputs
+    #- exhaustive #Check for missing cases in select statements
+
+    ###### DISABLED because golang will put the space back into //nolint: linter ######
+    #- nolintlint  #Reports ill-formed or insufficient nolint directives.
+
+    # Disabled for performance reasons - Ignores cache and takes 12+ minutes to run on the repo for _any_ change
+    #- whitespace #checks for unnecessary newlines at the start and end of functions, if, for, etc.
 
 linters-settings:
   errcheck:

internal/clients/builder.go

@@ -5,6 +5,7 @@ package clients
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log"
 	"time"
@@ -48,7 +49,7 @@ func Build(ctx context.Context, builder ClientBuilder) (*Client, error) {
 
 	// point folks towards the separate Azure Stack Provider when using Azure Stack
 	if builder.AuthConfig.Environment.IsAzureStack() {
-		return nil, fmt.Errorf(azureStackEnvironmentError)
+		return nil, errors.New(azureStackEnvironmentError)
 	}
 
 	var resourceManagerAuth, storageAuth, synapseAuth, batchManagementAuth, keyVaultAuth auth.Authorizer

internal/customermanagedkeys/key_vault_or_managed_hsm_key.go

@@ -132,7 +132,7 @@ func ExpandKeyVaultOrManagedHSMKey(d interface{}, requireVersion VersionType, ke
 
 // ExpandKeyVaultOrManagedHSMKeyWithCustomFieldKey
 // d: should be one of *pluginsdk.ResourceData or map[string]interface{}
-// if return nil, nil, it means no key_vault_key_id or managed_hsm_key_id is specified
+// if return nil, nil - it means no key_vault_key_id or managed_hsm_key_id is specified //nolint:dupword
 func ExpandKeyVaultOrManagedHSMKeyWithCustomFieldKey(d interface{}, requireVersion VersionType, keyVaultFieldName, hsmFieldName string, keyVaultEnv, hsmEnv environments.Api) (*KeyVaultOrManagedHSMKey, error) {
 	key := &KeyVaultOrManagedHSMKey{}
 	var err error

internal/sdk/README.md

@@ -28,7 +28,7 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2020-06-01/resources" // nolint: staticcheck
+	"github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2020-06-01/resources" //nolint: staticcheck
 	"github.com/hashicorp/go-azure-helpers/lang/response"
 	"github.com/hashicorp/go-azure-helpers/resourcemanager/location"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/sdk"

internal/sdk/wrapper_resource.go

@@ -5,6 +5,7 @@ package sdk
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -79,18 +80,18 @@ func (rw *ResourceWrapper) Resource() (*schema.Resource, error) {
 		},
 		Importer: pluginsdk.ImporterValidatingResourceIdThen(func(id string) error {
 			fn := rw.resource.IDValidationFunc()
-			warnings, errors := fn(id, "id")
+			warnings, errs := fn(id, "id")
 			if len(warnings) > 0 {
 				for _, warning := range warnings {
 					rw.logger.Warn(warning)
 				}
 			}
-			if len(errors) > 0 {
+			if len(errs) > 0 {
 				out := ""
-				for _, err := range errors {
+				for _, err := range errs {
 					out += err.Error()
 				}
-				return fmt.Errorf(out)
+				return errors.New(out)
 			}
 
 			return nil

internal/services/apimanagement/api_management_api_schema_resource.go

@@ -216,7 +216,7 @@ func resourceApiManagementApiSchemaDelete(d *pluginsdk.ResourceData, meta interf
 }
 
 func convert2Str(rawVal interface{}) (string, error) {
-	value := ""
+	var value string
 	if val, ok := rawVal.(string); ok {
 		value = val
 	} else {

internal/services/apimanagement/api_management_backend_resource_test.go

@@ -328,7 +328,8 @@ resource "azurerm_api_management_backend" "test" {
 }
 
 func (r ApiManagementAuthorizationBackendResource) serviceFabric(data acceptance.TestData) string {
-	return fmt.Sprintf(`
+	// nolint: dupword
+	return fmt.Sprintf(` 
 %s
 
 resource "azurerm_api_management_certificate" "test" {
@@ -394,6 +395,7 @@ resource "azurerm_api_management_backend" "test" {
 }
 
 func (r ApiManagementAuthorizationBackendResource) serviceFabricClientCertificateId(data acceptance.TestData) string {
+	// nolint: dupword
 	return fmt.Sprintf(`
 %s